-
Notifications
You must be signed in to change notification settings - Fork 407
Fix SDK installation suggestion for already-installed versions #9834
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
Merged
Merged
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
1aad9c7
Fix SDK installation suggestion for already-installed versions
phenning e14a428
Remove duplicate test
phenning c334d16
Update PR to use registry-based SDK detection
phenning 47f29d0
Cleanup usings
phenning 2444025
Addressed reveiew feedback
phenning File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,7 @@ | ||
| // Licensed to the .NET Foundation under one or more agreements. The .NET Foundation licenses this file to you under the MIT license. See the LICENSE.md file in the project root for more information. | ||
|
|
||
| using System.Text.Json; | ||
| using Microsoft.VisualStudio.ProjectSystem.VS.Setup; | ||
| using Microsoft.VisualStudio.Shell; | ||
| using Microsoft.VisualStudio.Shell.Interop; | ||
| using Microsoft.VisualStudio.Threading; | ||
|
|
@@ -18,6 +19,7 @@ internal sealed partial class ProjectRetargetHandler : IProjectRetargetHandler, | |
| private readonly IProjectThreadingService _projectThreadingService; | ||
| private readonly IVsService<SVsTrackProjectRetargeting, IVsTrackProjectRetargeting2> _projectRetargetingService; | ||
| private readonly IVsService<SVsSolution, IVsSolution> _solutionService; | ||
| private readonly IDotNetEnvironment _dotnetEnvironment; | ||
|
|
||
| private Guid _currentSdkDescriptionId = Guid.Empty; | ||
| private Guid _sdkRetargetId = Guid.Empty; | ||
|
|
@@ -28,13 +30,15 @@ public ProjectRetargetHandler( | |
| IFileSystem fileSystem, | ||
| IProjectThreadingService projectThreadingService, | ||
| IVsService<SVsTrackProjectRetargeting, IVsTrackProjectRetargeting2> projectRetargetingService, | ||
| IVsService<SVsSolution, IVsSolution> solutionService) | ||
| IVsService<SVsSolution, IVsSolution> solutionService, | ||
| IDotNetEnvironment dotnetEnvironment) | ||
| { | ||
| _releasesProvider = releasesProvider; | ||
| _fileSystem = fileSystem; | ||
| _projectThreadingService = projectThreadingService; | ||
| _projectRetargetingService = projectRetargetingService; | ||
| _solutionService = solutionService; | ||
| _dotnetEnvironment = dotnetEnvironment; | ||
| } | ||
|
|
||
| public Task<IProjectTargetChange?> CheckForRetargetAsync(RetargetCheckOptions options) | ||
|
|
@@ -89,6 +93,12 @@ public Task RetargetAsync(TextWriter outputLogger, RetargetOptions options, IPro | |
| return null; | ||
| } | ||
|
|
||
| // Check if the retarget is already installed globally | ||
| if (_dotnetEnvironment.IsSdkInstalled(retargetVersion)) | ||
| { | ||
| return null; | ||
| } | ||
|
|
||
| if (_currentSdkDescriptionId == Guid.Empty) | ||
| { | ||
| // register the current and retarget versions, note there is a bug in the current implementation | ||
|
|
@@ -142,7 +152,7 @@ public Task RetargetAsync(TextWriter outputLogger, RetargetOptions options, IPro | |
| { | ||
| try | ||
| { | ||
| using Stream stream = File.OpenRead(globalJsonPath); | ||
| using Stream stream = _fileSystem.OpenTextStream(globalJsonPath); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unrelated to this PR, but if you're in an IDE could you rename |
||
| using JsonDocument doc = await JsonDocument.ParseAsync(stream); | ||
| if (doc.RootElement.TryGetProperty("sdk", out JsonElement sdkProp) && | ||
| sdkProp.TryGetProperty("version", out JsonElement versionProp)) | ||
|
|
||
123 changes: 123 additions & 0 deletions
123
...crosoft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/Setup/DotNetEnvironment.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,123 @@ | ||
| // Licensed to the .NET Foundation under one or more agreements. The .NET Foundation licenses this file to you under the MIT license. See the LICENSE.md file in the project root for more information. | ||
|
|
||
| using System.Runtime.InteropServices; | ||
| using IFileSystem = Microsoft.VisualStudio.IO.IFileSystem; | ||
| using IRegistry = Microsoft.VisualStudio.ProjectSystem.VS.Utilities.IRegistry; | ||
|
|
||
| namespace Microsoft.VisualStudio.ProjectSystem.VS.Setup; | ||
|
|
||
| /// <summary> | ||
| /// Provides information about the .NET environment and installed SDKs by querying the Windows registry. | ||
| /// </summary> | ||
| [Export(typeof(IDotNetEnvironment))] | ||
| internal class DotNetEnvironment : IDotNetEnvironment | ||
| { | ||
| private readonly IFileSystem _fileSystem; | ||
| private readonly IRegistry _registry; | ||
| private readonly IEnvironment _environment; | ||
|
|
||
| [ImportingConstructor] | ||
| public DotNetEnvironment(IFileSystem fileSystem, IRegistry registry, IEnvironment environment) | ||
| { | ||
| _fileSystem = fileSystem; | ||
| _registry = registry; | ||
| _environment = environment; | ||
| } | ||
|
|
||
| /// <inheritdoc/> | ||
| public bool IsSdkInstalled(string sdkVersion) | ||
| { | ||
| try | ||
| { | ||
| string archSubKey = GetArchitectureSubKey(_environment.ProcessArchitecture); | ||
| string registryKey = $@"SOFTWARE\dotnet\Setup\InstalledVersions\{archSubKey}\sdk"; | ||
|
|
||
| // Get all value names from the sdk subkey | ||
| string[] installedVersions = _registry.GetValueNames( | ||
| Win32.RegistryHive.LocalMachine, | ||
| Win32.RegistryView.Registry32, | ||
| registryKey); | ||
|
|
||
| // Check if the requested SDK version is in the list | ||
| foreach (string installedVersion in installedVersions) | ||
| { | ||
| if (string.Equals(installedVersion, sdkVersion, StringComparison.OrdinalIgnoreCase)) | ||
| { | ||
| return true; | ||
| } | ||
| } | ||
|
|
||
| return false; | ||
| } | ||
| catch | ||
| { | ||
| // If we fail to check, assume the SDK is not installed | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| /// <inheritdoc/> | ||
| public string? GetDotNetHostPath() | ||
| { | ||
| // First check the registry | ||
| string archSubKey = GetArchitectureSubKey(_environment.ProcessArchitecture); | ||
| string registryKey = $@"SOFTWARE\dotnet\Setup\InstalledVersions\{archSubKey}"; | ||
|
|
||
| string? installLocation = _registry.GetValue( | ||
| Win32.RegistryHive.LocalMachine, | ||
| Win32.RegistryView.Registry32, | ||
| registryKey, | ||
| "InstallLocation"); | ||
|
|
||
| if (!string.IsNullOrEmpty(installLocation)) | ||
| { | ||
| string dotnetExePath = Path.Combine(installLocation, "dotnet.exe"); | ||
| if (_fileSystem.FileExists(dotnetExePath)) | ||
| { | ||
| return dotnetExePath; | ||
| } | ||
| } | ||
|
|
||
| // Fallback to Program Files | ||
| string? programFiles = _environment.GetFolderPath(Environment.SpecialFolder.ProgramFiles); | ||
| if (programFiles is not null) | ||
| { | ||
| string dotnetPath = Path.Combine(programFiles, "dotnet", "dotnet.exe"); | ||
|
|
||
| if (_fileSystem.FileExists(dotnetPath)) | ||
| { | ||
| return dotnetPath; | ||
| } | ||
| } | ||
|
|
||
| return null; | ||
| } | ||
|
|
||
| /// <inheritdoc/> | ||
| public string[]? GetInstalledRuntimeVersions(Architecture architecture) | ||
| { | ||
| // https://github.com/dotnet/designs/blob/96d2ddad13dcb795ff2c5c6a051753363bdfcf7d/accepted/2020/install-locations.md#globally-registered-install-location-new | ||
|
|
||
| string archSubKey = GetArchitectureSubKey(architecture); | ||
| string registryKey = $@"SOFTWARE\dotnet\Setup\InstalledVersions\{archSubKey}\sharedfx\Microsoft.NETCore.App"; | ||
|
|
||
| string[] valueNames = _registry.GetValueNames( | ||
| Win32.RegistryHive.LocalMachine, | ||
| Win32.RegistryView.Registry32, | ||
| registryKey); | ||
|
|
||
| return valueNames.Length == 0 ? null : valueNames; | ||
| } | ||
|
|
||
| private static string GetArchitectureSubKey(Architecture architecture) | ||
| { | ||
| return architecture switch | ||
| { | ||
| Architecture.X86 => "x86", | ||
| Architecture.X64 => "x64", | ||
| Architecture.Arm => "arm", | ||
| Architecture.Arm64 => "arm64", | ||
| _ => architecture.ToString().ToLower() | ||
| }; | ||
| } | ||
| } |
40 changes: 40 additions & 0 deletions
40
...rosoft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/Setup/IDotNetEnvironment.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,40 @@ | ||
| // Licensed to the .NET Foundation under one or more agreements. The .NET Foundation licenses this file to you under the MIT license. See the LICENSE.md file in the project root for more information. | ||
|
|
||
| namespace Microsoft.VisualStudio.ProjectSystem.VS.Setup; | ||
|
|
||
| /// <summary> | ||
| /// Provides information about the .NET environment and installed SDKs. | ||
| /// </summary> | ||
| [ProjectSystemContract(ProjectSystemContractScope.Global, ProjectSystemContractProvider.Private, Cardinality = ImportCardinality.ExactlyOne)] | ||
| internal interface IDotNetEnvironment | ||
| { | ||
| /// <summary> | ||
| /// Checks if a specific .NET SDK version is installed on the system. | ||
| /// </summary> | ||
| /// <param name="sdkVersion">The SDK version to check for (e.g., "8.0.415").</param> | ||
| /// <returns> | ||
| /// A task that represents the asynchronous operation. The task result contains | ||
| /// <see langword="true"/> if the SDK version is installed; otherwise, <see langword="false"/>. | ||
| /// </returns> | ||
| bool IsSdkInstalled(string sdkVersion); | ||
|
|
||
| /// <summary> | ||
| /// Gets the path to the dotnet.exe executable. | ||
| /// </summary> | ||
| /// <returns> | ||
| /// The full path to dotnet.exe if found; otherwise, <see langword="null"/>. | ||
| /// </returns> | ||
| string? GetDotNetHostPath(); | ||
|
|
||
| /// <summary> | ||
| /// Reads the list of installed .NET Core runtimes for the specified architecture from the registry. | ||
| /// </summary> | ||
| /// <remarks> | ||
| /// Returns runtimes installed both as standalone packages, and through VS Setup. | ||
| /// Values have the form <c>3.1.32</c>, <c>7.0.11</c>, <c>8.0.0-preview.7.23375.6</c>, <c>8.0.0-rc.1.23419.4</c>. | ||
| /// If results could not be determined, <see langword="null"/> is returned. | ||
| /// </remarks> | ||
| /// <param name="architecture">The runtime architecture to report results for.</param> | ||
| /// <returns>An array of runtime versions, or <see langword="null"/> if results could not be determined or no runtimes were found.</returns> | ||
| string[]? GetInstalledRuntimeVersions(System.Runtime.InteropServices.Architecture architecture); | ||
| } |
37 changes: 0 additions & 37 deletions
37
...o.ProjectSystem.Managed.VS/ProjectSystem/VS/Setup/NetCoreRuntimeVersionsRegistryReader.cs
This file was deleted.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
35 changes: 35 additions & 0 deletions
35
src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/Utilities/IRegistry.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| // Licensed to the .NET Foundation under one or more agreements. The .NET Foundation licenses this file to you under the MIT license. See the LICENSE.md file in the project root for more information. | ||
|
|
||
| using Microsoft.Win32; | ||
|
|
||
| namespace Microsoft.VisualStudio.ProjectSystem.VS.Utilities; | ||
|
|
||
| /// <summary> | ||
| /// Provides access to the Windows registry in a testable manner. | ||
| /// </summary> | ||
| [ProjectSystem.ProjectSystemContract(ProjectSystem.ProjectSystemContractScope.Global, ProjectSystem.ProjectSystemContractProvider.Private, Cardinality = ImportCardinality.ExactlyOne)] | ||
| internal interface IRegistry | ||
| { | ||
| /// <summary> | ||
| /// Opens a registry key with the specified path under the given base key. | ||
| /// </summary> | ||
| /// <param name="hive">The registry hive to open (e.g., LocalMachine, CurrentUser).</param> | ||
| /// <param name="view">The registry view to use (e.g., Registry32, Registry64).</param> | ||
| /// <param name="subKeyPath">The path to the subkey to open.</param> | ||
| /// <param name="valueName">The name of the value to retrieve.</param> | ||
| /// <returns> | ||
| /// The registry key value as a string if found; otherwise, <see langword="null"/>. | ||
| /// </returns> | ||
| string? GetValue(RegistryHive hive, RegistryView view, string subKeyPath, string valueName); | ||
|
|
||
| /// <summary> | ||
| /// Gets the names of all values under the specified registry key. | ||
| /// </summary> | ||
| /// <param name="hive">The registry hive to open (e.g., LocalMachine, CurrentUser).</param> | ||
| /// <param name="view">The registry view to use (e.g., Registry32, Registry64).</param> | ||
| /// <param name="subKeyPath">The path to the subkey to open.</param> | ||
| /// <returns> | ||
| /// An array of value names if the key exists; otherwise, an empty array. | ||
| /// </returns> | ||
| string[] GetValueNames(RegistryHive hive, RegistryView view, string subKeyPath); | ||
| } |
40 changes: 40 additions & 0 deletions
40
...osoft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/Utilities/RegistryService.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,40 @@ | ||
| // Licensed to the .NET Foundation under one or more agreements. The .NET Foundation licenses this file to you under the MIT license. See the LICENSE.md file in the project root for more information. | ||
|
|
||
| using Microsoft.Win32; | ||
|
|
||
| namespace Microsoft.VisualStudio.ProjectSystem.VS.Utilities; | ||
|
|
||
| /// <summary> | ||
| /// Provides access to the Windows registry. | ||
| /// </summary> | ||
| [Export(typeof(IRegistry))] | ||
| internal class RegistryService : IRegistry | ||
| { | ||
| /// <inheritdoc/> | ||
| public string? GetValue(RegistryHive hive, RegistryView view, string subKeyPath, string valueName) | ||
| { | ||
| using RegistryKey? subKey = OpenSubKey(hive, view, subKeyPath); | ||
| return subKey?.GetValue(valueName) as string; | ||
| } | ||
|
|
||
| /// <inheritdoc/> | ||
| public string[] GetValueNames(RegistryHive hive, RegistryView view, string subKeyPath) | ||
| { | ||
| using RegistryKey? subKey = OpenSubKey(hive, view, subKeyPath); | ||
| return subKey?.GetValueNames() ?? []; | ||
| } | ||
|
|
||
| private static RegistryKey? OpenSubKey(RegistryHive hive, RegistryView view, string subKeyPath) | ||
| { | ||
| try | ||
| { | ||
| using RegistryKey baseKey = RegistryKey.OpenBaseKey(hive, view); | ||
| return baseKey.OpenSubKey(subKeyPath); | ||
| } | ||
| catch (Exception ex) when (ex.IsCatchable()) | ||
| { | ||
| // Return null on catchable registry access errors | ||
| return null; | ||
| } | ||
| } | ||
| } |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
If
IsSdkInstalledAsyncis only checking global locations, maybe include that detail in the method name:IsSdkInstalledGloballyAsync