From 089cc3bb5a6712668f4554e088da47a133cf72be Mon Sep 17 00:00:00 2001 From: Laszlo Csomor Date: Fri, 7 Sep 2018 08:56:03 -0700 Subject: [PATCH] Windows: fix writing java.log Fixes https://github.com/bazelbuild/bazel/issues/6099 Fixes https://github.com/bazelbuild/bazel/issues/6098 Related to https://github.com/bazelbuild/bazel/issues/2576 Change-Id: Iabff91cca715d08e2977234e8615b2e0bf8a5dbc Closes #6100. Change-Id: Ib55e01b5a82315d2a5f1a861f56079013d1cfec8 PiperOrigin-RevId: 211983456 --- src/main/cpp/startup_options.cc | 6 +-- src/main/cpp/util/path_windows.cc | 37 ++++++++++++++----- .../integration/execution_phase_tests.sh | 15 +------- .../shell/integration/server_logging_test.sh | 3 -- 4 files changed, 33 insertions(+), 28 deletions(-) diff --git a/src/main/cpp/startup_options.cc b/src/main/cpp/startup_options.cc index c54728f92e0a2a..e71972ce5d4e0b 100644 --- a/src/main/cpp/startup_options.cc +++ b/src/main/cpp/startup_options.cc @@ -540,9 +540,9 @@ blaze_exit_code::ExitCode StartupOptions::AddJVMArguments( void StartupOptions::AddJVMLoggingArguments(std::vector *result) const { // Configure logging - const string propFile = - blaze_util::JoinPath(output_base, "javalog.properties"); - string java_log( + const string propFile = 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..07231475c814cb 100644 --- a/src/main/cpp/util/path_windows.cc +++ b/src/main/cpp/util/path_windows.cc @@ -119,18 +119,37 @@ 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; + << "PathAsJvmFlag(" << path << "): 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) { 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