Merge pull request #2181 from JoeRobich/use-msbuild-locator

Use the Microsoft.Build.Locator package for discovery

build/Packages.props

@@ -54,6 +54,8 @@
         <PackageReference Update="Microsoft.Extensions.Options" Version="$(MicrosoftExtensionPackageVersion)" />
         <PackageReference Update="Microsoft.Extensions.Options.ConfigurationExtensions" Version="$(MicrosoftExtensionPackageVersion)" />
 
+        <PackageReference Update="Microsoft.Build.Locator" Version="1.4.1" />
+
         <PackageReference Update="Microsoft.NETFramework.ReferenceAssemblies" Version="1.0.0" />
 
         <PackageReference Update="Microsoft.NET.Test.Sdk" Version="$(MicrosoftTestPackageVersion)" />

src/OmniSharp.Host/MSBuild/Discovery/MSBuildLocator.cs

@@ -1,46 +1,33 @@
 using System;
 using System.Collections.Immutable;
 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;
 using OmniSharp.Utilities;
+using MicrosoftBuildLocator = Microsoft.Build.Locator.MSBuildLocator;
 
 namespace OmniSharp.MSBuild.Discovery
 {
     internal class MSBuildLocator : DisposableObject, IMSBuildLocator
     {
-        private static readonly ImmutableHashSet<string> s_msbuildAssemblies = ImmutableHashSet.Create(StringComparer.OrdinalIgnoreCase,
-            "Microsoft.Build",
-            "Microsoft.Build.Framework",
-            "Microsoft.Build.Tasks.Core",
-            "Microsoft.Build.Utilities.Core");
-
         private readonly ILogger _logger;
-        private readonly IAssemblyLoader _assemblyLoader;
         private readonly ImmutableArray<MSBuildInstanceProvider> _providers;
 
         public MSBuildInstance RegisteredInstance { get; private set; }
 
         private MSBuildLocator(ILoggerFactory loggerFactory, IAssemblyLoader assemblyLoader, ImmutableArray<MSBuildInstanceProvider> providers)
         {
             _logger = loggerFactory.CreateLogger<MSBuildLocator>();
-            _assemblyLoader = assemblyLoader;
             _providers = providers;
         }
 
         protected override void DisposeCore(bool disposing)
         {
             if (RegisteredInstance != null)
             {
-                try
-                {
-                    AppDomain.CurrentDomain.AssemblyResolve -= Resolve;
-                }
-                catch (AppDomainUnloadedException){ } // Ignore if the AppDomain is going away (like during a test in xunit)
                 RegisteredInstance = null;
             }
         }
@@ -57,17 +44,14 @@ public static MSBuildLocator CreateDefault(ILoggerFactory loggerFactory, IAssemb
 
             return new MSBuildLocator(loggerFactory, assemblyLoader,
                 ImmutableArray.Create<MSBuildInstanceProvider>(
-                    new DevConsoleInstanceProvider(loggerFactory),
-                    new VisualStudioInstanceProvider(loggerFactory),
+                    new MicrosoftBuildLocatorInstanceProvider(loggerFactory),
                     new MonoInstanceProvider(loggerFactory),
                     new StandAloneInstanceProvider(loggerFactory),
                     new UserOverrideInstanceProvider(loggerFactory, msbuildConfiguration)));
         }
 
         public static MSBuildLocator CreateStandAlone(ILoggerFactory loggerFactory, IAssemblyLoader assemblyLoader)
-            => new MSBuildLocator(loggerFactory, assemblyLoader,
-                ImmutableArray.Create<MSBuildInstanceProvider>(
-                    new StandAloneInstanceProvider(loggerFactory)));
+            => new MSBuildLocator(loggerFactory, assemblyLoader, ImmutableArray.Create<MSBuildInstanceProvider>(new StandAloneInstanceProvider(loggerFactory)));
 
         public void RegisterInstance(MSBuildInstance instance)
         {
@@ -78,13 +62,6 @@ public void RegisterInstance(MSBuildInstance instance)
 
             RegisteredInstance = instance ?? throw new ArgumentNullException(nameof(instance));
 
-            foreach (var assemblyName in s_msbuildAssemblies)
-            {
-                LoadAssemblyByNameOnly(assemblyName);
-            }
-
-            AppDomain.CurrentDomain.AssemblyResolve += Resolve;
-
             if (instance.SetMSBuildExePathVariable)
             {
                 var msbuildExePath = Path.Combine(instance.MSBuildPath, "MSBuild.exe");
@@ -120,110 +97,13 @@ public void RegisterInstance(MSBuildInstance instance)
             }
 
             _logger.LogInformation(builder.ToString());
-        }
-
-        private Assembly Resolve(object sender, ResolveEventArgs e)
-        {
-            var assemblyName = new AssemblyName(e.Name);
-
-            _logger.LogDebug($"Attempting to resolve '{assemblyName}'");
-
-            return s_msbuildAssemblies.Contains(assemblyName.Name)
-                ? LoadAssemblyByNameOnly(assemblyName.Name)
-                : LoadAssemblyByFullName(assemblyName);
-        }
-
-        private Assembly LoadAssemblyByNameOnly(string assemblyName)
-        {
-            var assemblyPath = Path.Combine(RegisteredInstance.MSBuildPath, assemblyName + ".dll");
-            var result = File.Exists(assemblyPath)
-                ? _assemblyLoader.LoadFrom(assemblyPath)
-                : null;
-
-            if (result != null)
-            {
-                _logger.LogDebug($"SUCCESS: Resolved to '{assemblyPath}' (name-only).");
-            }
-
-            return result;
-        }
-
-        private Assembly LoadAssemblyByFullName(AssemblyName assemblyName)
-        {
-            var assemblyPath = Path.Combine(RegisteredInstance.MSBuildPath, assemblyName.Name + ".dll");
-            if (!File.Exists(assemblyPath))
-            {
-                _logger.LogDebug($"FAILURE: Could not locate '{assemblyPath}'.");
-                return null;
-            }
-
-            if (!TryGetAssemblyName(assemblyPath, out var resultAssemblyName))
-            {
-                _logger.LogDebug($"FAILURE: Could not retrieve {nameof(AssemblyName)} for '{assemblyPath}'.");
-                return null;
-            }
 
-            if (assemblyName.Name != resultAssemblyName.Name ||
-                assemblyName.Version != resultAssemblyName.Version ||
-                !AreEqual(assemblyName.GetPublicKeyToken(), resultAssemblyName.GetPublicKeyToken()))
+            if (!MicrosoftBuildLocator.CanRegister)
             {
-                _logger.LogDebug($"FAILURE: Found '{assemblyPath}' but name, '{resultAssemblyName}', did not match.");
-                return null;
+                return;
             }
 
-            // Note: don't bother testing culture. If the assembly has a different culture than what we're
-            // looking for, go ahead and use it.
-
-            var resultAssembly = _assemblyLoader.LoadFrom(assemblyPath);
-
-            if (resultAssembly != null)
-            {
-                _logger.LogDebug($"SUCCESS: Resolved to '{assemblyPath}'");
-            }
-
-            return resultAssembly;
-        }
-
-        private static bool AreEqual(byte[] array1, byte[] array2)
-        {
-            if (array1 == null)
-            {
-                return array2 == null;
-            }
-
-            if (array1 == null)
-            {
-                return false;
-            }
-
-            if (array1.Length != array2.Length)
-            {
-                return false;
-            }
-
-            for (int i = 0; i < array1.Length; i++)
-            {
-                if (array1[i] != array2[i])
-                {
-                    return false;
-                }
-            }
-
-            return true;
-        }
-
-        private static bool TryGetAssemblyName(string assemblyPath, out AssemblyName assemblyName)
-        {
-            try
-            {
-                assemblyName = AssemblyName.GetAssemblyName(assemblyPath);
-                return assemblyName != null;
-            }
-            catch
-            {
-                assemblyName = null;
-                return false;
-            }
+            MicrosoftBuildLocator.RegisterMSBuildPath(instance.MSBuildPath);
         }
 
         public ImmutableArray<MSBuildInstance> GetInstances()

src/OmniSharp.Host/MSBuild/Discovery/Providers/MicrosoftBuildLocatorInstanceProvider.cs

@@ -0,0 +1,51 @@
+using System;
+using System.Collections.Immutable;
+using System.IO;
+using System.Linq;
+using Microsoft.Extensions.Logging;
+using OmniSharp.Utilities;
+using MicrosoftBuildLocator = Microsoft.Build.Locator.MSBuildLocator;
+using MicrosoftDiscoveryType = Microsoft.Build.Locator.DiscoveryType;
+
+namespace OmniSharp.MSBuild.Discovery.Providers
+{
+    internal class MicrosoftBuildLocatorInstanceProvider : MSBuildInstanceProvider
+    {
+        public MicrosoftBuildLocatorInstanceProvider(ILoggerFactory loggerFactory)
+            : base(loggerFactory)
+        {
+        }
+
+        public override ImmutableArray<MSBuildInstance> GetInstances()
+        {
+            if (!PlatformHelper.IsWindows)
+            {
+                return NoInstances;
+            }
+
+            var instances = MicrosoftBuildLocator.QueryVisualStudioInstances();
+
+            return instances.Select(instance =>
+            {
+                var microsoftBuildPath = Path.Combine(instance.MSBuildPath, "Microsoft.Build.dll");
+                var version = GetMSBuildVersion(microsoftBuildPath);
+
+                return new MSBuildInstance(
+                    $"{instance.Name} {instance.Version}",
+                    instance.MSBuildPath,
+                    version,
+                    GetDiscoveryType(instance.DiscoveryType));
+            }).ToImmutableArray();
+
+            static DiscoveryType GetDiscoveryType(MicrosoftDiscoveryType discoveryType)
+            {
+                return discoveryType switch
+                {
+                    MicrosoftDiscoveryType.DeveloperConsole => DiscoveryType.DeveloperConsole,
+                    MicrosoftDiscoveryType.VisualStudioSetup => DiscoveryType.VisualStudioSetup,
+                    _ => throw new ArgumentException()
+                };
+            }
+        }
+    }
+}