Clean up MSBuild environment initialization and allow it to use Visua…

…l Studio 2017 if it's present

Fixes dotnet/vscode-csharp#1368

This change ensures that OmniSharp will use the MSBuild tools installed with Visual Studio 2017 if they're present on the machine. This allows OmniSharp to properly handle VS 2017 projects where the targets/tasks aren't include with OmniSharp's local MSBuild, such as WebApplication projects as reported in the bug listed above. If VS 2017 is not on the machine, OmniSharp will continue to use its local MSBuild which has a fallback to the Microsoft Build Tools for targets/tasks that it can't find.

I've also taken the opportunity to clean up a lot of the MSBuild environment initialized code and project file processing.

src/OmniSharp.Abstractions/Utilities/PlatformHelper.cs

@@ -14,6 +14,8 @@ public static class PlatformHelper
         public static string MonoFilePath => _monoPath.Value;
         public static string MonoXBuildFrameworksDirPath => _monoXBuildFrameworksDirPath.Value;
 
+        public static bool IsWindows => Path.DirectorySeparatorChar == '\\';
+
         private static string FindMonoPath()
         {
             // To locate Mono on unix, we use the 'which' command (https://en.wikipedia.org/wiki/Which_(Unix))

src/OmniSharp.MSBuild/Extensions.cs

@@ -0,0 +1,21 @@
+using System.Collections.Generic;
+
+namespace OmniSharp.MSBuild
+{
+    internal static class Extensions
+    {
+        public static void AddPropertyIfNeeded(this Dictionary<string, string> properties, string name, string userOptionValue, string environmentValue)
+        {
+            if (!string.IsNullOrWhiteSpace(userOptionValue))
+            {
+                // If the user set the option, we should use that.
+                properties.Add(name, userOptionValue);
+            }
+            else if (!string.IsNullOrWhiteSpace(environmentValue))
+            {
+                // If we have a custom environment value, we should use that.
+                properties.Add(name, environmentValue);
+            }
+        }
+    }
+}

src/OmniSharp.MSBuild/MSBuildEnvironment.cs

@@ -11,26 +11,38 @@ public static class MSBuildEnvironment
         public const string MSBuildSDKsPathName = "MSBuildSDKsPath";
 
         private static bool s_isInitialized;
-        private static string s_msbuildFolder;
+
+        private static bool s_usingVisualStudio;
+
+        private static string s_msbuildExePath;
         private static string s_msbuildExtensionsPath;
         private static string s_msbuildSDKsPath;
 
         public static bool IsInitialized => s_isInitialized;
 
-        public static string MSBuildFolder
+        public static bool UsingVisualStudio
+        {
+            get
+            {
+                ThrowIfNotInitialized();
+                return s_usingVisualStudio;
+            }
+        }
+
+        public static string MSBuildExePath
         {
             get
             {
-                EnsureInitialized();
-                return s_msbuildFolder;
+                ThrowIfNotInitialized();
+                return s_msbuildExePath;
             }
         }
 
         public static string MSBuildExtensionsPath
         {
             get
             {
-                EnsureInitialized();
+                ThrowIfNotInitialized();
                 return s_msbuildExtensionsPath;
             }
         }
@@ -39,82 +51,110 @@ public static string MSBuildSDKsPath
         {
             get
             {
-                EnsureInitialized();
+                ThrowIfNotInitialized();
                 return s_msbuildSDKsPath;
             }
         }
 
+        private static void ThrowIfNotInitialized()
+        {
+            if (!s_isInitialized)
+            {
+                throw new InvalidOperationException("MSBuild environment has not been initialized.");
+            }
+        }
+
         public static void Initialize(ILogger logger)
         {
             if (s_isInitialized)
             {
-                throw new InvalidOperationException("MSBuildEnvironment is already initialized");
+                throw new InvalidOperationException("MSBuild environment is already initialized.");
             }
 
-            // If OmniSharp is running normally, MSBuild is located in an 'msbuild' folder beneath OmniSharp.exe.
-            var msbuildFolder = Path.Combine(AppContext.BaseDirectory, "msbuild");
+            // If MSBuild can locate VS 2017 and set up a build environment, we don't need to do anything.
+            // MSBuild will take care of itself.
+            if (MSBuildHelpers.TryGetVisualStudioBuildEnvironment())
+            {
+                logger.LogInformation("MSBuild will use local Visual Studio installation.");
+                s_usingVisualStudio = true;
+                s_isInitialized = true;
+            }
+            else if (TryWithLocalMSBuild(logger, out var msbuildExePath, out var msbuildExtensionsPath, out var msbuildSDKsPath))
+            {
+                logger.LogInformation("MSBuild will use local OmniSharp installation.");
+                s_msbuildExePath = msbuildExePath;
+                s_msbuildExtensionsPath = msbuildExtensionsPath;
+                s_msbuildSDKsPath = msbuildSDKsPath;
+                s_isInitialized = true;
+            }
 
-            if (!Directory.Exists(msbuildFolder))
+            if (!s_isInitialized)
             {
-                // If the 'msbuild' folder does not exist beneath OmniSharp.exe, this is likely a development scenario,
-                // such as debugging or running unit tests. In that case, we use one of the .msbuild-* folders at the
-                // solution level.
-                msbuildFolder = FindMSBuildFolderFromSolution(logger);
+                logger.LogError("MSBuild environment could not be initialized.");
             }
+        }
 
-            if (msbuildFolder == null)
+        private static bool TryWithLocalMSBuild(ILogger logger, out string msbuildExePath, out string msbuildExtensionsPath, out string msbuildSDKsPath)
+        {
+            msbuildExePath = null;
+            msbuildExtensionsPath = null;
+            msbuildSDKsPath = null;
+
+            var msbuildDirectory = FindMSBuildDirectory(logger);
+            if (msbuildDirectory == null)
             {
-                logger.LogError("Could not locate MSBuild path. MSBuildProjectSystem will not function properly.");
-                return;
+                logger.LogError("Could not locate MSBuild directory.");
+                return false;
             }
 
             // Set the MSBUILD_EXE_PATH environment variable to the location of MSBuild.exe or MSBuild.dll.
-            var msbuildExePath = Path.Combine(msbuildFolder, "MSBuild.exe");
-            if (!File.Exists(msbuildExePath))
+            msbuildExePath = FindMSBuildExe(msbuildDirectory);
+            if (msbuildExePath == null)
             {
-                msbuildExePath = Path.Combine(msbuildFolder, "MSBuild.dll");
+                logger.LogError("Could not locate MSBuild executable");
+                return false;
             }
 
-            if (!File.Exists(msbuildExePath))
+            // Set the MSBuildExtensionsPath environment variable to the local msbuild directory.
+            msbuildExtensionsPath = msbuildDirectory;
+
+            // Set the MSBuildSDKsPath environment variable to the location of the SDKs.
+            msbuildSDKsPath = FindMSBuildSDKsPath(msbuildDirectory);
+            if (msbuildSDKsPath == null)
             {
-                logger.LogError("Could not locate MSBuild to set MSBUILD_EXE_PATH");
-                return;
+                logger.LogError("Could not locate MSBuild Sdks path");
+                return false;
             }
 
             Environment.SetEnvironmentVariable(MSBuildExePathName, msbuildExePath);
             logger.LogInformation($"{MSBuildExePathName} environment variable set to {msbuildExePath}");
 
-            // Set the MSBuildExtensionsPath environment variable to the msbuild folder.
-            Environment.SetEnvironmentVariable(MSBuildExtensionsPathName, msbuildFolder);
-            logger.LogInformation($"{MSBuildExtensionsPathName} environment variable set to {msbuildFolder}");
+            Environment.SetEnvironmentVariable(MSBuildExtensionsPathName, msbuildExtensionsPath);
+            logger.LogInformation($"{MSBuildExtensionsPathName} environment variable set to {msbuildExtensionsPath}");
 
-            // Set the MSBuildSDKsPath environment variable to the location of the SDKs.
-            var msbuildSdksFolder = Path.Combine(msbuildFolder, "Sdks");
-            if (Directory.Exists(msbuildSdksFolder))
-            {
-                s_msbuildSDKsPath = msbuildSdksFolder;
-                Environment.SetEnvironmentVariable(MSBuildSDKsPathName, msbuildSdksFolder);
-                logger.LogInformation($"{MSBuildSDKsPathName} environment variable set to {msbuildSdksFolder}");
-            }
-            else
-            {
-                logger.LogError($"Could not locate MSBuild Sdks path to set {MSBuildSDKsPathName}");
-            }
+            Environment.SetEnvironmentVariable(MSBuildSDKsPathName, msbuildSDKsPath);
+            logger.LogInformation($"{MSBuildSDKsPathName} environment variable set to {msbuildSDKsPath}");
 
-            s_msbuildFolder = msbuildFolder;
-            s_msbuildExtensionsPath = msbuildFolder;
-            s_isInitialized = true;
+            return true;
         }
 
-        private static void EnsureInitialized()
+        private static string FindMSBuildDirectory(ILogger logger)
         {
-            if (!s_isInitialized)
+            // If OmniSharp is running normally, MSBuild is located in an 'msbuild' folder beneath OmniSharp.exe.
+            var msbuildDirectory = Path.Combine(AppContext.BaseDirectory, "msbuild");
+
+            if (!Directory.Exists(msbuildDirectory))
             {
-                throw new InvalidOperationException("MSBuildEnvironment is not initialized");
+                // If the 'msbuild' folder does not exist beneath OmniSharp.exe, this is likely a development scenario,
+                // such as debugging or running unit tests. In that case, we use one of the .msbuild-* folders at the
+                // solution level.
+                msbuildDirectory = FindLocalMSBuildFromSolution(logger);
             }
+
+            return msbuildDirectory;
         }
 
-        private static string FindMSBuildFolderFromSolution(ILogger logger)
+        private static string FindLocalMSBuildFromSolution(ILogger logger)
         {
             // Try to locate the appropriate build-time msbuild folder by searching for
             // the OmniSharp solution relative to the current folder.
@@ -143,5 +183,32 @@ private static string FindMSBuildFolderFromSolution(ILogger logger)
                 ? result
                 : null;
         }
+
+        private static string FindMSBuildExe(string directory)
+        {
+            // We look for either MSBuild.exe or MSBuild.dll.
+            var msbuildExePath = Path.Combine(directory, "MSBuild.exe");
+            if (File.Exists(msbuildExePath))
+            {
+                return msbuildExePath;
+            }
+
+            msbuildExePath = Path.Combine(directory, "MSBuild.dll");
+            if (File.Exists(msbuildExePath))
+            {
+                return msbuildExePath;
+            }
+
+            return null;
+        }
+
+        private static string FindMSBuildSDKsPath(string directory)
+        {
+            var msbuildSDKsPath = Path.Combine(directory, "Sdks");
+
+            return Directory.Exists(msbuildSDKsPath)
+                ? msbuildSDKsPath
+                : null;
+        }
     }
 }

src/OmniSharp.MSBuild/MSBuildHelpers.cs

@@ -0,0 +1,80 @@
+using System;
+using System.Collections.Generic;
+using System.Reflection;
+using System.Text;
+using Microsoft.Build.Evaluation;
+using Microsoft.Build.Execution;
+using OmniSharp.Utilities;
+
+namespace OmniSharp.MSBuild
+{
+    public static class MSBuildHelpers
+    {
+        private static Assembly s_MicrosoftBuildAssembly;
+
+        private static Type s_BuildEnvironmentHelperType;
+        private static Type s_BuildEnvironmentType;
+
+        static MSBuildHelpers()
+        {
+            s_MicrosoftBuildAssembly = Assembly.Load(new AssemblyName("Microsoft.Build"));
+
+            s_BuildEnvironmentHelperType = s_MicrosoftBuildAssembly.GetType("Microsoft.Build.Shared.BuildEnvironmentHelper");
+            s_BuildEnvironmentType = s_MicrosoftBuildAssembly.GetType("Microsoft.Build.Shared.BuildEnvironment");
+        }
+
+        public static string GetBuildEnvironmentInfo()
+        {
+            var instanceProp = s_BuildEnvironmentHelperType.GetProperty("Instance");
+            var buildEnvironment = instanceProp.GetMethod.Invoke(null, null);
+
+            return DumpBuildEnvironment(buildEnvironment);
+        }
+
+        private static string DumpBuildEnvironment(object buildEnvironment)
+        {
+            var builder = new StringBuilder();
+
+            if (buildEnvironment != null)
+            {
+                const BindingFlags flags = BindingFlags.NonPublic | BindingFlags.Instance;
+
+                AppendPropertyValue(builder, "Mode", buildEnvironment, s_BuildEnvironmentType, flags);
+                AppendPropertyValue(builder, "RunningTests", buildEnvironment, s_BuildEnvironmentType, flags);
+                AppendPropertyValue(builder, "RunningInVisualStudio", buildEnvironment, s_BuildEnvironmentType, flags);
+                AppendPropertyValue(builder, "MSBuildToolsDirectory32", buildEnvironment, s_BuildEnvironmentType, flags);
+                AppendPropertyValue(builder, "MSBuildToolsDirectory64", buildEnvironment, s_BuildEnvironmentType, flags);
+                AppendPropertyValue(builder, "MSBuildSDKsPath", buildEnvironment, s_BuildEnvironmentType, flags);
+                AppendPropertyValue(builder, "CurrentMSBuildConfigurationFile", buildEnvironment, s_BuildEnvironmentType, flags);
+                AppendPropertyValue(builder, "CurrentMSBuildExePath", buildEnvironment, s_BuildEnvironmentType, flags);
+                AppendPropertyValue(builder, "CurrentMSBuildToolsDirectory", buildEnvironment, s_BuildEnvironmentType, flags);
+                AppendPropertyValue(builder, "VisualStudioInstallRootDirectory", buildEnvironment, s_BuildEnvironmentType, flags);
+                AppendPropertyValue(builder, "MSBuildExtensionsPath", buildEnvironment, s_BuildEnvironmentType, flags);
+            }
+
+            return builder.ToString();
+        }
+
+        private static void AppendPropertyValue(StringBuilder builder, string name, object instance, Type type, BindingFlags bindingFlags)
+        {
+            var propInfo = type.GetProperty(name, bindingFlags);
+            var propValue = propInfo.GetMethod.Invoke(instance, null);
+            builder.AppendLine($"{name}: {propValue}");
+        }
+
+        public static bool TryGetVisualStudioBuildEnvironment()
+        {
+            if (!PlatformHelper.IsWindows)
+            {
+                return false;
+            }
+
+            // Call Microsoft.Build.Shared.BuildEnvironmentHelper.TryFromSetupApi(...), which attempts
+            // to compute a build environment by looking for VS 2017.
+            var tryFromSetupApiMethod = s_BuildEnvironmentHelperType.GetMethod("TryFromSetupApi", BindingFlags.NonPublic | BindingFlags.Static);
+            var buildEnvironment = tryFromSetupApiMethod.Invoke(null, null);
+
+            return buildEnvironment != null;
+        }
+    }
+}

src/OmniSharp.MSBuild/MSBuildProjectSystem.cs

@@ -2,7 +2,6 @@
 using System.Collections.Generic;
 using System.Collections.Immutable;
 using System.Composition;
-using System.Diagnostics;
 using System.IO;
 using System.Linq;
 using System.Threading.Tasks;
@@ -80,14 +79,12 @@ public void Initalize(IConfiguration configuration)
             if (!MSBuildEnvironment.IsInitialized)
             {
                 MSBuildEnvironment.Initialize(_logger);
-            }
 
-            if (_options.WaitForDebugger)
-            {
-                Console.WriteLine($"Attach to process {Process.GetCurrentProcess().Id}");
-                while (!Debugger.IsAttached)
+                if (MSBuildEnvironment.IsInitialized &&
+                    _environment.LogLevel < LogLevel.Information)
                 {
-                    System.Threading.Thread.Sleep(100);
+                    var buildEnvironmentInfo = MSBuildHelpers.GetBuildEnvironmentInfo();
+                    _logger.LogDebug($"MSBuild environment: {Environment.NewLine}{buildEnvironmentInfo}");
                 }
             }
 

src/OmniSharp.MSBuild/Options/MSBuildOptions.cs

@@ -4,7 +4,6 @@ public class MSBuildOptions
     {
         public string ToolsVersion { get; set; }
         public string VisualStudioVersion { get; set; }
-        public bool WaitForDebugger { get; set; }
         public bool EnablePackageAutoRestore { get; set; }
 
         public string MSBuildExtensionsPath { get; set; }

src/OmniSharp.MSBuild/ProjectFile/ProjectFileInfo.PropertyNames.cs

@@ -29,6 +29,7 @@ private static class PropertyNames
             public const string TargetFrameworks = nameof(TargetFrameworks);
             public const string TargetPath = nameof(TargetPath);
             public const string VisualStudioVersion = nameof(VisualStudioVersion);
+            public const string VsInstallRoot = nameof(VsInstallRoot);
         }
     }
 }