commit 7e862eb2593c5d2934cf6a1d11c4a0aaf2d794fa
author Googler <twerth@google.com> Fri Apr 26 08:03:25 2024 -0700
committer Copybara-Service <copybara-worker@google.com> Fri Apr 26 08:05:24 2024 -0700
tree 5c43759779969e94ce8fda453c7aa99d5b99bed1
parent 5205dc1a00732fd16dba2256179025c5e74ac1cd

Fix escaping of arguments on windows when using `bazel run --script_path...`.

Fixes #21940.

PiperOrigin-RevId: 628407079
Change-Id: I403d0d881f244fd38c06a2585824bdbd3db52702

src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommandLine.java

diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommandLine.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommandLine.java
index 3d77ce6..a138ece 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommandLine.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommandLine.java
@@ -271,8 +271,6 @@
               .collect(joining("\n  "));
       String commandLine = getCommandLine(shExecutable, runUnderPrefix, args);
 
-      // TODO: https://github.com/bazelbuild/bazel/issues/21940 - This formatting and escaping is
-      // just about certainly wrong.
       StringBuilder result = new StringBuilder();
       result.append("@echo off\n");
       result.append("cd /d ").append(workingDir).append("\n");
@@ -292,7 +290,7 @@
         if (i == 0) {
           command.append(args.get(i).replace('/', '\\'));
         } else {
-          command.append(" ").append(args.get(i));
+          command.append(" ").append(ShellUtils.windowsEscapeArg(args.get(i)));
         }
       }
       if (runUnderPrefix == null) {

src/test/java/com/google/devtools/build/lib/runtime/commands/RunCommandLineTest.java

diff --git a/src/test/java/com/google/devtools/build/lib/runtime/commands/RunCommandLineTest.java b/src/test/java/com/google/devtools/build/lib/runtime/commands/RunCommandLineTest.java
index e68dfbd..7ca3390 100644
--- a/src/test/java/com/google/devtools/build/lib/runtime/commands/RunCommandLineTest.java
+++ b/src/test/java/com/google/devtools/build/lib/runtime/commands/RunCommandLineTest.java
@@ -166,7 +166,6 @@
 
             /* runUnderPrefix= */ "echo hello &&",
             ImmutableList.of("C:/executable", "argv1", "arg w spaces"));
-    // TODO: https://github.com/bazelbuild/bazel/issues/21940 - Fix escaping.
     assertThat(result)
         .isEqualTo(
             """
@@ -176,7 +175,7 @@
               SET UNSET_ME_TOO=
               SET ENV_VAR=val
               SET ENV_VAR_WITH_SPACES=foo bar
-              /bin/bash -c 'echo hello && C:\\executable argv1 arg w spaces' %*\
+              /bin/bash -c 'echo hello && C:\\executable argv1 "arg w spaces"' %*\
             """);
   }
 
@@ -191,7 +190,6 @@
             ImmutableSortedMap.of("ENV_VAR", "val", "ENV_VAR_WITH_SPACES", "foo bar"),
             /* runUnderPrefix= */ null,
             ImmutableList.of("C:/executable", "argv1", "arg w spaces"));
-    // TODO: https://github.com/bazelbuild/bazel/issues/21940 - Fix escaping.
     assertThat(result)
         .isEqualTo(
             """
@@ -201,7 +199,7 @@
               SET UNSET_ME_TOO=
               SET ENV_VAR=val
               SET ENV_VAR_WITH_SPACES=foo bar
-              C:\\executable argv1 arg w spaces %*\
+              C:\\executable argv1 "arg w spaces" %*\
             """);
   }
 }

src/test/shell/integration/run_test.sh

diff --git a/src/test/shell/integration/run_test.sh b/src/test/shell/integration/run_test.sh
index 7ebcb95..d3c274e 100755
--- a/src/test/shell/integration/run_test.sh
+++ b/src/test/shell/integration/run_test.sh
@@ -544,8 +544,8 @@
 
 function test_run_under_script_script_path() {
   if $is_windows; then
-    # TODO: https://github.com/bazelbuild/bazel/issues/21940 - Fix escaping, etc
-    # so that this test works on windows.
+    # TODO(https://github.com/bazelbuild/bazel/issues/22148): Fix --run_under
+    # paths under windows.
     return
   fi
   local -r pkg="pkg${LINENO}"
@@ -596,8 +596,8 @@
 
   bazel run --run_under="//$pkg:greetings friend &&" -- "//$pkg:farewell" buddy \
       >$TEST_log || fail "expected test to pass"
-  # TODO: bazel-team - This is just demonstrating how things are, it's probably
-  # not how we want them to be.
+  # TODO(https://github.com/bazelbuild/bazel/issues/22148): bazel-team - This is
+  # just demonstrating how things are, it's probably not how we want them to be.
   if "$is_windows"; then
     expect_log "hello there friend"
     expect_log "goodbye buddy"