Merge pull request #70240 from jasonmalinowski/parse-solution-files-oop

Load solutions out-of-proc

src/Features/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/HostWorkspace/BuildHostProcessManager.cs

@@ -41,7 +41,7 @@ public async Task<IBuildHost> GetBuildHostAsync(string projectFilePath, Cancella
         // Check if this is the case.
         if (neededBuildHostKind == BuildHostProcessKind.NetFramework)
         {
-            if (!await buildHost.IsProjectFileSupportedAsync(projectFilePath, cancellationToken))
+            if (!await buildHost.HasUsableMSBuildAsync(projectFilePath, cancellationToken))
             {
                 // It's not usable, so we'll fall back to the .NET Core one.
                 _logger?.LogWarning($"An installation of Visual Studio or the Build Tools for Visual Studio could not be found; {projectFilePath} will be loaded with the .NET Core SDK and may encounter errors.");
@@ -52,7 +52,7 @@ public async Task<IBuildHost> GetBuildHostAsync(string projectFilePath, Cancella
         return buildHost;
     }
 
-    private async Task<IBuildHost> GetBuildHostAsync(BuildHostProcessKind buildHostKind, CancellationToken cancellationToken)
+    public async Task<IBuildHost> GetBuildHostAsync(BuildHostProcessKind buildHostKind, CancellationToken cancellationToken)
     {
         using (await _gate.DisposableWaitAsync(cancellationToken).ConfigureAwait(false))
         {
@@ -211,7 +211,7 @@ private static BuildHostProcessKind GetKindForProject(string projectFilePath)
         return BuildHostProcessKind.NetFramework;
     }
 
-    private enum BuildHostProcessKind
+    public enum BuildHostProcessKind
     {
         NetCore,
         NetFramework

src/Features/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/HostWorkspace/LanguageServerProjectSystem.cs

@@ -6,8 +6,6 @@
 using System.Collections.Immutable;
 using System.Composition;
 using System.Diagnostics;
-using System.Runtime.CompilerServices;
-using Microsoft.Build.Locator;
 using Microsoft.CodeAnalysis.Collections;
 using Microsoft.CodeAnalysis.Host;
 using Microsoft.CodeAnalysis.Host.Mef;
@@ -16,7 +14,6 @@
 using Microsoft.CodeAnalysis.Options;
 using Microsoft.CodeAnalysis.ProjectSystem;
 using Microsoft.CodeAnalysis.Shared.TestHooks;
-using Microsoft.CodeAnalysis.Workspaces.MSBuild.BuildHost;
 using Microsoft.CodeAnalysis.Workspaces.ProjectSystem;
 using Microsoft.Extensions.Logging;
 using Microsoft.VisualStudio.Composition;
@@ -29,11 +26,9 @@ namespace Microsoft.CodeAnalysis.LanguageServer.HostWorkspace;
 internal sealed class LanguageServerProjectSystem
 {
     /// <summary>
-    /// A single gate for code that is adding work to <see cref="_projectsToLoadAndReload" /> and modifying <see cref="_msbuildLoaded" />.
-    /// This is just we don't have code simultaneously trying to load and unload solutions at once.
+    /// A single gate for code that is adding work to <see cref="_projectsToLoadAndReload" />. This is just we don't have code simultaneously trying to load and unload solutions at once.
     /// </summary>
     private readonly SemaphoreSlim _gate = new SemaphoreSlim(initialCount: 1);
-    private bool _msbuildLoaded = false;
 
     /// <summary>
     /// The suffix to use for the binary log name; incremented each time we have a new build. Should be incremented with <see cref="Interlocked.Increment(ref int)"/>.
@@ -88,28 +83,24 @@ public LanguageServerProjectSystem(
     }
 
     public async Task OpenSolutionAsync(string solutionFilePath)
-    {
-        if (await TryEnsureMSBuildLoadedAsync(Path.GetDirectoryName(solutionFilePath)!))
-            await OpenSolutionCoreAsync(solutionFilePath);
-    }
-
-    [MethodImpl(MethodImplOptions.NoInlining)] // Don't inline; the caller needs to ensure MSBuild is loaded before we can use MSBuild types here
-    private async Task OpenSolutionCoreAsync(string solutionFilePath)
     {
         using (await _gate.DisposableWaitAsync())
         {
             _logger.LogInformation($"Loading {solutionFilePath}...");
-            var solutionFile = Microsoft.Build.Construction.SolutionFile.Parse(solutionFilePath);
             _workspaceFactory.ProjectSystemProjectFactory.SolutionPath = solutionFilePath;
 
-            foreach (var project in solutionFile.ProjectsInOrder)
-            {
-                if (project.ProjectType == Microsoft.Build.Construction.SolutionProjectType.SolutionFolder)
-                {
-                    continue;
-                }
+            // We'll load solutions out-of-proc, since it's possible we might be running on a runtime that doesn't have a matching SDK installed,
+            // and we don't want any MSBuild registration to set environment variables in our process that might impact child processes.
+            await using var buildHostProcessManager = new BuildHostProcessManager(_loggerFactory);
+            var buildHost = await buildHostProcessManager.GetBuildHostAsync(BuildHostProcessManager.BuildHostProcessKind.NetCore, CancellationToken.None);
+
+            // If we don't have a .NET Core SDK on this machine at all, try .NET Framework
+            if (!await buildHost.HasUsableMSBuildAsync(solutionFilePath, CancellationToken.None))
+                buildHost = await buildHostProcessManager.GetBuildHostAsync(BuildHostProcessManager.BuildHostProcessKind.NetFramework, CancellationToken.None);
 
-                _projectsToLoadAndReload.AddWork(new ProjectToLoad(project.AbsolutePath, project.ProjectGuid));
+            foreach (var project in await buildHost.GetProjectsInSolutionAsync(solutionFilePath, CancellationToken.None))
+            {
+                _projectsToLoadAndReload.AddWork(new ProjectToLoad(project.ProjectPath, project.ProjectGuid));
             }
 
             // Wait for the in progress batch to complete and send a project initialized notification to the client.
@@ -133,53 +124,15 @@ public async Task OpenProjectsAsync(ImmutableArray<string> projectFilePaths)
         }
     }
 
-    private async Task<bool> TryEnsureMSBuildLoadedAsync(string workingDirectory)
-    {
-        using (await _gate.DisposableWaitAsync())
-        {
-            if (_msbuildLoaded)
-            {
-                return true;
-            }
-            else
-            {
-                var msbuildDiscoveryOptions = new VisualStudioInstanceQueryOptions { DiscoveryTypes = DiscoveryType.DotNetSdk, WorkingDirectory = workingDirectory };
-                var msbuildInstances = MSBuildLocator.QueryVisualStudioInstances(msbuildDiscoveryOptions);
-                var msbuildInstance = msbuildInstances.FirstOrDefault();
-
-                if (msbuildInstance != null)
-                {
-                    MSBuildLocator.RegisterInstance(msbuildInstance);
-                    _logger.LogInformation($"Loaded MSBuild in-process from {msbuildInstance.MSBuildPath}");
-                    _msbuildLoaded = true;
-
-                    return true;
-                }
-                else
-                {
-                    _logger.LogError($"Unable to find a MSBuild to use to load {workingDirectory}.");
-                    await ShowToastNotification.ShowToastNotificationAsync(LSP.MessageType.Error, LanguageServerResources.There_were_problems_loading_your_projects_See_log_for_details, CancellationToken.None, ShowToastNotification.ShowCSharpLogsCommand);
-
-                    return false;
-                }
-            }
-        }
-    }
-
     private async ValueTask LoadOrReloadProjectsAsync(ImmutableSegmentedList<ProjectToLoad> projectPathsToLoadOrReload, CancellationToken cancellationToken)
     {
         var stopwatch = Stopwatch.StartNew();
 
         // TODO: support configuration switching
 
         var binaryLogPath = GetMSBuildBinaryLogPath();
-        var runBuildInProcess = _globalOptionService.GetOption(LanguageServerProjectSystemOptionsStorage.LoadInProcess);
-
-        if (runBuildInProcess)
-            _logger.LogInformation("In-process project loading is enabled.");
 
-        await using var buildHostProcessManager = !runBuildInProcess ? new BuildHostProcessManager(_loggerFactory, binaryLogPath) : null;
-        var inProcessBuildHost = runBuildInProcess ? new BuildHost(_loggerFactory, binaryLogPath) : null;
+        await using var buildHostProcessManager = new BuildHostProcessManager(_loggerFactory, binaryLogPath);
 
         var displayedToast = 0;
 
@@ -191,7 +144,7 @@ private async ValueTask LoadOrReloadProjectsAsync(ImmutableSegmentedList<Project
             {
                 tasks.Add(Task.Run(async () =>
                 {
-                    var errorKind = await LoadOrReloadProjectAsync(projectToLoad, buildHostProcessManager, inProcessBuildHost, cancellationToken);
+                    var errorKind = await LoadOrReloadProjectAsync(projectToLoad, buildHostProcessManager, cancellationToken);
                     if (errorKind is LSP.MessageType.Error)
                     {
                         // We should display a toast when the value of displayedToast is 0.  This will also update the value to 1 meaning we won't send any more toasts.
@@ -210,9 +163,6 @@ private async ValueTask LoadOrReloadProjectsAsync(ImmutableSegmentedList<Project
         finally
         {
             _logger.LogInformation($"Completed (re)load of all projects in {stopwatch.Elapsed}");
-
-            if (inProcessBuildHost != null)
-                await inProcessBuildHost.ShutdownAsync();
         }
     }
 
@@ -229,14 +179,13 @@ private async ValueTask LoadOrReloadProjectsAsync(ImmutableSegmentedList<Project
         return binaryLogPath;
     }
 
-    private async Task<LSP.MessageType?> LoadOrReloadProjectAsync(ProjectToLoad projectToLoad, BuildHostProcessManager? buildHostProcessManager, BuildHost? inProcessBuildHost, CancellationToken cancellationToken)
+    private async Task<LSP.MessageType?> LoadOrReloadProjectAsync(ProjectToLoad projectToLoad, BuildHostProcessManager buildHostProcessManager, CancellationToken cancellationToken)
     {
         try
         {
             var projectPath = projectToLoad.Path;
 
-            // If we have a process manager, then get an OOP process; otherwise we're still using in-proc builds so just fetch one in-process
-            var buildHost = inProcessBuildHost ?? await buildHostProcessManager!.GetBuildHostAsync(projectPath, cancellationToken);
+            var buildHost = await buildHostProcessManager!.GetBuildHostAsync(projectPath, cancellationToken);
 
             if (await buildHost.IsProjectFileSupportedAsync(projectPath, cancellationToken))
             {

src/Features/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/Microsoft.CodeAnalysis.LanguageServer.csproj

@@ -64,8 +64,6 @@
   </ItemGroup>
 
   <ItemGroup>
-    <PackageReference Include="Microsoft.Build" Version="$(RefOnlyMicrosoftBuildVersion)" ExcludeAssets="Runtime" PrivateAssets="All" />
-    <PackageReference Include="Microsoft.Build.Locator" Version="$(MicrosoftBuildLocatorVersion)" />
     <PackageReference Include="Microsoft.Extensions.Logging" Version="$(MicrosoftExtensionsLoggingVersion)" />
     <PackageReference Include="Microsoft.Extensions.Logging.Abstractions" Version="$(MicrosoftExtensionsLoggingAbstractionsVersion)" />
     <PackageReference Include="Microsoft.Extensions.Logging.Console" Version="$(MicrosoftExtensionsLoggingConsoleVersion)" />

src/Features/LanguageServer/Protocol/Features/Options/LanguageServerProjectSystemOptionsStorage.cs

@@ -14,11 +14,5 @@ internal static class LanguageServerProjectSystemOptionsStorage
         /// A folder to log binlogs to when running design-time builds.
         /// </summary>
         public static readonly Option2<string?> BinaryLogPath = new Option2<string?>("dotnet_binary_log_path", defaultValue: null, s_optionGroup);
-
-        /// <summary>
-        /// Whether we are doing design-time builds in-process; this is only to offer a fallback if the OOP builds are broken, and should be removed once
-        /// we don't have folks using this.
-        /// </summary>
-        public static readonly Option2<bool> LoadInProcess = new("dotnet_load_in_process", defaultValue: false, s_optionGroup);
     }
 }

src/Features/LanguageServer/Protocol/Handler/Configuration/DidChangeConfigurationNotificationHandler_OptionList.cs

@@ -61,7 +61,6 @@ internal partial class DidChangeConfigurationNotificationHandler
             LspOptionsStorage.LspEnableReferencesCodeLens,
             LspOptionsStorage.LspEnableTestsCodeLens,
             // Project system
-            LanguageServerProjectSystemOptionsStorage.BinaryLogPath,
-            LanguageServerProjectSystemOptionsStorage.LoadInProcess);
+            LanguageServerProjectSystemOptionsStorage.BinaryLogPath);
     }
 }

src/Features/LanguageServer/ProtocolUnitTests/Configuration/DidChangeConfigurationNotificationHandlerTest.cs

@@ -145,7 +145,6 @@ public void VerifyLspClientOptionNames()
                 "code_lens.dotnet_enable_references_code_lens",
                 "code_lens.dotnet_enable_tests_code_lens",
                 "projects.dotnet_binary_log_path",
-                "projects.dotnet_load_in_process",
             }.OrderBy(name => name);
 
             Assert.Equal(expectedNames, actualNames);

src/VisualStudio/Core/Test.Next/Options/VisualStudioOptionStorageTests.cs

@@ -226,7 +226,6 @@ public void OptionHasStorageIfNecessary(string configName)
             "dotnet_style_operator_placement_when_wrapping",                                // Doesn't have VS UI. TODO: https://github.com/dotnet/roslyn/issues/66062
             "dotnet_style_prefer_foreach_explicit_cast_in_source",                          // For a small customer segment, doesn't warrant VS UI.
             "dotnet_binary_log_path",                                                       // VSCode only option for the VS Code project system; does not apply to VS
-            "dotnet_load_in_process",                                                       // VSCode only option for the VS Code project system; does not apply to VS
             "dotnet_lsp_using_devkit",                                                      // VSCode internal only option.  Does not need any UI.
             "dotnet_enable_references_code_lens",                                           // VSCode only option.  Does not apply to VS.
             "dotnet_enable_tests_code_lens",                                                // VSCode only option.  Does not apply to VS.

src/Workspaces/Core/MSBuild.BuildHost/BuildHost.cs

@@ -9,6 +9,7 @@
 using System.Runtime.CompilerServices;
 using System.Threading;
 using System.Threading.Tasks;
+using Microsoft.Build.Construction;
 using Microsoft.Build.Locator;
 using Microsoft.Build.Logging;
 using Microsoft.CodeAnalysis.MSBuild;
@@ -32,7 +33,7 @@ public BuildHost(ILoggerFactory loggerFactory, string? binaryLogPath)
         _binaryLogPath = binaryLogPath;
     }
 
-    private bool TryEnsureMSBuildLoaded(string projectFilePath)
+    private bool TryEnsureMSBuildLoaded(string projectOrSolutionFilePath)
     {
         lock (_gate)
         {
@@ -56,7 +57,7 @@ private bool TryEnsureMSBuildLoaded(string projectFilePath)
 
             // Locate the right SDK for this particular project; MSBuildLocator ensures in this case the first one is the preferred one.
             // TODO: we should pick the appropriate instance back in the main process and just use the one chosen here.
-            var options = new VisualStudioInstanceQueryOptions { DiscoveryTypes = DiscoveryType.DotNetSdk, WorkingDirectory = Path.GetDirectoryName(projectFilePath) };
+            var options = new VisualStudioInstanceQueryOptions { DiscoveryTypes = DiscoveryType.DotNetSdk, WorkingDirectory = Path.GetDirectoryName(projectOrSolutionFilePath) };
             instance = MSBuildLocator.QueryVisualStudioInstances(options).FirstOrDefault();
 
 #endif
@@ -97,19 +98,41 @@ private void CreateBuildManager()
         }
     }
 
-    public Task<bool> IsProjectFileSupportedAsync(string projectFilePath, CancellationToken cancellationToken)
+    public Task<bool> HasUsableMSBuildAsync(string projectOrSolutionFilePath, CancellationToken cancellationToken)
     {
-        if (!TryEnsureMSBuildLoaded(projectFilePath))
-            return Task.FromResult(false);
+        return Task.FromResult(TryEnsureMSBuildLoaded(projectOrSolutionFilePath));
+    }
 
+    private void EnsureMSBuildLoaded(string projectFilePath)
+    {
+        Contract.ThrowIfFalse(TryEnsureMSBuildLoaded(projectFilePath), $"We don't have an MSBuild to use; {nameof(HasUsableMSBuildAsync)} should have been called first to check.");
+    }
+
+    public Task<ImmutableArray<(string ProjectPath, string ProjectGuid)>> GetProjectsInSolutionAsync(string solutionFilePath, CancellationToken cancellationToken)
+    {
+        EnsureMSBuildLoaded(solutionFilePath);
+        return Task.FromResult(GetProjectsInSolution(solutionFilePath));
+    }
+
+    [MethodImpl(MethodImplOptions.NoInlining)] // Do not inline this, since this uses MSBuild types which are being loaded by the caller
+    private static ImmutableArray<(string ProjectPath, string ProjectGuid)> GetProjectsInSolution(string solutionFilePath)
+    {
+        return SolutionFile.Parse(solutionFilePath).ProjectsInOrder
+            .Where(static p => p.ProjectType != SolutionProjectType.SolutionFolder)
+            .SelectAsArray(static p => (p.AbsolutePath, p.ProjectGuid));
+    }
+
+    public Task<bool> IsProjectFileSupportedAsync(string projectFilePath, CancellationToken cancellationToken)
+    {
+        EnsureMSBuildLoaded(projectFilePath);
         CreateBuildManager();
 
         return Task.FromResult(TryGetLoaderForPath(projectFilePath) is not null);
     }
 
     public async Task<IRemoteProjectFile> LoadProjectFileAsync(string projectFilePath, CancellationToken cancellationToken)
     {
-        Contract.ThrowIfFalse(TryEnsureMSBuildLoaded(projectFilePath), $"We don't have an MSBuild to use; {nameof(IsProjectFileSupportedAsync)} should have been called first.");
+        EnsureMSBuildLoaded(projectFilePath);
         CreateBuildManager();
 
         var projectLoader = TryGetLoaderForPath(projectFilePath);

src/Workspaces/Core/MSBuild.BuildHost/IBuildHost.cs

@@ -2,6 +2,7 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
+using System.Collections.Immutable;
 using System.Threading;
 using System.Threading.Tasks;
 
@@ -13,7 +14,14 @@ namespace Microsoft.CodeAnalysis.Workspaces.MSBuild.BuildHost;
 internal interface IBuildHost
 {
     /// <summary>
-    /// Returns whether this project's is supported by this host, considering both the project language and also MSBuild availability.
+    /// Returns true if this build host was able to discover a usable MSBuild instance. This should be called before calling other methods.
+    /// </summary>
+    Task<bool> HasUsableMSBuildAsync(string projectOrSolutionFilePath, CancellationToken cancellationToken);
+
+    Task<ImmutableArray<(string ProjectPath, string ProjectGuid)>> GetProjectsInSolutionAsync(string solutionFilePath, CancellationToken cancellationToken);
+
+    /// <summary>
+    /// Returns whether this project's is supported by this host.
     /// </summary>
     Task<bool> IsProjectFileSupportedAsync(string projectFilePath, CancellationToken cancellationToken);