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

Update condition for max version limitation & add package tags #265

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion .github/workflows/pull-request.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ on:
branches: [ master ]
pull_request:
branches:
- master
- main
- 'release/**'

jobs:
Expand Down
11 changes: 6 additions & 5 deletions src/MSBuildLocator/DotNetSdkLocationHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ internal static class DotNetSdkLocationHelper
private static readonly string ExeName = IsWindows ? "dotnet.exe" : "dotnet";
private static readonly Lazy<IList<string>> s_dotnetPathCandidates = new(() => ResolveDotnetPathCandidates());

public static VisualStudioInstance? GetInstance(string dotNetSdkPath)
public static VisualStudioInstance? GetInstance(string dotNetSdkPath, bool allowQueryAllRuntimeVersions)
{
if (string.IsNullOrWhiteSpace(dotNetSdkPath) || !File.Exists(Path.Combine(dotNetSdkPath, "Microsoft.Build.dll")))
{
Expand Down Expand Up @@ -56,8 +56,9 @@ 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 (major > Environment.Version.Major ||
(major == Environment.Version.Major && minor > Environment.Version.Minor))
if (!allowQueryAllRuntimeVersions &&
(major > Environment.Version.Major ||
(major == Environment.Version.Major && minor > Environment.Version.Minor)))
{
return null;
}
Expand All @@ -69,11 +70,11 @@ internal static class DotNetSdkLocationHelper
discoveryType: DiscoveryType.DotNetSdk);
}

public static IEnumerable<VisualStudioInstance> GetInstances(string workingDirectory)
public static IEnumerable<VisualStudioInstance> GetInstances(string workingDirectory, bool allowQueryAllRuntimes)
{
foreach (var basePath in GetDotNetBasePaths(workingDirectory))
{
var dotnetSdk = GetInstance(basePath);
var dotnetSdk = GetInstance(basePath, allowQueryAllRuntimes);
if (dotnetSdk != null)
{
yield return dotnetSdk;
Expand Down
10 changes: 9 additions & 1 deletion src/MSBuildLocator/MSBuildLocator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,14 @@ public static class MSBuildLocator
/// </summary>
public static bool IsRegistered => s_registeredHandler != null;

/// <summary>
/// Allow discovery of .NET SDK versions that are unlikely to be successfully loaded in the current process.
/// </summary>
/// <remarks>
/// 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 static bool AllowQueryAllRuntimeVersions { get; set; } = false;
Comment on lines +46 to +52
Copy link
Member

Choose a reason for hiding this comment

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

Sorry I only noticed this PR now, but @YuliiaKovalova should a flag like this have been added to the VisualStudioInstanceQueryOptions rather than being a global static? Otherwise it's a bit unclear what a library like Roslyn's MSBuildWorkspace should do here since technically we're just a library and won't ever be registering MSBuild in the calling applications' process at this point -- do we set this and set it back to the old value, and pray there wasn't another thread doing something?

Copy link
Member

Choose a reason for hiding this comment

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

Oh, I completely missed that move! I agree with @jasonmalinowski that this made more sense on VisualStudioInstanceQueryOptions.


/// <summary>
/// Gets a value indicating whether an instance of MSBuild can be registered.
/// </summary>
Expand Down Expand Up @@ -353,7 +361,7 @@ private static IEnumerable<VisualStudioInstance> GetInstances(VisualStudioInstan
#endif

#if NETCOREAPP
foreach (var dotnetSdk in DotNetSdkLocationHelper.GetInstances(options.WorkingDirectory))
foreach (var dotnetSdk in DotNetSdkLocationHelper.GetInstances(options.WorkingDirectory, AllowQueryAllRuntimeVersions))
yield return dotnetSdk;
#endif
}
Expand Down
3 changes: 2 additions & 1 deletion src/MSBuildLocator/Microsoft.Build.Locator.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@

<Title>MSBuild Locator</Title>
<Description>Package that assists in locating and using a copy of MSBuild installed as part of Visual Studio 2017 or higher or .NET Core SDK 2.1 or higher.</Description>

<PackageTags>msbuildlocator;locator;buildlocator</PackageTags>

<EnablePackageValidation>true</EnablePackageValidation>
<PackageValidationBaselineVersion>1.6.1</PackageValidationBaselineVersion>
</PropertyGroup>
Expand Down
2 changes: 1 addition & 1 deletion version.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"version": "1.6",
"version": "1.7",
"assemblyVersion": "1.0.0.0",
"publicReleaseRefSpec": [
"^refs/heads/release/.*"
Expand Down
Loading