Skip to content

Commit 19d26c0

Browse files
JanProvaznikJanProvaznik
authored
revert refactors of commandline parsing (#12788)
reverts #12693 #12704 ### Context unblocking VMR insertion dotnet/dotnet#3091 ### Changes Made ### Testing ### Notes
1 parent 8dedbe4 commit 19d26c0

15 files changed

+833
-769
lines changed

‎src/Build.UnitTests/Utilities_Tests.cs‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,9 +80,17 @@ public void CommentsInPreprocessing()
8080

8181
env.SetEnvironmentVariable("MSBUILDLOADALLFILESASWRITEABLE", "1");
8282

83+
#if FEATURE_GET_COMMANDLINE
8384
MSBuildApp.Execute(@"c:\bin\msbuild.exe """ + inputFile.Path +
8485
(NativeMethodsShared.IsUnixLike ? @""" -pp:""" : @""" /pp:""") + outputFile.Path + @"""")
8586
.ShouldBe(MSBuildApp.ExitType.Success);
87+
#else
88+
Assert.Equal(
89+
MSBuildApp.ExitType.Success,
90+
MSBuildApp.Execute(
91+
new[] { @"c:\bin\msbuild.exe", '"' + inputFile.Path + '"',
92+
'"' + (NativeMethodsShared.IsUnixLike ? "-pp:" : "/pp:") + outputFile.Path + '"'}));
93+
#endif
8694

8795
bool foundDoNotModify = false;
8896
foreach (string line in File.ReadLines(outputFile.Path))

‎src/Build/BackEnd/Client/MSBuildClient.cs‎

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,11 @@ public sealed class MSBuildClient
4848
/// The command line to process.
4949
/// The first argument on the command line is assumed to be the name/path of the executable, and is ignored.
5050
/// </summary>
51+
#if FEATURE_GET_COMMANDLINE
5152
private readonly string _commandLine;
53+
#else
54+
private readonly string[] _commandLine;
55+
#endif
5256

5357
/// <summary>
5458
/// The MSBuild client execution result.
@@ -108,7 +112,13 @@ public sealed class MSBuildClient
108112
/// on the command line is assumed to be the name/path of the executable, and is ignored</param>
109113
/// <param name="msbuildLocation"> Full path to current MSBuild.exe if executable is MSBuild.exe,
110114
/// or to version of MSBuild.dll found to be associated with the current process.</param>
111-
public MSBuildClient(string commandLine, string msbuildLocation)
115+
public MSBuildClient(
116+
#if FEATURE_GET_COMMANDLINE
117+
string commandLine,
118+
#else
119+
string[] commandLine,
120+
#endif
121+
string msbuildLocation)
112122
{
113123
_serverEnvironmentVariables = new();
114124
_exitResult = new();
@@ -152,7 +162,12 @@ private void CreateNodePipeStream()
152162
public MSBuildClientExitResult Execute(CancellationToken cancellationToken)
153163
{
154164
// Command line in one string used only in human readable content.
155-
string descriptiveCommandLine = _commandLine;
165+
string descriptiveCommandLine =
166+
#if FEATURE_GET_COMMANDLINE
167+
_commandLine;
168+
#else
169+
string.Join(" ", _commandLine);
170+
#endif
156171

157172
CommunicationsUtilities.Trace("Executing build with command line '{0}'", descriptiveCommandLine);
158173

‎src/Build/BackEnd/Node/OutOfProcServerNode.cs‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,12 @@ public sealed class OutOfProcServerNode : INode, INodePacketFactory, INodePacket
2525
/// <summary>
2626
/// A callback used to execute command line build.
2727
/// </summary>
28-
public delegate (int exitCode, string exitType) BuildCallback(string commandLine);
28+
public delegate (int exitCode, string exitType) BuildCallback(
29+
#if FEATURE_GET_COMMANDLINE
30+
string commandLine);
31+
#else
32+
string[] commandLine);
33+
#endif
2934

3035
private readonly BuildCallback _buildFunction;
3136

‎src/Build/BackEnd/Node/ServerNodeBuildCommand.cs‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,11 @@ namespace Microsoft.Build.BackEnd
1414
/// </summary>
1515
internal sealed class ServerNodeBuildCommand : INodePacket
1616
{
17+
#if FEATURE_GET_COMMANDLINE
1718
private string _commandLine = default!;
19+
#else
20+
private string[] _commandLine = default!;
21+
#endif
1822
private string _startupDirectory = default!;
1923
private Dictionary<string, string> _buildProcessEnvironment = default!;
2024
private CultureInfo _culture = default!;
@@ -30,7 +34,11 @@ internal sealed class ServerNodeBuildCommand : INodePacket
3034
/// <summary>
3135
/// Command line including arguments
3236
/// </summary>
37+
#if FEATURE_GET_COMMANDLINE
3338
public string CommandLine => _commandLine;
39+
#else
40+
public string[] CommandLine => _commandLine;
41+
#endif
3442

3543
/// <summary>
3644
/// The startup directory
@@ -71,7 +79,11 @@ private ServerNodeBuildCommand()
7179
}
7280

7381
public ServerNodeBuildCommand(
82+
#if FEATURE_GET_COMMANDLINE
7483
string commandLine,
84+
#else
85+
string[] commandLine,
86+
#endif
7587
string startupDirectory,
7688
Dictionary<string, string> buildProcessEnvironment,
7789
CultureInfo culture, CultureInfo uiCulture,

‎src/Build/CompatibilitySuppressions.xml‎

Lines changed: 1 addition & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -1,48 +1,3 @@
11
<?xml version="1.0" encoding="utf-8"?>
22
<!-- https://learn.microsoft.com/dotnet/fundamentals/package-validation/diagnostic-ids -->
3-
<Suppressions xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:xsd="http://www.w3.org/2001/XMLSchema">
4-
<!-- MSBuildClient is in experimental namespace and not used outside the MSBuild -->
5-
<!-- OutOfProcServerNode.BuildCallback is in experimental namespace and not used outside the MSBuild -->
6-
<Suppression>
7-
<DiagnosticId>CP0002</DiagnosticId>
8-
<Target>M:Microsoft.Build.Experimental.MSBuildClient.#ctor(System.String[],System.String)</Target>
9-
<Left>lib/net10.0/Microsoft.Build.dll</Left>
10-
<Right>lib/net10.0/Microsoft.Build.dll</Right>
11-
<IsBaselineSuppression>true</IsBaselineSuppression>
12-
</Suppression>
13-
<Suppression>
14-
<DiagnosticId>CP0002</DiagnosticId>
15-
<Target>M:Microsoft.Build.Experimental.OutOfProcServerNode.BuildCallback.BeginInvoke(System.String[],System.AsyncCallback,System.Object)</Target>
16-
<Left>lib/net10.0/Microsoft.Build.dll</Left>
17-
<Right>lib/net10.0/Microsoft.Build.dll</Right>
18-
<IsBaselineSuppression>true</IsBaselineSuppression>
19-
</Suppression>
20-
<Suppression>
21-
<DiagnosticId>CP0002</DiagnosticId>
22-
<Target>M:Microsoft.Build.Experimental.OutOfProcServerNode.BuildCallback.Invoke(System.String[])</Target>
23-
<Left>lib/net10.0/Microsoft.Build.dll</Left>
24-
<Right>lib/net10.0/Microsoft.Build.dll</Right>
25-
<IsBaselineSuppression>true</IsBaselineSuppression>
26-
</Suppression>
27-
<Suppression>
28-
<DiagnosticId>CP0002</DiagnosticId>
29-
<Target>M:Microsoft.Build.Experimental.MSBuildClient.#ctor(System.String[],System.String)</Target>
30-
<Left>ref/net10.0/Microsoft.Build.dll</Left>
31-
<Right>ref/net10.0/Microsoft.Build.dll</Right>
32-
<IsBaselineSuppression>true</IsBaselineSuppression>
33-
</Suppression>
34-
<Suppression>
35-
<DiagnosticId>CP0002</DiagnosticId>
36-
<Target>M:Microsoft.Build.Experimental.OutOfProcServerNode.BuildCallback.BeginInvoke(System.String[],System.AsyncCallback,System.Object)</Target>
37-
<Left>ref/net10.0/Microsoft.Build.dll</Left>
38-
<Right>ref/net10.0/Microsoft.Build.dll</Right>
39-
<IsBaselineSuppression>true</IsBaselineSuppression>
40-
</Suppression>
41-
<Suppression>
42-
<DiagnosticId>CP0002</DiagnosticId>
43-
<Target>M:Microsoft.Build.Experimental.OutOfProcServerNode.BuildCallback.Invoke(System.String[])</Target>
44-
<Left>ref/net10.0/Microsoft.Build.dll</Left>
45-
<Right>ref/net10.0/Microsoft.Build.dll</Right>
46-
<IsBaselineSuppression>true</IsBaselineSuppression>
47-
</Suppression>
48-
</Suppressions>
3+
<Suppressions xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:xsd="http://www.w3.org/2001/XMLSchema" />

‎src/Directory.BeforeCommon.targets‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
<DefineConstants>$(DefineConstants);FEATURE_ENVIRONMENT_SYSTEMDIRECTORY</DefineConstants>
3232
<DefineConstants>$(DefineConstants);FEATURE_FILE_TRACKER</DefineConstants>
3333
<DefineConstants Condition="'$(MachineIndependentBuild)' != 'true'">$(DefineConstants);FEATURE_GAC</DefineConstants>
34+
<DefineConstants>$(DefineConstants);FEATURE_GET_COMMANDLINE</DefineConstants>
3435
<DefineConstants>$(DefineConstants);FEATURE_HANDLEPROCESSCORRUPTEDSTATEEXCEPTIONS</DefineConstants>
3536
<DefineConstants>$(DefineConstants);FEATURE_HTTP_LISTENER</DefineConstants>
3637
<DefineConstants Condition="'$(MachineIndependentBuild)' != 'true'">$(DefineConstants);FEATURE_INSTALLED_MSBUILD</DefineConstants>

‎src/MSBuild.UnitTests/CommandLineSwitches_Tests.cs‎

Lines changed: 21 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -622,8 +622,7 @@ public void FeatureAvailibilitySwitchIdentificationTest(string switchName)
622622
public void TargetsSwitchParameter()
623623
{
624624
CommandLineSwitches switches = new CommandLineSwitches();
625-
CommandLineParser parser = new CommandLineParser();
626-
parser.GatherCommandLineSwitches(new List<string>() { "/targets:targets.txt" }, switches);
625+
MSBuildApp.GatherCommandLineSwitches(new List<string>() { "/targets:targets.txt" }, switches);
627626

628627
switches.HaveErrors().ShouldBeFalse();
629628
switches[CommandLineSwitches.ParameterizedSwitch.Targets].ShouldBe(new[] { "targets.txt" });
@@ -633,8 +632,7 @@ public void TargetsSwitchParameter()
633632
public void TargetsSwitchDoesNotSupportMultipleOccurrences()
634633
{
635634
CommandLineSwitches switches = new CommandLineSwitches();
636-
CommandLineParser parser = new CommandLineParser();
637-
parser.GatherCommandLineSwitches(new List<string>() { "/targets /targets" }, switches);
635+
MSBuildApp.GatherCommandLineSwitches(new List<string>() { "/targets /targets" }, switches);
638636

639637
switches.HaveErrors().ShouldBeTrue();
640638
}
@@ -711,9 +709,8 @@ public void LowPrioritySwitchIdentificationTests(string lowpriority)
711709
public void GraphBuildSwitchCanHaveParameters()
712710
{
713711
CommandLineSwitches switches = new CommandLineSwitches();
714-
CommandLineParser parser = new CommandLineParser();
715712

716-
parser.GatherCommandLineSwitches(new List<string> { "/graph", "/graph:true; NoBuild ;; ;", "/graph:foo" }, switches);
713+
MSBuildApp.GatherCommandLineSwitches(new List<string> { "/graph", "/graph:true; NoBuild ;; ;", "/graph:foo" }, switches);
717714

718715
switches[CommandLineSwitches.ParameterizedSwitch.GraphBuild].ShouldBe(new[] { "true", " NoBuild ", " ", "foo" });
719716

@@ -724,9 +721,8 @@ public void GraphBuildSwitchCanHaveParameters()
724721
public void GraphBuildSwitchCanBeParameterless()
725722
{
726723
CommandLineSwitches switches = new CommandLineSwitches();
727-
CommandLineParser parser = new CommandLineParser();
728724

729-
parser.GatherCommandLineSwitches(new List<string> { "/graph" }, switches);
725+
MSBuildApp.GatherCommandLineSwitches(new List<string> { "/graph" }, switches);
730726

731727
switches[CommandLineSwitches.ParameterizedSwitch.GraphBuild].ShouldBe(Array.Empty<string>());
732728

@@ -737,9 +733,8 @@ public void GraphBuildSwitchCanBeParameterless()
737733
public void InputResultsCachesSupportsMultipleOccurrence()
738734
{
739735
CommandLineSwitches switches = new CommandLineSwitches();
740-
CommandLineParser parser = new CommandLineParser();
741736

742-
parser.GatherCommandLineSwitches(new List<string>() { "/irc", "/irc:a;b", "/irc:c;d" }, switches);
737+
MSBuildApp.GatherCommandLineSwitches(new List<string>() { "/irc", "/irc:a;b", "/irc:c;d" }, switches);
743738

744739
switches[CommandLineSwitches.ParameterizedSwitch.InputResultsCaches].ShouldBe(new[] { null, "a", "b", "c", "d" });
745740

@@ -750,9 +745,8 @@ public void InputResultsCachesSupportsMultipleOccurrence()
750745
public void OutputResultsCache()
751746
{
752747
CommandLineSwitches switches = new CommandLineSwitches();
753-
CommandLineParser parser = new CommandLineParser();
754748

755-
parser.GatherCommandLineSwitches(new List<string>() { "/orc:a" }, switches);
749+
MSBuildApp.GatherCommandLineSwitches(new List<string>() { "/orc:a" }, switches);
756750

757751
switches[CommandLineSwitches.ParameterizedSwitch.OutputResultsCache].ShouldBe(new[] { "a" });
758752

@@ -763,9 +757,8 @@ public void OutputResultsCache()
763757
public void OutputResultsCachesDoesNotSupportMultipleOccurrences()
764758
{
765759
CommandLineSwitches switches = new CommandLineSwitches();
766-
CommandLineParser parser = new CommandLineParser();
767760

768-
parser.GatherCommandLineSwitches(new List<string>() { "/orc:a", "/orc:b" }, switches);
761+
MSBuildApp.GatherCommandLineSwitches(new List<string>() { "/orc:a", "/orc:b" }, switches);
769762

770763
switches.HaveErrors().ShouldBeTrue();
771764
}
@@ -1295,9 +1288,8 @@ public void ExtractAnyLoggerParameterPickLast()
12951288
public void ProcessWarnAsErrorSwitchNotSpecified()
12961289
{
12971290
CommandLineSwitches commandLineSwitches = new CommandLineSwitches();
1298-
CommandLineParser parser = new CommandLineParser();
12991291

1300-
parser.GatherCommandLineSwitches(new List<string>(new[] { "" }), commandLineSwitches);
1292+
MSBuildApp.GatherCommandLineSwitches(new List<string>(new[] { "" }), commandLineSwitches);
13011293

13021294
Assert.Null(MSBuildApp.ProcessWarnAsErrorSwitch(commandLineSwitches));
13031295
}
@@ -1311,9 +1303,8 @@ public void ProcessWarnAsErrorSwitchWithCodes()
13111303
ISet<string> expectedWarningsAsErrors = new HashSet<string>(StringComparer.OrdinalIgnoreCase) { "a", "B", "c", "D", "e" };
13121304

13131305
CommandLineSwitches commandLineSwitches = new CommandLineSwitches();
1314-
CommandLineParser parser = new CommandLineParser();
13151306

1316-
parser.GatherCommandLineSwitches(new List<string>(new[]
1307+
MSBuildApp.GatherCommandLineSwitches(new List<string>(new[]
13171308
{
13181309
"\"/warnaserror: a,B ; c \"", // Leading, trailing, leading and trailing whitespace
13191310
"/warnaserror:A,b,C", // Repeats of different case
@@ -1337,9 +1328,8 @@ public void ProcessWarnAsErrorSwitchWithCodes()
13371328
public void ProcessWarnAsErrorSwitchEmptySwitchClearsSet()
13381329
{
13391330
CommandLineSwitches commandLineSwitches = new CommandLineSwitches();
1340-
CommandLineParser parser = new CommandLineParser();
13411331

1342-
parser.GatherCommandLineSwitches(new List<string>(new[]
1332+
MSBuildApp.GatherCommandLineSwitches(new List<string>(new[]
13431333
{
13441334
"/warnaserror:a;b;c",
13451335
"/warnaserror",
@@ -1361,9 +1351,8 @@ public void ProcessWarnAsErrorSwitchValuesAfterEmptyAddOn()
13611351
ISet<string> expectedWarningsAsErors = new HashSet<string>(StringComparer.OrdinalIgnoreCase) { "e", "f", "g" };
13621352

13631353
CommandLineSwitches commandLineSwitches = new CommandLineSwitches();
1364-
CommandLineParser parser = new CommandLineParser();
13651354

1366-
parser.GatherCommandLineSwitches(new List<string>(new[]
1355+
MSBuildApp.GatherCommandLineSwitches(new List<string>(new[]
13671356
{
13681357
"/warnaserror:a;b;c",
13691358
"/warnaserror",
@@ -1384,9 +1373,8 @@ public void ProcessWarnAsErrorSwitchValuesAfterEmptyAddOn()
13841373
public void ProcessWarnAsErrorSwitchEmpty()
13851374
{
13861375
CommandLineSwitches commandLineSwitches = new CommandLineSwitches();
1387-
CommandLineParser parser = new CommandLineParser();
13881376

1389-
parser.GatherCommandLineSwitches(new List<string>(new[] { "/warnaserror" }), commandLineSwitches);
1377+
MSBuildApp.GatherCommandLineSwitches(new List<string>(new[] { "/warnaserror" }), commandLineSwitches);
13901378

13911379
ISet<string> actualWarningsAsErrors = MSBuildApp.ProcessWarnAsErrorSwitch(commandLineSwitches);
13921380

@@ -1402,11 +1390,10 @@ public void ProcessWarnAsErrorSwitchEmpty()
14021390
public void ProcessWarnAsMessageSwitchEmpty()
14031391
{
14041392
CommandLineSwitches commandLineSwitches = new CommandLineSwitches();
1405-
CommandLineParser parser = new CommandLineParser();
14061393

14071394
// Set "expanded" content to match the placeholder so the verify can use the exact resource string as "expected."
14081395
string command = "{0}";
1409-
parser.GatherCommandLineSwitches(new List<string>(new[] { "/warnasmessage" }), commandLineSwitches, command);
1396+
MSBuildApp.GatherCommandLineSwitches(new List<string>(new[] { "/warnasmessage" }), commandLineSwitches, command);
14101397

14111398
VerifySwitchError(commandLineSwitches, "/warnasmessage", AssemblyResources.GetString("MissingWarnAsMessageParameterError"));
14121399
}
@@ -1423,15 +1410,13 @@ public void ProcessEnvironmentVariableSwitch()
14231410
env.SetEnvironmentVariable("ENVIRONMENTVARIABLE", string.Empty);
14241411

14251412
CommandLineSwitches commandLineSwitches = new();
1426-
CommandLineParser parser = new CommandLineParser();
1427-
14281413
string fullCommandLine = "msbuild validProject.csproj %ENVIRONMENTVARIABLE%";
1429-
parser.GatherCommandLineSwitches(new List<string>() { "validProject.csproj", "%ENVIRONMENTVARIABLE%" }, commandLineSwitches, fullCommandLine);
1414+
MSBuildApp.GatherCommandLineSwitches(new List<string>() { "validProject.csproj", "%ENVIRONMENTVARIABLE%" }, commandLineSwitches, fullCommandLine);
14301415
VerifySwitchError(commandLineSwitches, "%ENVIRONMENTVARIABLE%", String.Format(AssemblyResources.GetString("EnvironmentVariableAsSwitch"), fullCommandLine));
14311416

14321417
commandLineSwitches = new();
14331418
fullCommandLine = "msbuild %ENVIRONMENTVARIABLE% validProject.csproj";
1434-
parser.GatherCommandLineSwitches(new List<string>() { "%ENVIRONMENTVARIABLE%", "validProject.csproj" }, commandLineSwitches, fullCommandLine);
1419+
MSBuildApp.GatherCommandLineSwitches(new List<string>() { "%ENVIRONMENTVARIABLE%", "validProject.csproj" }, commandLineSwitches, fullCommandLine);
14351420
VerifySwitchError(commandLineSwitches, "%ENVIRONMENTVARIABLE%", String.Format(AssemblyResources.GetString("EnvironmentVariableAsSwitch"), fullCommandLine));
14361421
}
14371422
}
@@ -1445,9 +1430,8 @@ public void ProcessWarnAsMessageSwitchWithCodes()
14451430
ISet<string> expectedWarningsAsMessages = new HashSet<string>(StringComparer.OrdinalIgnoreCase) { "a", "B", "c", "D", "e" };
14461431

14471432
CommandLineSwitches commandLineSwitches = new CommandLineSwitches();
1448-
CommandLineParser parser = new CommandLineParser();
14491433

1450-
parser.GatherCommandLineSwitches(new List<string>(new[]
1434+
MSBuildApp.GatherCommandLineSwitches(new List<string>(new[]
14511435
{
14521436
"\"/warnasmessage: a,B ; c \"", // Leading, trailing, leading and trailing whitespace
14531437
"/warnasmessage:A,b,C", // Repeats of different case
@@ -1471,9 +1455,8 @@ public void ProcessWarnAsMessageSwitchWithCodes()
14711455
public void ProcessProfileEvaluationEmpty()
14721456
{
14731457
CommandLineSwitches commandLineSwitches = new CommandLineSwitches();
1474-
CommandLineParser parser = new CommandLineParser();
14751458

1476-
parser.GatherCommandLineSwitches(new List<string>(new[] { "/profileevaluation" }), commandLineSwitches);
1459+
MSBuildApp.GatherCommandLineSwitches(new List<string>(new[] { "/profileevaluation" }), commandLineSwitches);
14771460
commandLineSwitches[CommandLineSwitches.ParameterizedSwitch.ProfileEvaluation][0].ShouldBe("no-file");
14781461
}
14791462

@@ -1565,7 +1548,11 @@ public void ProcessInvalidTargetSwitch()
15651548
using TestEnvironment testEnvironment = TestEnvironment.Create();
15661549
string project = testEnvironment.CreateTestProjectWithFiles("project.proj", projectContent).ProjectFile;
15671550

1551+
#if FEATURE_GET_COMMANDLINE
15681552
MSBuildApp.Execute(@"msbuild.exe " + project + " /t:foo.bar").ShouldBe(MSBuildApp.ExitType.SwitchError);
1553+
#else
1554+
MSBuildApp.Execute(new[] { @"msbuild.exe", project, "/t:foo.bar" }).ShouldBe(MSBuildApp.ExitType.SwitchError);
1555+
#endif
15691556
}
15701557

15711558
/// <summary>

0 commit comments

Comments
 (0)