Windows: fix native test wrapper's arg. escaping

src/test/py/bazel/BUILD

@@ -164,6 +164,16 @@ py_test(
         "//src/conditions:windows": [":test_base"],
         "//conditions:default": [],
     }),
+    data = select({
+        "//src/conditions:windows": [":printargs"],
+        "//conditions:default": [],
+    }),
+)
+
+cc_binary(
+    name = "printargs",
+    srcs = ["printargs.cc"],
+    testonly = 1,
 )
 
 py_test(

src/test/py/bazel/printargs.cc

@@ -0,0 +1,7 @@
+#include <stdio.h>
+int main(int argc, char** argv) {
+  for (int i = 1; i < argc; ++i) {
+    printf("arg=(%s)\n", argv[i]);
+  }
+  return 0;
+}

src/test/py/bazel/test_wrapper_test.py

@@ -81,9 +81,9 @@ def _CreateMockWorkspace(self):
         '    srcs = ["unexported.bat"],',
         ')',
         'sh_test(',
-        '    name = "testargs_test.bat",',
-        '    srcs = ["testargs.bat"],',
-        '    args = ["foo", "a b", "", "bar"],',
+        '    name = "testargs_test.exe",',
+        '    srcs = ["testargs.exe"],',
+        r'    args = ["foo", "a b", "", "\"c d\"", "\"\"", "bar"],',
         ')',
         'py_test(',
         '    name = "undecl_test",',
@@ -134,21 +134,11 @@ def _CreateMockWorkspace(self):
             '@echo BAD=%TEST_UNDECLARED_OUTPUTS_MANIFEST%',
         ],
         executable=True)
-    self.ScratchFile(
-        'foo/testargs.bat',
-        [
-            '@echo arg=(%~nx0)',  # basename of $0
-            '@echo arg=(%1)',
-            '@echo arg=(%2)',
-            '@echo arg=(%3)',
-            '@echo arg=(%4)',
-            '@echo arg=(%5)',
-            '@echo arg=(%6)',
-            '@echo arg=(%7)',
-            '@echo arg=(%8)',
-            '@echo arg=(%9)',
-        ],
-        executable=True)
+
+    self.CopyFile(
+        src_path = self.Rlocation("io_bazel/src/test/py/bazel/printargs.exe"),
+        dst_path = "foo/testargs.exe",
+        executable = True)
 
     # A single white pixel as an ".ico" file. /usr/bin/file should identify this
     # as "image/x-icon".
@@ -385,7 +375,7 @@ def _AssertTestArgs(self, flag, expected):
 
     exit_code, stdout, stderr = self.RunBazel([
         'test',
-        '//foo:testargs_test.bat',
+        '//foo:testargs_test.exe',
         '-t-',
         '--test_output=all',
         '--test_arg=baz',
@@ -568,24 +558,20 @@ def testTestExecutionWithTestSetupSh(self):
     self._AssertTestArgs(
         flag,
         [
-            '(testargs_test.bat)',
             '(foo)',
             '(a)',
             '(b)',
+            '(c d)',
+            '()',
             '(bar)',
-            # Note: debugging shows that test-setup.sh receives more-or-less
-            # good arguments (let's ignore issues #6276 and #6277 for now), but
-            # mangles the last few.
-            # I (laszlocsomor@) don't know the reason (as of 2018-10-01) but
-            # since I'm planning to phase out test-setup.sh on Windows in favor
-            # of the native test wrapper, I don't intend to debug this further.
-            # The test is here merely to guard against unwanted future change of
-            # behavior.
             '(baz)',
-            '("\\"x)',
-            '(y\\"")',
-            '("\\\\\\")',
-            '(qux")'
+            '("x y")',
+            # I (laszlocsomor@) don't know the exact reason (as of 2019-04-05)
+            # why () and (qux) are mangled as they are, but since I'm planning
+            # to phase out test-setup.sh on Windows in favor of the native test
+            # wrapper, I don't intend to debug this further.  The test is here
+            # merely to guard against unwanted future change of behavior.
+            '(\\" qux)'
         ])
     self._AssertUndeclaredOutputs(flag)
     self._AssertUndeclaredOutputsAnnotations(flag)
@@ -606,7 +592,6 @@ def testTestExecutionWithTestWrapperExe(self):
     self._AssertTestArgs(
         flag,
         [
-            '(testargs_test.bat)',
             '(foo)',
             # TODO(laszlocsomor): assert that "a b" is passed as one argument,
             # not two, after https://github.com/bazelbuild/bazel/issues/6277
@@ -616,12 +601,13 @@ def testTestExecutionWithTestWrapperExe(self):
             # TODO(laszlocsomor): assert that the empty string argument is
             # passed, after https://github.com/bazelbuild/bazel/issues/6276
             # is fixed.
+            '(c d)',
+            '()',
             '(bar)',
             '(baz)',
             '("x y")',
             '("")',
             '(qux)',
-            '()'
         ])
     self._AssertUndeclaredOutputs(flag)
     self._AssertUndeclaredOutputsAnnotations(flag)

src/tools/launcher/util/BUILD

@@ -1,6 +1,6 @@
 package(default_visibility = ["//src/tools/launcher:__subpackages__"])
 
-load("//src/tools/launcher:win_rules.bzl", "cc_library", "cc_binary", "cc_test")
+load("//src/tools/launcher:win_rules.bzl", "cc_binary", "cc_library", "cc_test")
 
 filegroup(
     name = "srcs",
@@ -19,6 +19,10 @@ cc_library(
     name = "util",
     srcs = ["launcher_util.cc"],
     hdrs = ["launcher_util.h"],
+    visibility = [
+        "//src/tools/launcher:__subpackages__",
+        "//tools/test:__pkg__",
+    ],
     deps = ["//src/main/cpp/util:filesystem"],
 )
 

tools/test/BUILD

@@ -80,6 +80,7 @@ cc_library(
             "//src/main/cpp/util:strings",
             "//src/main/native/windows:lib-file",
             "//src/main/native/windows:lib-util",
+            "//src/tools/launcher/util",
             "//third_party/ijar:zip",
             "@bazel_tools//tools/cpp/runfiles",
         ],

tools/test/windows/tw.cc

@@ -43,6 +43,7 @@
 #include "src/main/cpp/util/strings.h"
 #include "src/main/native/windows/file.h"
 #include "src/main/native/windows/util.h"
+#include "src/tools/launcher/util/launcher_util.h"
 #include "third_party/ijar/common.h"
 #include "third_party/ijar/platform_utils.h"
 #include "third_party/ijar/zip.h"
@@ -1118,7 +1119,7 @@ bool AddCommandLineArg(const wchar_t* arg, const size_t arg_size,
 }
 
 bool CreateCommandLine(const Path& path,
-                       const std::vector<const wchar_t*>& args,
+                       const std::vector<std::wstring>& args,
                        std::unique_ptr<WCHAR[]>* result) {
   // kMaxCmdline value: see lpCommandLine parameter of CreateProcessW.
   static constexpr size_t kMaxCmdline = 32767;
@@ -1132,9 +1133,9 @@ bool CreateCommandLine(const Path& path,
     return false;
   }
 
-  for (const auto arg : args) {
-    if (!AddCommandLineArg(arg, wcslen(arg), false, result->get(), kMaxCmdline,
-                           &total_len)) {
+  for (const std::wstring& arg : args) {
+    if (!AddCommandLineArg(arg.c_str(), arg.size(), false, result->get(),
+                           kMaxCmdline, &total_len)) {
       return false;
     }
   }
@@ -1145,7 +1146,7 @@ bool CreateCommandLine(const Path& path,
   return true;
 }
 
-bool StartSubprocess(const Path& path, const std::vector<const wchar_t*>& args,
+bool StartSubprocess(const Path& path, const std::vector<std::wstring>& args,
                      const Path& outerr, std::unique_ptr<Tee>* tee,
                      LARGE_INTEGER* start_time,
                      bazel::windows::AutoHandle* process) {
@@ -1342,7 +1343,7 @@ bool CreateUndeclaredOutputsAnnotations(const Path& undecl_annot_dir,
 
 bool ParseArgs(int argc, wchar_t** argv, Path* out_argv0,
                std::wstring* out_test_path_arg,
-               std::vector<const wchar_t*>* out_args) {
+               std::vector<std::wstring>* out_args) {
   if (!out_argv0->Set(argv[0])) {
     return false;
   }
@@ -1358,7 +1359,7 @@ bool ParseArgs(int argc, wchar_t** argv, Path* out_argv0,
   out_args->clear();
   out_args->reserve(argc - 1);
   for (int i = 1; i < argc; i++) {
-    out_args->push_back(argv[i]);
+    out_args->push_back(bazel::launcher::WindowsEscapeArg2(argv[i]));
   }
   return true;
 }
@@ -1434,7 +1435,7 @@ bool TeeImpl::MainFunc() const {
 }
 
 int RunSubprocess(const Path& test_path,
-                  const std::vector<const wchar_t*>& args,
+                  const std::vector<std::wstring>& args,
                   const Path& test_outerr, Duration* test_duration) {
   std::unique_ptr<Tee> tee;
   bazel::windows::AutoHandle process;
@@ -1871,7 +1872,7 @@ int TestWrapperMain(int argc, wchar_t** argv) {
   std::wstring test_path_arg;
   Path test_path, exec_root, srcdir, tmpdir, test_outerr, xml_log;
   UndeclaredOutputs undecl;
-  std::vector<const wchar_t*> args;
+  std::vector<std::wstring> args;
   if (!ParseArgs(argc, argv, &argv0, &test_path_arg, &args) ||
       !PrintTestLogStartMarker() ||
       !FindTestBinary(argv0, test_path_arg, &test_path) ||