Optionally allow Bazel to pass JVM options containing spaces directly through to the JVM instead of (almost certainly incorrectly) splitting the options along spaces.

This allows us to pass non-quote-delimited strings to the JVM, which is necessary for things like -XX:OnOutOfMemoryError="kill -3 %p" (normally bash strips those quotes, but they're not stripped when passed via --host_jvm_args).

--
MOS_MIGRATED_REVID=107820087

6 files changed, 55 insertions, 14 deletions

diff --git a/src/main/cpp/blaze.cc b/src/main/cpp/blaze.cc
index 911bd3d469..ae8af0d424 100644
--- a/src/main/cpp/blaze.cc
+++ b/src/main/cpp/blaze.cc
@@ -261,8 +261,25 @@ static vector<string> GetArgumentArray() {
 
   vector<string> user_options;
 
-  blaze_util::SplitQuotedStringUsing(globals->options.host_jvm_args, ' ',
-                                     &user_options);
+  if (globals->options.preserve_spaces_in_host_jvm_args) {
+    user_options.insert(user_options.begin(),
+                        globals->options.host_jvm_args.begin(),
+                        globals->options.host_jvm_args.end());
+  } else {
+    for (const auto &arg : globals->options.host_jvm_args) {
+      // int num_segments =
+      blaze_util::SplitQuotedStringUsing(arg, ' ', &user_options);
+      // TODO(janakr): Enable this warning when users have been migrated.
+      //       if (num_segments > 1) {
+      //         fprintf(stderr, "WARNING: You are passing multiple jvm options"
+      //             " under a single --host_jvm_args option: %s. This will stop
+      //             working "
+      //             "soon. Instead, pass each option under its own
+      //             --host_jvm_args "
+      //             "option.\n", arg);
+      //
+    }
+  }
 
   // Add JVM arguments particular to building blaze64 and particular JVM
   // versions.
@@ -350,8 +367,13 @@ static vector<string> GetArgumentArray() {
   if (!globals->options.host_jvm_profile.empty()) {
     result.push_back("--host_jvm_profile=" + globals->options.host_jvm_profile);
   }
+  if (globals->options.preserve_spaces_in_host_jvm_args) {
+    result.push_back("--experimental_preserve_spaces_in_host_jvm_args");
+  }
   if (!globals->options.host_jvm_args.empty()) {
-    result.push_back("--host_jvm_args=" + globals->options.host_jvm_args);
+    for (const auto &arg : globals->options.host_jvm_args) {
+      result.push_back("--host_jvm_args=" + arg);
+    }
   }
 
   if (globals->options.invocation_policy != NULL &&
diff --git a/src/main/cpp/blaze_startup_options.h b/src/main/cpp/blaze_startup_options.h
index f29ab3fcb0..e61af4b47d 100644
--- a/src/main/cpp/blaze_startup_options.h
+++ b/src/main/cpp/blaze_startup_options.h
@@ -126,7 +126,9 @@ class BlazeStartupOptions {
 
   string host_jvm_profile;
 
-  string host_jvm_args;
+  bool preserve_spaces_in_host_jvm_args;
+
+  std::vector<string> host_jvm_args;
 
   bool batch;
 
diff --git a/src/main/cpp/blaze_startup_options_common.cc b/src/main/cpp/blaze_startup_options_common.cc
index 914f7ab4e5..8a5f6f1121 100644
--- a/src/main/cpp/blaze_startup_options_common.cc
+++ b/src/main/cpp/blaze_startup_options_common.cc
@@ -41,6 +41,8 @@ void BlazeStartupOptions::Init() {
   block_for_lock = true;
   host_jvm_debug = false;
   host_javabase = "";
+  // TODO(janakr): change this to true when ready, then delete it.
+  preserve_spaces_in_host_jvm_args = false;
   batch = false;
   batch_cpu_scheduling = false;
   blaze_cpu = false;
@@ -73,6 +75,7 @@ void BlazeStartupOptions::Copy(
   lhs->host_jvm_debug = rhs.host_jvm_debug;
   lhs->host_jvm_profile = rhs.host_jvm_profile;
   lhs->host_javabase = rhs.host_javabase;
+  lhs->preserve_spaces_in_host_jvm_args = rhs.preserve_spaces_in_host_jvm_args;
   lhs->host_jvm_args = rhs.host_jvm_args;
   lhs->batch = rhs.batch;
   lhs->batch_cpu_scheduling = rhs.batch_cpu_scheduling;
@@ -126,13 +129,13 @@ blaze_exit_code::ExitCode BlazeStartupOptions::ProcessArg(
     // and re-execing.
     host_javabase = MakeAbsolute(value);
     option_sources["host_javabase"] = rcfile;
+  } else if (GetNullaryOption(
+                 arg, "--experimental_preserve_spaces_in_host_jvm_args")) {
+    preserve_spaces_in_host_jvm_args = true;
+    option_sources["preserve_spaces_in_host_jvm_args"] = rcfile;
   } else if ((value = GetUnaryOption(arg, next_arg,
                                      "--host_jvm_args")) != NULL) {
-    if (host_jvm_args.empty()) {
-      host_jvm_args = value;
-    } else {
-      host_jvm_args = host_jvm_args + " " + value;
-    }
+    host_jvm_args.push_back(value);
     option_sources["host_jvm_args"] = rcfile;  // NB: This is incorrect
   } else if ((value = GetUnaryOption(arg, next_arg, "--blaze_cpu")) != NULL) {
     blaze_cpu = true;
diff --git a/src/main/cpp/util/strings.cc b/src/main/cpp/util/strings.cc
index 214f887aee..3df0b9ed2b 100644
--- a/src/main/cpp/util/strings.cc
+++ b/src/main/cpp/util/strings.cc
@@ -128,11 +128,12 @@ void SplitStringUsing(
   }
 }
 
-void SplitQuotedStringUsing(const string &contents, const char delimeter,
-                            std::vector<string> *output) {
+size_t SplitQuotedStringUsing(const string &contents, const char delimeter,
+                              std::vector<string> *output) {
   size_t len = contents.length();
   size_t start = 0;
   size_t quote = string::npos;  // quote position
+  size_t num_segments = 0;
 
   for (size_t pos = 0; pos < len; ++pos) {
     if (start == pos && contents[start] == delimeter) {
@@ -147,13 +148,16 @@ void SplitQuotedStringUsing(const string &contents, const char delimeter,
     } else if (quote == string::npos && contents[pos] == delimeter) {
       output->push_back(string(contents, start, pos - start));
       start = pos + 1;
+      num_segments++;
     }
   }
 
   // A trailing element
   if (start < len) {
     output->push_back(string(contents, start));
+    num_segments++;
   }
+  return num_segments;
 }
 
 void Replace(const string &oldsub, const string &newsub, string *str) {
diff --git a/src/main/cpp/util/strings.h b/src/main/cpp/util/strings.h
index 18d53b15f2..0dca21929d 100644
--- a/src/main/cpp/util/strings.h
+++ b/src/main/cpp/util/strings.h
@@ -80,9 +80,9 @@ std::vector<string> Split(const string &contents, const char delimeter);
 void SplitStringUsing(
     const string &contents, const char delimeter, std::vector<string> *output);
 
-// Same as above, but adds results to output.
-void SplitQuotedStringUsing(const string &contents, const char delimeter,
-                            std::vector<string> *output);
+// Same as above, but adds results to output. Returns number of elements added.
+size_t SplitQuotedStringUsing(const string &contents, const char delimeter,
+                              std::vector<string> *output);
 
 // Global replace of oldsub with newsub.
 void Replace(const string &oldsub, const string &newsub, string *str);
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 69d69da39d..2621a53035 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
@@ -213,6 +213,16 @@ public class BlazeServerStartupOptions extends OptionsBase {
       help = "Unused.")
   public String unusedSkyframe;
 
+  @Option(
+    name = "experimental_preserve_spaces_in_host_jvm_args",
+    defaultValue = "false",
+    category = "undocumented",
+    help =
+        "If this option is true, each argument to --host_jvm_args is considered a single JVM "
+            + "flag, even if it has spaces in it."
+  )
+  public boolean unusedPreserveSpacesInHostJvmArgs;
+
   @Option(name = "fatal_event_bus_exceptions",
       defaultValue = "false",  // NOTE: purely decorative!
       category = "undocumented",