nuke-build
ProcessTasks.StartShell should not add quotes on windows.
Usage Information
8.1.2, net8.0, Windows
Description
As you might know, correct quoting of arguments can be a nightmare. Nuke always adds double quotes when launching processes via ProcessTasks.StartShell which can lead to wrong arguments passed when launching a batch file on windows.
While I'm not sure about the overall impact of such a change, I think Nuke should not add double quotes on Windows. cmd.exe seem to handle things fine if you do not add any quotes around the overall command while it breaks actual quotes when you need pass arguments with spaces to your script.
Reproduction Steps
Nuke:
var bat = (RootDirectory / "test.bat");
bat.WriteAllText(
"""
@echo off
SET Arg1=%~1
echo Arg1=%Arg1%
""");
var cmdArgs = new Arguments();
cmdArgs.Add("{value}", RootDirectory / "with spaces");
ProcessTasks.StartShell($"{bat} {cmdArgs.RenderForExecution()}").AssertWaitForExit();
This launches a command like
C:\Windows\System32\cmd.exe /c "C:\myproject\test.bat \"C:\myproject\with spaces\""
While a command line like this would deliver the expected result:
C:\Windows\System32\cmd.exe /c C:\myproject\test.bat "C:\myproject\with spaces"
Expected Behavior
Arg1=C:\myproject\with spaces
Actual Behavior
Arg1=\"C:\myproject\with spaces\"
Regression?
No response
Known Workarounds
protected virtual IProcess RunBatch(
string command,
string? workingDirectory = null,
IReadOnlyDictionary<string, string>? environmentVariables = null,
int? timeout = null,
bool? logOutput = null,
bool? logInvocation = null,
Action<OutputType, string>? logger = null,
Func<string, string>? outputFilter = null)
{
return ProcessTasks.StartProcess(
toolPath: ToolPathResolver.GetPathExecutable(IsWin ? "cmd" : "bash"),
arguments: IsWin ? $"/c {command}" : $"-c {command.DoubleQuote()}",
workingDirectory,
environmentVariables,
timeout,
logOutput,
logInvocation,
logger,
outputFilter);
}
Could you help with a pull-request?
Yes
Some other things I noticed: Things might be tricky when the launched application also has spaces. What seems to work really as expected is:
- Add
/sto cmd.exe - Wrap the argument with double quotes
- Do not escape any quotes inside the command
C:\Windows\System32\cmd.exe /s /c ""C:\myproject\test.bat" "C:\myproject\with spaces""
Arg1=C:\myproject\with spaces
Something like this should deliver the expected result:
public static IProcess StartShell(
string command,
string? workingDirectory = null,
IReadOnlyDictionary<string, string>? environmentVariables = null,
int? timeout = null,
bool? logOutput = null,
bool? logInvocation = null,
Action<OutputType, string>? logger = null,
Func<string, string>? outputFilter = null)
{
return ProcessTasks.StartProcess(
toolPath: ToolPathResolver.GetPathExecutable(IsWin ? "cmd" : "bash"),
arguments: IsWin ? $"/s /c \"{command}\"" : $"-c {command.DoubleQuote()}",
workingDirectory,
environmentVariables,
timeout,
logOutput,
logInvocation,
logger,
outputFilter);
}
Related docs for the option and the exact behavior. If we don't want to go for /s nuke would need to respect the special logic described and quote accordingly.
/S Modifies the treatment of string after /C or /K (see below)
...
If /C or /K is specified, then the remainder of the command line after
the switch is processed as a command line, where the following logic is
used to process quote (") characters:
1. If all of the following conditions are met, then quote characters
on the command line are preserved:
- no /S switch
- exactly two quote characters
- no special characters between the two quote characters,
where special is one of: &<>()@^|
- there are one or more whitespace characters between the
two quote characters
- the string between the two quote characters is the name
of an executable file.
2. Otherwise, old behavior is to see if the first character is
a quote character and if so, strip the leading character and
remove the last quote character on the command line, preserving
any text after the last quote character.
The same seems to be true for ProcessTasks.StartProcess. How is one supposed to run custom CLI tools with parameters that have spaces in NUKE?
The same seems to be true for
ProcessTasks.StartProcess. How is one supposed to run custom CLI tools with parameters that have spaces in NUKE?
I'm not sure if it's the right way, I've definitely had problems with arguments with spaces if I use
Tool.Invoke("-d \"argument with spaces\"");
where it seems to eat the extra quotes and you get each word as a separate argument. Tracing through it doesn't seem to suggest that it should be doing that though.
What I have found is using something very messy like
ArgumentStringHandler args = new ArgumentStringHandler(5, 0, out valid);
args.AppendLiteral("arg1");
args.AppendLiteral(" arg2");
args.AppendLiteral(" -i c:\\file1.json");
args.AppendLiteral(" -o C:\\output.json");
args.AppendLiteral(" -d \"This argument has spaces\"");
tool.Invoke(arguments: args);
works for us. The leading spaces for AppendLiteral is necessary, and the arguments to the constructor for ArgumentStringHandler avoids an exception where it tries to parse formatted arguments when none exists (the 0, specifically).
I personally don't recommend that one uses the ArgumentStringHandler, but instead use the old Arguments class instead. We have created a wrapper around ArgumentStringHandler that mimics the exact old behaviour. Using the ArgumentStringHandler by default does not really work for anything other than basic commands.
var args = new ToolArguments()
.Add("arg1")
.Add("arg2")
.Add("-i {value}", "c:\\file1.json")
.Add("-o {value}", "C:\\output.json")
.Add("-d {value}", "This argument has spaces")
tool.Invoke(arguments: args);
[PublicAPI]
public class ToolArguments
{
private readonly List<(string Name, string? Value, string? Format)> Arguments = new();
public ToolArguments()
{
}
public ToolArguments(string argument)
{
Add(argument);
}
public ToolArguments Add(string argumentFormat)
{
Arguments.Add((argumentFormat, null, null));
return this;
}
public ToolArguments Add(string argumentFormat, bool? value)
=> value.HasValue && (value.Value || argumentFormat.Contains("{value}"))
? Add(argumentFormat, value.Value.ToString(), null)
: this;
public ToolArguments Add<T>(string argumentFormat, T? value, bool secret = false) where T : struct
=> value.HasValue ? Add(argumentFormat, value.Value, secret) : this;
public ToolArguments Add(string argumentFormat, object? value, bool secret = false)
=> Add(argumentFormat, value?.ToString(), secret);
public ToolArguments Add<T>(
string argumentFormat,
IEnumerable<T>? values,
char separator = ' ',
char? disallowed = null)
{
var list = values?.ToList();
if (list == null || list.Count == 0)
{
return this;
}
var joinedValues = list.Select(value => value!.ToString().DoubleQuoteIfNeeded(separator, disallowed, ' ')).Join(separator);
return Add(argumentFormat, joinedValues, "nq");
}
public ToolArguments Add(
string argumentFormat,
string? value,
bool secret = false)
{
var format = secret ? "r" : null;
return Add(argumentFormat, value, format);
}
private ToolArguments Add(string argumentFormat, string? value, string? format)
{
if (value == null)
{
return this;
}
Arguments.Add((argumentFormat, value, format));
return this;
}
public ArgumentStringHandler RenderForExecution()
{
ArgumentStringHandler builder = $"";
for (var i = 0; i < Arguments.Count; i++)
{
var (key, value, format) = Arguments[i];
if (i is not 0)
{
builder.AppendLiteral(" ");
}
var hasValuePlaceholder = key.Contains("{value}");
if (hasValuePlaceholder)
{
key = key.Replace("{value}", string.Empty);
}
builder.AppendLiteral(key);
if (hasValuePlaceholder && !string.IsNullOrWhiteSpace(value))
{
builder.AppendFormatted(value, format: format);
}
}
return builder;
}
public static implicit operator ArgumentStringHandler(ToolArguments arguments)
{
return arguments.RenderForExecution();
}
}
