Windows: remove incompatible_windows_style_arg_escaping
In this PR:
- Removed the
`--incompatible_windows_style_arg_escaping` flag
and all code for its "false" case. The flag is
flipped to true.
- In WindowsProcessesTest, the
testDoesNotQuoteArgWithDoubleQuote method is
replaced by testQuotesArgWithDoubleQuote, i.e.
the test logic is reversed. The new test shows
the correct behavior, the old test was wrong.
See https://github.com/bazelbuild/bazel/issues/7122
See https://github.com/bazelbuild/bazel/issues/7454
RELNOTES[INC]: The --incompatible_windows_style_arg_escaping flag is flipped to "true", and the "false" case unsupported. Bazel no longer accepts this flag.
Closes #8003.
PiperOrigin-RevId: 255929367
diff --git a/src/main/cpp/bazel_startup_options.cc b/src/main/cpp/bazel_startup_options.cc
index b6cce11..2ec8c8c 100644
--- a/src/main/cpp/bazel_startup_options.cc
+++ b/src/main/cpp/bazel_startup_options.cc
@@ -28,10 +28,8 @@
use_system_rc(true),
use_workspace_rc(true),
use_home_rc(true),
- use_master_bazelrc_(true),
- incompatible_windows_style_arg_escaping(true) {
+ use_master_bazelrc_(true) {
RegisterNullaryStartupFlag("home_rc");
- RegisterNullaryStartupFlag("incompatible_windows_style_arg_escaping");
RegisterNullaryStartupFlag("master_bazelrc");
RegisterNullaryStartupFlag("system_rc");
RegisterNullaryStartupFlag("workspace_rc");
@@ -106,14 +104,6 @@
}
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;
@@ -147,13 +137,4 @@
}
}
-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 5eba030..d84cfd2 100644
--- a/src/main/cpp/bazel_startup_options.h
+++ b/src/main/cpp/bazel_startup_options.h
@@ -26,8 +26,6 @@
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;
@@ -41,13 +39,6 @@
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 718a3dc..ebef4cc 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
@@ -1125,10 +1125,9 @@
return OS.getCurrent() == OS.WINDOWS ? new WindowsFileSystem() : new UnixFileSystem();
}
- private static SubprocessFactory subprocessFactoryImplementation(
- BlazeServerStartupOptions startupOptions) {
+ private static SubprocessFactory subprocessFactoryImplementation() {
if (!"0".equals(System.getProperty("io.bazel.EnableJni")) && OS.getCurrent() == OS.WINDOWS) {
- return new WindowsSubprocessFactory(startupOptions.windowsStyleArgEscaping);
+ return WindowsSubprocessFactory.INSTANCE;
} else {
return JavaSubprocessFactory.INSTANCE;
}
@@ -1234,7 +1233,7 @@
"No module set the default hash function.", ExitCode.BLAZE_INTERNAL_ERROR, e);
}
Path.setFileSystemForSerialization(fs);
- SubprocessBuilder.setDefaultSubprocessFactory(subprocessFactoryImplementation(startupOptions));
+ SubprocessBuilder.setDefaultSubprocessFactory(subprocessFactoryImplementation());
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 5294198..31a2f8c 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
@@ -466,25 +466,6 @@
public boolean unlimitCoredumps;
@Option(
- name = "incompatible_windows_style_arg_escaping",
- defaultValue = "true", // 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;
-
- @Option(
name = "macos_qos_class",
defaultValue = "default", // Only for documentation; value is set and used by the client.
documentationCategory = OptionDocumentationCategory.BAZEL_CLIENT_OPTIONS,
diff --git a/src/main/java/com/google/devtools/build/lib/windows/WindowsSubprocessFactory.java b/src/main/java/com/google/devtools/build/lib/windows/WindowsSubprocessFactory.java
index 01e7588..4e3a91a 100644
--- a/src/main/java/com/google/devtools/build/lib/windows/WindowsSubprocessFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/windows/WindowsSubprocessFactory.java
@@ -32,11 +32,7 @@
* A subprocess factory that uses the Win32 API.
*/
public class WindowsSubprocessFactory implements SubprocessFactory {
- private final boolean windowsStyleArgEscaping;
-
- public WindowsSubprocessFactory(boolean windowsStyleArgEscaping) {
- this.windowsStyleArgEscaping = windowsStyleArgEscaping;
- }
+ public static final WindowsSubprocessFactory INSTANCE = new WindowsSubprocessFactory();
@Override
public Subprocess create(SubprocessBuilder builder) throws IOException {
@@ -74,21 +70,17 @@
}
private String escapeArgvRest(List<String> argv) {
- if (windowsStyleArgEscaping) {
- StringBuilder result = new StringBuilder();
- boolean first = true;
- for (String arg : argv) {
- if (first) {
- first = false;
- } else {
- result.append(" ");
- }
- result.append(ShellUtils.windowsEscapeArg(arg));
+ StringBuilder result = new StringBuilder();
+ boolean first = true;
+ for (String arg : argv) {
+ if (first) {
+ first = false;
+ } else {
+ result.append(" ");
}
- return result.toString();
- } else {
- return WindowsProcesses.quoteCommandLine(argv);
+ result.append(ShellUtils.windowsEscapeArg(arg));
}
+ return result.toString();
}
public static String processArgv0(String argv0) {
diff --git a/src/main/java/com/google/devtools/build/lib/windows/jni/WindowsProcesses.java b/src/main/java/com/google/devtools/build/lib/windows/jni/WindowsProcesses.java
index 7e00ddb..63d72c5 100644
--- a/src/main/java/com/google/devtools/build/lib/windows/jni/WindowsProcesses.java
+++ b/src/main/java/com/google/devtools/build/lib/windows/jni/WindowsProcesses.java
@@ -14,7 +14,6 @@
package com.google.devtools.build.lib.windows.jni;
-import java.util.List;
/** Process management on Windows. */
public class WindowsProcesses {
@@ -215,59 +214,4 @@
}
private static native int nativeGetpid();
-
- // TODO(laszlocsomor): Replace this method with ShellUtils.windowsEscapeArg in order to fix
- // https://github.com/bazelbuild/bazel/issues/7122
- public static String quoteCommandLine(List<String> argv) {
- StringBuilder result = new StringBuilder();
- for (int iArg = 0; iArg < argv.size(); iArg++) {
- if (iArg != 0) {
- result.append(" ");
- }
- String arg = argv.get(iArg);
- if (arg.isEmpty()) {
- result.append("\"\"");
- continue;
- }
- boolean hasSpace = arg.contains(" ");
- if (!arg.contains("\"") && !arg.contains("\\") && !hasSpace) {
- // fast path. Just append the input string.
- result.append(arg);
- } else {
- // We need to quote things if the argument contains a space.
- if (hasSpace) {
- result.append("\"");
- }
-
- for (int iChar = 0; iChar < arg.length(); iChar++) {
- boolean lastChar = iChar == arg.length() - 1;
- switch (arg.charAt(iChar)) {
- case '"':
- // Escape double quotes
- result.append("\\\"");
- break;
- case '\\':
- // Backslashes at the end of the string are quoted if we add quotes
- if (lastChar) {
- result.append(hasSpace ? "\\\\" : "\\");
- } else {
- // Backslashes everywhere else are quoted if they are followed by a
- // quote or a backslash
- result.append(
- arg.charAt(iChar + 1) == '"' || arg.charAt(iChar + 1) == '\\' ? "\\\\" : "\\");
- }
- break;
- default:
- result.append(arg.charAt(iChar));
- }
- }
- // Add ending quotes if we added a quote at the beginning.
- if (hasSpace) {
- result.append("\"");
- }
- }
- }
-
- return result.toString();
- }
}
diff --git a/src/test/cpp/bazel_startup_options_test.cc b/src/test/cpp/bazel_startup_options_test.cc
index 5f7cf4d..0326441 100644
--- a/src/test/cpp/bazel_startup_options_test.cc
+++ b/src/test/cpp/bazel_startup_options_test.cc
@@ -87,7 +87,6 @@
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/java/com/google/devtools/build/lib/windows/WindowsProcessesTest.java b/src/test/java/com/google/devtools/build/lib/windows/WindowsProcessesTest.java
index 91e4fe4..e1b4ec7 100644
--- a/src/test/java/com/google/devtools/build/lib/windows/WindowsProcessesTest.java
+++ b/src/test/java/com/google/devtools/build/lib/windows/WindowsProcessesTest.java
@@ -18,7 +18,8 @@
import static java.nio.charset.StandardCharsets.UTF_16LE;
import static java.nio.charset.StandardCharsets.UTF_8;
-import com.google.common.collect.ImmutableList;
+import com.google.common.base.Joiner;
+import com.google.devtools.build.lib.shell.ShellUtils;
import com.google.devtools.build.lib.testutil.TestSpec;
import com.google.devtools.build.lib.util.OS;
import com.google.devtools.build.lib.windows.jni.WindowsProcesses;
@@ -65,16 +66,26 @@
}
}
+ private static List<String> quoteArgs(List<String> argv, String... args) {
+ for (String arg : args) {
+ argv.add(ShellUtils.windowsEscapeArg(arg));
+ }
+ return argv;
+ }
+
+ private static List<String> quoteArgs(String... args) {
+ List<String> argv = new ArrayList<>();
+ return quoteArgs(argv, args);
+ }
+
private String mockArgs(String... args) {
List<String> argv = new ArrayList<>();
argv.add("-jar");
argv.add(mockSubprocess);
- for (String arg : args) {
- argv.add(arg);
- }
+ quoteArgs(argv, args);
- return WindowsProcesses.quoteCommandLine(argv);
+ return Joiner.on(" ").join(argv);
}
private void assertNoProcessError() throws Exception {
@@ -87,35 +98,32 @@
@Test
public void testDoesNotQuoteSimpleArg() throws Exception {
- assertThat(WindowsProcesses.quoteCommandLine(ImmutableList.of("x", "a"))).isEqualTo("x a");
+ assertThat(quoteArgs("x", "a")).containsExactly("x", "a").inOrder();
}
@Test
public void testQuotesEmptyArg() throws Exception {
- assertThat(WindowsProcesses.quoteCommandLine(ImmutableList.of("x", ""))).isEqualTo("x \"\"");
+ assertThat(quoteArgs("x", "")).containsExactly("x", "\"\"").inOrder();
}
@Test
public void testQuotesArgWithSpace() throws Exception {
- assertThat(WindowsProcesses.quoteCommandLine(ImmutableList.of("x", "a b")))
- .isEqualTo("x \"a b\"");
+ assertThat(quoteArgs("x", "a b")).containsExactly("x", "\"a b\"").inOrder();
}
@Test
public void testDoesNotQuoteArgWithBackslash() throws Exception {
- assertThat(WindowsProcesses.quoteCommandLine(ImmutableList.of("x", "a\\b")))
- .isEqualTo("x a\\b");
+ assertThat(quoteArgs("x", "a\\b")).containsExactly("x", "a\\b").inOrder();
}
@Test
public void testDoesNotQuoteArgWithSingleQuote() throws Exception {
- assertThat(WindowsProcesses.quoteCommandLine(ImmutableList.of("x", "a'b"))).isEqualTo("x a'b");
+ assertThat(quoteArgs("x", "a'b")).containsExactly("x", "a'b").inOrder();
}
@Test
- public void testDoesNotQuoteArgWithDoubleQuote() throws Exception {
- assertThat(WindowsProcesses.quoteCommandLine(ImmutableList.of("x", "a\"b")))
- .isEqualTo("x a\\\"b");
+ public void testQuotesArgWithDoubleQuote() throws Exception {
+ assertThat(quoteArgs("x", "a\"b", "y")).containsExactly("x", "\"a\\\"b\"", "y").inOrder();
}
@Test
diff --git a/src/test/java/com/google/devtools/build/lib/windows/WindowsSubprocessTest.java b/src/test/java/com/google/devtools/build/lib/windows/WindowsSubprocessTest.java
index d392230..77ccd85 100644
--- a/src/test/java/com/google/devtools/build/lib/windows/WindowsSubprocessTest.java
+++ b/src/test/java/com/google/devtools/build/lib/windows/WindowsSubprocessTest.java
@@ -17,17 +17,14 @@
import static com.google.common.truth.Truth.assertThat;
import static java.nio.charset.StandardCharsets.UTF_8;
-import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.devtools.build.lib.shell.ShellUtils;
import com.google.devtools.build.lib.shell.Subprocess;
import com.google.devtools.build.lib.shell.SubprocessBuilder;
import com.google.devtools.build.lib.testutil.TestSpec;
import com.google.devtools.build.lib.util.OS;
-import com.google.devtools.build.lib.windows.jni.WindowsProcesses;
import com.google.devtools.build.runfiles.Runfiles;
import java.io.File;
-import java.util.function.Function;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
@@ -65,9 +62,9 @@
}
}
- private void assertSystemRootIsSetByDefault(boolean windowsStyleArgEscaping) throws Exception {
- SubprocessBuilder subprocessBuilder =
- new SubprocessBuilder(new WindowsSubprocessFactory(windowsStyleArgEscaping));
+ @Test
+ public void testSystemRootIsSetByDefault() throws Exception {
+ SubprocessBuilder subprocessBuilder = new SubprocessBuilder(WindowsSubprocessFactory.INSTANCE);
subprocessBuilder.setWorkingDirectory(new File("."));
subprocessBuilder.setArgv(mockBinary, "-jar", mockSubprocess, "O$SYSTEMROOT");
process = subprocessBuilder.start();
@@ -80,18 +77,8 @@
}
@Test
- public void testSystemRootIsSetByDefaultNoWindowsStyleArgEscaping() throws Exception {
- assertSystemRootIsSetByDefault(false);
- }
-
- @Test
- public void testSystemRootIsSetByDefaultWithWindowsStyleArgEscaping() throws Exception {
- assertSystemRootIsSetByDefault(true);
- }
-
- private void assertSystemDriveIsSetByDefault(boolean windowsStyleArgEscaping) throws Exception {
- SubprocessBuilder subprocessBuilder =
- new SubprocessBuilder(new WindowsSubprocessFactory(windowsStyleArgEscaping));
+ public void testSystemDriveIsSetByDefault() throws Exception {
+ SubprocessBuilder subprocessBuilder = new SubprocessBuilder(WindowsSubprocessFactory.INSTANCE);
subprocessBuilder.setWorkingDirectory(new File("."));
subprocessBuilder.setArgv(mockBinary, "-jar", mockSubprocess, "O$SYSTEMDRIVE");
process = subprocessBuilder.start();
@@ -104,18 +91,8 @@
}
@Test
- public void testSystemDriveIsSetByDefaultNoWindowsStyleArgEscaping() throws Exception {
- assertSystemDriveIsSetByDefault(false);
- }
-
- @Test
- public void testSystemDriveIsSetByDefaultWithWindowsStyleArgEscaping() throws Exception {
- assertSystemDriveIsSetByDefault(true);
- }
-
- private void assertSystemRootIsSet(boolean windowsStyleArgEscaping) throws Exception {
- SubprocessBuilder subprocessBuilder =
- new SubprocessBuilder(new WindowsSubprocessFactory(windowsStyleArgEscaping));
+ public void testSystemRootIsSet() throws Exception {
+ SubprocessBuilder subprocessBuilder = new SubprocessBuilder(WindowsSubprocessFactory.INSTANCE);
subprocessBuilder.setWorkingDirectory(new File("."));
subprocessBuilder.setArgv(mockBinary, "-jar", mockSubprocess, "O$SYSTEMROOT");
// Case shouldn't matter on Windows
@@ -130,18 +107,8 @@
}
@Test
- public void testSystemRootIsSetNoWindowsStyleArgEscaping() throws Exception {
- assertSystemRootIsSet(false);
- }
-
- @Test
- public void testSystemRootIsSetWithWindowsStyleArgEscaping() throws Exception {
- assertSystemRootIsSet(true);
- }
-
- private void assertSystemDriveIsSet(boolean windowsStyleArgEscaping) throws Exception {
- SubprocessBuilder subprocessBuilder =
- new SubprocessBuilder(new WindowsSubprocessFactory(windowsStyleArgEscaping));
+ public void testSystemDriveIsSet() throws Exception {
+ SubprocessBuilder subprocessBuilder = new SubprocessBuilder(WindowsSubprocessFactory.INSTANCE);
subprocessBuilder.setWorkingDirectory(new File("."));
subprocessBuilder.setArgv(mockBinary, "-jar", mockSubprocess, "O$SYSTEMDRIVE");
// Case shouldn't matter on Windows
@@ -155,16 +122,6 @@
assertThat(new String(buf, UTF_8).trim()).isEqualTo("X:");
}
- @Test
- public void testSystemDriveIsSetNoWindowsStyleArgEscaping() throws Exception {
- assertSystemDriveIsSet(false);
- }
-
- @Test
- public void testSystemDriveIsSetWithWindowsStyleArgEscaping() throws Exception {
- assertSystemDriveIsSet(true);
- }
-
/**
* An argument and its command-line-escaped counterpart.
*
@@ -181,9 +138,7 @@
};
/** Asserts that a subprocess correctly receives command line arguments. */
- private void assertSubprocessReceivesArgsAsIntended(
- boolean windowsStyleArgEscaping, Function<String, String> escaper, ArgPair... args)
- throws Exception {
+ private void assertSubprocessReceivesArgsAsIntended(ArgPair... args) throws Exception {
// Look up the path of the printarg.exe utility.
String printArgExe =
runfiles.rlocation("io_bazel/src/test/java/com/google/devtools/build/lib/printarg.exe");
@@ -191,11 +146,11 @@
for (ArgPair arg : args) {
// Assert that the command-line encoding logic works as intended.
- assertThat(escaper.apply(arg.original)).isEqualTo(arg.escaped);
+ assertThat(ShellUtils.windowsEscapeArg(arg.original)).isEqualTo(arg.escaped);
// Create a separate subprocess just for this argument.
SubprocessBuilder subprocessBuilder =
- new SubprocessBuilder(new WindowsSubprocessFactory(windowsStyleArgEscaping));
+ new SubprocessBuilder(WindowsSubprocessFactory.INSTANCE);
subprocessBuilder.setWorkingDirectory(new File("."));
subprocessBuilder.setArgv(printArgExe, arg.original);
process = subprocessBuilder.start();
@@ -212,25 +167,8 @@
}
@Test
- public void testSubprocessReceivesArgsAsIntendedNoWindowsStyleArgEscaping() throws Exception {
+ public void testSubprocessReceivesArgsAsIntended() throws Exception {
assertSubprocessReceivesArgsAsIntended(
- false,
- x -> WindowsProcesses.quoteCommandLine(ImmutableList.of(x)),
- new ArgPair("", "\"\""),
- new ArgPair(" ", "\" \""),
- new ArgPair("foo", "foo"),
- new ArgPair("foo\\bar", "foo\\bar"),
- new ArgPair("foo bar", "\"foo bar\""));
- // TODO(laszlocsomor): the escaping logic in WindowsProcesses.quoteCommandLine is wrong, because
- // it fails to properly escape things like a single backslash followed by a quote, e.g. a\"b
- // Fix the escaping logic and add more test here.
- }
-
- @Test
- public void testSubprocessReceivesArgsAsIntendedWithWindowsStyleArgEscaping() throws Exception {
- assertSubprocessReceivesArgsAsIntended(
- true,
- x -> ShellUtils.windowsEscapeArg(x),
new ArgPair("", "\"\""),
new ArgPair(" ", "\" \""),
new ArgPair("\"", "\"\\\"\""),
diff --git a/src/test/py/bazel/test_wrapper_test.py b/src/test/py/bazel/test_wrapper_test.py
index 77fdb01..9a756d4 100644
--- a/src/test/py/bazel/test_wrapper_test.py
+++ b/src/test/py/bazel/test_wrapper_test.py
@@ -346,10 +346,6 @@
bazel_bin = bazel_bin[0]
exit_code, stdout, stderr = self.RunBazel([
- # --[no]incompatible_windows_style_arg_escaping affects what arguments
- # the test receives. Run with --incompatible_windows_style_arg_escaping
- # to test for future (as of 2019-04-05) behavior.
- '--incompatible_windows_style_arg_escaping',
'test',
'//foo:testargs_test',
'-t-',
diff --git a/src/test/shell/bazel/windows_arg_esc_test.sh b/src/test/shell/bazel/windows_arg_esc_test.sh
index 2667a8d..dd5be79 100755
--- a/src/test/shell/bazel/windows_arg_esc_test.sh
+++ b/src/test/shell/bazel/windows_arg_esc_test.sh
@@ -87,9 +87,11 @@
return [DefaultInfo(executable = output)]
def _impl1(ctx):
+ # See https://github.com/bazelbuild/bazel/issues/7122
return _impl(ctx, '${1+"$@"}', "foo1")
def _impl2(ctx):
+ # See https://github.com/bazelbuild/bazel/issues/7122
return _impl(ctx, '${1+"$@"} ', "foo2")
args1_test = rule(implementation = _impl1, test = True)
@@ -115,58 +117,16 @@
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
-
+function test_windows_style_arg_escaping() {
create_pkg
- bazel $flag build foo:x --verbose_failures 2>$TEST_log \
+ bazel 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
+ assert_command_succeeded foo1
- # 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 \
+ bazel 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"
