Repository: bazelbuild/bazel
Issue: Windows, Bazel 0.17.0rc2, regression: bazel no longer writes java.log

### Description of the problem / feature request:

0.17.0 rc2 regression over rc1: Bazel on Windows no longer writes `<output_base>/java.log`.

### Bugs: what's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

Build anything with 0.17.0 rc1, then run `bazel info output_base`, list the reported directory, see `java.log` is there.
Do the same with rc2, see that `java.log` is not there.

### What operating system are you running Bazel on?

Windows 10 v1709

Issue comments:
I found the root cause: similarly to https://github.com/bazelbuild/bazel/issues/2576, it's the java log path. Turns out we must pass the path with escaped backslashes instead of forward slashes. Working on a fix.
How come it worked in 0.17rc1 then?
I don't know yet. What I do know is that (a) the fix solves this issue, and (b) the fix removes an exception from the `<output_base>/server/jvm.out` that I've been aware of for a long time, but which never seemed to result in any problems (until now?).

Pull request review comments:
Isn't this quadratic? Mind you, not that the strings in question are very long (cue PATH_MAX), but if there is a simple way...
You're fully right. Let me fix that.
nit: `std::count` could be used to count these numbers, but I'm not at all sure if it would result in simpler code.
nit: you overwrite it anyway, why zero it out?
I'm not sure if a blackbelt in C++ would be able to do it using the standard library, but I sure don't know how. string splitting/joining is apparently not in the standard library and this doesn't require retreating to a monastery for years to make sense of, so go ahead.
Thanks for the idea! I'd need to use `count_if` and I think the resulting code would be neither more readable nor more efficient.
Thanks! I'm also not aware of a good library function for this.
AFAIK this is how you reserve space in a string.
That sentiment, I can heartily agree with! :)

Patch that resolved the issue:
diff --git a/src/main/cpp/startup_options.cc b/src/main/cpp/startup_options.cc
index 05ec6de69ac7b5..fbf34679d32e55 100644
--- a/src/main/cpp/startup_options.cc
+++ b/src/main/cpp/startup_options.cc
@@ -537,8 +537,9 @@ blaze_exit_code::ExitCode StartupOptions::AddJVMArguments(
 void StartupOptions::AddJVMLoggingArguments(std::vector<string> *result) const {
   // Configure logging
   const string propFile =
-      blaze_util::JoinPath(output_base, "javalog.properties");
-  string java_log(
+      blaze_util::PathAsJvmFlag(
+          blaze_util::JoinPath(output_base, "javalog.properties"));
+  const string java_log(
       blaze_util::PathAsJvmFlag(blaze_util::JoinPath(output_base, "java.log")));
   if (!blaze_util::WriteFile("handlers=java.util.logging.FileHandler\n"
                              ".level=INFO\n"
diff --git a/src/main/cpp/util/path_windows.cc b/src/main/cpp/util/path_windows.cc
index c7afe4021d3578..bb1b227e1a56f0 100644
--- a/src/main/cpp/util/path_windows.cc
+++ b/src/main/cpp/util/path_windows.cc
@@ -119,18 +119,38 @@ bool CompareAbsolutePaths(const std::string& a, const std::string& b) {
 }
 
 std::string PathAsJvmFlag(const std::string& path) {
-  std::string spath;
+  std::string cpath;
   std::string error;
-  if (!AsShortWindowsPath(path, &spath, &error)) {
+  if (!AsWindowsPath(path, &cpath, &error)) {
     BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR)
         << "PathAsJvmFlag(" << path
-        << "): AsShortWindowsPath failed: " << error;
+        << "): AsWindowsPath failed: " << error;
   }
-  // Convert backslashes to forward slashes, in order to avoid the JVM parsing
-  // Windows paths as if they contained escaped characters.
-  // See https://github.com/bazelbuild/bazel/issues/2576
-  std::replace(spath.begin(), spath.end(), '\\', '/');
-  return spath;
+  // Convert forward slashes and backslashes to double (escaped) backslashes, so
+  // they are safe to pass on the command line to the JVM and the JVM won't
+  // misinterpret them.
+  // See https://github.com/bazelbuild/bazel/issues/2576 and
+  // https://github.com/bazelbuild/bazel/issues/6098
+  size_t separators = 0;
+  for (const auto& c : cpath) {
+    if (c == '/' || c == '\\') {
+      separators++;
+    }
+  }
+  // In the result we replace each '/' and '\' with "\\", i.e. the total size
+  // *increases* by `separators`.
+  // Create a string of that size, filled with zeroes.
+  std::string result(/* count */ cpath.size() + separators, '\0');
+  std::string::size_type i = 0;
+  for (const auto& c : cpath) {
+    if (c == '/' || c == '\\') {
+      result[i++] = '\\';
+      result[i++] = '\\';
+    } else {
+      result[i++] = c;
+    }
+  }
+  return result;
 }
 
 void AddUncPrefixMaybe(std::wstring* path) {


Test patch:
diff --git a/src/test/shell/integration/execution_phase_tests.sh b/src/test/shell/integration/execution_phase_tests.sh
index 74b3e1b14d98e2..71951a21f55f21 100755
--- a/src/test/shell/integration/execution_phase_tests.sh
+++ b/src/test/shell/integration/execution_phase_tests.sh
@@ -105,9 +105,6 @@ function assert_cache_stats() {
 #### TESTS #############################################################
 
 function test_cache_computed_file_digests_behavior() {
-  # Does not work on Windows, https://github.com/bazelbuild/bazel/issues/6098
-  return
-
   local -r pkg="${FUNCNAME}"
   mkdir -p "$pkg" || fail "could not create \"$pkg\""
 
@@ -168,9 +165,8 @@ EOF
   assert_cache_stats "miss count" 1  # volatile-status.txt
 }
 
-function test_cache_computed_file_digests_uncaught_changes() {
-  return
-
+function DISABLED_test_cache_computed_file_digests_uncaught_changes() {
+  # Does not work on Windows, https://github.com/bazelbuild/bazel/issues/6098
   local timestamp=201703151112.13  # Fixed timestamp to mark our file with.
 
   mkdir -p package || fail "mkdir failed"
@@ -230,9 +226,6 @@ EOF
 }
 
 function test_cache_computed_file_digests_ui() {
-  # Does not work on Windows, https://github.com/bazelbuild/bazel/issues/6098
-  return
-
   local -r pkg="${FUNCNAME}"
   mkdir -p "$pkg" || fail "could not create \"$pkg\""
 
@@ -257,8 +250,6 @@ function test_cache_computed_file_digests_ui() {
 }
 
 function test_jobs_default_auto() {
-  return
-
   local -r pkg="${FUNCNAME}"
   mkdir -p "$pkg" || fail "could not create \"$pkg\""
 
@@ -279,8 +270,6 @@ function test_jobs_default_auto() {
 }
 
 function test_analysis_warning_cached() {
-  return
-
   mkdir -p "foo" "bar" || fail "Could not create directories"
   cat > foo/BUILD <<'EOF' || fail "foo/BUILD"
 cc_library(
diff --git a/src/test/shell/integration/server_logging_test.sh b/src/test/shell/integration/server_logging_test.sh
index 931a4dedaeaa9a..bee416098fc3c8 100755
--- a/src/test/shell/integration/server_logging_test.sh
+++ b/src/test/shell/integration/server_logging_test.sh
@@ -67,9 +67,6 @@ fi
 #### TESTS #############################################################
 
 function test_log_file_uses_single_line_formatter() {
-  # Does not work on Windows, https://github.com/bazelbuild/bazel/issues/6098
-  return
-
   local client_log="$(bazel info output_base)/java.log"
 
   # Construct a regular expression to match a sample message in the log using