-
Notifications
You must be signed in to change notification settings - Fork 418
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
Check for MSBuild version is higher than required by the .NET SDK #1875
Merged
Merged
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
d969cdc
Check for MSBuild versiosn higher than required by the .NET SDK
JoeRobich ecbd81d
Use VisualStudioSetup discovery type when logging VS version warnings
JoeRobich d20d4cd
Merge branch 'master' into check-minimum-msbuid
filipw 0bf47f2
Fix replace all mistake
JoeRobich 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 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 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 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,13 +1,20 @@ | ||
using System.IO; | ||
using System; | ||
using System.IO; | ||
using Microsoft.Extensions.Logging; | ||
using OmniSharp.Services; | ||
|
||
namespace OmniSharp.MSBuild.Discovery | ||
{ | ||
internal static class Extensions | ||
{ | ||
public static void RegisterDefaultInstance(this IMSBuildLocator msbuildLocator, ILogger logger) | ||
private static readonly Version s_minimumMSBuildVersion = new Version(16, 3); | ||
|
||
public static void RegisterDefaultInstance(this IMSBuildLocator msbuildLocator, ILogger logger, DotNetInfo dotNetInfo = null) | ||
{ | ||
var bestInstanceFound = GetBestInstance(msbuildLocator, logger, out var invalidVSFound, out var vsWithoutSdkResolver); | ||
var minimumMSBuildVersion = GetSdkMinimumMSBuildVersion(dotNetInfo, logger); | ||
logger.LogDebug($".NET SDK requires MSBuild instances version {minimumMSBuildVersion} or higher"); | ||
|
||
var bestInstanceFound = GetBestInstance(msbuildLocator, minimumMSBuildVersion, logger, out var invalidVSFound, out var vsWithoutSdkResolver); | ||
|
||
if (bestInstanceFound != null) | ||
{ | ||
|
@@ -16,7 +23,7 @@ public static void RegisterDefaultInstance(this IMSBuildLocator msbuildLocator, | |
if (invalidVSFound && bestInstanceFound.DiscoveryType == DiscoveryType.StandAlone) | ||
{ | ||
logger.LogWarning( | ||
@"It looks like you have Visual Studio lower than VS 2019 16.3 installed. | ||
$@"It looks like you have Visual Studio lower than VS 2019 {s_minimumMSBuildVersion} installed. | ||
Try updating Visual Studio to the most recent release to enable better MSBuild support." | ||
); | ||
} | ||
|
@@ -29,6 +36,38 @@ Try updating Visual Studio 2019 installation with .NET Core SDK to enable better | |
); | ||
} | ||
|
||
if (bestInstanceFound.Version < minimumMSBuildVersion) | ||
{ | ||
if (bestInstanceFound.DiscoveryType == DiscoveryType.StandAlone) | ||
{ | ||
logger.LogWarning( | ||
$@"It looks like the included version of MSBuild is lower than {minimumMSBuildVersion} which is the minimum supported by the configured .NET Core Sdk. | ||
Try installing MSBuild version {minimumMSBuildVersion} or higher to enable better .NET Core Sdk support." | ||
); | ||
} | ||
else if (bestInstanceFound.DiscoveryType == DiscoveryType.Mono) | ||
{ | ||
logger.LogWarning( | ||
$@"It looks like you have Mono installed which contains a MSBuild lower than {minimumMSBuildVersion} which is the minimum supported by the configured .NET Core Sdk. | ||
Try updating Mono to the latest stable or preview version to enable better .NET Core Sdk support." | ||
); | ||
} | ||
else if (bestInstanceFound.DiscoveryType == DiscoveryType.VisualStudioSetup) | ||
{ | ||
logger.LogWarning( | ||
$@"It looks like you have Visual Studio 2019 installed which contains a MSBuild lower than {minimumMSBuildVersion} which is the minimum supported by the configured .NET Core Sdk. | ||
Try updating Visual Studio to version {minimumMSBuildVersion} or higher to enable better .NET Core Sdk support." | ||
); | ||
} | ||
else if (bestInstanceFound.DiscoveryType == DiscoveryType.UserOverride) | ||
{ | ||
logger.LogWarning( | ||
$@"It looks like you have overridden the version of MSBuild with a version lower than {minimumMSBuildVersion} which is the minimum supported by the configured .NET Core Sdk. | ||
Try updating your MSBuild to version {minimumMSBuildVersion} or higher to enable better .NET Core Sdk support." | ||
); | ||
} | ||
} | ||
|
||
msbuildLocator.RegisterInstance(bestInstanceFound); | ||
} | ||
else | ||
|
@@ -52,15 +91,15 @@ public static bool HasDotNetSdksResolvers(this MSBuildInstance instance) | |
} | ||
|
||
/// <summary> | ||
/// Checks if it is MSBuild from Visual Studio 2017 RTM that cannot be used. | ||
/// Checks if the discovered Visual Studio instance includes a version of MSBuild lower than our minimum supported version. | ||
/// </summary> | ||
public static bool IsInvalidVisualStudio(this MSBuildInstance instance) | ||
=> (instance.Version.Major < 16 || | ||
(instance.Version.Major == 16 && instance.Version.Minor < 3)) | ||
=> (instance.Version.Major < s_minimumMSBuildVersion.Major || | ||
(instance.Version.Major == s_minimumMSBuildVersion.Major && instance.Version.Minor < s_minimumMSBuildVersion.Minor)) | ||
&& (instance.DiscoveryType == DiscoveryType.DeveloperConsole | ||
|| instance.DiscoveryType == DiscoveryType.VisualStudioSetup); | ||
|
||
public static MSBuildInstance GetBestInstance(this IMSBuildLocator msbuildLocator, ILogger logger, out bool invalidVSFound, out bool vsWithoutSdkResolver) | ||
public static MSBuildInstance GetBestInstance(this IMSBuildLocator msbuildLocator, Version minimumMSBuildVersion, ILogger logger, out bool invalidVSFound, out bool vsWithoutSdkResolver) | ||
{ | ||
invalidVSFound = false; | ||
vsWithoutSdkResolver = false; | ||
|
@@ -69,7 +108,7 @@ public static MSBuildInstance GetBestInstance(this IMSBuildLocator msbuildLocato | |
|
||
foreach (var instance in msbuildLocator.GetInstances()) | ||
{ | ||
var score = GetInstanceFeatureScore(instance); | ||
var score = GetInstanceFeatureScore(instance, minimumMSBuildVersion); | ||
|
||
logger.LogDebug($"MSBuild instance {instance.Name} {instance.Version} scored at {score}"); | ||
|
||
|
@@ -88,7 +127,7 @@ public static MSBuildInstance GetBestInstance(this IMSBuildLocator msbuildLocato | |
return bestMatchInstance; | ||
} | ||
|
||
private static int GetInstanceFeatureScore(MSBuildInstance i) | ||
private static int GetInstanceFeatureScore(MSBuildInstance i, Version minimumMSBuildVersion) | ||
{ | ||
var score = 0; | ||
|
||
|
@@ -112,5 +151,28 @@ private static int GetInstanceFeatureScore(MSBuildInstance i) | |
|
||
return score; | ||
} | ||
|
||
public static Version GetSdkMinimumMSBuildVersion(DotNetInfo dotNetInfo, ILogger logger) | ||
{ | ||
if (dotNetInfo is null | ||
|| string.IsNullOrWhiteSpace(dotNetInfo.SdksPath) | ||
|| dotNetInfo.SdkVersion is null) | ||
{ | ||
return s_minimumMSBuildVersion; | ||
} | ||
|
||
var version = dotNetInfo.SdkVersion; | ||
var sdksPath = dotNetInfo.SdksPath; | ||
var minimumVersionPath = Path.Combine(sdksPath, version.ToNormalizedString(), "minimumMSBuildVersion"); | ||
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. this is really there?! that's new to me! 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. I was happy to find it =) |
||
|
||
if (!File.Exists(minimumVersionPath)) | ||
{ | ||
logger.LogDebug($"Unable to locate minimumMSBuildVersion file at '{minimumVersionPath}'"); | ||
return s_minimumMSBuildVersion; | ||
} | ||
|
||
var minimumVersionText = File.ReadAllText(minimumVersionPath); | ||
return Version.Parse(minimumVersionText); | ||
} | ||
} | ||
} |
This file contains 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 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 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 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 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 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 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
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.
I am not sure if it's possible to have a different global.json with different SDK versions per project - but I think it is not, so it should be fine to run this against the startup directory