From 5f6828916c1b20e0a69a373a6b4045f694e4359f Mon Sep 17 00:00:00 2001 From: Dustin Campbell Date: Fri, 7 Apr 2017 14:32:13 -0700 Subject: [PATCH] Clean up MSBuild environment initialization and allow it to use Visual Studio 2017 if it's present Fixes https://github.com/OmniSharp/omnisharp-vscode/issues/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. --- .../Utilities/PlatformHelper.cs | 2 + src/OmniSharp.MSBuild/Extensions.cs | 21 +++ src/OmniSharp.MSBuild/MSBuildEnvironment.cs | 157 +++++++++++++----- src/OmniSharp.MSBuild/MSBuildHelpers.cs | 80 +++++++++ src/OmniSharp.MSBuild/MSBuildProjectSystem.cs | 11 +- .../Options/MSBuildOptions.cs | 1 - .../ProjectFileInfo.PropertyNames.cs | 1 + .../ProjectFile/ProjectFileInfo.cs | 79 +++++---- src/OmniSharp/Program.cs | 2 +- 9 files changed, 260 insertions(+), 94 deletions(-) create mode 100644 src/OmniSharp.MSBuild/Extensions.cs create mode 100644 src/OmniSharp.MSBuild/MSBuildHelpers.cs diff --git a/src/OmniSharp.Abstractions/Utilities/PlatformHelper.cs b/src/OmniSharp.Abstractions/Utilities/PlatformHelper.cs index 5863159cce..387d2d0e15 100644 --- a/src/OmniSharp.Abstractions/Utilities/PlatformHelper.cs +++ b/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)) diff --git a/src/OmniSharp.MSBuild/Extensions.cs b/src/OmniSharp.MSBuild/Extensions.cs new file mode 100644 index 0000000000..a3b578938a --- /dev/null +++ b/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 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); + } + } + } +} diff --git a/src/OmniSharp.MSBuild/MSBuildEnvironment.cs b/src/OmniSharp.MSBuild/MSBuildEnvironment.cs index 98e5992f7f..6fc5129356 100644 --- a/src/OmniSharp.MSBuild/MSBuildEnvironment.cs +++ b/src/OmniSharp.MSBuild/MSBuildEnvironment.cs @@ -11,18 +11,30 @@ 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; } } @@ -30,7 +42,7 @@ 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; + } } } diff --git a/src/OmniSharp.MSBuild/MSBuildHelpers.cs b/src/OmniSharp.MSBuild/MSBuildHelpers.cs new file mode 100644 index 0000000000..a4a65e2c67 --- /dev/null +++ b/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; + } + } +} diff --git a/src/OmniSharp.MSBuild/MSBuildProjectSystem.cs b/src/OmniSharp.MSBuild/MSBuildProjectSystem.cs index 885b10900d..486129ee38 100644 --- a/src/OmniSharp.MSBuild/MSBuildProjectSystem.cs +++ b/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}"); } } diff --git a/src/OmniSharp.MSBuild/Options/MSBuildOptions.cs b/src/OmniSharp.MSBuild/Options/MSBuildOptions.cs index 05c90bae79..6fa8059e7e 100644 --- a/src/OmniSharp.MSBuild/Options/MSBuildOptions.cs +++ b/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; } diff --git a/src/OmniSharp.MSBuild/ProjectFile/ProjectFileInfo.PropertyNames.cs b/src/OmniSharp.MSBuild/ProjectFile/ProjectFileInfo.PropertyNames.cs index d0e9013adc..7eb647986f 100644 --- a/src/OmniSharp.MSBuild/ProjectFile/ProjectFileInfo.PropertyNames.cs +++ b/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); } } } diff --git a/src/OmniSharp.MSBuild/ProjectFile/ProjectFileInfo.cs b/src/OmniSharp.MSBuild/ProjectFile/ProjectFileInfo.cs index c5e7c378e4..2fe3faa34b 100644 --- a/src/OmniSharp.MSBuild/ProjectFile/ProjectFileInfo.cs +++ b/src/OmniSharp.MSBuild/ProjectFile/ProjectFileInfo.cs @@ -123,46 +123,7 @@ public static ProjectFileInfo Create( options = options ?? new MSBuildOptions(); - var globalProperties = new Dictionary - { - { PropertyNames.DesignTimeBuild, "true" }, - { PropertyNames.BuildProjectReferences, "false" }, - { PropertyNames._ResolveReferenceDependencies, "true" }, - { PropertyNames.SolutionDir, solutionDirectory + Path.DirectorySeparatorChar } - }; - - if (!string.IsNullOrWhiteSpace(options.MSBuildExtensionsPath)) - { - globalProperties.Add(PropertyNames.MSBuildExtensionsPath, options.MSBuildExtensionsPath); - } - else if (!string.IsNullOrWhiteSpace(MSBuildEnvironment.MSBuildExtensionsPath)) - { - globalProperties.Add(PropertyNames.MSBuildExtensionsPath, MSBuildEnvironment.MSBuildExtensionsPath); - } - - if (!string.IsNullOrWhiteSpace(options.MSBuildSDKsPath)) - { - globalProperties.Add(PropertyNames.MSBuildSDKsPath, options.MSBuildSDKsPath); - } - else if (!string.IsNullOrWhiteSpace(MSBuildEnvironment.MSBuildSDKsPath)) - { - globalProperties.Add(PropertyNames.MSBuildSDKsPath, MSBuildEnvironment.MSBuildSDKsPath); - } - - if (PlatformHelper.IsMono) - { - var monoXBuildFrameworksDirPath = PlatformHelper.MonoXBuildFrameworksDirPath; - if (monoXBuildFrameworksDirPath != null) - { - logger.LogDebug($"Using TargetFrameworkRootPath: {monoXBuildFrameworksDirPath}"); - globalProperties.Add(PropertyNames.TargetFrameworkRootPath, monoXBuildFrameworksDirPath); - } - } - - if (!string.IsNullOrWhiteSpace(options.VisualStudioVersion)) - { - globalProperties.Add(PropertyNames.VisualStudioVersion, options.VisualStudioVersion); - } + var globalProperties = GetGlobalProperties(options, solutionDirectory, logger); var collection = new ProjectCollection(globalProperties); @@ -231,6 +192,44 @@ public static ProjectFileInfo Create( sourceFiles, references, projectReferences, analyzers, packageReferences); } + private static Dictionary GetGlobalProperties(MSBuildOptions options, string solutionDirectory, ILogger logger) + { + var globalProperties = new Dictionary + { + { PropertyNames.DesignTimeBuild, "true" }, + { PropertyNames.BuildProjectReferences, "false" }, + { PropertyNames._ResolveReferenceDependencies, "true" }, + { PropertyNames.SolutionDir, solutionDirectory + Path.DirectorySeparatorChar } + }; + + globalProperties.AddPropertyIfNeeded( + PropertyNames.MSBuildExtensionsPath, + userOptionValue: options.MSBuildExtensionsPath, + environmentValue: MSBuildEnvironment.MSBuildExtensionsPath); + + globalProperties.AddPropertyIfNeeded( + PropertyNames.MSBuildSDKsPath, + userOptionValue: options.MSBuildSDKsPath, + environmentValue: MSBuildEnvironment.MSBuildSDKsPath); + + globalProperties.AddPropertyIfNeeded( + PropertyNames.VisualStudioVersion, + userOptionValue: options.VisualStudioVersion, + environmentValue: null); + + if (PlatformHelper.IsMono) + { + var monoXBuildFrameworksDirPath = PlatformHelper.MonoXBuildFrameworksDirPath; + if (monoXBuildFrameworksDirPath != null) + { + logger.LogDebug($"Using TargetFrameworkRootPath: {monoXBuildFrameworksDirPath}"); + globalProperties.Add(PropertyNames.TargetFrameworkRootPath, monoXBuildFrameworksDirPath); + } + } + + return globalProperties; + } + private static bool ReferenceSourceTargetIsNotProjectReference(ProjectItemInstance item) { return item.GetMetadataValue(MetadataNames.ReferenceSourceTarget) != ItemNames.ProjectReference; diff --git a/src/OmniSharp/Program.cs b/src/OmniSharp/Program.cs index 85ba5bae67..c7ec854065 100644 --- a/src/OmniSharp/Program.cs +++ b/src/OmniSharp/Program.cs @@ -11,7 +11,7 @@ public static int Main(string[] args) if (argsList.Contains("--debug")) { argsList.Remove("--debug"); - Console.WriteLine($"Attach debugger to process {Process.GetCurrentProcess().Id} to continue. .."); + Console.WriteLine($"Attach debugger to process {Process.GetCurrentProcess().Id} to continue..."); while (!Debugger.IsAttached) { Thread.Sleep(100);