Repository: bazelbuild/bazel
Issue: incompatible_windows_style_arg_escaping: enables correct subprocess argument escaping on Windows
Labels: area-Windows
incompatible-change
team-OSS

# Description

The option `--incompatible_windows_style_arg_escaping` enables correct subprocess argument escaping on Windows. This flag has NO effect on other platforms.

When enabled, [WindowsSubprocessFactory will use ShellUtils.windowsEscapeArg to escape command line arguments](https://github.com/bazelbuild/bazel/blob/1da48619d94ccc9fda59c114c6cd14904dbc6e34/src/main/java/com/google/devtools/build/lib/windows/WindowsSubprocessFactory.java#L77). This is correct, as [verified by tests](https://github.com/bazelbuild/bazel/blob/1da48619d94ccc9fda59c114c6cd14904dbc6e34/src/test/java/com/google/devtools/build/lib/windows/WindowsSubprocessTest.java#L230).

When disabled, [WindowsSubprocessFactory will use ShellUtils.quoteCommandLine](https://github.com/bazelbuild/bazel/blob/1da48619d94ccc9fda59c114c6cd14904dbc6e34/src/main/java/com/google/devtools/build/lib/windows/WindowsSubprocessFactory.java#L90). This is buggy, as shown by https://github.com/bazelbuild/bazel/issues/7122.

# Migration recipe

None, as of 2019-02-18.

We don't expect any breakages when this flag is enabled. However if it breaks your build, please let us know so we can help fixing it and provide a migration recipe.

# Rollout plan

- Bazel 0.23.0 will not support this flag.
- Bazel 0.24.0 is expected to support this flag, with default value being `false`.
- Bazel 0.25.0 is expected to flip this flag to true.

Issue comments:
Fixed by https://github.com/laszlocsomor/bazel/commit/363016efe0973f12f28ac42e43c79c84bb0a05df. Closing issue. 
No, https://github.com/laszlocsomor/bazel/commit/363016efe0973f12f28ac42e43c79c84bb0a05df was not merged! This flag is not a breaking change in 0.25
Sorry about that, thanks for updating the labels
I re-added breaking change 26 because that will put this bug in the queue for the next release manager to check up on it. No pressure if it doesn't get flipped, the protocol is to extend the window in that case. But I think if it doesn't have a breaking change label it will fall through the cracks eventually. 
Good to know. Thank you!
This flag is marked as `migration-0.25`, causing it to be run by the bazelisk incompatible change pipeline. But this flag is not available (at least under linux), causing the build to [fail](https://buildkite.com/bazel/bazelisk-plus-incompatible-flags/builds/104#5b8edad8-db18-440d-a0cb-08614a27c374/1958).
We can remove the migration tag; the flag is now flipped (but not yet removed).

This flag is a startup flag, which Bazelisk cannot handle.
I don't see any failure in the log you linked, can you check the link again?
The link goes to line 1958 of the Subpar build on 14.04 (OpenJDK 8). Copy/pasting the section:

```
Running Bazel with --incompatible_windows_style_arg_escaping | 0s
-- | --
  |  
  | bazel test --flaky_test_attempts=3 --build_tests_only --local_test_jobs=12 --show_progress_rate_limit=5 --curses=yes --color=yes --verbose_failures --keep_going --jobs=32 --announce_rc --experimental_multi_threaded_digest --experimental_repository_cache_hardlinks --disk_cache= --sandbox_tmpfs_path=/tmp --experimental_build_event_json_file_path_conversion=false --build_event_json_file=/tmp/tmp117gcv3x/test_bep.json --remote_timeout=60 --remote_max_connections=200 --remote_http_cache=https://storage.googleapis.com/bazel-untrusted-buildkite-cache/e0979f59aee862bb5636c07921415ff395dc7b7a24c297265429c5fae66e56f6 --google_default_credentials --test_env=HOME --sandbox_writable_path=/home/bazel/.cache/bazelisk ... --incompatible_windows_style_arg_escaping
  | ERROR: Unrecognized option: --incompatible_windows_style_arg_escaping
  | INFO: Invocation ID: 8c007530-88e6-423f-b651-02fe88d5071c
```

Bazelisk being unable to handle startup options is a good reason for this to fail the pipeline. I'll ignore.
Maybe we should label issues for startup flags such that bazelisk can exclude issues with the label.
@laurentlb can you fix the labels here for bazelisk, with 0.25.2 this fails because the flag isn't recognized
removed

Pull request review comments:

Patch that resolved the issue:
diff --git a/src/main/cpp/bazel_startup_options.cc b/src/main/cpp/bazel_startup_options.cc
index f4d494fe8d0d1b..b6cce11db7a6fb 100644
--- a/src/main/cpp/bazel_startup_options.cc
+++ b/src/main/cpp/bazel_startup_options.cc
@@ -29,7 +29,7 @@ BazelStartupOptions::BazelStartupOptions(
       use_workspace_rc(true),
       use_home_rc(true),
       use_master_bazelrc_(true),
-      incompatible_windows_style_arg_escaping(false) {
+      incompatible_windows_style_arg_escaping(true) {
   RegisterNullaryStartupFlag("home_rc");
   RegisterNullaryStartupFlag("incompatible_windows_style_arg_escaping");
   RegisterNullaryStartupFlag("master_bazelrc");
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 d4de1f90b855c7..52941982d7cfa1 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
@@ -467,7 +467,7 @@ public String getTypeDescription() {
 
   @Option(
       name = "incompatible_windows_style_arg_escaping",
-      defaultValue = "false", // NOTE: purely decorative, rc files are read by the client.
+      defaultValue = "true", // NOTE: purely decorative, rc files are read by the client.
       documentationCategory = OptionDocumentationCategory.EXECUTION_STRATEGY,
       effectTags = {
         OptionEffectTag.ACTION_COMMAND_LINES,


Test patch: