From 3bf3fa6ebd7c9b5974905ec2627a06d40d564a0c Mon Sep 17 00:00:00 2001 From: Dustin Campbell Date: Fri, 10 Nov 2017 07:15:34 -0800 Subject: [PATCH] Add back code that sets MSBuildSDKsPath variable for each project --- src/OmniSharp.MSBuild/MSBuildProjectSystem.cs | 7 +- .../ProjectFile/ProjectFileInfo.cs | 79 +++++++++--------- src/OmniSharp.MSBuild/SdksPathResolver.cs | 80 +++++++++++++++++++ .../ProjectFileInfoTests.cs | 3 +- 4 files changed, 128 insertions(+), 41 deletions(-) create mode 100644 src/OmniSharp.MSBuild/SdksPathResolver.cs diff --git a/src/OmniSharp.MSBuild/MSBuildProjectSystem.cs b/src/OmniSharp.MSBuild/MSBuildProjectSystem.cs index 6f9ba7e17f..4a847036aa 100644 --- 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(), _msbuildInstance, _options, diagnostics); + project = ProjectFileInfo.Create(projectFilePath, _environment.TargetDirectory, _loggerFactory.CreateLogger(), _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(); - var newProjectFileInfo = oldProjectFileInfo.Reload(_environment.TargetDirectory, _loggerFactory.CreateLogger(), _msbuildInstance, _options, diagnostics); + var newProjectFileInfo = oldProjectFileInfo.Reload(_environment.TargetDirectory, _loggerFactory.CreateLogger(), _msbuildInstance, _sdksPathResolver, _options, diagnostics); if (newProjectFileInfo != null) { diff --git a/src/OmniSharp.MSBuild/ProjectFile/ProjectFileInfo.cs b/src/OmniSharp.MSBuild/ProjectFile/ProjectFileInfo.cs index 44dc067a2b..544d5399e4 100644 --- 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 diagnostics = null) + MSBuildInstance msbuildInstance, SdksPathResolver sdksPathResolver, MSBuildOptions options = null, ICollection 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 diagnostics, out ImmutableArray targetFrameworks) + MSBuildInstance msbuildInstance, SdksPathResolver sdksPathResolver, MSBuildOptions options, ICollection diagnostics, out ImmutableArray 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 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 diagnostics = null) + MSBuildInstance msbuildInstance, SdksPathResolver sdksPathResolver, MSBuildOptions options = null, ICollection 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 new file mode 100644 index 0000000000..3024e8e6fe --- /dev/null +++ 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 index 71210e14d4..469328ecaf 100644 --- 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(); + var sdksPathResolver = host.GetExport(); - return ProjectFileInfo.Create(projectFilePath, testProject.Directory, this._logger, msbuildLocator.RegisteredInstance); + return ProjectFileInfo.Create(projectFilePath, testProject.Directory, this._logger, msbuildLocator.RegisteredInstance, sdksPathResolver); } [Fact]