diff --git a/src/OmniSharp.Abstractions/MSBuild/Discovery/DiscoveryType.cs b/src/OmniSharp.Abstractions/MSBuild/Discovery/DiscoveryType.cs index ff10a866c6..fc2ff8d211 100644 --- a/src/OmniSharp.Abstractions/MSBuild/Discovery/DiscoveryType.cs +++ b/src/OmniSharp.Abstractions/MSBuild/Discovery/DiscoveryType.cs @@ -5,6 +5,7 @@ public enum DiscoveryType StandAlone = 0, DeveloperConsole = 1, VisualStudioSetup = 2, - Mono = 3 + Mono = 3, + UserOverride = 4 } } diff --git a/src/OmniSharp.Host/CompositionHostBuilder.cs b/src/OmniSharp.Host/CompositionHostBuilder.cs index 9b86c5c28a..f794bf6d34 100644 --- a/src/OmniSharp.Host/CompositionHostBuilder.cs +++ b/src/OmniSharp.Host/CompositionHostBuilder.cs @@ -135,7 +135,9 @@ public static IServiceProvider CreateDefaultServiceProvider( services.AddSingleton(sp => MSBuildLocator.CreateDefault( loggerFactory: sp.GetService(), - assemblyLoader: sp.GetService())); + assemblyLoader: sp.GetService(), + msbuildConfiguration: configuration.GetSection("msbuild"))); + // Setup the options from configuration services.Configure(configuration); diff --git a/src/OmniSharp.Host/MSBuild/Discovery/Extensions.cs b/src/OmniSharp.Host/MSBuild/Discovery/Extensions.cs index 9325d73039..93d121b678 100644 --- a/src/OmniSharp.Host/MSBuild/Discovery/Extensions.cs +++ b/src/OmniSharp.Host/MSBuild/Discovery/Extensions.cs @@ -79,6 +79,10 @@ private static int GetInstanceFeatureScore(MSBuildInstance i) { var score = 0; + // user override gets highest priority + if (i.DiscoveryType == DiscoveryType.UserOverride) + return int.MaxValue; + if (i.HasDotNetSdksResolvers()) score++; @@ -90,6 +94,12 @@ private static int GetInstanceFeatureScore(MSBuildInstance i) if (i.DiscoveryType == DiscoveryType.StandAlone) score--; + // VS 2019 should be preferred + if (i.Version.Major >= 16) + score++; + else + score--; + return score; } } diff --git a/src/OmniSharp.Host/MSBuild/Discovery/MSBuildLocator.cs b/src/OmniSharp.Host/MSBuild/Discovery/MSBuildLocator.cs index 339206c6c8..0abbf7a4a9 100644 --- a/src/OmniSharp.Host/MSBuild/Discovery/MSBuildLocator.cs +++ b/src/OmniSharp.Host/MSBuild/Discovery/MSBuildLocator.cs @@ -3,6 +3,7 @@ using System.IO; using System.Reflection; using System.Text; +using Microsoft.Extensions.Configuration; using Microsoft.Extensions.Logging; using OmniSharp.MSBuild.Discovery.Providers; using OmniSharp.Services; @@ -21,9 +22,8 @@ internal class MSBuildLocator : DisposableObject, IMSBuildLocator private readonly ILogger _logger; private readonly IAssemblyLoader _assemblyLoader; private readonly ImmutableArray _providers; - private MSBuildInstance _registeredInstance; - public MSBuildInstance RegisteredInstance => _registeredInstance; + public MSBuildInstance RegisteredInstance { get; private set; } private MSBuildLocator(ILoggerFactory loggerFactory, IAssemblyLoader assemblyLoader, ImmutableArray providers) { @@ -34,20 +34,21 @@ private MSBuildLocator(ILoggerFactory loggerFactory, IAssemblyLoader assemblyLoa protected override void DisposeCore(bool disposing) { - if (_registeredInstance != null) + if (RegisteredInstance != null) { AppDomain.CurrentDomain.AssemblyResolve -= Resolve; - _registeredInstance = null; + RegisteredInstance = null; } } - public static MSBuildLocator CreateDefault(ILoggerFactory loggerFactory, IAssemblyLoader assemblyLoader) + public static MSBuildLocator CreateDefault(ILoggerFactory loggerFactory, IAssemblyLoader assemblyLoader, IConfiguration msbuildConfiguration) => new MSBuildLocator(loggerFactory, assemblyLoader, ImmutableArray.Create( new DevConsoleInstanceProvider(loggerFactory), new VisualStudioInstanceProvider(loggerFactory), new MonoInstanceProvider(loggerFactory), - new StandAloneInstanceProvider(loggerFactory, allowMonoPaths: true))); + new StandAloneInstanceProvider(loggerFactory, allowMonoPaths: true), + new UserOverrideInstanceProvider(loggerFactory, msbuildConfiguration))); public static MSBuildLocator CreateStandAlone(ILoggerFactory loggerFactory, IAssemblyLoader assemblyLoader, bool allowMonoPaths) => new MSBuildLocator(loggerFactory, assemblyLoader, @@ -56,12 +57,12 @@ public static MSBuildLocator CreateStandAlone(ILoggerFactory loggerFactory, IAss public void RegisterInstance(MSBuildInstance instance) { - if (_registeredInstance != null) + if (RegisteredInstance != null) { throw new InvalidOperationException("An MSBuild instance is already registered."); } - _registeredInstance = instance ?? throw new ArgumentNullException(nameof(instance)); + RegisteredInstance = instance ?? throw new ArgumentNullException(nameof(instance)); foreach (var assemblyName in s_msbuildAssemblies) { @@ -120,7 +121,7 @@ private Assembly Resolve(object sender, ResolveEventArgs e) private Assembly LoadAssemblyByNameOnly(string assemblyName) { - var assemblyPath = Path.Combine(_registeredInstance.MSBuildPath, assemblyName + ".dll"); + var assemblyPath = Path.Combine(RegisteredInstance.MSBuildPath, assemblyName + ".dll"); var result = File.Exists(assemblyPath) ? _assemblyLoader.LoadFrom(assemblyPath) : null; @@ -135,7 +136,7 @@ private Assembly LoadAssemblyByNameOnly(string assemblyName) private Assembly LoadAssemblyByFullName(AssemblyName assemblyName) { - var assemblyPath = Path.Combine(_registeredInstance.MSBuildPath, assemblyName.Name + ".dll"); + var assemblyPath = Path.Combine(RegisteredInstance.MSBuildPath, assemblyName.Name + ".dll"); if (!File.Exists(assemblyPath)) { _logger.LogDebug($"FAILURE: Could not locate '{assemblyPath}'."); diff --git a/src/OmniSharp.Host/MSBuild/Discovery/Providers/MSBuildOverrideOptions.cs b/src/OmniSharp.Host/MSBuild/Discovery/Providers/MSBuildOverrideOptions.cs new file mode 100644 index 0000000000..18a68f5654 --- /dev/null +++ b/src/OmniSharp.Host/MSBuild/Discovery/Providers/MSBuildOverrideOptions.cs @@ -0,0 +1,13 @@ +using System.Collections.Generic; + +namespace OmniSharp.MSBuild.Discovery.Providers +{ + internal class MSBuildOverrideOptions + { + public Dictionary PropertyOverrides { get; set; } + + public string MSBuildPath { get; set; } + + public string Name { get; set; } + } +} diff --git a/src/OmniSharp.Host/MSBuild/Discovery/Providers/MonoInstanceProvider.cs b/src/OmniSharp.Host/MSBuild/Discovery/Providers/MonoInstanceProvider.cs index fcfa21e93d..3e57d91736 100644 --- a/src/OmniSharp.Host/MSBuild/Discovery/Providers/MonoInstanceProvider.cs +++ b/src/OmniSharp.Host/MSBuild/Discovery/Providers/MonoInstanceProvider.cs @@ -69,9 +69,9 @@ public override ImmutableArray GetInstances() return NoInstances; } - if (monoVersion < new Version("5.2.0")) + if (monoVersion < new Version("5.18.1")) { - Logger.LogDebug($"Found Mono MSBuild but it could not be used because it is version {monoVersion} and in needs to be >= 5.2.0"); + Logger.LogDebug($"Found Mono MSBuild but it could not be used because it is version {monoVersion} and in needs to be >= 5.18.1"); return NoInstances; } @@ -112,7 +112,7 @@ public override ImmutableArray GetInstances() new MSBuildInstance( nameof(DiscoveryType.Mono), toolsPath, - new Version(15, 0), + new Version(16, 0), DiscoveryType.Mono, propertyOverrides.ToImmutable())); } diff --git a/src/OmniSharp.Host/MSBuild/Discovery/Providers/StandAloneInstanceProvider.cs b/src/OmniSharp.Host/MSBuild/Discovery/Providers/StandAloneInstanceProvider.cs index e7bf372fdb..e0e7b44c82 100644 --- a/src/OmniSharp.Host/MSBuild/Discovery/Providers/StandAloneInstanceProvider.cs +++ b/src/OmniSharp.Host/MSBuild/Discovery/Providers/StandAloneInstanceProvider.cs @@ -65,7 +65,7 @@ public override ImmutableArray GetInstances() new MSBuildInstance( nameof(DiscoveryType.StandAlone), toolsPath, - new Version(15, 0), + new Version(16, 0), // we now ship with embedded MsBuild 16.x DiscoveryType.StandAlone, propertyOverrides.ToImmutable(), setMSBuildExePathVariable: true)); diff --git a/src/OmniSharp.Host/MSBuild/Discovery/Providers/UserOverrideInstanceProvider.cs b/src/OmniSharp.Host/MSBuild/Discovery/Providers/UserOverrideInstanceProvider.cs new file mode 100644 index 0000000000..30374e9a1a --- /dev/null +++ b/src/OmniSharp.Host/MSBuild/Discovery/Providers/UserOverrideInstanceProvider.cs @@ -0,0 +1,37 @@ +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.Logging; +using System; +using System.Collections.Immutable; + +namespace OmniSharp.MSBuild.Discovery.Providers +{ + internal class UserOverrideInstanceProvider : MSBuildInstanceProvider + { + private readonly MSBuildOverrideOptions _options; + + public UserOverrideInstanceProvider(ILoggerFactory loggerFactory, IConfiguration configuration) + : base(loggerFactory) + { + _options = configuration.GetSection("msbuildoverride").Get(); + } + + public override ImmutableArray GetInstances() + { + if (_options == null || string.IsNullOrWhiteSpace(_options.MSBuildPath)) + { + return ImmutableArray.Empty; + } + + var builder = ImmutableArray.CreateBuilder(); + builder.Add( + new MSBuildInstance( + _options.Name ?? $"Overridden MSBuild from {_options.MSBuildPath}", + _options.MSBuildPath, + new Version(99, 0), + DiscoveryType.UserOverride, + _options.PropertyOverrides?.ToImmutableDictionary())); + return builder.ToImmutable(); + + } + } +} diff --git a/src/OmniSharp.MSBuild/Options/MSBuildOptions.cs b/src/OmniSharp.MSBuild/Options/MSBuildOptions.cs index bf03468862..3fb0491ae0 100644 --- a/src/OmniSharp.MSBuild/Options/MSBuildOptions.cs +++ b/src/OmniSharp.MSBuild/Options/MSBuildOptions.cs @@ -1,6 +1,6 @@ namespace OmniSharp.Options { - internal class MSBuildOptions + public class MSBuildOptions { public string ToolsVersion { get; set; } public string VisualStudioVersion { get; set; } diff --git a/tests/OmniSharp.MSBuild.Tests/MSBuildSelectionTests.cs b/tests/OmniSharp.MSBuild.Tests/MSBuildSelectionTests.cs index d527ee4a62..069003ebdb 100644 --- a/tests/OmniSharp.MSBuild.Tests/MSBuildSelectionTests.cs +++ b/tests/OmniSharp.MSBuild.Tests/MSBuildSelectionTests.cs @@ -23,7 +23,7 @@ public void RegisterDefaultInstanceFindsTheBestInstanceAvailable() new MSBuildInstance( "Test Instance", TestIO.GetRandomTempFolderPath(), - Version.Parse("15.1.2.3"), + Version.Parse("16.1.2.3"), DiscoveryType.VisualStudioSetup ).AddDotNetCoreToFakeInstance(), GetStandAloneMSBuildInstance() @@ -50,7 +50,7 @@ public void RegisterDefaultInstanceFindsTheBestInstanceAvailableEvenWithOtherVal new MSBuildInstance( "Valid Test Instance", TestIO.GetRandomTempFolderPath(), - Version.Parse("15.3.2.1"), + Version.Parse("16.3.2.1"), DiscoveryType.VisualStudioSetup ), GetInvalidMsBuildInstance(), @@ -59,7 +59,7 @@ public void RegisterDefaultInstanceFindsTheBestInstanceAvailableEvenWithOtherVal new MSBuildInstance( "Another Valid Test Instance", TestIO.GetRandomTempFolderPath(), - Version.Parse("15.1.2.3"), + Version.Parse("16.1.2.3"), DiscoveryType.VisualStudioSetup ).AddDotNetCoreToFakeInstance(), GetStandAloneMSBuildInstance() @@ -81,6 +81,53 @@ public void RegisterDefaultInstanceFindsTheBestInstanceAvailableEvenWithOtherVal msbuildLocator.DeleteFakeInstancesFolders(); } + [Fact] + public void RegisterDefaultInstanceFindsUserOverrideAvailableEvenWithOtherValidInstances() + { + var msBuildInstances = new[] + { + new MSBuildInstance( + "Valid Test Instance", + TestIO.GetRandomTempFolderPath(), + Version.Parse("16.3.2.1"), + DiscoveryType.VisualStudioSetup + ), + GetInvalidMsBuildInstance(), + + // Valid + Dotnet Core + new MSBuildInstance( + "Another Valid Test Instance", + TestIO.GetRandomTempFolderPath(), + Version.Parse("16.1.2.3"), + DiscoveryType.VisualStudioSetup + ).AddDotNetCoreToFakeInstance(), + GetStandAloneMSBuildInstance(), + + // user override + new MSBuildInstance( + "Manually Overridden", + TestIO.GetRandomTempFolderPath(), + Version.Parse("99.0.0"), + DiscoveryType.UserOverride + ).AddDotNetCoreToFakeInstance(), + }; + + var msbuildLocator = new MSFakeLocator(msBuildInstances); + + var logger = LoggerFactory.CreateLogger( + nameof(RegisterDefaultInstanceFindsUserOverrideAvailableEvenWithOtherValidInstances) + ); + + // test + msbuildLocator.RegisterDefaultInstance(logger); + + Assert.NotNull(msbuildLocator.RegisteredInstance); + Assert.Same(msBuildInstances[4], msbuildLocator.RegisteredInstance); + + // clean up + msbuildLocator.DeleteFakeInstancesFolders(); + } + [Fact] public void RegisterDefaultInstanceStillPrefersTheFirstInstance() { @@ -89,7 +136,7 @@ public void RegisterDefaultInstanceStillPrefersTheFirstInstance() new MSBuildInstance( "Test Instance", TestIO.GetRandomTempFolderPath(), - Version.Parse("15.1.2.3"), + Version.Parse("16.1.2.3"), DiscoveryType.VisualStudioSetup ), GetStandAloneMSBuildInstance() @@ -108,12 +155,39 @@ public void RegisterDefaultInstanceStillPrefersTheFirstInstance() msbuildLocator.DeleteFakeInstancesFolders(); } + [Fact] + public void StandAloneIsPreferredOverVS2017() + { + var msBuildInstances = new[] + { + new MSBuildInstance( + "Test Instance", + TestIO.GetRandomTempFolderPath(), + Version.Parse("15.1.2.3"), + DiscoveryType.VisualStudioSetup + ), + GetStandAloneMSBuildInstance() + }; + + var msbuildLocator = new MSFakeLocator(msBuildInstances); + var logger = LoggerFactory.CreateLogger(nameof(StandAloneIsPreferredOverVS2017)); + + // test + msbuildLocator.RegisterDefaultInstance(logger); + + Assert.NotNull(msbuildLocator.RegisteredInstance); + Assert.Same(msBuildInstances[1], msbuildLocator.RegisteredInstance); + + // clean up + msbuildLocator.DeleteFakeInstancesFolders(); + } + private static MSBuildInstance GetStandAloneMSBuildInstance() { return new MSBuildInstance( "Stand Alone :(", TestIO.GetRandomTempFolderPath(), - Version.Parse("99.0.0.0"), + Version.Parse("16.0.0.0"), DiscoveryType.StandAlone ); }