Windows: fix native test wrapper's arg. escaping
The native test wrapper now correctly escapes the
arguments for the subprocess, using
bazel::launcher::WindowsEscapeArg2 from the native
launcher.
The test_wrapper_test now uses a mock C++ binary
instead of a .bat file to verify the test
arguments. Doing so trades the weird command line
flag parsing logic of cmd.exe (inherent in using a
.bat file) for the saner C++ flag parsing one.
Fixes https://github.com/bazelbuild/bazel/issues/7956
Unblocks https://github.com/bazelbuild/bazel/issues/6622
Closes #7957.
PiperOrigin-RevId: 242446422
diff --git a/src/test/py/bazel/BUILD b/src/test/py/bazel/BUILD
index d5f59d6..31432e1 100644
--- a/src/test/py/bazel/BUILD
+++ b/src/test/py/bazel/BUILD
@@ -156,6 +156,10 @@
"//src/conditions:windows": ["test_wrapper_test.py"],
"//conditions:default": ["empty_test.py"],
}),
+ data = select({
+ "//src/conditions:windows": [":printargs"],
+ "//conditions:default": [],
+ }),
main = select({
"//src/conditions:windows": "test_wrapper_test.py",
"//conditions:default": "empty_test.py",
@@ -166,6 +170,12 @@
}),
)
+cc_binary(
+ name = "printargs",
+ testonly = 1,
+ srcs = ["printargs.cc"],
+)
+
py_test(
name = "first_time_use_test",
srcs = ["first_time_use_test.py"],
diff --git a/src/test/py/bazel/printargs.cc b/src/test/py/bazel/printargs.cc
new file mode 100644
index 0000000..315ac48
--- /dev/null
+++ b/src/test/py/bazel/printargs.cc
@@ -0,0 +1,23 @@
+// Copyright 2019 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+// Prints the command line arguments, one on each line, as arg=(<ARG>).
+// This program aids testing the flags Bazel passes on the command line.
+#include <stdio.h>
+int main(int argc, char** argv) {
+ for (int i = 1; i < argc; ++i) {
+ printf("arg=(%s)\n", argv[i]);
+ }
+ return 0;
+}
diff --git a/src/test/py/bazel/test_wrapper_test.py b/src/test/py/bazel/test_wrapper_test.py
index 7665f68..49e88e3 100644
--- a/src/test/py/bazel/test_wrapper_test.py
+++ b/src/test/py/bazel/test_wrapper_test.py
@@ -81,9 +81,9 @@
' 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,20 +134,10 @@
'@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)',
- ],
+
+ 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
@@ -385,7 +375,7 @@
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 @@
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 @@
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 @@
# 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)
diff --git a/src/tools/launcher/util/BUILD b/src/tools/launcher/util/BUILD
index 5077e23..e3aa79a 100644
--- a/src/tools/launcher/util/BUILD
+++ b/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 @@
name = "util",
srcs = ["launcher_util.cc"],
hdrs = ["launcher_util.h"],
+ visibility = [
+ "//src/tools/launcher:__subpackages__",
+ "//tools/test:__pkg__",
+ ],
deps = ["//src/main/cpp/util:filesystem"],
)
diff --git a/tools/test/BUILD b/tools/test/BUILD
index 735e2fb..6deb7dd 100644
--- a/tools/test/BUILD
+++ b/tools/test/BUILD
@@ -80,6 +80,7 @@
"//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",
],
diff --git a/tools/test/windows/tw.cc b/tools/test/windows/tw.cc
index aec0853..d577a02 100644
--- a/tools/test/windows/tw.cc
+++ b/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"
@@ -1117,8 +1118,7 @@
}
}
-bool CreateCommandLine(const Path& path,
- const std::vector<const wchar_t*>& args,
+bool CreateCommandLine(const Path& path, 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 +1132,9 @@
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 +1145,7 @@
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 +1342,7 @@
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 +1358,7 @@
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;
}
@@ -1433,8 +1433,7 @@
return true;
}
-int RunSubprocess(const Path& test_path,
- const std::vector<const wchar_t*>& args,
+int RunSubprocess(const Path& test_path, const std::vector<std::wstring>& args,
const Path& test_outerr, Duration* test_duration) {
std::unique_ptr<Tee> tee;
bazel::windows::AutoHandle process;
@@ -1871,7 +1870,7 @@
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) ||
