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

Check for MSBuild version is higher than required by the .NET SDK #1875

Merged
merged 4 commits into from
Aug 8, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
29 changes: 26 additions & 3 deletions src/OmniSharp.Abstractions/Services/DotNetInfo.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System;
using System.Collections.Generic;
using System.Text.RegularExpressions;
using NuGet.Versioning;

namespace OmniSharp.Services
Expand All @@ -16,13 +17,15 @@ public class DotNetInfo
public string OSPlatform { get; }
public string RID { get; }
public string BasePath { get; }
public SemanticVersion SdkVersion { get; }
public string SdksPath { get; }

private DotNetInfo()
{
IsEmpty = true;
}

private DotNetInfo(string version, string osName, string osVersion, string osPlatform, string rid, string basePath)
private DotNetInfo(string version, string osName, string osVersion, string osPlatform, string rid, string basePath, string sdkVersion, string sdksPath)
{
IsEmpty = false;

Expand All @@ -35,6 +38,11 @@ private DotNetInfo(string version, string osName, string osVersion, string osPla
OSPlatform = osPlatform;
RID = rid;
BasePath = basePath;

SdkVersion = SemanticVersion.TryParse(sdkVersion, out var sdkValue)
? sdkValue
: null;
SdksPath = sdksPath;
}

public static DotNetInfo Parse(List<string> lines)
Expand All @@ -50,6 +58,8 @@ public static DotNetInfo Parse(List<string> lines)
var osPlatform = string.Empty;
var rid = string.Empty;
var basePath = string.Empty;
var sdkVersion = string.Empty;
var sdksPath = string.Empty;

foreach (var line in lines)
{
Expand Down Expand Up @@ -84,19 +94,32 @@ public static DotNetInfo Parse(List<string> lines)
basePath = value;
}
}
else if (string.IsNullOrEmpty(sdkVersion))
{
var getSdkVersionAndPath = new Regex(@"^\s*(\d+\.\d+\.\d+)\s\[(.*)\]\s*$", RegexOptions.Multiline);
var match = getSdkVersionAndPath.Match(line);

if (match.Success)
{
sdkVersion = match.Groups[1].Value;
sdksPath = match.Groups[2].Value;
}
}
}

if (string.IsNullOrWhiteSpace(version) &&
string.IsNullOrWhiteSpace(osName) &&
string.IsNullOrWhiteSpace(osVersion) &&
string.IsNullOrWhiteSpace(osPlatform) &&
string.IsNullOrWhiteSpace(rid) &&
string.IsNullOrWhiteSpace(basePath))
string.IsNullOrWhiteSpace(basePath) &&
string.IsNullOrWhiteSpace(sdkVersion) &&
string.IsNullOrWhiteSpace(sdksPath))
{
return Empty;
}

return new DotNetInfo(version, osName, osVersion, osPlatform, rid, basePath);
return new DotNetInfo(version, osName, osVersion, osPlatform, rid, basePath, sdkVersion, sdksPath);
}
}
}
5 changes: 3 additions & 2 deletions src/OmniSharp.Host/CompositionHostBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public CompositionHostBuilder(
_exportDescriptorProviders = exportDescriptorProviders ?? Array.Empty<ExportDescriptorProvider>();
}

public CompositionHost Build()
public CompositionHost Build(string workingDirectory)
{
var options = _serviceProvider.GetRequiredService<IOptionsMonitor<OmniSharpOptions>>();
var memoryCache = _serviceProvider.GetRequiredService<IMemoryCache>();
Expand All @@ -64,7 +64,8 @@ public CompositionHost Build()
// This is for tests, where the MSBuild instance may be registered early.
if (msbuildLocator.RegisteredInstance == null)
{
msbuildLocator.RegisterDefaultInstance(logger);
var dotNetInfo = dotNetCliService.GetInfo(workingDirectory);
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 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

msbuildLocator.RegisterDefaultInstance(logger, dotNetInfo);
}

config = config
Expand Down
73 changes: 64 additions & 9 deletions src/OmniSharp.Host/MSBuild/Discovery/Extensions.cs
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)
{
Expand All @@ -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."
);
}
Expand All @@ -29,6 +36,31 @@ Try updating Visual Studio 2019 installation with .NET Core SDK to enable better
);
}

if (bestInstanceFound.Version < minimumMSBuildVersion)
{
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.StandAlone)
{
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
Expand All @@ -55,12 +87,12 @@ public static bool HasDotNetSdksResolvers(this MSBuildInstance instance)
/// Checks if it is MSBuild from Visual Studio 2017 RTM that cannot be used.
JoeRobich marked this conversation as resolved.
Show resolved Hide resolved
/// </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;
Expand All @@ -69,7 +101,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}");

Expand All @@ -88,7 +120,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;

Expand All @@ -112,5 +144,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");
Copy link
Member

Choose a reason for hiding this comment

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

this is really there?! that's new to me!

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 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);
}
}
}
2 changes: 1 addition & 1 deletion src/OmniSharp.Http/Startup.cs
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ public IServiceProvider ConfigureServices(IServiceCollection services)
_compositionHost = new CompositionHostBuilder(serviceProvider)
.WithOmniSharpAssemblies()
.WithAssemblies(assemblyLoader.LoadByAssemblyNameOrPath(logger, plugins).ToArray())
.Build();
.Build(_environment.TargetDirectory);

return serviceProvider;
}
Expand Down
8 changes: 4 additions & 4 deletions src/OmniSharp.LanguageServerProtocol/LanguageServerHost.cs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ public LanguageServerHost(
.WithOutput(output)
.ConfigureLogging(x => x
.AddLanguageProtocolLogging()
// .SetMinimumLevel(application.LogLevel)
// .SetMinimumLevel(application.LogLevel)
)
.OnInitialize(Initialize)
.WithServices(ConfigureServices);
Expand Down Expand Up @@ -202,7 +202,7 @@ IServiceCollection services
.WithAssemblies(typeof(LanguageServerHost).Assembly)
.WithAssemblies(assemblyLoader.LoadByAssemblyNameOrPath(logger, plugins.AssemblyNames).ToArray());

return (serviceProvider, compositionHostBuilder.Build());
return (serviceProvider, compositionHostBuilder.Build(environment.TargetDirectory));
}

internal static RequestHandlers ConfigureCompositionHost(ILanguageServer server,
Expand Down Expand Up @@ -241,7 +241,7 @@ internal static RequestHandlers ConfigureCompositionHost(ILanguageServer server,

logger.LogTrace(
"Configured Document Selectors {@DocumentSelectors}",
documentSelectors.Select(x => new {x.language, x.selector})
documentSelectors.Select(x => new { x.language, x.selector })
);

var omnisharpRequestHandlers =
Expand Down Expand Up @@ -365,7 +365,7 @@ private static IDictionary<string, Lazy<LanguageProtocolInteropHandler>> Initial

IDictionary<string, Lazy<LanguageProtocolInteropHandler>> endpointHandlers = null;
var updateBufferEndpointHandler = new Lazy<LanguageProtocolInteropHandler<UpdateBufferRequest, object>>(
() => (LanguageProtocolInteropHandler<UpdateBufferRequest, object>) endpointHandlers[
() => (LanguageProtocolInteropHandler<UpdateBufferRequest, object>)endpointHandlers[
OmniSharpEndpoints.UpdateBuffer].Value);
var languagePredicateHandler = new LanguagePredicateHandler(projectSystems);
var projectSystemPredicateHandler = new StaticLanguagePredicateHandler("Projects");
Expand Down
2 changes: 1 addition & 1 deletion src/OmniSharp.Stdio/Host.cs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public Host(

_logger.LogInformation($"Starting OmniSharp on {Platform.Current}");

_compositionHost = compositionHostBuilder.Build();
_compositionHost = compositionHostBuilder.Build(_environment.TargetDirectory);
_cachedStringBuilder = new CachedStringBuilder();

var handlers = Initialize();
Expand Down
2 changes: 1 addition & 1 deletion tests/OmniSharp.Http.Tests/EndpointMiddlewareFacts.cs
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ private PlugInHost CreatePlugInHost(params Assembly[] assemblies)
var serviceProvider = TestServiceProvider.Create(this.TestOutput, new OmniSharpEnvironment());
var compositionHost = new CompositionHostBuilder(serviceProvider)
.WithAssemblies(assemblies)
.Build();
.Build(workingDirectory: null);

return new PlugInHost(serviceProvider, compositionHost);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ public AbstractMSBuildTestFixture(ITestOutputHelper output)

// Some tests require MSBuild to be discovered early
// to ensure that the Microsoft.Build.* assemblies can be located
_msbuildLocator.RegisterDefaultInstance(this.LoggerFactory.CreateLogger("MSBuildTests"));
_msbuildLocator.RegisterDefaultInstance(this.LoggerFactory.CreateLogger("MSBuildTests"), dotNetInfo: null);
}

public void Dispose()
Expand Down
14 changes: 7 additions & 7 deletions tests/OmniSharp.MSBuild.Tests/MSBuildSelectionTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public void RegisterDefaultInstanceFindsTheBestInstanceAvailable()
var logger = LoggerFactory.CreateLogger(nameof(RegisterDefaultInstanceFindsTheBestInstanceAvailable));

// test
msbuildLocator.RegisterDefaultInstance(logger);
msbuildLocator.RegisterDefaultInstance(logger, dotNetInfo: null);

Assert.NotNull(msbuildLocator.RegisteredInstance);
Assert.Same(msBuildInstances[1], msbuildLocator.RegisteredInstance);
Expand Down Expand Up @@ -72,7 +72,7 @@ public void RegisterDefaultInstanceFindsTheBestInstanceAvailableEvenWithOtherVal
);

// test
msbuildLocator.RegisterDefaultInstance(logger);
msbuildLocator.RegisterDefaultInstance(logger, dotNetInfo: null);

Assert.NotNull(msbuildLocator.RegisteredInstance);
Assert.Same(msBuildInstances[2], msbuildLocator.RegisteredInstance);
Expand Down Expand Up @@ -112,7 +112,7 @@ public void RegisterDefaultInstanceFindsTheNewestInstanceAvailableEvenWithOtherV
);

// test
msbuildLocator.RegisterDefaultInstance(logger);
msbuildLocator.RegisterDefaultInstance(logger, dotNetInfo: null);

Assert.NotNull(msbuildLocator.RegisteredInstance);
Assert.Same(msBuildInstances[2], msbuildLocator.RegisteredInstance);
Expand Down Expand Up @@ -159,7 +159,7 @@ public void RegisterDefaultInstanceFindsUserOverrideAvailableEvenWithOtherValidI
);

// test
msbuildLocator.RegisterDefaultInstance(logger);
msbuildLocator.RegisterDefaultInstance(logger, dotNetInfo: null);

Assert.NotNull(msbuildLocator.RegisteredInstance);
Assert.Same(msBuildInstances[4], msbuildLocator.RegisteredInstance);
Expand All @@ -186,7 +186,7 @@ public void RegisterDefaultInstancePrefersSupportedVSLowerVersionInstanceOverSta
var logger = LoggerFactory.CreateLogger(nameof(RegisterDefaultInstancePrefersSupportedVSLowerVersionInstanceOverStandAlone));

// test
msbuildLocator.RegisterDefaultInstance(logger);
msbuildLocator.RegisterDefaultInstance(logger, dotNetInfo: null);

Assert.NotNull(msbuildLocator.RegisteredInstance);
Assert.Same(msBuildInstances[0], msbuildLocator.RegisteredInstance);
Expand Down Expand Up @@ -215,7 +215,7 @@ public void RegisterDefaultInstancePrefersStandAloneOverSupportedVSInstanceWitho
var logger = LoggerFactory.CreateLogger(nameof(RegisterDefaultInstancePrefersStandAloneOverSupportedVSInstanceWithoutDotnetCore));

// test
msbuildLocator.RegisterDefaultInstance(logger);
msbuildLocator.RegisterDefaultInstance(logger, dotNetInfo: null);

Assert.NotNull(msbuildLocator.RegisteredInstance);
Assert.Same(msBuildInstances[1], msbuildLocator.RegisteredInstance);
Expand Down Expand Up @@ -246,7 +246,7 @@ public void StandAloneIsPreferredOverUnsupportedVS(string vsVersion)
var logger = LoggerFactory.CreateLogger(nameof(StandAloneIsPreferredOverUnsupportedVS));

// test
msbuildLocator.RegisterDefaultInstance(logger);
msbuildLocator.RegisterDefaultInstance(logger, dotNetInfo: null);

Assert.NotNull(msbuildLocator.RegisteredInstance);
Assert.Same(msBuildInstances[1], msbuildLocator.RegisteredInstance);
Expand Down
2 changes: 1 addition & 1 deletion tests/TestUtility/OmniSharpTestHost.cs
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ public static OmniSharpTestHost Create(
[CallerMemberName] string callerName = "")
{
var compositionHost = new CompositionHostBuilder(serviceProvider, s_lazyAssemblies.Value, additionalExports)
.Build();
.Build(workingDirectory: null);

WorkspaceInitializer.Initialize(serviceProvider, compositionHost);

Expand Down