Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Select-VSSetupInstance -Latest - is it about install date or version? #40

Closed
nightroman opened this issue Mar 15, 2018 · 18 comments · Fixed by #53
Closed

Select-VSSetupInstance -Latest - is it about install date or version? #40

nightroman opened this issue Mar 15, 2018 · 18 comments · Fixed by #53
Assignees
Labels

Comments

@nightroman
Copy link

Select-VSSetupInstance -Latest - is it about install date or version?

The help says

-Latest [<SwitchParameter>]
    Select the most recently installed instance with the highest version (within the optional `-Version` range).

so it is not clear whether it is about date or version.

If we take a look at the code
then it looks like it is about installation dates and versions are not checked.

If it is true, is it by design that Latest deals with installation dates, not versions?

In any case, the help might be more clear about what is checked.

NB (Just for cross-reference) Loosely related to this issue nightroman/Invoke-Build#122

@heaths
Copy link
Member

heaths commented Mar 15, 2018

"the most recent" means most recently installed, which will have a newer InstallDate. I should be checking for the highest version, and if more than one instance has the same version I should pick the one with the latest InstallDate. But now that you call attention to it, I see it's not sorting based on version like I did in vswhere. I'll get that fixed and make sure I have a test covering it.

@heaths heaths added bug and removed question labels Mar 15, 2018
heaths added a commit that referenced this issue Mar 16, 2018
Fixes #40 and updates the test data to make sure we're properly sorting and selecting. Also made a corresponding test-only change to Microsoft/vswhere to make sure this case isn't regressed there (was already handled correctly).
heaths added a commit that referenced this issue Mar 16, 2018
Fixes #40 and updates the test data to make sure we're properly sorting and selecting. Also made a corresponding test-only change to Microsoft/vswhere to make sure this case isn't regressed there (was already handled correctly).
b-higginbotham added a commit to b-higginbotham/vssetup.powershell that referenced this issue Jun 21, 2019
The InstanceComparer that was added as part of microsoft#41 to fix microsoft#40 isn't actually able to solve the problem due to the implementation of the ProcessRecord method in SelectInstanceCommand. InstanceComparer is used to sort instances by version and then date, but when iterating over all instances, the latestInstance variable is only set when the latestInstance date is less than the date of the current instance in the loop.

This completely ignores the previous OrderBy that was done using InstanceComparer. The simplest solution at this point is to always set the latestInstance variable when the Latest flag is set because the collection is already sorted in the desired order and no additional comparisons are necessary.
@pssewell21
Copy link

@heaths, I have been experiencing an issue where the -Latest flag is picking my VS 2017 instance that was installed after my VS 2019 instance and I believe this discussion is relevant. @Hadronicus appears to have a solution in pull request #53 to this. I noticed that his pull request has been pending approval for over 5 weeks. Are you waiting on something to approve the pull request? Is there additional work that needs to be done for you to approve it that I'm just not seeing communication for? I'm happy to contribute to the effort if needed. Thanks.

heaths pushed a commit that referenced this issue Jul 31, 2019
The InstanceComparer that was added as part of #41 to fix #40 isn't actually able to solve the problem due to the implementation of the ProcessRecord method in SelectInstanceCommand. InstanceComparer is used to sort instances by version and then date, but when iterating over all instances, the latestInstance variable is only set when the latestInstance date is less than the date of the current instance in the loop.

This completely ignores the previous OrderBy that was done using InstanceComparer. The simplest solution at this point is to always set the latestInstance variable when the Latest flag is set because the collection is already sorted in the desired order and no additional comparisons are necessary.
@heaths
Copy link
Member

heaths commented Jul 31, 2019

To be honest, I missed this somehow. I've merged this and will get the changes published. Thank you for your contribution!

@pssewell21
Copy link

Thank you @heaths and @Hadronicus.

@heaths
Copy link
Member

heaths commented Jul 31, 2019

Unfortunately, we're having a publishing issue at the moment I'm trying to sort out. For now, can you expand the ZIP release and make sure all your existing scenarios work? We have code coverage and detected no regressions, but I'd like to get hands on a prerelease before we push a release (once I get all that working again).

@pssewell21
Copy link

I'll be able to test this tomorrow. Thanks for the info.

@pssewell21
Copy link

I've been running into issues getting Visual Studio's installer to update to the newly released version that are currently blocking me from testing this. I'll provide an update as soon as I have one to give.

@heaths
Copy link
Member

heaths commented Aug 1, 2019

The installation of our installer is pretty much just deleting a directory (we make a backup), unzip, and run a finalization process. We've not heard reports of this failing. Could you open a feedback ticket from within the Installer (if you still have one installed) or from within Visual Studio if already installed? That will collected relevant data (at least from within the Installer) we'd need to diagnose the issue. But as long as the query API is installed (which actually installs with the products we install so we retain error handling, download robustness, telemetry, etc.) this module will work. I haven't made any significant changes to the query API in many months. Both these cmdlets and vswhere are designed to work with even the original versions of the query API and rarely make use of new features, which we try to use best-effort if necessary.

@pssewell21
Copy link

@pssewell21
Copy link

Update: I installed the prerelease module in my environment. Get-VsSetupInstance gets the following:

DisplayName: Visual Studio Enterprise 2019
InstallationVersion: 16.2.29123.88
InstallDate: 7/31/2019 5:23:27PM

DisplayName: Visual Studio Enterprise 2017
InstallationVersion: 15.9.28307.770
InstallDate: 8/1/2019 3:01:17PM

Get-VsSetupInstance | Select-VsSetupInstance -Latest 

is returning the 2017 instance instead of the 2019 instance as I would expect it should. I'll take a look into the change @Hadronicus made as I would expect that his change would have returned the 2019 instance when the latest flag is used. I'll plan to look at this when I come in tomorrow. Thanks for the pre-release to try out.

@heaths
Copy link
Member

heaths commented Aug 1, 2019

I was able to repro the problem and will be sure to add an integration test. The problem occurs when piping instances to Select-VSSetupInstance because records come in one at a time but needed to be sorted all together. If you pass all instances as a single parameter value to Select-VSSetupInstance -Instance it works correctly. As such, all sorting should just be done in EndProcessing. I'll make the changes but we're still having problems publishing at the moment.

@heaths heaths reopened this Aug 1, 2019
@heaths heaths self-assigned this Aug 1, 2019
heaths added a commit that referenced this issue Aug 4, 2019
@heaths heaths closed this as completed in 0f58b5c Aug 4, 2019
@pssewell21
Copy link

@heaths, any idea what the timeline looks like on getting this change into a released version of the tool? Thanks.

@heaths
Copy link
Member

heaths commented Sep 21, 2019

Unfortunately, powershellgallery.com is still giving us issues in production. I've pinged the owner again for a status update.

@heaths
Copy link
Member

heaths commented Sep 21, 2019

Reopening until published to keep on my radar.

@heaths heaths reopened this Sep 21, 2019
@heaths
Copy link
Member

heaths commented Oct 1, 2019

Okay, finally got the powershellgallery.com issue resolved and https://www.powershellgallery.com/packages/VSSetup/2.2.15-g62f3905dea was published. Can you give that a try and make sure everything is working as expected? Then I can publish a release version.

@pssewell21
Copy link

I'll look into this tomorrow when I get into work. Thanks for the support @heaths

@pssewell21
Copy link

Just tested this and it appears to behave as I would expect in my environment now! Thanks for your assistance @heaths. I apologize for the delayed response, I just got to this.

@heaths heaths mentioned this issue Oct 4, 2019
heaths added a commit that referenced this issue Oct 4, 2019
Release fix for #40
@heaths
Copy link
Member

heaths commented Oct 4, 2019

2.2.16 with this fix has been released.

@heaths heaths closed this as completed Oct 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants