-
Notifications
You must be signed in to change notification settings - Fork 461
Update and futureproofing of ConfigureVisualStudio #1833
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
Conversation
Visual Studio contains a copy of mingit that it will use for Git operations in its UI (Team Explorer, etc.), which can be overridden by a registry key. Previously, gvfs mount would set the registry key to point to the user's installed version of git (presumably the .vfs fork), but it only set the registry key for versions 15.0 and 16.0 of Visual Studio. This commit instead looks for registry keys for versions of Visual Studio at least 15.0, in the same format as the known keys, and sets the key for them. The current version of Visual Studio is 17.0, and this is fix is confirmed to work for it. With luck this will also work for future versions of Visual Studio, but there's no guarantee of that.
dscho
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! In microsoft/git, we extended this list, too albeit by hard-coding the version numbers.
| * revisited in the future if VS changes how it stores this. */ | ||
| var vsVersions = Registry.CurrentUser.OpenSubKey(VSRegistryKeyRoot) | ||
| ?.GetSubKeyNames() | ||
| .Where(name => float.TryParse(name, out var version) && version >= 15.0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The equality arm of >= 15.0 unnerves me.. comparing equality of two floating-point numbers. But, if this has been tested for VS15, then I guess this is ok.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could also use System.Version.TryParse and compare the .Major version parts.
mjcheetham
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Upon second thought.. I think I'd prefer avoiding floats here and using Version.Major integer comparison instead.
Interesting! I'd have expected this to already cover the users that were having issues with this. On my machine those registry keys for up to 20.0 exist, but GitPath value is only set for 15.0 and 16.0. I've uninstalled/reinstalled git several times testing things, so I wonder if those values were added on first install, removed on uninstall, but not added back on later reinstall. Edit: Did some experimenting - if I install microsoft/git using the installer from the releases page, clicking through the installer UI, then it sets those keys. If I install it with winget (silent install), it does not set the keys. Edit2: After some more experimenting, it's the /COMPONENTS=icons,ext,gitlfs,assoc,assoc_sh,consolefont,windowsterminal,scalar in winget config that is causing the post-install script not to run. Edit3: The default set of components also includes autoupdate. Since the custom installer is explicitly inserted after |
|
Created microsoft/git#739 to track the winget installer issue. |
dscho
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
mjcheetham
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the changes! Just one thing to note.. this change now means that you must install Visual Studio before VFS for Git, otherwise these registry subkey versions will not be present and therefore the GitSourceControl key will not be written. I assume this is acceptable for your use case @tyrielv?
Yes, and also this runs on mount (not just on install) so a remount would fix it. |
Visual Studio contains a copy of mingit that it will use for Git operations in its UI (Team Explorer, etc.), which can be overridden by a registry key.
Previously, gvfs mount would set the registry key to point to the user's installed version of git (presumably the .vfs fork), but it only set the registry key for versions 15.0 and 16.0 of Visual Studio.
This commit instead looks for registry keys for versions of Visual Studio at least 15.0, in the same format as the known keys, and sets the key for them.
The current version of Visual Studio is 17.0, and this is fix is confirmed to work for it.
With luck this will also work for future versions of Visual Studio, but there's no guarantee of that.