CmdLine parsing was extracted from XMake and the implementation is visible to dotnet (attempt 2)

src/Build.UnitTests/Utilities_Tests.cs

@@ -80,17 +80,9 @@ public void CommentsInPreprocessing()
 
             env.SetEnvironmentVariable("MSBUILDLOADALLFILESASWRITEABLE", "1");
 
-#if FEATURE_GET_COMMANDLINE
-            MSBuildApp.Execute(@"c:\bin\msbuild.exe """ + inputFile.Path +
-                (NativeMethodsShared.IsUnixLike ? @""" -pp:""" : @""" /pp:""") + outputFile.Path + @"""")
-                .ShouldBe(MSBuildApp.ExitType.Success);
-#else
             Assert.Equal(
                 MSBuildApp.ExitType.Success,
-                MSBuildApp.Execute(
-                    new[] { @"c:\bin\msbuild.exe", '"' + inputFile.Path + '"',
-                '"' + (NativeMethodsShared.IsUnixLike ? "-pp:" : "/pp:") + outputFile.Path + '"'}));
-#endif
+                MSBuildApp.Execute([ @"c:\bin\msbuild.exe", '"' + inputFile.Path + '"', '"' + (NativeMethodsShared.IsUnixLike ? "-pp:" : "/pp:") + outputFile.Path + '"']));
 
             bool foundDoNotModify = false;
             foreach (string line in File.ReadLines(outputFile.Path))

src/Build/BackEnd/Client/MSBuildClient.cs

@@ -48,11 +48,7 @@ public sealed class MSBuildClient
         /// The command line to process.
         /// The first argument on the command line is assumed to be the name/path of the executable, and is ignored.
         /// </summary>
-#if FEATURE_GET_COMMANDLINE
-        private readonly string _commandLine;
-#else
         private readonly string[] _commandLine;
-#endif
 
         /// <summary>
         /// The MSBuild client execution result.
@@ -112,13 +108,7 @@ public sealed class MSBuildClient
         /// on the command line is assumed to be the name/path of the executable, and is ignored</param>
         /// <param name="msbuildLocation"> Full path to current MSBuild.exe if executable is MSBuild.exe,
         /// or to version of MSBuild.dll found to be associated with the current process.</param>
-        public MSBuildClient(
-#if FEATURE_GET_COMMANDLINE
-            string commandLine,
-#else
-            string[] commandLine,
-#endif
-            string msbuildLocation)
+        public MSBuildClient(string[] commandLine, string msbuildLocation)
         {
             _serverEnvironmentVariables = new();
             _exitResult = new();
@@ -162,12 +152,7 @@ private void CreateNodePipeStream()
         public MSBuildClientExitResult Execute(CancellationToken cancellationToken)
         {
             // Command line in one string used only in human readable content.
-            string descriptiveCommandLine =
-#if FEATURE_GET_COMMANDLINE
-                _commandLine;
-#else
-                string.Join(" ", _commandLine);
-#endif
+            string descriptiveCommandLine = string.Join(" ", _commandLine);
 
             CommunicationsUtilities.Trace("Executing build with command line '{0}'", descriptiveCommandLine);
 

src/Build/BackEnd/Node/OutOfProcServerNode.cs

@@ -25,12 +25,7 @@ public sealed class OutOfProcServerNode : INode, INodePacketFactory, INodePacket
         /// <summary>
         /// A callback used to execute command line build.
         /// </summary>
-        public delegate (int exitCode, string exitType) BuildCallback(
-#if FEATURE_GET_COMMANDLINE
-            string commandLine);
-#else
-            string[] commandLine);
-#endif
+        public delegate (int exitCode, string exitType) BuildCallback(string[] commandLine);
 
         private readonly BuildCallback _buildFunction;
 

src/Build/BackEnd/Node/ServerNodeBuildCommand.cs

@@ -14,11 +14,7 @@ namespace Microsoft.Build.BackEnd
     /// </summary>
     internal sealed class ServerNodeBuildCommand : INodePacket
     {
-#if FEATURE_GET_COMMANDLINE
-        private string _commandLine = default!;
-#else
         private string[] _commandLine = default!;
-#endif
         private string _startupDirectory = default!;
         private Dictionary<string, string> _buildProcessEnvironment = default!;
         private CultureInfo _culture = default!;
@@ -34,11 +30,7 @@ internal sealed class ServerNodeBuildCommand : INodePacket
         /// <summary>
         /// Command line including arguments
         /// </summary>
-#if FEATURE_GET_COMMANDLINE
-        public string CommandLine => _commandLine;
-#else
         public string[] CommandLine => _commandLine;
-#endif
 
         /// <summary>
         /// The startup directory
@@ -79,11 +71,7 @@ private ServerNodeBuildCommand()
         }
 
         public ServerNodeBuildCommand(
-#if FEATURE_GET_COMMANDLINE
-            string commandLine,
-#else
             string[] commandLine,
-#endif
             string startupDirectory,
             Dictionary<string, string> buildProcessEnvironment,
             CultureInfo culture, CultureInfo uiCulture,

src/Build/CompatibilitySuppressions.xml

@@ -1,3 +1,48 @@
 <?xml version="1.0" encoding="utf-8"?>
 <!-- https://learn.microsoft.com/dotnet/fundamentals/package-validation/diagnostic-ids -->
-<Suppressions xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:xsd="http://www.w3.org/2001/XMLSchema" />
+<Suppressions xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:xsd="http://www.w3.org/2001/XMLSchema">
+  <!-- MSBuildClient is in experimental namespace and not used outside the MSBuild -->
+  <!-- OutOfProcServerNode.BuildCallback is in experimental namespace and not used outside the MSBuild -->
+  <Suppression>
+    <DiagnosticId>CP0002</DiagnosticId>
+    <Target>M:Microsoft.Build.Experimental.MSBuildClient.#ctor(System.String,System.String)</Target>
+    <Left>lib/net472/Microsoft.Build.dll</Left>
+    <Right>lib/net472/Microsoft.Build.dll</Right>
+    <IsBaselineSuppression>true</IsBaselineSuppression>
+  </Suppression>
+  <Suppression>
+    <DiagnosticId>CP0002</DiagnosticId>
+    <Target>M:Microsoft.Build.Experimental.OutOfProcServerNode.BuildCallback.BeginInvoke(System.String,System.AsyncCallback,System.Object)</Target>
+    <Left>lib/net472/Microsoft.Build.dll</Left>
+    <Right>lib/net472/Microsoft.Build.dll</Right>
+    <IsBaselineSuppression>true</IsBaselineSuppression>
+  </Suppression>
+  <Suppression>
+    <DiagnosticId>CP0002</DiagnosticId>
+    <Target>M:Microsoft.Build.Experimental.OutOfProcServerNode.BuildCallback.Invoke(System.String)</Target>
+    <Left>lib/net472/Microsoft.Build.dll</Left>
+    <Right>lib/net472/Microsoft.Build.dll</Right>
+    <IsBaselineSuppression>true</IsBaselineSuppression>
+  </Suppression>
+  <Suppression>
+    <DiagnosticId>CP0002</DiagnosticId>
+    <Target>M:Microsoft.Build.Experimental.MSBuildClient.#ctor(System.String,System.String)</Target>
+    <Left>ref/net472/Microsoft.Build.dll</Left>
+    <Right>ref/net472/Microsoft.Build.dll</Right>
+    <IsBaselineSuppression>true</IsBaselineSuppression>
+  </Suppression>
+  <Suppression>
+    <DiagnosticId>CP0002</DiagnosticId>
+    <Target>M:Microsoft.Build.Experimental.OutOfProcServerNode.BuildCallback.BeginInvoke(System.String,System.AsyncCallback,System.Object)</Target>
+    <Left>ref/net472/Microsoft.Build.dll</Left>
+    <Right>ref/net472/Microsoft.Build.dll</Right>
+    <IsBaselineSuppression>true</IsBaselineSuppression>
+  </Suppression>
+  <Suppression>
+    <DiagnosticId>CP0002</DiagnosticId>
+    <Target>M:Microsoft.Build.Experimental.OutOfProcServerNode.BuildCallback.Invoke(System.String)</Target>
+    <Left>ref/net472/Microsoft.Build.dll</Left>
+    <Right>ref/net472/Microsoft.Build.dll</Right>
+    <IsBaselineSuppression>true</IsBaselineSuppression>
+  </Suppression>
+</Suppressions>

src/Directory.BeforeCommon.targets

@@ -31,7 +31,6 @@
     <DefineConstants>$(DefineConstants);FEATURE_ENVIRONMENT_SYSTEMDIRECTORY</DefineConstants>
     <DefineConstants>$(DefineConstants);FEATURE_FILE_TRACKER</DefineConstants>
     <DefineConstants Condition="'$(MachineIndependentBuild)' != 'true'">$(DefineConstants);FEATURE_GAC</DefineConstants>
-    <DefineConstants>$(DefineConstants);FEATURE_GET_COMMANDLINE</DefineConstants>
     <DefineConstants>$(DefineConstants);FEATURE_HANDLEPROCESSCORRUPTEDSTATEEXCEPTIONS</DefineConstants>
     <DefineConstants>$(DefineConstants);FEATURE_HTTP_LISTENER</DefineConstants>
     <DefineConstants Condition="'$(MachineIndependentBuild)' != 'true'">$(DefineConstants);FEATURE_INSTALLED_MSBUILD</DefineConstants>

src/MSBuild.UnitTests/CommandLineSwitches_Tests.cs

@@ -1548,11 +1548,7 @@ public void ProcessInvalidTargetSwitch()
             using TestEnvironment testEnvironment = TestEnvironment.Create();
             string project = testEnvironment.CreateTestProjectWithFiles("project.proj", projectContent).ProjectFile;
 
-#if FEATURE_GET_COMMANDLINE
-            MSBuildApp.Execute(@"msbuild.exe " + project + " /t:foo.bar").ShouldBe(MSBuildApp.ExitType.SwitchError);
-#else
-            MSBuildApp.Execute(new[] { @"msbuild.exe", project, "/t:foo.bar" }).ShouldBe(MSBuildApp.ExitType.SwitchError);
-#endif
+            MSBuildApp.Execute([@"msbuild.exe", project, "/t:foo.bar"]).ShouldBe(MSBuildApp.ExitType.SwitchError);
         }
 
         /// <summary>

src/MSBuild.UnitTests/ProjectSchemaValidationHandler_Tests.cs

@@ -52,7 +52,7 @@ public void VerifyInvalidProjectSchema()
                     ");
                 string quotedProjectFilename = "\"" + projectFilename + "\"";
 
-                Assert.Equal(MSBuildApp.ExitType.InitializationError, MSBuildApp.Execute(@"c:\foo\msbuild.exe " + quotedProjectFilename + " /validate:\"" + msbuildTempXsdFilenames[0] + "\""));
+                Assert.Equal(MSBuildApp.ExitType.InitializationError, MSBuildApp.Execute([@"c:\foo\msbuild.exe", quotedProjectFilename, $"/validate:\"{msbuildTempXsdFilenames[0]}\""]));
             }
             finally
             {
@@ -95,7 +95,7 @@ public void VerifyInvalidSchemaItself1()
                     ");
                 string quotedProjectFile = "\"" + projectFilename + "\"";
 
-                Assert.Equal(MSBuildApp.ExitType.InitializationError, MSBuildApp.Execute(@"c:\foo\msbuild.exe " + quotedProjectFile + " /validate:\"" + invalidSchemaFile + "\""));
+                Assert.Equal(MSBuildApp.ExitType.InitializationError, MSBuildApp.Execute([@"c:\foo\msbuild.exe", quotedProjectFile, $"/validate:\"{invalidSchemaFile}\""]));
             }
             finally
             {
@@ -155,7 +155,7 @@ public void VerifyInvalidSchemaItself2()
 
                 string quotedProjectFile = "\"" + projectFilename + "\"";
 
-                Assert.Equal(MSBuildApp.ExitType.InitializationError, MSBuildApp.Execute(@"c:\foo\msbuild.exe " + quotedProjectFile + " /validate:\"" + invalidSchemaFile + "\""));
+                Assert.Equal(MSBuildApp.ExitType.InitializationError, MSBuildApp.Execute([@"c:\foo\msbuild.exe", quotedProjectFile, $"/validate:\"{invalidSchemaFile}\""]));
             }
             finally
             {
@@ -203,7 +203,7 @@ public void VerifyValidProjectSchema()
                 msbuildTempXsdFilenames = PrepareSchemaFiles();
                 string quotedProjectFile = "\"" + projectFilename + "\"";
 
-                Assert.Equal(MSBuildApp.ExitType.Success, MSBuildApp.Execute(@"c:\foo\msbuild.exe " + quotedProjectFile + " /validate:\"" + msbuildTempXsdFilenames[0] + "\""));
+                Assert.Equal(MSBuildApp.ExitType.Success, MSBuildApp.Execute([@"c:\foo\msbuild.exe", quotedProjectFile, $"/validate:\"{msbuildTempXsdFilenames[0]}\""]));
 
                 // ProjectSchemaValidationHandler.VerifyProjectSchema
                 //    (
@@ -256,7 +256,7 @@ public void VerifyInvalidImportNotCaughtBySchema()
                 msbuildTempXsdFilenames = PrepareSchemaFiles();
                 string quotedProjectFile = "\"" + projectFilename + "\"";
 
-                Assert.Equal(MSBuildApp.ExitType.Success, MSBuildApp.Execute(@"c:\foo\msbuild.exe " + quotedProjectFile + " /validate:\"" + msbuildTempXsdFilenames[0] + "\""));
+                Assert.Equal(MSBuildApp.ExitType.Success, MSBuildApp.Execute([@"c:\foo\msbuild.exe", quotedProjectFile, "/validate:\"{msbuildTempXsdFilenames[0]}\""]));
 
                 // ProjectSchemaValidationHandler.VerifyProjectSchema
                 //    (

src/MSBuild.UnitTests/XMake_Tests.cs

@@ -562,12 +562,7 @@ public void GetLengthOfSwitchIndicatorTest()
         [InlineData(@"/h")]
         public void Help(string indicator)
         {
-            MSBuildApp.Execute(
-#if FEATURE_GET_COMMANDLINE
-                @$"c:\bin\msbuild.exe {indicator} ")
-#else
-                new[] { @"c:\bin\msbuild.exe", indicator })
-#endif
+            MSBuildApp.Execute([@"c:\bin\msbuild.exe", indicator])
             .ShouldBe(MSBuildApp.ExitType.Success);
         }
 
@@ -660,19 +655,11 @@ public void VersionSwitchDisableChangeWave()
         public void ErrorCommandLine()
         {
             string oldValueForMSBuildLoadMicrosoftTargetsReadOnly = Environment.GetEnvironmentVariable("MSBuildLoadMicrosoftTargetsReadOnly");
-#if FEATURE_GET_COMMANDLINE
-            MSBuildApp.Execute(@"c:\bin\msbuild.exe -junk").ShouldBe(MSBuildApp.ExitType.SwitchError);
-
-            MSBuildApp.Execute(@"msbuild.exe -t").ShouldBe(MSBuildApp.ExitType.SwitchError);
-
-            MSBuildApp.Execute(@"msbuild.exe @bogus.rsp").ShouldBe(MSBuildApp.ExitType.InitializationError);
-#else
-            MSBuildApp.Execute(new[] { @"c:\bin\msbuild.exe", "-junk" }).ShouldBe(MSBuildApp.ExitType.SwitchError);
 
-            MSBuildApp.Execute(new[] { @"msbuild.exe", "-t" }).ShouldBe(MSBuildApp.ExitType.SwitchError);
+            MSBuildApp.Execute([@"c:\bin\msbuild.exe", "-junk"]).ShouldBe(MSBuildApp.ExitType.SwitchError);
+            MSBuildApp.Execute([@"msbuild.exe", "-t"]).ShouldBe(MSBuildApp.ExitType.SwitchError);
+            MSBuildApp.Execute([@"msbuild.exe", "@bogus.rsp"]).ShouldBe(MSBuildApp.ExitType.InitializationError);
 
-            MSBuildApp.Execute(new[] { @"msbuild.exe", "@bogus.rsp" }).ShouldBe(MSBuildApp.ExitType.InitializationError);
-#endif
             Environment.SetEnvironmentVariable("MSBuildLoadMicrosoftTargetsReadOnly", oldValueForMSBuildLoadMicrosoftTargetsReadOnly);
         }
 
@@ -1153,11 +1140,7 @@ public void TestEnvironmentTest()
                     sw.WriteLine(projectString);
                 }
                 // Should pass
-#if FEATURE_GET_COMMANDLINE
-                MSBuildApp.Execute(@"c:\bin\msbuild.exe " + quotedProjectFileName).ShouldBe(MSBuildApp.ExitType.Success);
-#else
-                MSBuildApp.Execute(new[] { @"c:\bin\msbuild.exe", quotedProjectFileName }).ShouldBe(MSBuildApp.ExitType.Success);
-#endif
+                MSBuildApp.Execute([@"c:\bin\msbuild.exe", quotedProjectFileName]).ShouldBe(MSBuildApp.ExitType.Success);
             }
             finally
             {
@@ -1190,21 +1173,15 @@ public void MSBuildEngineLogger()
                 {
                     sw.WriteLine(projectString);
                 }
-#if FEATURE_GET_COMMANDLINE
                 // Should pass
-                MSBuildApp.Execute(@$"c:\bin\msbuild.exe /logger:FileLogger,""Microsoft.Build, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a"";""LogFile={logFile}"" /verbosity:detailed " + quotedProjectFileName).ShouldBe(MSBuildApp.ExitType.Success);
-
-#else
-                // Should pass
-                MSBuildApp.Execute(
-                    new[]
-                        {
+                MSBuildApp
+                    .Execute([
                             NativeMethodsShared.IsWindows ? @"c:\bin\msbuild.exe" : "/msbuild.exe",
                             @$"/logger:FileLogger,""Microsoft.Build, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a"";""LogFile={logFile}""",
                             "/verbosity:detailed",
-                            quotedProjectFileName
-                        }).ShouldBe(MSBuildApp.ExitType.Success);
-#endif
+                            quotedProjectFileName])
+                    .ShouldBe(MSBuildApp.ExitType.Success);
+
                 File.Exists(logFile).ShouldBeTrue();
 
                 var logFileContents = File.ReadAllText(logFile);
@@ -2946,11 +2923,7 @@ public void ThrowsWhenMaxCpuCountTooLargeForMultiThreadedAndForceAllTasksOutOfPr
             testEnvironment.SetEnvironmentVariable("MSBUILDFORCEALLTASKSOUTOFPROC", "1");
             string project = testEnvironment.CreateTestProjectWithFiles("project.proj", projectContent).ProjectFile;
 
-#if FEATURE_GET_COMMANDLINE
-            MSBuildApp.Execute(@"c:\bin\msbuild.exe " + project + " / m:257 /mt").ShouldBe(MSBuildApp.ExitType.SwitchError);
-#else
-            MSBuildApp.Execute(new[] { @"c:\bin\msbuild.exe", project, "/m:257 /mt" }).ShouldBe(MSBuildApp.ExitType.SwitchError);
-#endif
+            MSBuildApp.Execute([@"c:\bin\msbuild.exe", project, "/m:257 /mt"]).ShouldBe(MSBuildApp.ExitType.SwitchError);
         }
 
         private string CopyMSBuild()

src/MSBuild/MSBuildClientApp.cs

@@ -34,43 +34,30 @@ internal static class MSBuildClientApp
         /// <remarks>
         /// The locations of msbuild exe/dll and dotnet.exe would be automatically detected if called from dotnet or msbuild cli. Calling this function from other executables might not work.
         /// </remarks>
-        public static MSBuildApp.ExitType Execute(
-#if FEATURE_GET_COMMANDLINE
-            string commandLine,
-#else
-            string[] commandLine,
-#endif
-            CancellationToken cancellationToken)
+        public static MSBuildApp.ExitType Execute(string[] commandLineArgs, CancellationToken cancellationToken)
         {
             string msbuildLocation = BuildEnvironmentHelper.Instance.CurrentMSBuildExePath;
 
             return Execute(
-                commandLine,
+                commandLineArgs,
                 msbuildLocation,
                 cancellationToken);
         }
 
         /// <summary>
         /// This is the entry point for the MSBuild client.
         /// </summary>
-        /// <param name="commandLine">The command line to process. The first argument
+        /// <param name="commandLineArgs">The command line to process. The first argument
         /// on the command line is assumed to be the name/path of the executable, and
         /// is ignored.</param>
         /// <param name="msbuildLocation"> Full path to current MSBuild.exe if executable is MSBuild.exe,
         /// or to version of MSBuild.dll found to be associated with the current process.</param>
         /// <param name="cancellationToken">Cancellation token.</param>
         /// <returns>A value of type <see cref="MSBuildApp.ExitType"/> that indicates whether the build succeeded,
         /// or the manner in which it failed.</returns>
-        public static MSBuildApp.ExitType Execute(
-#if FEATURE_GET_COMMANDLINE
-            string commandLine,
-#else
-            string[] commandLine,
-#endif
-            string msbuildLocation,
-            CancellationToken cancellationToken)
+        public static MSBuildApp.ExitType Execute(string[] commandLineArgs, string msbuildLocation, CancellationToken cancellationToken)
         {
-            MSBuildClient msbuildClient = new MSBuildClient(commandLine, msbuildLocation);
+            MSBuildClient msbuildClient = new MSBuildClient(commandLineArgs, msbuildLocation);
             MSBuildClientExitResult exitResult = msbuildClient.Execute(cancellationToken);
 
             if (exitResult.MSBuildClientExitType == MSBuildClientExitType.ServerBusy ||
@@ -84,7 +71,7 @@ public static MSBuildApp.ExitType Execute(
                 }
 
                 // Server is busy, fallback to old behavior.
-                return MSBuildApp.Execute(commandLine);
+                return MSBuildApp.Execute(commandLineArgs);
             }
 
             if (exitResult.MSBuildClientExitType == MSBuildClientExitType.Success &&