Skip to content

Commit

Permalink
Handle escaping command arguments in cmd_bat
Browse files Browse the repository at this point in the history
This pull request stops arguments escaping for commands run using cmd.exe since the result of `ShellUtils.windowsEscapeArg()` method is not accepted by cmd.exe.

Fixes: #11975

Closes #12530.

PiperOrigin-RevId: 344010635
  • Loading branch information
mai93 authored and copybara-github committed Nov 24, 2020
1 parent 913a985 commit 47e832a
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,7 @@
import java.util.Map;
import java.util.TreeMap;

/**
* A subprocess factory that uses the Win32 API.
*/
/** A subprocess factory that uses the Win32 API. */
public class WindowsSubprocessFactory implements SubprocessFactory {
public static final WindowsSubprocessFactory INSTANCE = new WindowsSubprocessFactory();

Expand All @@ -39,7 +37,10 @@ public Subprocess create(SubprocessBuilder builder) throws IOException {

// DO NOT quote argv0, createProcess will do it for us.
String argv0 = processArgv0(argv.get(0));
String argvRest = argv.size() > 1 ? escapeArgvRest(argv.subList(1, argv.size())) : "";
String argvRest =
argv.size() > 1
? escapeArgvRest(argv.subList(1, argv.size()), argv0.equals("cmd.exe"))
: "";
byte[] env = convertEnvToNative(builder.getEnv());

String stdoutPath = getRedirectPath(builder.getStdout(), builder.getStdoutFile());
Expand Down Expand Up @@ -68,7 +69,7 @@ public Subprocess create(SubprocessBuilder builder) throws IOException {
builder.getTimeoutMillis());
}

private String escapeArgvRest(List<String> argv) {
private static String escapeArgvRest(List<String> argv, boolean isCmd) {
StringBuilder result = new StringBuilder();
boolean first = true;
for (String arg : argv) {
Expand All @@ -77,7 +78,11 @@ private String escapeArgvRest(List<String> argv) {
} else {
result.append(" ");
}
result.append(ShellUtils.windowsEscapeArg(arg));
if (isCmd) {
result.append(arg);
} else {
result.append(ShellUtils.windowsEscapeArg(arg));
}
}
return result.toString();
}
Expand All @@ -98,7 +103,7 @@ public static String processArgv0(String argv0) {
private static String getRedirectPath(StreamAction action, File file) {
switch (action) {
case DISCARD:
return "NUL"; // That's /dev/null on Windows
return "NUL"; // That's /dev/null on Windows

case REDIRECT:
return file.getPath();
Expand Down
50 changes: 50 additions & 0 deletions src/test/py/bazel/genrule_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,56 @@ def testCommandFailsEagerlyInPowershell(self):
'The term \'command_not_exist\' is not recognized as the name of a cmdlet',
''.join(stderr))

def testCopyWithSpacesWithBatch(self):
if not self.IsWindows():
return
self.ScratchFile('WORKSPACE')
self.ScratchFile('foo/BUILD', [
'genrule(',
' name = "x",',
' srcs = ["hello source"],',
' outs = ["hello copied"],',
' cmd_bat = "copy \\"$<\\" \\"$@\\"",',
')',
])
self.ScratchFile('foo/hello source', ['hello world'])

exit_code, stdout, stderr = self.RunBazel(['info', 'bazel-bin'])
self.AssertExitCode(exit_code, 0, stderr)
bazel_bin = stdout[0]

exit_code, _, stderr = self.RunBazel(['build', '//foo:x'])
self.AssertExitCode(exit_code, 0, stderr)

copied = os.path.join(bazel_bin, 'foo', 'hello copied')
self.assertTrue(os.path.exists(copied))
self.AssertFileContentContains(copied, 'hello world')

def testCopyWithSpacesWithPowershell(self):
if not self.IsWindows():
return
self.ScratchFile('WORKSPACE')
self.ScratchFile('foo/BUILD', [
'genrule(',
' name = "x",',
' srcs = ["hello source"],',
' outs = ["hello copied"],',
' cmd_ps = "cp \\"$<\\" \\"$@\\"",',
')',
])
self.ScratchFile('foo/hello source', ['hello world'])

exit_code, stdout, stderr = self.RunBazel(['info', 'bazel-bin'])
self.AssertExitCode(exit_code, 0, stderr)
bazel_bin = stdout[0]

exit_code, _, stderr = self.RunBazel(['build', '//foo:x'])
self.AssertExitCode(exit_code, 0, stderr)

copied = os.path.join(bazel_bin, 'foo', 'hello copied')
self.assertTrue(os.path.exists(copied))
self.AssertFileContentContains(copied, 'hello world')


if __name__ == '__main__':
unittest.main()

0 comments on commit 47e832a

Please sign in to comment.