Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide nice error when an environment variable is not expanded on the command line, including the full command line for all switch errors Fixes #7210 #7213

Merged
merged 28 commits into from
Feb 18, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
66ae06a
Ignore env variable switches
Forgind Jan 4, 2022
df38393
Log command line on switch error
Forgind Jan 5, 2022
ab4b6ba
Add hack to string
Forgind Jan 5, 2022
4bf3613
Always point to the environment variable
Forgind Jan 5, 2022
11a2359
Create new error
Forgind Jan 5, 2022
062b313
Move check earlier
Forgind Jan 5, 2022
41427e4
Add test for new behavior
Forgind Jan 5, 2022
76afef4
Delete unused method
Forgind Jan 5, 2022
44a87c9
Include rsps in command line
Forgind Jan 6, 2022
4d9d090
Use TestEnvironment
Forgind Jan 6, 2022
ba4283c
Include response file paths in command line
Forgind Jan 7, 2022
1e9d5c7
Fix resource string
Forgind Jan 7, 2022
6174c75
Build
Forgind Jan 7, 2022
ad31551
Adjust format
Forgind Jan 10, 2022
4753c7d
Merge branch 'main' into ignore-environment-variable-switches
Forgind Jan 10, 2022
af1f39c
Make it easier on the loc team
Forgind Jan 14, 2022
596a7c1
Even easier
Forgind Jan 14, 2022
5af7815
Properly make easier
Forgind Jan 14, 2022
55e9955
Release properly
Forgind Jan 15, 2022
7fcf177
Use ' ' and make tests pass
Forgind Jan 20, 2022
e624a5e
Merge branch 'main' into ignore-environment-variable-switches
Forgind Jan 31, 2022
5113a67
PR comments + added test
Forgind Feb 8, 2022
f50182a
Little TestEnvironment cleanup
Forgind Feb 8, 2022
50894f0
GC SwitchesFromResponseFile
Forgind Feb 8, 2022
4244db2
Ensure initialized
Forgind Feb 8, 2022
1c6c2c3
Keep switches in order
Forgind Feb 14, 2022
af3d14b
Merge branch 'main' into ignore-environment-variable-switches
Forgind Feb 16, 2022
0d7e80f
Add commandLine arg
Forgind Feb 16, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions src/MSBuild/CommandLineSwitches.cs
Original file line number Diff line number Diff line change
Expand Up @@ -438,6 +438,29 @@ internal void SetParameterlessSwitch(ParameterlessSwitch parameterlessSwitch, st
// list of recognized switch parameter separators -- for switches that take multiple parameters
private static readonly char[] s_parameterSeparators = { ',', ';' };

/// <summary>
/// Called when a recognized switch that takes parameters is detected on the command line,
/// but an invalid switch of the same kind was detected first. Here we pretend the first
/// switch didn't exist and override it.
/// </summary>
/// <param name="parameterizedSwitch"></param>
/// <param name="switchParameters"></param>
/// <param name="multipleParametersAllowed"></param>
/// <param name="unquoteParameters"></param>
/// <returns>true, if the given parameters were successfully stored</returns>
internal bool OverrideParameterizedSwitch(
ParameterizedSwitch parameterizedSwitch,
string commandLineArg,
string switchParameters,
bool multipleParametersAllowed,
bool unquoteParameters,
bool emptyParametersAllowed
)
{
_parameterizedSwitches[(int)parameterizedSwitch].commandLineArg = null;
return SetParameterizedSwitch(parameterizedSwitch, commandLineArg, switchParameters, multipleParametersAllowed, unquoteParameters, emptyParametersAllowed);
}

/// <summary>
/// Called when a recognized switch that takes parameters is detected on the command line.
/// </summary>
Expand Down
26 changes: 26 additions & 0 deletions src/MSBuild/XMake.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2008,6 +2008,27 @@ bool allowEmptyParameters
}
}
}
else if (parameterizedSwitch == CommandLineSwitches.ParameterizedSwitch.Project && switchParameters.Length > 0 &&
(IsEnvironmentVariable(commandLineSwitches.GetParameterizedSwitchCommandLineArg(CommandLineSwitches.ParameterizedSwitch.Project)) ||
IsEnvironmentVariable(switchParameters.Substring(1))))
{
if (IsEnvironmentVariable(commandLineSwitches.GetParameterizedSwitchCommandLineArg(CommandLineSwitches.ParameterizedSwitch.Project)))
{
if (switchParameters.Length > 0)
{
switchParameters = switchParameters.Substring(1);
}

if (!commandLineSwitches.OverrideParameterizedSwitch(parameterizedSwitch, unquotedCommandLineArg, switchParameters, multipleParametersAllowed, unquoteParameters, allowEmptyParameters))
{
// if parsing revealed there were no real parameters, flag an error, unless the parameters are optional
if (missingParametersErrorMessage != null)
{
commandLineSwitches.SetSwitchError(missingParametersErrorMessage, unquotedCommandLineArg);
}
}
}
}
else
{
commandLineSwitches.SetSwitchError(duplicateSwitchErrorMessage, unquotedCommandLineArg);
Expand All @@ -2019,6 +2040,11 @@ bool allowEmptyParameters
}
}

private static bool IsEnvironmentVariable(string s)
{
return s.StartsWith("$") || (s.StartsWith("%") && s.EndsWith("%") && s.Length > 1);
}

/// <summary>
/// The name of the auto-response file.
/// </summary>
Expand Down