Eliminate build process hop by loading MSBuild.dll into CLI process

eng/Versions.props

@@ -95,9 +95,11 @@
     <MicrosoftBuildRuntimePackageVersion>$(MicrosoftBuildPackageVersion)</MicrosoftBuildRuntimePackageVersion>
     <MicrosoftBuildLocalizationPackageVersion>16.10.0-preview-21175-01</MicrosoftBuildLocalizationPackageVersion>
     <MicrosoftBuildUtilitiesCorePackageVersion>$(MicrosoftBuildPackageVersion)</MicrosoftBuildUtilitiesCorePackageVersion>
+    <MicrosoftBuildTasksCorePackageVersion>$(MicrosoftBuildPackageVersion)</MicrosoftBuildTasksCorePackageVersion>
     <MicrosoftBuildVersion>$(MicrosoftBuildPackageVersion)</MicrosoftBuildVersion>
     <MicrosoftBuildFrameworkVersion>$(MicrosoftBuildFrameworkPackageVersion)</MicrosoftBuildFrameworkVersion>
     <MicrosoftBuildUtilitiesCoreVersion>$(MicrosoftBuildUtilitiesCorePackageVersion)</MicrosoftBuildUtilitiesCoreVersion>
+    <MicrosoftBuildTasksCoreVersion>$(MicrosoftBuildTasksCorePackageVersion)</MicrosoftBuildTasksCoreVersion>
   </PropertyGroup>
   <PropertyGroup>
     <!-- Dependencies from https://github.com/dotnet/templating -->

src/Cli/Microsoft.DotNet.Cli.Utils/MSBuildForwardingAppWithoutLogging.cs

@@ -1,6 +1,8 @@
 // Copyright (c) .NET Foundation and contributors. All rights reserved.
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
 
+#if NET
+
 using System;
 using System.Collections.Generic;
 using System.Diagnostics;
@@ -13,14 +15,26 @@ namespace Microsoft.DotNet.Cli.Utils
 {
     internal class MSBuildForwardingAppWithoutLogging
     {
+        private static readonly bool AlwaysExecuteMSBuildOutOfProc = Env.GetEnvironmentVariableAsBool("DOTNET_CLI_RUN_MSBUILD_OUTOFPROC");
+
         private const string MSBuildExeName = "MSBuild.dll";
 
         private const string SdksDirectoryName = "Sdks";
 
-        private readonly ForwardingAppImplementation _forwardingApp;
+        // Null if we're running MSBuild in-proc.
+        private ForwardingAppImplementation _forwardingApp;
+
+        // Command line arguments we're asked to forward to MSBuild.
+        private readonly IEnumerable<string> _argsToForward;
 
         internal static string MSBuildExtensionsPathTestHook = null;
 
+        // Path to the MSBuild binary to use.
+        public string MSBuildPath { get; }
+
+        // True if, given current state of the class, MSBuild would be executed in its own process.
+        public bool ExecuteMSBuildOutOfProc => _forwardingApp != null;
+
         private readonly Dictionary<string, string> _msbuildRequiredEnvironmentVariables =
             new Dictionary<string, string>
             {
@@ -33,27 +47,106 @@ internal class MSBuildForwardingAppWithoutLogging
             new List<string> { "-maxcpucount", "-verbosity:m" };
 
         public MSBuildForwardingAppWithoutLogging(IEnumerable<string> argsToForward, string msbuildPath = null)
+        {
+            string defaultMSBuildPath = GetMSBuildExePath();
+
+            _argsToForward = argsToForward;
+            MSBuildPath = msbuildPath ?? defaultMSBuildPath;
+
+            // If DOTNET_CLI_RUN_MSBUILD_OUTOFPROC is set or we're asked to execute a non-default binary, call MSBuild out-of-proc.
+            if (AlwaysExecuteMSBuildOutOfProc || !string.Equals(MSBuildPath, defaultMSBuildPath, StringComparison.OrdinalIgnoreCase))
+            {
+                InitializeForOutOfProcForwarding();
+            }
+        }
+
+        private void InitializeForOutOfProcForwarding()
         {
             _forwardingApp = new ForwardingAppImplementation(
-                msbuildPath ?? GetMSBuildExePath(),
-                _msbuildRequiredParameters.Concat(argsToForward.Select(Escape)),
+                MSBuildPath,
+                _msbuildRequiredParameters.Concat(_argsToForward.Select(Escape)),
                 environmentVariables: _msbuildRequiredEnvironmentVariables);
         }
 
-        public virtual ProcessStartInfo GetProcessStartInfo()
+        public ProcessStartInfo GetProcessStartInfo()
+        {
+            Debug.Assert(_forwardingApp != null, "Can't get ProcessStartInfo when not executing out-of-proc");
+            return _forwardingApp.GetProcessStartInfo();
+        }
+
+        public string[] GetAllArguments()
         {
-            return _forwardingApp
-                .GetProcessStartInfo();
+            return _msbuildRequiredParameters.Concat(_argsToForward.Select(Escape)).ToArray();
         }
 
         public void EnvironmentVariable(string name, string value)
         {
-            _forwardingApp.WithEnvironmentVariable(name, value);
+            if (_forwardingApp != null)
+            {
+                _forwardingApp.WithEnvironmentVariable(name, value);
+            }
+            else
+            {
+                _msbuildRequiredEnvironmentVariables.Add(name, value);
+            }
+
+            if (value == string.Empty || value == "\0")
+            {
+                // Unlike ProcessStartInfo.EnvironmentVariables, Environment.SetEnvironmentVariable can't set a variable
+                // to an empty value, so we just fall back to calling MSBuild out-of-proc if we encounter this case.
+                // https://github.com/dotnet/runtime/issues/50554
+                InitializeForOutOfProcForwarding();
+            }
         }
 
         public int Execute()
         {
-            return GetProcessStartInfo().Execute();
+            if (_forwardingApp != null)
+            {
+                return GetProcessStartInfo().Execute();
+            }
+            else
+            {
+                return ExecuteInProc(GetAllArguments());
+            }
+        }
+
+        public int ExecuteInProc(string[] arguments)
+        {
+            // Save current environment variables before overwriting them.
+            Dictionary<string, string> savedEnvironmentVariables = new Dictionary<string, string>();
+            try
+            {
+                foreach (KeyValuePair<string, string> kvp in _msbuildRequiredEnvironmentVariables)
+                {
+                    savedEnvironmentVariables[kvp.Key] = Environment.GetEnvironmentVariable(kvp.Key);
+                    Environment.SetEnvironmentVariable(kvp.Key, kvp.Value);
+                }
+
+                try
+                {
+                    // Execute MSBuild in the current process by calling its Main method.
+                    return Microsoft.Build.CommandLine.MSBuildApp.Main(arguments);
+                }
+                catch (Exception exception)
+                {
+                    // MSBuild, like all well-behaved CLI tools, handles all exceptions. In the unlikely case
+                    // that something still escapes, we print the exception and fail the call. Non-localized
+                    // string is OK here.
+                    Console.Error.Write("Unhandled exception: ");
+                    Console.Error.WriteLine(exception.ToString());
+
+                    return unchecked((int)0xe0434352); // EXCEPTION_COMPLUS
+                }
+            }
+            finally
+            {
+                // Restore saved environment variables.
+                foreach (KeyValuePair<string, string> kvp in savedEnvironmentVariables)
+                {
+                    Environment.SetEnvironmentVariable(kvp.Key, kvp.Value);
+                }
+            }
         }
 
         private static string Escape(string arg) =>
@@ -99,3 +192,4 @@ private static bool IsRestoreSources(string arg)
     }
 }
 
+#endif

src/Cli/Microsoft.DotNet.Cli.Utils/Microsoft.DotNet.Cli.Utils.csproj

@@ -14,7 +14,22 @@
     <EmbeddedResource Update="**\*.resx" GenerateSource="true" />
   </ItemGroup>
 
+  <Target Name="VerifyMSBuildDependency" BeforeTargets="ResolveAssemblyReferences" Condition="'$(TargetFramework)' == '$(SdkTargetFramework)'">
+    <PropertyGroup>
+      <MSBuildPathInPackage>$(PkgMicrosoft_Build_Runtime)\contentFiles\any\net5.0\MSBuild.dll</MSBuildPathInPackage>
+    </PropertyGroup>
+    <Error Condition="!Exists('$(MSBuildPathInPackage)')" Text="Something moved around in Microsoft.Build.Runtime, adjust code here accordingly." />
+    <ItemGroup>
+      <Reference Include="$(MSBuildPathInPackage)" />
+    </ItemGroup>
+  </Target>
+
   <ItemGroup>
+    <PackageReference Include="Microsoft.Build.Runtime"
+        ExcludeAssets="all"
+        Version="$(MicrosoftBuildRuntimePackageVersion)"
+        Condition="'$(TargetFramework)' == '$(SdkTargetFramework)'"
+        GeneratePathProperty="true" />
     <PackageReference Include="Microsoft.Extensions.DependencyModel" Version="$(MicrosoftExtensionsDependencyModelPackageVersion)" />
     <PackageReference Include="NuGet.Versioning" Version="$(NuGetVersioningPackageVersion)" />
     <PackageReference Include="NuGet.Packaging" Version="$(NuGetPackagingPackageVersion)" />

src/Cli/dotnet/CommandFactory/CommandResolution/ProjectToolsCommandResolver.cs

@@ -399,9 +399,40 @@ internal void GenerateDepsJsonFile(
             Reporter.Verbose.WriteLine(string.Format(LocalizableStrings.MSBuildArgs,
                 ArgumentEscaper.EscapeAndConcatenateArgArrayForProcessStart(args)));
 
-            var result = new MSBuildForwardingAppWithoutLogging(args, msBuildExePath)
-                .GetProcessStartInfo()
-                .ExecuteAndCaptureOutput(out string stdOut, out string stdErr);
+            int result;
+            string stdOut;
+            string stdErr;
+
+            var forwardingAppWithoutLogging = new MSBuildForwardingAppWithoutLogging(args, msBuildExePath);
+            if (forwardingAppWithoutLogging.ExecuteMSBuildOutOfProc)
+            {
+                result = forwardingAppWithoutLogging
+                    .GetProcessStartInfo()
+                    .ExecuteAndCaptureOutput(out stdOut, out stdErr);
+            }
+            else
+            {
+                // Execute and capture output of MSBuild running in-process.
+                var outWriter = new StringWriter();
+                var errWriter = new StringWriter();
+                var savedOutWriter = Console.Out;
+                var savedErrWriter = Console.Error;
+                try
+                {
+                    Console.SetOut(outWriter);
+                    Console.SetError(errWriter);
+
+                    result = forwardingAppWithoutLogging.Execute();
+
+                    stdOut = outWriter.ToString();
+                    stdErr = errWriter.ToString();
+                }
+                finally
+                {
+                    Console.SetOut(savedOutWriter);
+                    Console.SetError(savedErrWriter);
+                }
+            }
 
             if (result != 0)
             {

src/Cli/dotnet/PerformanceLogEventSource.cs

@@ -264,11 +264,11 @@ internal void MemoryConfiguration(int memoryLoad, int availablePhysicalMB, int t
         }
 
         [NonEvent]
-        internal void LogMSBuildStart(ProcessStartInfo startInfo)
+        internal void LogMSBuildStart(string fileName, string arguments)
         {
             if (IsEnabled())
             {
-                MSBuildStart($"{startInfo.FileName} {startInfo.Arguments}");
+                MSBuildStart($"{fileName} {arguments}");
             }
         }
 

src/Cli/dotnet/commands/dotnet-msbuild/MSBuildForwardingApp.cs

@@ -69,17 +69,42 @@ public ProcessStartInfo GetProcessStartInfo()
             return _forwardingAppWithoutLogging.GetProcessStartInfo();
         }
 
+        /// <summary>
+        /// Test hook returning concatenated and escaped command line arguments that would be passed to MSBuild.
+        /// </summary>
+        internal string GetArgumentsToMSBuild()
+        {
+            var argumentsUnescaped = _forwardingAppWithoutLogging.GetAllArguments();
+            return Cli.Utils.ArgumentEscaper.EscapeAndConcatenateArgArrayForProcessStart(argumentsUnescaped);
+        }
+
         public virtual int Execute()
         {
             // Ignore Ctrl-C for the remainder of the command's execution
             // Forwarding commands will just spawn the child process and exit
             Console.CancelKeyPress += (sender, e) => { e.Cancel = true; };
 
-            ProcessStartInfo startInfo = GetProcessStartInfo();
+            int exitCode;
+            if (_forwardingAppWithoutLogging.ExecuteMSBuildOutOfProc)
+            {
+                ProcessStartInfo startInfo = GetProcessStartInfo();
 
-            PerformanceLogEventSource.Log.LogMSBuildStart(startInfo);
-            int exitCode = startInfo.Execute();
-            PerformanceLogEventSource.Log.MSBuildStop(exitCode);
+                PerformanceLogEventSource.Log.LogMSBuildStart(startInfo.FileName, startInfo.Arguments);
+                exitCode = startInfo.Execute();
+                PerformanceLogEventSource.Log.MSBuildStop(exitCode);
+            }
+            else
+            {
+                string[] arguments = _forwardingAppWithoutLogging.GetAllArguments();
+                if (PerformanceLogEventSource.Log.IsEnabled())
+                {
+                    PerformanceLogEventSource.Log.LogMSBuildStart(
+                        _forwardingAppWithoutLogging.MSBuildPath,
+                        ArgumentEscaper.EscapeAndConcatenateArgArrayForProcessStart(arguments));
+                }
+                exitCode = _forwardingAppWithoutLogging.ExecuteInProc(arguments);
+                PerformanceLogEventSource.Log.MSBuildStop(exitCode);
+            }
 
             return exitCode;
         }

src/Tests/Microsoft.NET.TestFramework/TestContext.cs

@@ -208,8 +208,10 @@ public static void Initialize(TestCommandLine commandLine)
                 "MSBuildSDKsPath",
                 Path.Combine(testContext.ToolsetUnderTest.SdksPath));
 
+#if NETCOREAPP
             DotNet.Cli.Utils.MSBuildForwardingAppWithoutLogging.MSBuildExtensionsPathTestHook =
                 testContext.ToolsetUnderTest.SdkFolderUnderTest;
+#endif
         }
 
         public static string GetRepoRoot()

src/Tests/dotnet.Tests/ParserTests/MSBuildArgumentCommandLineParserTests.cs

@@ -31,7 +31,7 @@ public void MSBuildArgumentsAreForwardedCorrectly(string[] arguments, bool build
             RestoringCommand command = buildCommand ? 
                 BuildCommand.FromArgs(arguments) : 
                 PublishCommand.FromArgs(arguments);
-            command.GetProcessStartInfo().Arguments.Split(' ')
+            command.GetArgumentsToMSBuild().Split(' ')
                 .Should()
                 .Contain(arguments);
         }

src/Tests/dotnet.Tests/dotnet-msbuild/DotnetMsbuildInProcTests.cs

@@ -60,22 +60,12 @@ private string[] GetArgsForMSBuild(Func<bool> sentinelExists, out Telemetry.Tele
 
             MSBuildForwardingApp msBuildForwardingApp = new MSBuildForwardingApp(Enumerable.Empty<string>());
 
-            object forwardingAppWithoutLogging = msBuildForwardingApp
+            var forwardingAppWithoutLogging = msBuildForwardingApp
                 .GetType()
                 .GetField("_forwardingAppWithoutLogging", BindingFlags.Instance | BindingFlags.NonPublic)
-                ?.GetValue(msBuildForwardingApp);
+                ?.GetValue(msBuildForwardingApp) as Cli.Utils.MSBuildForwardingAppWithoutLogging;
 
-            FieldInfo forwardingAppFieldInfo = forwardingAppWithoutLogging
-                .GetType()
-                .GetField("_forwardingApp", BindingFlags.Instance | BindingFlags.NonPublic);
-
-            object forwardingApp = forwardingAppFieldInfo?.GetValue(forwardingAppWithoutLogging);
-
-            FieldInfo allArgsFieldinfo = forwardingApp?
-                .GetType()
-                .GetField("_allArgs", BindingFlags.Instance | BindingFlags.NonPublic);
-
-            return allArgsFieldinfo?.GetValue(forwardingApp) as string[];
+            return forwardingAppWithoutLogging?.GetAllArguments();
         }
     }
     public sealed class MockFirstTimeUseNoticeSentinel : IFirstTimeUseNoticeSentinel

src/Tests/dotnet.Tests/dotnet-msbuild/GivenDotnetBuildInvocation.cs

@@ -9,7 +9,7 @@ namespace Microsoft.DotNet.Cli.MSBuild.Tests
 {
     public class GivenDotnetBuildInvocation : IClassFixture<NullCurrentSessionIdFixture>
     {
-        const string ExpectedPrefix = "exec <msbuildpath> -maxcpucount -verbosity:m";
+        const string ExpectedPrefix = "-maxcpucount -verbosity:m";
 
         private static readonly string WorkingDirectory =
             TestPathUtilities.FormatAbsolutePath(nameof(GivenDotnetBuildInvocation));
@@ -46,8 +46,8 @@ public void MsbuildInvocationIsCorrect(string[] args, string expectedAdditionalA
 
                 command.SeparateRestoreCommand.Should().BeNull();
 
-                command.GetProcessStartInfo()
-                    .Arguments.Should()
+                command.GetArgumentsToMSBuild()
+                    .Should()
                     .Be($"{ExpectedPrefix} -restore -consoleloggerparameters:Summary{expectedAdditionalArgs}");
             });
         }
@@ -76,12 +76,12 @@ public void MsbuildInvocationIsCorrectForSeparateRestore(
                 var msbuildPath = "<msbuildpath>";
                 var command = BuildCommand.FromArgs(args, msbuildPath);
 
-                command.SeparateRestoreCommand.GetProcessStartInfo()
-                    .Arguments.Should()
+                command.SeparateRestoreCommand.GetArgumentsToMSBuild()
+                    .Should()
                     .Be($"{ExpectedPrefix} {expectedAdditionalArgsForRestore}");
 
-                command.GetProcessStartInfo()
-                    .Arguments.Should()
+                command.GetArgumentsToMSBuild()
+                    .Should()
                     .Be($"{ExpectedPrefix} -nologo -consoleloggerparameters:Summary{expectedAdditionalArgs}");
             });