Merge pull request #1021 from DustinCampbell/fix-sdks-path

Add back code that sets MSBuildSDKsPath environment variable for each project
OmniSharp · Nov 10, 2017 · 4b389b0 · 4b389b0
2 parents ff36173 + 3bf3fa6
commit 4b389b0
Show file tree

Hide file tree

Showing 4 changed files with 128 additions and 41 deletions.
diff --git a/src/OmniSharp.MSBuild/MSBuildProjectSystem.cs b/src/OmniSharp.MSBuild/MSBuildProjectSystem.cs
@@ -33,6 +33,7 @@ public class MSBuildProjectSystem : IProjectSystem
         private readonly OmniSharpWorkspace _workspace;
         private readonly MSBuildInstance _msbuildInstance;
         private readonly DotNetCliService _dotNetCli;
+        private readonly SdksPathResolver _sdksPathResolver;
         private readonly MetadataFileReferenceCache _metadataFileReferenceCache;
         private readonly IEventEmitter _eventEmitter;
         private readonly IFileSystemWatcher _fileSystemWatcher;
@@ -57,6 +58,7 @@ public MSBuildProjectSystem(
             OmniSharpWorkspace workspace,
             IMSBuildLocator msbuildLocator,
             DotNetCliService dotNetCliService,
+            SdksPathResolver sdksPathResolver,
             MetadataFileReferenceCache metadataFileReferenceCache,
             IEventEmitter eventEmitter,
             IFileSystemWatcher fileSystemWatcher,
@@ -66,6 +68,7 @@ public MSBuildProjectSystem(
             _workspace = workspace;
             _msbuildInstance = msbuildLocator.RegisteredInstance;
             _dotNetCli = dotNetCliService;
+            _sdksPathResolver = sdksPathResolver;
             _metadataFileReferenceCache = metadataFileReferenceCache;
             _eventEmitter = eventEmitter;
             _fileSystemWatcher = fileSystemWatcher;
@@ -315,7 +318,7 @@ private ProjectFileInfo LoadProject(string projectFilePath)
 
             try
             {
-                project = ProjectFileInfo.Create(projectFilePath, _environment.TargetDirectory, _loggerFactory.CreateLogger<ProjectFileInfo>(), _msbuildInstance, _options, diagnostics);
+                project = ProjectFileInfo.Create(projectFilePath, _environment.TargetDirectory, _loggerFactory.CreateLogger<ProjectFileInfo>(), _msbuildInstance, _sdksPathResolver, _options, diagnostics);
 
                 if (project == null)
                 {
@@ -341,7 +344,7 @@ private void OnProjectChanged(string projectFilePath, bool allowAutoRestore)
                 if (_projects.TryGetValue(projectFilePath, out var oldProjectFileInfo))
                 {
                     var diagnostics = new List<MSBuildDiagnosticsMessage>();
-                    var newProjectFileInfo = oldProjectFileInfo.Reload(_environment.TargetDirectory, _loggerFactory.CreateLogger<ProjectFileInfo>(), _msbuildInstance, _options, diagnostics);
+                    var newProjectFileInfo = oldProjectFileInfo.Reload(_environment.TargetDirectory, _loggerFactory.CreateLogger<ProjectFileInfo>(), _msbuildInstance, _sdksPathResolver, _options, diagnostics);
 
                     if (newProjectFileInfo != null)
                     {

diff --git a/src/OmniSharp.MSBuild/ProjectFile/ProjectFileInfo.cs b/src/OmniSharp.MSBuild/ProjectFile/ProjectFileInfo.cs
@@ -71,14 +71,14 @@ private ProjectFileInfo(
 
         public static ProjectFileInfo Create(
             string filePath, string solutionDirectory, ILogger logger,
-            MSBuildInstance msbuildInstance, MSBuildOptions options = null, ICollection<MSBuildDiagnosticsMessage> diagnostics = null)
+            MSBuildInstance msbuildInstance, SdksPathResolver sdksPathResolver, MSBuildOptions options = null, ICollection<MSBuildDiagnosticsMessage> diagnostics = null)
         {
             if (!File.Exists(filePath))
             {
                 return null;
             }
 
-            var projectInstance = LoadProject(filePath, solutionDirectory, logger, msbuildInstance, options, diagnostics, out var targetFrameworks);
+            var projectInstance = LoadProject(filePath, solutionDirectory, logger, msbuildInstance, sdksPathResolver, options, diagnostics, out var targetFrameworks);
             if (projectInstance == null)
             {
                 return null;
@@ -92,50 +92,53 @@ public static ProjectFileInfo Create(
 
         private static ProjectInstance LoadProject(
             string filePath, string solutionDirectory, ILogger logger,
-            MSBuildInstance msbuildInstance, MSBuildOptions options, ICollection<MSBuildDiagnosticsMessage> diagnostics, out ImmutableArray<string> targetFrameworks)
+            MSBuildInstance msbuildInstance, SdksPathResolver sdksPathResolver, MSBuildOptions options, ICollection<MSBuildDiagnosticsMessage> diagnostics, out ImmutableArray<string> targetFrameworks)
         {
-            options = options ?? new MSBuildOptions();
+            using (sdksPathResolver.SetSdksPathEnvironmentVariable(filePath))
+            {
+                options = options ?? new MSBuildOptions();
 
-            var globalProperties = GetGlobalProperties(msbuildInstance, options, solutionDirectory, logger);
+                var globalProperties = GetGlobalProperties(msbuildInstance, options, solutionDirectory, logger);
 
-            var collection = new ProjectCollection(globalProperties);
+                var collection = new ProjectCollection(globalProperties);
 
-            var toolsVersion = options.ToolsVersion;
-            if (string.IsNullOrEmpty(toolsVersion) || Version.TryParse(toolsVersion, out _))
-            {
-                toolsVersion = collection.DefaultToolsVersion;
-            }
+                var toolsVersion = options.ToolsVersion;
+                if (string.IsNullOrEmpty(toolsVersion) || Version.TryParse(toolsVersion, out _))
+                {
+                    toolsVersion = collection.DefaultToolsVersion;
+                }
 
-            toolsVersion = GetLegalToolsetVersion(toolsVersion, collection.Toolsets);
+                toolsVersion = GetLegalToolsetVersion(toolsVersion, collection.Toolsets);
 
-            // Evaluate the MSBuild project
-            var project = collection.LoadProject(filePath, toolsVersion);
+                // Evaluate the MSBuild project
+                var project = collection.LoadProject(filePath, toolsVersion);
 
-            var targetFramework = project.GetPropertyValue(PropertyNames.TargetFramework);
-            targetFrameworks = PropertyConverter.SplitList(project.GetPropertyValue(PropertyNames.TargetFrameworks), ';');
+                var targetFramework = project.GetPropertyValue(PropertyNames.TargetFramework);
+                targetFrameworks = PropertyConverter.SplitList(project.GetPropertyValue(PropertyNames.TargetFrameworks), ';');
 
-            // If the project supports multiple target frameworks and specific framework isn't
-            // selected, we must pick one before execution. Otherwise, the ResolveReferences
-            // target might not be available to us.
-            if (string.IsNullOrWhiteSpace(targetFramework) && targetFrameworks.Length > 0)
-            {
-                // For now, we'll just pick the first target framework. Eventually, we'll need to
-                // do better and potentially allow OmniSharp hosts to select a target framework.
-                targetFramework = targetFrameworks[0];
-                project.SetProperty(PropertyNames.TargetFramework, targetFramework);
-            }
-            else if (!string.IsNullOrWhiteSpace(targetFramework) && targetFrameworks.Length == 0)
-            {
-                targetFrameworks = ImmutableArray.Create(targetFramework);
-            }
+                // If the project supports multiple target frameworks and specific framework isn't
+                // selected, we must pick one before execution. Otherwise, the ResolveReferences
+                // target might not be available to us.
+                if (string.IsNullOrWhiteSpace(targetFramework) && targetFrameworks.Length > 0)
+                {
+                    // For now, we'll just pick the first target framework. Eventually, we'll need to
+                    // do better and potentially allow OmniSharp hosts to select a target framework.
+                    targetFramework = targetFrameworks[0];
+                    project.SetProperty(PropertyNames.TargetFramework, targetFramework);
+                }
+                else if (!string.IsNullOrWhiteSpace(targetFramework) && targetFrameworks.Length == 0)
+                {
+                    targetFrameworks = ImmutableArray.Create(targetFramework);
+                }
 
-            var projectInstance = project.CreateProjectInstance();
-            var buildResult = projectInstance.Build(new string[] { TargetNames.Compile, TargetNames.CoreCompile },
-                new[] { new MSBuildLogForwarder(logger, diagnostics) });
+                var projectInstance = project.CreateProjectInstance();
+                var buildResult = projectInstance.Build(new string[] { TargetNames.Compile, TargetNames.CoreCompile },
+                    new[] { new MSBuildLogForwarder(logger, diagnostics) });
 
-            return buildResult
-                ? projectInstance
-                : null;
+                return buildResult
+                    ? projectInstance
+                    : null;
+            }
         }
 
         private static string GetLegalToolsetVersion(string toolsVersion, ICollection<Toolset> toolsets)
@@ -250,9 +253,9 @@ private static ProjectData CreateProjectData(ProjectInstance projectInstance, Im
 
         public ProjectFileInfo Reload(
             string solutionDirectory, ILogger logger,
-            MSBuildInstance msbuildInstance, MSBuildOptions options = null, ICollection<MSBuildDiagnosticsMessage> diagnostics = null)
+            MSBuildInstance msbuildInstance, SdksPathResolver sdksPathResolver, MSBuildOptions options = null, ICollection<MSBuildDiagnosticsMessage> diagnostics = null)
         {
-            var projectInstance = LoadProject(FilePath, solutionDirectory, logger, msbuildInstance, options, diagnostics, out var targetFrameworks);
+            var projectInstance = LoadProject(FilePath, solutionDirectory, logger, msbuildInstance, sdksPathResolver, options, diagnostics, out var targetFrameworks);
             if (projectInstance == null)
             {
                 return null;

diff --git a/src/OmniSharp.MSBuild/SdksPathResolver.cs b/src/OmniSharp.MSBuild/SdksPathResolver.cs
@@ -0,0 +1,80 @@
+using System;
+using System.Composition;
+using System.IO;
+using OmniSharp.Services;
+
+namespace OmniSharp.MSBuild
+{
+    [Export, Shared]
+    public class SdksPathResolver
+    {
+        private const string MSBuildSDKsPath = nameof(MSBuildSDKsPath);
+        private const string Sdks = nameof(Sdks);
+
+        private readonly DotNetCliService _dotNetCli;
+
+        [ImportingConstructor]
+        public SdksPathResolver(DotNetCliService dotNetCli)
+        {
+            _dotNetCli = dotNetCli;
+        }
+
+        public bool TryGetSdksPath(string projectFilePath, out string sdksPath)
+        {
+            var projectFileDirectory = Path.GetDirectoryName(projectFilePath);
+
+            var info = _dotNetCli.GetInfo(projectFileDirectory);
+
+            if (info.IsEmpty || string.IsNullOrWhiteSpace(info.BasePath))
+            {
+                sdksPath = null;
+                return false;
+            }
+
+            sdksPath = Path.Combine(info.BasePath, Sdks);
+
+            if (Directory.Exists(sdksPath))
+            {
+                return true;
+            }
+
+            sdksPath = null;
+            return false;
+        }
+
+        public IDisposable SetSdksPathEnvironmentVariable(string projectFilePath)
+        {
+            if (!TryGetSdksPath(projectFilePath, out var sdksPath))
+            {
+                return NullDisposable.Instance;
+            }
+
+            var oldMSBuildSDKsPath = Environment.GetEnvironmentVariable(MSBuildSDKsPath);
+            Environment.SetEnvironmentVariable(MSBuildSDKsPath, sdksPath);
+
+            return new ResetSdksPathEnvironmentVariable(oldMSBuildSDKsPath);
+        }
+
+        private class NullDisposable : IDisposable
+        {
+            public static IDisposable Instance { get; } = new NullDisposable();
+
+            public void Dispose() { }
+        }
+
+        private class ResetSdksPathEnvironmentVariable : IDisposable
+        {
+            private readonly string _oldValue;
+
+            public ResetSdksPathEnvironmentVariable(string oldValue)
+            {
+                _oldValue = oldValue;
+            }
+
+            public void Dispose()
+            {
+                Environment.SetEnvironmentVariable(MSBuildSDKsPath, _oldValue);
+            }
+        }
+    }
+}
diff --git a/tests/OmniSharp.MSBuild.Tests/ProjectFileInfoTests.cs b/tests/OmniSharp.MSBuild.Tests/ProjectFileInfoTests.cs
@@ -26,8 +26,9 @@ public ProjectFileInfoTests(ITestOutputHelper output)
         private ProjectFileInfo CreateProjectFileInfo(OmniSharpTestHost host, ITestProject testProject, string projectFilePath)
         {
             var msbuildLocator = host.GetExport<IMSBuildLocator>();
+            var sdksPathResolver = host.GetExport<SdksPathResolver>();
 
-            return ProjectFileInfo.Create(projectFilePath, testProject.Directory, this._logger, msbuildLocator.RegisteredInstance);
+            return ProjectFileInfo.Create(projectFilePath, testProject.Directory, this._logger, msbuildLocator.RegisteredInstance, sdksPathResolver);
         }
 
         [Fact]