Windows: add --incompatible_windows_style_arg_escaping
Add the --incompatible_windows_style_arg_escaping
flag (default: false). This flag has no effect on
platforms other than Windows.
Semantics:
- When true: subprocess arguments are escaped with
ShellUtils.windowsEscapeArg. This is the correct
behavior.
- When false: subprocess arguments are escaped
with ShellUtils.quoteCommandLine. This is the
default behavior on all platforms, but it is
incorrect on Windows.
Incompatible flag: https://github.com/bazelbuild/bazel/issues/7454
Related: https://github.com/bazelbuild/bazel/issues/7122
RELNOTES[NEW]: Added --incompatible_windows_style_arg_escaping flag: enables correct subprocess argument escaping on Windows. (No-op on other platforms.)
PiperOrigin-RevId: 234581069
diff --git a/src/main/cpp/bazel_startup_options.cc b/src/main/cpp/bazel_startup_options.cc
index 2ec8c8c..f4d494f 100644
--- a/src/main/cpp/bazel_startup_options.cc
+++ b/src/main/cpp/bazel_startup_options.cc
@@ -28,8 +28,10 @@
use_system_rc(true),
use_workspace_rc(true),
use_home_rc(true),
- use_master_bazelrc_(true) {
+ use_master_bazelrc_(true),
+ incompatible_windows_style_arg_escaping(false) {
RegisterNullaryStartupFlag("home_rc");
+ RegisterNullaryStartupFlag("incompatible_windows_style_arg_escaping");
RegisterNullaryStartupFlag("master_bazelrc");
RegisterNullaryStartupFlag("system_rc");
RegisterNullaryStartupFlag("workspace_rc");
@@ -104,6 +106,14 @@
}
use_master_bazelrc_ = false;
option_sources["blazerc"] = rcfile;
+ } else if (GetNullaryOption(arg,
+ "--incompatible_windows_style_arg_escaping")) {
+ incompatible_windows_style_arg_escaping = true;
+ option_sources["incompatible_windows_style_arg_escaping"] = rcfile;
+ } else if (GetNullaryOption(arg,
+ "--noincompatible_windows_style_arg_escaping")) {
+ incompatible_windows_style_arg_escaping = false;
+ option_sources["incompatible_windows_style_arg_escaping"] = rcfile;
} else {
*is_processed = false;
return blaze_exit_code::SUCCESS;
@@ -137,4 +147,13 @@
}
}
+void BazelStartupOptions::AddExtraOptions(
+ std::vector<std::string> *result) const {
+ if (incompatible_windows_style_arg_escaping) {
+ result->push_back("--incompatible_windows_style_arg_escaping");
+ } else {
+ result->push_back("--noincompatible_windows_style_arg_escaping");
+ }
+}
+
} // namespace blaze
diff --git a/src/main/cpp/bazel_startup_options.h b/src/main/cpp/bazel_startup_options.h
index d84cfd2..5eba030 100644
--- a/src/main/cpp/bazel_startup_options.h
+++ b/src/main/cpp/bazel_startup_options.h
@@ -26,6 +26,8 @@
public:
explicit BazelStartupOptions(const WorkspaceLayout *workspace_layout);
+ void AddExtraOptions(std::vector<std::string> *result) const override;
+
blaze_exit_code::ExitCode ProcessArgExtra(
const char *arg, const char *next_arg, const std::string &rcfile,
const char **value, bool *is_processed, std::string *error) override;
@@ -39,6 +41,13 @@
bool use_home_rc;
// TODO(b/36168162): Remove the master rc flag.
bool use_master_bazelrc_;
+
+ // Whether Windows-style subprocess argument escaping is enabled on Windows,
+ // or the (buggy) Bash-style is used.
+ // This flag only affects builds on Windows, and it's a no-op on other
+ // platforms.
+ // See https://github.com/bazelbuild/bazel/issues/7122
+ bool incompatible_windows_style_arg_escaping;
};
} // namespace blaze
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java
index e6e30fa..b500053 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java
@@ -1046,9 +1046,10 @@
return OS.getCurrent() == OS.WINDOWS ? new WindowsFileSystem() : new UnixFileSystem();
}
- private static SubprocessFactory subprocessFactoryImplementation() {
+ private static SubprocessFactory subprocessFactoryImplementation(
+ BlazeServerStartupOptions startupOptions) {
if (!"0".equals(System.getProperty("io.bazel.EnableJni")) && OS.getCurrent() == OS.WINDOWS) {
- return new WindowsSubprocessFactory(false);
+ return new WindowsSubprocessFactory(startupOptions.windowsStyleArgEscaping);
} else {
return JavaSubprocessFactory.INSTANCE;
}
@@ -1155,7 +1156,7 @@
"No module set the default hash function.", ExitCode.BLAZE_INTERNAL_ERROR, e);
}
Path.setFileSystemForSerialization(fs);
- SubprocessBuilder.setDefaultSubprocessFactory(subprocessFactoryImplementation());
+ SubprocessBuilder.setDefaultSubprocessFactory(subprocessFactoryImplementation(startupOptions));
Path outputUserRootPath = fs.getPath(outputUserRoot);
Path installBasePath = fs.getPath(installBase);
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeServerStartupOptions.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeServerStartupOptions.java
index bd0d705..1c60aa7 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeServerStartupOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeServerStartupOptions.java
@@ -464,4 +464,23 @@
+ " flag in your bazelrc once and forget about it so that you get coredumps when you"
+ " actually encounter a condition that triggers them.")
public boolean unlimitCoredumps;
+
+ @Option(
+ name = "incompatible_windows_style_arg_escaping",
+ defaultValue = "false", // NOTE: purely decorative, rc files are read by the client.
+ documentationCategory = OptionDocumentationCategory.EXECUTION_STRATEGY,
+ effectTags = {
+ OptionEffectTag.ACTION_COMMAND_LINES,
+ OptionEffectTag.EXECUTION,
+ },
+ metadataTags = {
+ OptionMetadataTag.INCOMPATIBLE_CHANGE,
+ OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES,
+ },
+ help =
+ "On Linux/macOS/non-Windows: no-op. On Windows: if true, then subprocess arguments are"
+ + " escaped Windows-style. When false, the arguments are escaped Bash-style. The"
+ + " Bash-style is buggy, the Windows-style is correct. See"
+ + " https://github.com/bazelbuild/bazel/issues/7122")
+ public boolean windowsStyleArgEscaping;
}
diff --git a/src/test/cpp/bazel_startup_options_test.cc b/src/test/cpp/bazel_startup_options_test.cc
index 1e91902..18467a3 100644
--- a/src/test/cpp/bazel_startup_options_test.cc
+++ b/src/test/cpp/bazel_startup_options_test.cc
@@ -87,6 +87,7 @@
ExpectIsNullaryOption(options, "fatal_event_bus_exceptions");
ExpectIsNullaryOption(options, "home_rc");
ExpectIsNullaryOption(options, "host_jvm_debug");
+ ExpectIsNullaryOption(options, "incompatible_windows_style_arg_escaping");
ExpectIsNullaryOption(options, "shutdown_on_low_sys_mem");
ExpectIsNullaryOption(options, "ignore_all_rc_files");
ExpectIsNullaryOption(options, "master_bazelrc");
diff --git a/src/test/shell/bazel/BUILD b/src/test/shell/bazel/BUILD
index b491f11..dc9f303 100644
--- a/src/test/shell/bazel/BUILD
+++ b/src/test/shell/bazel/BUILD
@@ -799,3 +799,10 @@
],
deps = ["@bazel_tools//tools/bash/runfiles"],
)
+
+sh_test(
+ name = "windows_arg_esc_test",
+ srcs = ["windows_arg_esc_test.sh"],
+ data = [":test-deps"],
+ deps = ["@bazel_tools//tools/bash/runfiles"],
+)
diff --git a/src/test/shell/bazel/windows_arg_esc_test.sh b/src/test/shell/bazel/windows_arg_esc_test.sh
new file mode 100755
index 0000000..2667a8d
--- /dev/null
+++ b/src/test/shell/bazel/windows_arg_esc_test.sh
@@ -0,0 +1,172 @@
+#!/bin/bash
+#
+# 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.
+
+# --- begin runfiles.bash initialization ---
+# Copy-pasted from Bazel's Bash runfiles library (tools/bash/runfiles/runfiles.bash).
+set -euo pipefail
+if [[ ! -d "${RUNFILES_DIR:-/dev/null}" && ! -f "${RUNFILES_MANIFEST_FILE:-/dev/null}" ]]; then
+ if [[ -f "$0.runfiles_manifest" ]]; then
+ export RUNFILES_MANIFEST_FILE="$0.runfiles_manifest"
+ elif [[ -f "$0.runfiles/MANIFEST" ]]; then
+ export RUNFILES_MANIFEST_FILE="$0.runfiles/MANIFEST"
+ elif [[ -f "$0.runfiles/bazel_tools/tools/bash/runfiles/runfiles.bash" ]]; then
+ export RUNFILES_DIR="$0.runfiles"
+ fi
+fi
+if [[ -f "${RUNFILES_DIR:-/dev/null}/bazel_tools/tools/bash/runfiles/runfiles.bash" ]]; then
+ source "${RUNFILES_DIR}/bazel_tools/tools/bash/runfiles/runfiles.bash"
+elif [[ -f "${RUNFILES_MANIFEST_FILE:-/dev/null}" ]]; then
+ source "$(grep -m1 "^bazel_tools/tools/bash/runfiles/runfiles.bash " \
+ "$RUNFILES_MANIFEST_FILE" | cut -d ' ' -f 2-)"
+else
+ echo >&2 "ERROR: cannot find @bazel_tools//tools/bash/runfiles:runfiles.bash"
+ exit 1
+fi
+# --- end runfiles.bash initialization ---
+
+source "$(rlocation "io_bazel/src/test/shell/integration_test_setup.sh")" \
+ || { echo "integration_test_setup.sh not found!" >&2; exit 1; }
+
+# `uname` returns the current platform, e.g "MSYS_NT-10.0" or "Linux".
+# `tr` converts all upper case letters to lower case.
+# `case` matches the result if the `uname | tr` expression to string prefixes
+# that use the same wildcards as names do in Bash, i.e. "msys*" matches strings
+# starting with "msys", and "*" matches everything (it's the default case).
+case "$(uname -s | tr [:upper:] [:lower:])" in
+msys*)
+ # As of 2019-02-18, Bazel on Windows only supports MSYS Bash.
+ declare -r is_windows=true
+ ;;
+*)
+ declare -r is_windows=false
+ ;;
+esac
+
+if "$is_windows"; then
+ # Disable MSYS path conversion that converts path-looking command arguments to
+ # Windows paths (even if they arguments are not in fact paths).
+ export MSYS_NO_PATHCONV=1
+ export MSYS2_ARG_CONV_EXCL="*"
+fi
+
+
+function create_pkg() {
+ if [[ -d foo ]]; then
+ return
+ fi
+
+ mkdir foo || fail "mkdir foo"
+
+ cat >foo/BUILD <<'EOF'
+load(":rules.bzl", "args1_test", "args2_test")
+args1_test(name = "x")
+args2_test(name = "y")
+EOF
+
+ cat >foo/rules.bzl <<'EOF'
+def _impl(ctx, cmd, out):
+ output = ctx.actions.declare_file(out)
+ ctx.actions.run_shell(
+ outputs = [output],
+ command = cmd,
+ arguments = ['echo', 'a', 'b', 'c'],
+ )
+ return [DefaultInfo(executable = output)]
+
+def _impl1(ctx):
+ return _impl(ctx, '${1+"$@"}', "foo1")
+
+def _impl2(ctx):
+ return _impl(ctx, '${1+"$@"} ', "foo2")
+
+args1_test = rule(implementation = _impl1, test = True)
+
+args2_test = rule(implementation = _impl2, test = True)
+EOF
+}
+
+function assert_foo1_failed() {
+ # The command fails because Bazel invokes it incorrectly.
+ expect_log "error executing shell command"
+ expect_log "\$: command not found"
+ expect_not_log "output .*foo/foo1.* was not created"
+}
+
+function assert_command_succeeded() {
+ local -r output=$1
+
+ # The command succeeds, though the build fails because the command does not
+ # create any outputs. This is expected.
+ expect_log "From.* foo/${output}"
+ expect_log "output .*foo/${output}.* was not created"
+ expect_log "not all outputs were created"
+}
+
+function assert_build() {
+ local -r enable_windows_style_arg_escaping=$1
+
+ if $enable_windows_style_arg_escaping; then
+ local -r flag="--incompatible_windows_style_arg_escaping"
+ local -r expect_foo1_fails="false"
+ else
+ local -r flag="--noincompatible_windows_style_arg_escaping"
+ local -r expect_foo1_fails="$is_windows"
+ fi
+
+ create_pkg
+
+ bazel $flag build foo:x --verbose_failures 2>$TEST_log \
+ && fail "expected failure" || true
+ if $expect_foo1_fails; then
+ # foo1's command does not work on Windows without the Windows-style argument
+ # escaping, see https://github.com/bazelbuild/bazel/issues/7122
+ assert_foo1_failed
+ else
+ assert_command_succeeded foo1
+ fi
+
+ # foo2's command works on Windows even with the incorrect escaping semantics,
+ # see https://github.com/bazelbuild/bazel/issues/7122
+ bazel $flag build foo:y --verbose_failures 2>$TEST_log \
+ && fail "expected failure" || true
+ assert_command_succeeded foo2
+}
+
+function test_nowindows_style_arg_escaping() {
+ # On Windows: assert that with --noincompatible_windows_style_arg_escaping,
+ # Bazel incorrectly escapes the shell command arguments of "//foo:x" and
+ # reports a bogus error ("/usr/bin/bash: $: command not found"), but it
+ # correctly invokes the command for "//foo:y".
+ #
+ # On other platforms: assert that Bazel correctly invokes the commands for
+ # both "//foo:x" and "//foo:y". This is not affected by
+ # --noincompatible_windows_style_arg_escaping.
+ # The test should behave the same as test_windows_style_arg_escaping.
+ assert_build false
+}
+
+function test_windows_style_arg_escaping() {
+ # On Windows: assert that with --incompatible_windows_style_arg_escaping,
+ # Bazel correctly invokes the commands for both "//foo:x" and "//foo:y".
+ #
+ # On other platforms: assert that Bazel correctly invokes the commands for
+ # both "//foo:x" and "//foo:y". This is not affected by
+ # --incompatible_windows_style_arg_escaping.
+ # The test should behave the same as test_nowindows_style_arg_escaping.
+ assert_build true
+}
+
+run_suite "Windows argument escaping test"
