Windows, bazel run: fix a bug in PR bazelbuild#8241

To "bazel run" a binary, the RunCommand creates an
ExecRequest and sends it to the Bazel client,
which then fulfills the request.

In PR bazelbuild#8241, RunCommand merged all command line
arguments and added that to the request as a
single argv entry. The client uses the first argv
element as the executable, but if all arguments
are joined into one string then argv0 contains the
whole argument vector as a single string.

Unfortunately the test didn't catch this because
it didn't attempt passing any arguments.

Fixes bazelbuild#8229
See bazelbuild#8240

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

@@ -504,9 +504,10 @@ public BlazeCommandResult exec(CommandEnvironment env, OptionsParsingResult opti
             ByteString.copyFrom(workingDir.getPathString(), StandardCharsets.ISO_8859_1));
 
     if (OS.getCurrent() == OS.WINDOWS && runOptions.bashlessRun) {
-      String joinedCommands =
-          Joiner.on(" ").join(Iterables.transform(cmdLine, x -> ShellUtils.windowsEscapeArg(x)));
-      execDescription.addArgv(ByteString.copyFrom(joinedCommands, StandardCharsets.ISO_8859_1));
+      for (String arg : cmdLine) {
+        execDescription.addArgv(
+            ByteString.copyFrom(ShellUtils.windowsEscapeArg(arg), StandardCharsets.ISO_8859_1));
+      }
     } else {
       PathFragment shExecutable = ShToolchain.getPath(configuration);
       if (shExecutable.isEmpty()) {

src/test/py/bazel/first_time_use_test.py

@@ -31,23 +31,44 @@ def testNoPythonRequirement(self):
       if 'python' in line and 'not found on PATH' in line:
         self._FailWithOutput(stdout + stderr)
 
+  def _AssertBazelRunBinaryOutput(self, exit_code, stdout, stderr, flag):
+    self.AssertExitCode(exit_code, 0, stderr)
+    found_hello = found_arg_a = found_arg_bc = False
+    for line in stdout + stderr:
+      if 'ERROR' in line and 'needs a shell' in line:
+        self._FailWithOutput(['flag=' + flag] + stdout + stderr)
+      if not found_hello and 'hello python' in line:
+        found_hello = True
+      elif not found_arg_a and 'arg[1]=(a)':
+        found_arg_a = True
+      elif not found_arg_bc and 'arg[2]=(b c)':
+        found_arg_bc = True
+        break
+    if not found_hello or not found_arg_a or not found_arg_bc:
+      self._FailWithOutput(['flag=' + flag] + stdout + stderr)
+
   def testNoBashRequiredForSimpleBazelRun(self):
     """Regression test for https://github.com/bazelbuild/bazel/issues/8229."""
     self.ScratchFile('WORKSPACE')
     self.ScratchFile('foo/BUILD', [
         'py_binary(',
         '    name = "x",'
         '    srcs = ["x.py"],',
+        '    args = ["a", "\'b c\'"],',
         ')',
     ])
     self.ScratchFile('foo/x.py', [
         'from __future__ import print_function',
+        'import sys',
         'print("hello python")',
+        'for i in range(len(sys.argv)):',
+        '    print("arg%d=(%s)" % (i, sys.argv[i]))',
     ])
 
     if test_base.TestBase.IsWindows():
       exit_code, stdout, stderr = self.RunBazel([
           'run',
+          # Run without shell but with shell-based "bazel run". Should fail.
           '--shell_executable=',
           '--noincompatible_windows_bashless_run_command',
           '//foo:x',
@@ -61,22 +82,16 @@ def testNoBashRequiredForSimpleBazelRun(self):
       if not found_error:
         self._FailWithOutput(stdout + stderr)
 
+      flag = '--incompatible_windows_bashless_run_command'
       exit_code, stdout, stderr = self.RunBazel([
           'run',
+          # Run without shell and with bashless "bazel run". Should succeed.
           '--shell_executable=',
-          '--incompatible_windows_bashless_run_command',
+          flag,
           '//foo:x',
       ])
-      self.AssertExitCode(exit_code, 0, stderr)
-      found_output = False
-      for line in stdout + stderr:
-        if 'ERROR' in line and 'needs a shell' in line:
-          self._FailWithOutput(stdout + stderr)
-        if 'hello python' in line:
-          found_output = True
-          break
-      if not found_output:
-        self._FailWithOutput(stdout + stderr)
+
+      self._AssertBazelRunBinaryOutput(exit_code, stdout, stderr, flag)
     else:
       # The --incompatible_windows_bashless_run_command should be a no-op on
       # platforms other than Windows.
@@ -86,6 +101,8 @@ def testNoBashRequiredForSimpleBazelRun(self):
       ]:
         exit_code, stdout, stderr = self.RunBazel([
             'run',
+            # Run fails because we provide no shell. Platforms other than
+            # Windows always use Bash for "bazel run".
             '--shell_executable=',
             flag,
             '//foo:x',
@@ -99,6 +116,10 @@ def testNoBashRequiredForSimpleBazelRun(self):
         if not found_error:
           self._FailWithOutput(['flag=' + flag] + stdout + stderr)
 
+        # Run succeeds because there is a shell.
+        exit_code, stdout, stderr = self.RunBazel(['run', flag, '//foo:x'])
+        self._AssertBazelRunBinaryOutput(exit_code, stdout, stderr, flag)
+
 
 if __name__ == '__main__':
   unittest.main()