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.

Sign up for GitHub

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

Return SDKs from all dotnet location #329

Merged
merged 7 commits into from
Mar 31, 2025
Merged

Return SDKs from all dotnet location #329

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
de9afce
Return SDKs from all dotnet location
JoeRobich Mar 19, 2025
6314d67
Preserve ordering of returned SDKs across dotnet instances.
JoeRobich Mar 20, 2025
2424986
Yield return all the available SDKs
JoeRobich Mar 20, 2025
302e031
Add query option to search all dotnet locations
JoeRobich Mar 25, 2025
52a422e
Fix up comment wording
JoeRobich Mar 28, 2025
298418b
Use TryAdd
JoeRobich Mar 28, 2025
4a8eac4
bump the version to 1.8
YuliiaKovalova Mar 31, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
165 changes: 87 additions & 78 deletions src/MSBuildLocator/DotNetSdkLocationHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,8 @@ internal static class DotNetSdkLocationHelper
// in the .NET 5 SDK rely on the .NET 5.0 runtime. Assuming the runtime that shipped with a particular SDK has the same version,
// this ensures that we don't choose an SDK that doesn't work with the runtime of the chosen application. This is not guaranteed
// to always work but should work for now.
if (!allowQueryAllRuntimeVersions &&
(major > Environment.Version.Major ||
if (!allowQueryAllRuntimeVersions &&
(major > Environment.Version.Major ||
(major == Environment.Version.Major && minor > Environment.Version.Minor)))
{
return null;
Expand All @@ -70,46 +70,104 @@ internal static class DotNetSdkLocationHelper
discoveryType: DiscoveryType.DotNetSdk);
}

public static IEnumerable<VisualStudioInstance> GetInstances(string workingDirectory, bool allowQueryAllRuntimes)
{
foreach (var basePath in GetDotNetBasePaths(workingDirectory))
public static IEnumerable<VisualStudioInstance> GetInstances(string workingDirectory, bool allowQueryAllRuntimes, bool allowAllDotnetLocations)
{
string? bestSdkPath;
string[] allAvailableSdks;
try
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inlined GetDotNetBasePaths

{
AddUnmanagedDllResolver();

bestSdkPath = GetSdkFromGlobalSettings(workingDirectory);
allAvailableSdks = GetAllAvailableSDKs(allowAllDotnetLocations).ToArray();
}
finally
{
RemoveUnmanagedDllResolver();
}

Dictionary<Version, VisualStudioInstance?> versionInstanceMap = new();
foreach (var basePath in allAvailableSdks)
{
var dotnetSdk = GetInstance(basePath, allowQueryAllRuntimes);
if (dotnetSdk != null)
{
yield return dotnetSdk;
// We want to return the best SDK first
if (dotnetSdk.VisualStudioRootPath == bestSdkPath)
{
// We will add a null entry to the map to ensure we do not add the same SDK from a different location.
versionInstanceMap[dotnetSdk.Version] = null;
yield return dotnetSdk;
}

// Only add an SDK once, even if it's installed in multiple locations.
versionInstanceMap.TryAdd(dotnetSdk.Version, dotnetSdk);
}
}
}

private static IEnumerable<string> GetDotNetBasePaths(string workingDirectory)
{
try
// We want to return the newest SDKs first. Using OfType will remove the null entry added if we found the best SDK.
var instances = versionInstanceMap.Values.OfType<VisualStudioInstance>().OrderByDescending(i => i.Version);
Comment on lines +108 to +109
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am finding this sort confusing with respect to your comment above about keeping the precedence order. That is just per-version, but you'd prefer the overall results sorted by version?

Can you go into more detail on your use case?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was trying to preserve the behavior from this comment:

// We want to return the newest SDKs first, however, so iterate over the list in reverse order.

But you are right. I cannot guarantee this will maintain the precedence ordering.

We intend to use this from the MSBuildWorkspace buildhost where we are looking for an SDK and we don't really care where it comes from. If they have pinned a version then we would like to find it, otherwise we would take the SDK with the highest version from any install location.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah that makes sense I think. The question I have is whether we should embed the sort here, or push it to the caller's layer. I don't think I have super strong opinions so this is fine.

foreach (var instance in instances)
{
AddUnmanagedDllResolver();
yield return instance;
}

string? bestSDK = GetSdkFromGlobalSettings(workingDirectory);
if (!string.IsNullOrEmpty(bestSDK))
// Returns the list of all available SDKs ordered by ascending version.
static IEnumerable<string> GetAllAvailableSDKs(bool allowAllDotnetLocations)
{
bool foundSdks = false;
string[]? resolvedPaths = null;
foreach (string dotnetPath in s_dotnetPathCandidates.Value)
{
yield return bestSDK;
}
int rc = NativeMethods.hostfxr_get_available_sdks(exe_dir: dotnetPath, result: (key, value) => resolvedPaths = value);

string[] dotnetPaths = GetAllAvailableSDKs();
// We want to return the newest SDKs first, however, so iterate over the list in reverse order.
// If basePath is disqualified because it was later
// than the runtime version, this ensures that RegisterDefaults will return the latest valid
// SDK instead of the earliest installed.
for (int i = dotnetPaths.Length - 1; i >= 0; i--)
{
if (dotnetPaths[i] != bestSDK)
if (rc == 0 && resolvedPaths != null)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previous behavior was that we would not throw if resolvedPaths was an empty array.

{
yield return dotnetPaths[i];
foundSdks = true;

foreach (string path in resolvedPaths)
{
yield return path;
}

if (resolvedPaths.Length > 0 && !allowAllDotnetLocations)
{
break;
}
}
}

// Errors are automatically printed to stderr. We should not continue to try to output anything if we failed.
if (!foundSdks)
{
throw new InvalidOperationException(SdkResolutionExceptionMessage(nameof(NativeMethods.hostfxr_get_available_sdks)));
}
}
finally

// Determines the directory location of the SDK accounting for global.json and multi-level lookup policy.
static string? GetSdkFromGlobalSettings(string workingDirectory)
{
RemoveUnmanagedDllResolver();
string? resolvedSdk = null;
foreach (string dotnetPath in s_dotnetPathCandidates.Value)
{
int rc = NativeMethods.hostfxr_resolve_sdk2(exe_dir: dotnetPath, working_dir: workingDirectory, flags: 0, result: (key, value) =>
{
if (key == NativeMethods.hostfxr_resolve_sdk2_result_key_t.resolved_sdk_dir)
{
resolvedSdk = value;
}
});

if (rc == 0)
{
SetEnvironmentVariableIfEmpty("DOTNET_HOST_PATH", Path.Combine(dotnetPath, ExeName));
return resolvedSdk;
}
}

return string.IsNullOrEmpty(resolvedSdk)
? throw new InvalidOperationException(SdkResolutionExceptionMessage(nameof(NativeMethods.hostfxr_resolve_sdk2)))
: resolvedSdk;
}
}

Expand Down Expand Up @@ -158,7 +216,7 @@ private static IntPtr HostFxrResolver(Assembly assembly, string libraryName)
};

var orderedVersions = fileEnumerable.Where(v => v != null).Select(v => v!).OrderByDescending(f => f).ToList();

foreach (SemanticVersion hostFxrVersion in orderedVersions)
{
string hostFxrAssembly = Path.Combine(hostFxrRoot, hostFxrVersion.OriginalValue, hostFxrLibName);
Expand All @@ -178,35 +236,6 @@ private static IntPtr HostFxrResolver(Assembly assembly, string libraryName)
}

private static string SdkResolutionExceptionMessage(string methodName) => $"Failed to find all versions of .NET Core MSBuild. Call to {methodName}. There may be more details in stderr.";

/// <summary>
/// Determines the directory location of the SDK accounting for
/// global.json and multi-level lookup policy.
/// </summary>
private static string? GetSdkFromGlobalSettings(string workingDirectory)
{
string? resolvedSdk = null;
foreach (string dotnetPath in s_dotnetPathCandidates.Value)
{
int rc = NativeMethods.hostfxr_resolve_sdk2(exe_dir: dotnetPath, working_dir: workingDirectory, flags: 0, result: (key, value) =>
{
if (key == NativeMethods.hostfxr_resolve_sdk2_result_key_t.resolved_sdk_dir)
{
resolvedSdk = value;
}
});

if (rc == 0)
{
SetEnvironmentVariableIfEmpty("DOTNET_HOST_PATH", Path.Combine(dotnetPath, ExeName));
return resolvedSdk;
}
}

return string.IsNullOrEmpty(resolvedSdk)
? throw new InvalidOperationException(SdkResolutionExceptionMessage(nameof(NativeMethods.hostfxr_resolve_sdk2)))
: resolvedSdk;
}

private static IList<string> ResolveDotnetPathCandidates()
{
Expand Down Expand Up @@ -256,7 +285,7 @@ void AddIfValid(string? path)
// 32-bit architecture has (x86) suffix
string envVarName = (IntPtr.Size == 4) ? "DOTNET_ROOT(x86)" : "DOTNET_ROOT";
var dotnetPath = FindDotnetPathFromEnvVariable(envVarName);

return dotnetPath;
}

Expand Down Expand Up @@ -285,26 +314,6 @@ void AddIfValid(string? path)
return dotnetPath;
}

/// <summary>
/// Returns the list of all available SDKs ordered by ascending version.
/// </summary>
private static string[] GetAllAvailableSDKs()
{
string[]? resolvedPaths = null;
foreach (string dotnetPath in s_dotnetPathCandidates.Value)
{
int rc = NativeMethods.hostfxr_get_available_sdks(exe_dir: dotnetPath, result: (key, value) => resolvedPaths = value);

if (rc == 0 && resolvedPaths != null && resolvedPaths.Length > 0)
{
break;
}
}

// Errors are automatically printed to stderr. We should not continue to try to output anything if we failed.
return resolvedPaths ?? throw new InvalidOperationException(SdkResolutionExceptionMessage(nameof(NativeMethods.hostfxr_get_available_sdks)));
}

/// <summary>
/// This native method call determines the actual location of path, including
/// resolving symbolic links.
Expand All @@ -321,7 +330,7 @@ private static string[] GetAllAvailableSDKs()
private static string? FindDotnetPathFromEnvVariable(string environmentVariable)
{
string? dotnetPath = Environment.GetEnvironmentVariable(environmentVariable);

return string.IsNullOrEmpty(dotnetPath) ? null : ValidatePath(dotnetPath);
}

Expand Down
17 changes: 13 additions & 4 deletions src/MSBuildLocator/MSBuildLocator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,14 @@ public static class MSBuildLocator
/// </remarks.
public static bool AllowQueryAllRuntimeVersions { get; set; } = false;

/// <summary>
/// Allow discovery of .NET SDK versions from all discovered dotnet install locations.
/// </summary>
/// <remarks>
/// Defaults to <see langword="false"/>. Set this to <see langword="true"/> only if you do not mind behaving differently than the dotnet muxer.
/// </remarks.
public static bool AllowQueryAllDotnetLocations { get; set; } = false;

/// <summary>
/// Gets a value indicating whether an instance of MSBuild can be registered.
/// </summary>
Expand Down Expand Up @@ -200,7 +208,7 @@ private static void RegisterMSBuildPathsInternally(string[] msbuildSearchPaths)
{
if (string.IsNullOrWhiteSpace(msbuildSearchPaths[i]))
{
nullOrWhiteSpaceExceptions.Add(new ArgumentException($"Value at position {i+1} may not be null or whitespace", nameof(msbuildSearchPaths)));
nullOrWhiteSpaceExceptions.Add(new ArgumentException($"Value at position {i + 1} may not be null or whitespace", nameof(msbuildSearchPaths)));
}
}
if (nullOrWhiteSpaceExceptions.Count > 0)
Expand Down Expand Up @@ -266,7 +274,7 @@ private static void RegisterMSBuildPathsInternally(string[] msbuildSearchPaths)

AppDomain.CurrentDomain.AssemblyResolve += s_registeredHandler;
#else
s_registeredHandler = (_, assemblyName) =>
s_registeredHandler = (_, assemblyName) =>
{
return TryLoadAssembly(assemblyName);
};
Expand Down Expand Up @@ -377,7 +385,8 @@ private static IEnumerable<VisualStudioInstance> GetInstances(VisualStudioInstan
#if NETCOREAPP
// AllowAllRuntimeVersions was added to VisualStudioInstanceQueryOptions for fulfilling Roslyn's needs. One of the properties will be removed in v2.0.
bool allowAllRuntimeVersions = AllowQueryAllRuntimeVersions || options.AllowAllRuntimeVersions;
foreach (var dotnetSdk in DotNetSdkLocationHelper.GetInstances(options.WorkingDirectory, allowAllRuntimeVersions))
bool allowAllDotnetLocations = AllowQueryAllDotnetLocations || options.AllowAllDotnetLocations;
foreach (var dotnetSdk in DotNetSdkLocationHelper.GetInstances(options.WorkingDirectory, allowAllRuntimeVersions, allowAllDotnetLocations))
yield return dotnetSdk;
#endif
}
Expand All @@ -404,7 +413,7 @@ private static VisualStudioInstance GetDevConsoleInstance()
Version.TryParse(versionString, out version);
}

if(version != null)
if (version != null)
{
return new VisualStudioInstance("DEVCONSOLE", path, version, DiscoveryType.DeveloperConsole);
}
Expand Down
8 changes: 8 additions & 0 deletions src/MSBuildLocator/VisualStudioInstanceQueryOptions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,14 @@ public class VisualStudioInstanceQueryOptions
/// Defaults to <see langword="false"/>. Set this to <see langword="true"/> only if your application has special logic to handle loading an incompatible SDK, such as launching a new process with the target SDK's runtime.
/// </remarks.
public bool AllowAllRuntimeVersions { get; set; } = false;

/// <summary>
/// Allow discovery of .NET SDK versions from all discovered dotnet install locations.
/// </summary>
/// <remarks>
/// Defaults to <see langword="false"/>. Set this to <see langword="true"/> only if you do not mind behaving differently than a command-line dotnet invocation.
/// </remarks.
public bool AllowAllDotnetLocations { get; set; } = false;
#endif

/// <summary>
Expand Down
2 changes: 1 addition & 1 deletion version.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"version": "1.7",
"version": "1.8",
"assemblyVersion": "1.0.0.0",
"publicReleaseRefSpec": [
"^refs/heads/release/.*"
Expand Down
Loading