Skip to content

Commit a525c6a

Browse files
authored
Fix SDK installation suggestion for already-installed versions (#9834)
* Fix SDK installation suggestion for already-installed versions Previously, the project retargeting handler would suggest installing an SDK version that was already installed on the system. This occurred because the check for installed SDKs was not properly implemented. Changes: - Add SdkInstallationService to detect globally installed .NET SDKs by querying the Windows registry for both 32-bit and 64-bit SDK installations - Add IRegistry abstraction and RegistryService implementation for testable registry access - Update ProjectRetargetHandler to check if the retarget SDK version is already installed before suggesting installation - Add IEnvironment abstraction and EnvironmentService for testable environment variable access - Add comprehensive unit tests for SdkInstallationService, RegistryService, and EnvironmentService - Simplify IRegistryMock test helper to use string-based keys instead of complex tuple structures - Fix JsonDocument.Parse to use ParseAsync for proper async/await pattern in ProjectRetargetHandler * Remove duplicate test * Update PR to use registry-based SDK detection Replace CLI-based SDK detection with direct Windows registry queries for improved performance and reliability. Key changes: - Create IDotNetEnvironment service to centralize .NET environment queries - Check installed SDK versions via registry (SOFTWARE\dotnet\Setup\InstalledVersions\{arch}\sdk) - Detect .NET runtime versions via registry - Locate dotnet.exe host path with registry fallback to Program Files - Refactor SetupComponentRegistrationService to use IDotNetEnvironment instead of NetCoreRuntimeVersionsRegistryReader - Add ExceptionExtensions.IsCatchable() for safer exception handling This eliminates the need to spawn dotnet.exe processes for SDK detection * Cleanup usings * Addressed reveiew feedback * merged IEnvironmentHelper into IEnvironment * change IsSdkInstalledAsync to IsSdkInstalled * removed unneeded Is64BitOperatingSystem * Moved ExceptionExtensions to non-VS assembly, fixed namespace
1 parent d4615c8 commit a525c6a

File tree

25 files changed

+1433
-102
lines changed

25 files changed

+1433
-102
lines changed

src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/Debug/ProjectLaunchTargetsProvider.cs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
using Microsoft.VisualStudio.ProjectSystem.Debug;
99
using Microsoft.VisualStudio.ProjectSystem.HotReload;
1010
using Microsoft.VisualStudio.ProjectSystem.Properties;
11-
using Microsoft.VisualStudio.ProjectSystem.Utilities;
1211
using Microsoft.VisualStudio.ProjectSystem.VS.HotReload;
1312
using Microsoft.VisualStudio.Shell.Interop;
1413
using Newtonsoft.Json;
@@ -36,7 +35,7 @@ internal class ProjectLaunchTargetsProvider :
3635
private readonly IUnconfiguredProjectVsServices _unconfiguredProjectVsServices;
3736
private readonly IDebugTokenReplacer _tokenReplacer;
3837
private readonly IFileSystem _fileSystem;
39-
private readonly IEnvironmentHelper _environment;
38+
private readonly IEnvironment _environment;
4039
private readonly IActiveDebugFrameworkServices _activeDebugFramework;
4140
private readonly IProjectThreadingService _threadingService;
4241
private readonly IVsUIService<IVsDebugger10> _debugger;
@@ -50,7 +49,7 @@ public ProjectLaunchTargetsProvider(
5049
ConfiguredProject project,
5150
IDebugTokenReplacer tokenReplacer,
5251
IFileSystem fileSystem,
53-
IEnvironmentHelper environment,
52+
IEnvironment environment,
5453
IActiveDebugFrameworkServices activeDebugFramework,
5554
IOutputTypeChecker outputTypeChecker,
5655
IProjectThreadingService threadingService,

src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/Retargeting/ProjectRetargetHandler.cs

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// 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.
22

33
using System.Text.Json;
4+
using Microsoft.VisualStudio.ProjectSystem.VS.Setup;
45
using Microsoft.VisualStudio.Shell;
56
using Microsoft.VisualStudio.Shell.Interop;
67
using Microsoft.VisualStudio.Threading;
@@ -18,6 +19,7 @@ internal sealed partial class ProjectRetargetHandler : IProjectRetargetHandler,
1819
private readonly IProjectThreadingService _projectThreadingService;
1920
private readonly IVsService<SVsTrackProjectRetargeting, IVsTrackProjectRetargeting2> _projectRetargetingService;
2021
private readonly IVsService<SVsSolution, IVsSolution> _solutionService;
22+
private readonly IDotNetEnvironment _dotnetEnvironment;
2123

2224
private Guid _currentSdkDescriptionId = Guid.Empty;
2325
private Guid _sdkRetargetId = Guid.Empty;
@@ -28,13 +30,15 @@ public ProjectRetargetHandler(
2830
IFileSystem fileSystem,
2931
IProjectThreadingService projectThreadingService,
3032
IVsService<SVsTrackProjectRetargeting, IVsTrackProjectRetargeting2> projectRetargetingService,
31-
IVsService<SVsSolution, IVsSolution> solutionService)
33+
IVsService<SVsSolution, IVsSolution> solutionService,
34+
IDotNetEnvironment dotnetEnvironment)
3235
{
3336
_releasesProvider = releasesProvider;
3437
_fileSystem = fileSystem;
3538
_projectThreadingService = projectThreadingService;
3639
_projectRetargetingService = projectRetargetingService;
3740
_solutionService = solutionService;
41+
_dotnetEnvironment = dotnetEnvironment;
3842
}
3943

4044
public Task<IProjectTargetChange?> CheckForRetargetAsync(RetargetCheckOptions options)
@@ -89,6 +93,12 @@ public Task RetargetAsync(TextWriter outputLogger, RetargetOptions options, IPro
8993
return null;
9094
}
9195

96+
// Check if the retarget is already installed globally
97+
if (_dotnetEnvironment.IsSdkInstalled(retargetVersion))
98+
{
99+
return null;
100+
}
101+
92102
if (_currentSdkDescriptionId == Guid.Empty)
93103
{
94104
// 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
142152
{
143153
try
144154
{
145-
using Stream stream = File.OpenRead(globalJsonPath);
155+
using Stream stream = _fileSystem.OpenTextStream(globalJsonPath);
146156
using JsonDocument doc = await JsonDocument.ParseAsync(stream);
147157
if (doc.RootElement.TryGetProperty("sdk", out JsonElement sdkProp) &&
148158
sdkProp.TryGetProperty("version", out JsonElement versionProp))
Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
// 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.
2+
3+
using System.Runtime.InteropServices;
4+
using IFileSystem = Microsoft.VisualStudio.IO.IFileSystem;
5+
using IRegistry = Microsoft.VisualStudio.ProjectSystem.VS.Utilities.IRegistry;
6+
7+
namespace Microsoft.VisualStudio.ProjectSystem.VS.Setup;
8+
9+
/// <summary>
10+
/// Provides information about the .NET environment and installed SDKs by querying the Windows registry.
11+
/// </summary>
12+
[Export(typeof(IDotNetEnvironment))]
13+
internal class DotNetEnvironment : IDotNetEnvironment
14+
{
15+
private readonly IFileSystem _fileSystem;
16+
private readonly IRegistry _registry;
17+
private readonly IEnvironment _environment;
18+
19+
[ImportingConstructor]
20+
public DotNetEnvironment(IFileSystem fileSystem, IRegistry registry, IEnvironment environment)
21+
{
22+
_fileSystem = fileSystem;
23+
_registry = registry;
24+
_environment = environment;
25+
}
26+
27+
/// <inheritdoc/>
28+
public bool IsSdkInstalled(string sdkVersion)
29+
{
30+
try
31+
{
32+
string archSubKey = GetArchitectureSubKey(_environment.ProcessArchitecture);
33+
string registryKey = $@"SOFTWARE\dotnet\Setup\InstalledVersions\{archSubKey}\sdk";
34+
35+
// Get all value names from the sdk subkey
36+
string[] installedVersions = _registry.GetValueNames(
37+
Win32.RegistryHive.LocalMachine,
38+
Win32.RegistryView.Registry32,
39+
registryKey);
40+
41+
// Check if the requested SDK version is in the list
42+
foreach (string installedVersion in installedVersions)
43+
{
44+
if (string.Equals(installedVersion, sdkVersion, StringComparison.OrdinalIgnoreCase))
45+
{
46+
return true;
47+
}
48+
}
49+
50+
return false;
51+
}
52+
catch
53+
{
54+
// If we fail to check, assume the SDK is not installed
55+
return false;
56+
}
57+
}
58+
59+
/// <inheritdoc/>
60+
public string? GetDotNetHostPath()
61+
{
62+
// First check the registry
63+
string archSubKey = GetArchitectureSubKey(_environment.ProcessArchitecture);
64+
string registryKey = $@"SOFTWARE\dotnet\Setup\InstalledVersions\{archSubKey}";
65+
66+
string? installLocation = _registry.GetValue(
67+
Win32.RegistryHive.LocalMachine,
68+
Win32.RegistryView.Registry32,
69+
registryKey,
70+
"InstallLocation");
71+
72+
if (!string.IsNullOrEmpty(installLocation))
73+
{
74+
string dotnetExePath = Path.Combine(installLocation, "dotnet.exe");
75+
if (_fileSystem.FileExists(dotnetExePath))
76+
{
77+
return dotnetExePath;
78+
}
79+
}
80+
81+
// Fallback to Program Files
82+
string? programFiles = _environment.GetFolderPath(Environment.SpecialFolder.ProgramFiles);
83+
if (programFiles is not null)
84+
{
85+
string dotnetPath = Path.Combine(programFiles, "dotnet", "dotnet.exe");
86+
87+
if (_fileSystem.FileExists(dotnetPath))
88+
{
89+
return dotnetPath;
90+
}
91+
}
92+
93+
return null;
94+
}
95+
96+
/// <inheritdoc/>
97+
public string[]? GetInstalledRuntimeVersions(Architecture architecture)
98+
{
99+
// https://github.com/dotnet/designs/blob/96d2ddad13dcb795ff2c5c6a051753363bdfcf7d/accepted/2020/install-locations.md#globally-registered-install-location-new
100+
101+
string archSubKey = GetArchitectureSubKey(architecture);
102+
string registryKey = $@"SOFTWARE\dotnet\Setup\InstalledVersions\{archSubKey}\sharedfx\Microsoft.NETCore.App";
103+
104+
string[] valueNames = _registry.GetValueNames(
105+
Win32.RegistryHive.LocalMachine,
106+
Win32.RegistryView.Registry32,
107+
registryKey);
108+
109+
return valueNames.Length == 0 ? null : valueNames;
110+
}
111+
112+
private static string GetArchitectureSubKey(Architecture architecture)
113+
{
114+
return architecture switch
115+
{
116+
Architecture.X86 => "x86",
117+
Architecture.X64 => "x64",
118+
Architecture.Arm => "arm",
119+
Architecture.Arm64 => "arm64",
120+
_ => architecture.ToString().ToLower()
121+
};
122+
}
123+
}
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
// 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.
2+
3+
namespace Microsoft.VisualStudio.ProjectSystem.VS.Setup;
4+
5+
/// <summary>
6+
/// Provides information about the .NET environment and installed SDKs.
7+
/// </summary>
8+
[ProjectSystemContract(ProjectSystemContractScope.Global, ProjectSystemContractProvider.Private, Cardinality = ImportCardinality.ExactlyOne)]
9+
internal interface IDotNetEnvironment
10+
{
11+
/// <summary>
12+
/// Checks if a specific .NET SDK version is installed on the system.
13+
/// </summary>
14+
/// <param name="sdkVersion">The SDK version to check for (e.g., "8.0.415").</param>
15+
/// <returns>
16+
/// A task that represents the asynchronous operation. The task result contains
17+
/// <see langword="true"/> if the SDK version is installed; otherwise, <see langword="false"/>.
18+
/// </returns>
19+
bool IsSdkInstalled(string sdkVersion);
20+
21+
/// <summary>
22+
/// Gets the path to the dotnet.exe executable.
23+
/// </summary>
24+
/// <returns>
25+
/// The full path to dotnet.exe if found; otherwise, <see langword="null"/>.
26+
/// </returns>
27+
string? GetDotNetHostPath();
28+
29+
/// <summary>
30+
/// Reads the list of installed .NET Core runtimes for the specified architecture from the registry.
31+
/// </summary>
32+
/// <remarks>
33+
/// Returns runtimes installed both as standalone packages, and through VS Setup.
34+
/// 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>.
35+
/// If results could not be determined, <see langword="null"/> is returned.
36+
/// </remarks>
37+
/// <param name="architecture">The runtime architecture to report results for.</param>
38+
/// <returns>An array of runtime versions, or <see langword="null"/> if results could not be determined or no runtimes were found.</returns>
39+
string[]? GetInstalledRuntimeVersions(System.Runtime.InteropServices.Architecture architecture);
40+
}

src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/Setup/NetCoreRuntimeVersionsRegistryReader.cs

Lines changed: 0 additions & 37 deletions
This file was deleted.

src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/Setup/SetupComponentRegistrationService.cs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ public SetupComponentRegistrationService(
4343
IVsService<SVsSetupCompositionService, IVsSetupCompositionService> vsSetupCompositionService,
4444
ISolutionService solutionService,
4545
IProjectFaultHandlerService projectFaultHandlerService,
46+
IDotNetEnvironment dotnetEnvironment,
4647
JoinableTaskContext joinableTaskContext)
4748
: base(new(joinableTaskContext))
4849
{
@@ -51,9 +52,9 @@ public SetupComponentRegistrationService(
5152
_solutionService = solutionService;
5253
_projectFaultHandlerService = projectFaultHandlerService;
5354

54-
_installedRuntimeComponentIds = new Lazy<HashSet<string>?>(FindInstalledRuntimeComponentIds);
55+
_installedRuntimeComponentIds = new Lazy<HashSet<string>?>(() => FindInstalledRuntimeComponentIds(dotnetEnvironment));
5556

56-
static HashSet<string>? FindInstalledRuntimeComponentIds()
57+
static HashSet<string>? FindInstalledRuntimeComponentIds(IDotNetEnvironment dotnetEnvironment)
5758
{
5859
// Workaround for https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1460328
5960
// VS Setup doesn't know about runtimes installed outside of VS. Deep detection is not suggested for performance reasons.
@@ -66,7 +67,7 @@ public SetupComponentRegistrationService(
6667
// TODO consider the architecture of the project itself
6768
Architecture architecture = RuntimeInformation.ProcessArchitecture;
6869

69-
string[]? runtimeVersions = NetCoreRuntimeVersionsRegistryReader.ReadRuntimeVersionsInstalledInLocalMachine(architecture);
70+
string[]? runtimeVersions = dotnetEnvironment.GetInstalledRuntimeVersions(architecture);
7071

7172
if (runtimeVersions is null)
7273
{
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
// 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.
2+
3+
using Microsoft.Win32;
4+
5+
namespace Microsoft.VisualStudio.ProjectSystem.VS.Utilities;
6+
7+
/// <summary>
8+
/// Provides access to the Windows registry in a testable manner.
9+
/// </summary>
10+
[ProjectSystem.ProjectSystemContract(ProjectSystem.ProjectSystemContractScope.Global, ProjectSystem.ProjectSystemContractProvider.Private, Cardinality = ImportCardinality.ExactlyOne)]
11+
internal interface IRegistry
12+
{
13+
/// <summary>
14+
/// Opens a registry key with the specified path under the given base key.
15+
/// </summary>
16+
/// <param name="hive">The registry hive to open (e.g., LocalMachine, CurrentUser).</param>
17+
/// <param name="view">The registry view to use (e.g., Registry32, Registry64).</param>
18+
/// <param name="subKeyPath">The path to the subkey to open.</param>
19+
/// <param name="valueName">The name of the value to retrieve.</param>
20+
/// <returns>
21+
/// The registry key value as a string if found; otherwise, <see langword="null"/>.
22+
/// </returns>
23+
string? GetValue(RegistryHive hive, RegistryView view, string subKeyPath, string valueName);
24+
25+
/// <summary>
26+
/// Gets the names of all values under the specified registry key.
27+
/// </summary>
28+
/// <param name="hive">The registry hive to open (e.g., LocalMachine, CurrentUser).</param>
29+
/// <param name="view">The registry view to use (e.g., Registry32, Registry64).</param>
30+
/// <param name="subKeyPath">The path to the subkey to open.</param>
31+
/// <returns>
32+
/// An array of value names if the key exists; otherwise, an empty array.
33+
/// </returns>
34+
string[] GetValueNames(RegistryHive hive, RegistryView view, string subKeyPath);
35+
}
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
// 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.
2+
3+
using Microsoft.Win32;
4+
5+
namespace Microsoft.VisualStudio.ProjectSystem.VS.Utilities;
6+
7+
/// <summary>
8+
/// Provides access to the Windows registry.
9+
/// </summary>
10+
[Export(typeof(IRegistry))]
11+
internal class RegistryService : IRegistry
12+
{
13+
/// <inheritdoc/>
14+
public string? GetValue(RegistryHive hive, RegistryView view, string subKeyPath, string valueName)
15+
{
16+
using RegistryKey? subKey = OpenSubKey(hive, view, subKeyPath);
17+
return subKey?.GetValue(valueName) as string;
18+
}
19+
20+
/// <inheritdoc/>
21+
public string[] GetValueNames(RegistryHive hive, RegistryView view, string subKeyPath)
22+
{
23+
using RegistryKey? subKey = OpenSubKey(hive, view, subKeyPath);
24+
return subKey?.GetValueNames() ?? [];
25+
}
26+
27+
private static RegistryKey? OpenSubKey(RegistryHive hive, RegistryView view, string subKeyPath)
28+
{
29+
try
30+
{
31+
using RegistryKey baseKey = RegistryKey.OpenBaseKey(hive, view);
32+
return baseKey.OpenSubKey(subKeyPath);
33+
}
34+
catch (Exception ex) when (ex.IsCatchable())
35+
{
36+
// Return null on catchable registry access errors
37+
return null;
38+
}
39+
}
40+
}

0 commit comments

Comments
 (0)