Support explicitly specifying a location for jvm.out

Allows users to monitor server output without needing to fish the output base.
Windows support is copied more or less verbatim from recommendations, I unfortunately
don't know how to test this on windows.

PiperOrigin-RevId: 183674130

8 files changed, 84 insertions, 18 deletions

diff --git a/src/main/cpp/blaze.cc b/src/main/cpp/blaze.cc
index f159def72f..8c85bb62d0 100644
--- a/src/main/cpp/blaze.cc
+++ b/src/main/cpp/blaze.cc
@@ -425,6 +425,10 @@ static vector<string> GetArgumentArray() {
   result.push_back("--workspace_directory=" +
                    blaze::ConvertPath(globals->workspace));
 
+  if (!globals->options->server_jvm_out.empty()) {
+    result.push_back("--server_jvm_out=" + globals->options->server_jvm_out);
+  }
+
   if (globals->options->allow_configurable_attributes) {
     result.push_back("--allow_configurable_attributes");
   }
@@ -616,7 +620,8 @@ static int StartServer(const WorkspaceLayout *workspace_layout,
   // we can still print errors to the terminal.
   GoToWorkspace(workspace_layout);
 
-  return ExecuteDaemon(exe, jvm_args_vector, globals->jvm_log_file, server_dir,
+  return ExecuteDaemon(exe, jvm_args_vector, globals->jvm_log_file,
+                       globals->jvm_log_file_append, server_dir,
                        server_startup);
 }
 
@@ -775,9 +780,15 @@ static void StartServerAndConnect(const WorkspaceLayout *workspace_layout,
     std::this_thread::sleep_until(next_attempt_time);
     if (!server_startup->IsStillAlive()) {
       globals->option_processor->PrintStartupOptionsProvenanceMessage();
-      fprintf(stderr, "\nServer crashed during startup. Now printing '%s':\n",
-              globals->jvm_log_file.c_str());
-      WriteFileToStderrOrDie(globals->jvm_log_file.c_str());
+      fprintf(stderr, "\nServer crashed during startup. ");
+      if (globals->jvm_log_file_append) {
+        // Don't dump the log if we were appending - the user should know where
+        // to find it, and who knows how much content they may have accumulated.
+        fprintf(stderr, "See '%s'\n", globals->jvm_log_file.c_str());
+      } else {
+        fprintf(stderr, "Now printing '%s':\n", globals->jvm_log_file.c_str());
+        WriteFileToStderrOrDie(globals->jvm_log_file.c_str());
+      }
       exit(blaze_exit_code::INTERNAL_ERROR);
     }
   }
@@ -1257,8 +1268,14 @@ static void ComputeBaseDirectories(const WorkspaceLayout *workspace_layout,
 
   globals->lockfile =
       blaze_util::JoinPath(globals->options->output_base, "lock");
-  globals->jvm_log_file =
+  if (!globals->options->server_jvm_out.empty()) {
+    globals->jvm_log_file = globals->options->server_jvm_out;
+    globals->jvm_log_file_append = true;
+  } else {
+    globals->jvm_log_file =
       blaze_util::JoinPath(globals->options->output_base, "server/jvm.out");
+    globals->jvm_log_file_append = false;
+  }
 }
 
 // Prepares the environment to be suitable to start a JVM.
diff --git a/src/main/cpp/blaze_util_platform.h b/src/main/cpp/blaze_util_platform.h
index 80b95e1485..42706b50e9 100644
--- a/src/main/cpp/blaze_util_platform.h
+++ b/src/main/cpp/blaze_util_platform.h
@@ -102,13 +102,15 @@ class BlazeServerStartup {
 };
 
 // Starts a daemon process with its standard output and standard error
-// redirected to the file "daemon_output". Sets server_startup to an object
-// that can be used to query if the server is still alive. The PID of the
-// daemon started is written into server_dir, both as a symlink (for legacy
-// reasons) and as a file, and returned to the caller.
+// redirected (and conditionally appended) to the file "daemon_output". Sets
+// server_startup to an object that can be used to query if the server is
+// still alive. The PID of the daemon started is written into server_dir,
+// both as a symlink (for legacy reasons) and as a file, and returned to the
+// caller.
 int ExecuteDaemon(const std::string& exe,
                   const std::vector<std::string>& args_vector,
                   const std::string& daemon_output,
+                  const bool daemon_output_append,
                   const std::string& server_dir,
                   BlazeServerStartup** server_startup);
 
diff --git a/src/main/cpp/blaze_util_posix.cc b/src/main/cpp/blaze_util_posix.cc
index 967fde603b..fff8028712 100644
--- a/src/main/cpp/blaze_util_posix.cc
+++ b/src/main/cpp/blaze_util_posix.cc
@@ -195,7 +195,8 @@ bool SymlinkDirectories(const string &target, const string &link) {
 // Causes the current process to become a daemon (i.e. a child of
 // init, detached from the terminal, in its own session.)  We don't
 // change cwd, though.
-static void Daemonize(const char* daemon_output) {
+static void Daemonize(const char* daemon_output,
+                      const bool daemon_output_append) {
   // Don't call die() or exit() in this function; we're already in a
   // child process so it won't work as expected.  Just don't do
   // anything that can possibly fail. :)
@@ -216,7 +217,9 @@ static void Daemonize(const char* daemon_output) {
 
   open("/dev/null", O_RDONLY);  // stdin
   // stdout:
-  if (open(daemon_output, O_WRONLY | O_CREAT | O_TRUNC, 0666) == -1) {
+  int out_flags =
+      O_WRONLY | O_CREAT | (daemon_output_append ? O_APPEND : O_TRUNC);
+  if (open(daemon_output, out_flags, 0666) == -1) {
     // In a daemon, no-one can hear you scream.
     open("/dev/null", O_WRONLY);
   }
@@ -338,7 +341,9 @@ void WriteSystemSpecificProcessIdentifier(
 // localized here.
 int ExecuteDaemon(const string& exe,
                   const std::vector<string>& args_vector,
-                  const string& daemon_output, const string& server_dir,
+                  const string& daemon_output,
+                  const bool daemon_output_append,
+                  const string& server_dir,
                   BlazeServerStartup** server_startup) {
   int fds[2];
 
@@ -380,7 +385,7 @@ int ExecuteDaemon(const string& exe,
     // before ExecuteDaemon() to understand why.
     close(fds[0]);  // ...child keeps the other.
 
-    Daemonize(daemon_output_chars);
+    Daemonize(daemon_output_chars, daemon_output_append);
 
     pid_t server_pid = getpid();
     WriteToFdWithRetryEintr(fds[1], &server_pid, sizeof server_pid,
diff --git a/src/main/cpp/blaze_util_windows.cc b/src/main/cpp/blaze_util_windows.cc
index 56e510b643..cec6160779 100644
--- a/src/main/cpp/blaze_util_windows.cc
+++ b/src/main/cpp/blaze_util_windows.cc
@@ -449,7 +449,8 @@ static void WriteProcessStartupTime(const string& server_dir, HANDLE process) {
 }
 
 static HANDLE CreateJvmOutputFile(const wstring& path,
-                                  SECURITY_ATTRIBUTES* sa) {
+                                  SECURITY_ATTRIBUTES* sa,
+                                  bool daemon_out_append) {
   // If the previous server process was asked to be shut down (but not killed),
   // it takes a while for it to comply, so wait until the JVM output file that
   // it held open is closed. There seems to be no better way to wait for a file
@@ -461,10 +462,16 @@ static HANDLE CreateJvmOutputFile(const wstring& path,
         /* dwDesiredAccess */ GENERIC_READ | GENERIC_WRITE,
         /* dwShareMode */ FILE_SHARE_READ,
         /* lpSecurityAttributes */ sa,
-        /* dwCreationDisposition */ CREATE_ALWAYS,
+        /* dwCreationDisposition */
+            daemon_out_append ? OPEN_ALWAYS : CREATE_ALWAYS,
         /* dwFlagsAndAttributes */ FILE_ATTRIBUTE_NORMAL,
         /* hTemplateFile */ NULL);
     if (handle != INVALID_HANDLE_VALUE) {
+      if (daemon_out_append
+          && !SetFilePointerEx(handle, {0}, NULL, FILE_END)) {
+        fprintf(stderr, "Could not seek to end of file (%s)\n", path.c_str());
+        return INVALID_HANDLE_VALUE;
+      }
       return handle;
     }
     if (GetLastError() != ERROR_SHARING_VIOLATION &&
@@ -503,7 +510,8 @@ class ProcessHandleBlazeServerStartup : public BlazeServerStartup {
 
 
 int ExecuteDaemon(const string& exe, const std::vector<string>& args_vector,
-                  const string& daemon_output, const string& server_dir,
+                  const string& daemon_output, const bool daemon_out_append,
+                  const string& server_dir,
                   BlazeServerStartup** server_startup) {
   wstring wdaemon_output;
   if (!blaze_util::AsAbsoluteWindowsPath(daemon_output, &wdaemon_output)) {
@@ -527,7 +535,8 @@ int ExecuteDaemon(const string& exe, const std::vector<string>& args_vector,
          "ExecuteDaemon(%s): CreateFileA(NUL)", exe.c_str());
   }
 
-  AutoHandle stdout_file(CreateJvmOutputFile(wdaemon_output.c_str(), &sa));
+  AutoHandle stdout_file(CreateJvmOutputFile(wdaemon_output.c_str(), &sa,
+                                             daemon_out_append));
   if (!stdout_file.IsValid()) {
     pdie(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR,
          "ExecuteDaemon(%s): CreateJvmOutputFile(%ls)", exe.c_str(),
diff --git a/src/main/cpp/global_variables.h b/src/main/cpp/global_variables.h
index 061e2837a9..6baa6a9576 100644
--- a/src/main/cpp/global_variables.h
+++ b/src/main/cpp/global_variables.h
@@ -53,7 +53,12 @@ struct GlobalVariables {
   // Used to make concurrent invocations of this program safe.
   std::string lockfile;  // = <output_base>/lock
 
-  std::string jvm_log_file;  // = <output_base>/server/jvm.out
+  // Whrere to write the server's JVM's output. Default value is
+  // <output_base>/server/jvm.out.
+  std::string jvm_log_file;
+
+  // Whether or not the jvm_log_file should be opened with O_APPEND.
+  bool jvm_log_file_append;
 
   std::string cwd;
 
diff --git a/src/main/cpp/startup_options.cc b/src/main/cpp/startup_options.cc
index c6fb606a3c..fa2f1e02b4 100644
--- a/src/main/cpp/startup_options.cc
+++ b/src/main/cpp/startup_options.cc
@@ -150,6 +150,7 @@ StartupOptions::StartupOptions(const string &product_name,
   RegisterUnaryStartupFlag("max_idle_secs");
   RegisterUnaryStartupFlag("output_base");
   RegisterUnaryStartupFlag("output_user_root");
+  RegisterUnaryStartupFlag("server_jvm_out");
 }
 
 StartupOptions::~StartupOptions() {}
@@ -201,6 +202,10 @@ blaze_exit_code::ExitCode StartupOptions::ProcessArg(
                                      "--output_user_root")) != NULL) {
     output_user_root = MakeAbsolute(value);
     option_sources["output_user_root"] = rcfile;
+  } else if ((value = GetUnaryOption(arg, next_arg,
+                                     "--server_jvm_out")) != NULL) {
+    server_jvm_out = MakeAbsolute(value);
+    option_sources["server_jvm_out"] = rcfile;
   } else if (GetNullaryOption(arg, "--deep_execroot")) {
     deep_execroot = true;
     option_sources["deep_execroot"] = rcfile;
diff --git a/src/main/cpp/startup_options.h b/src/main/cpp/startup_options.h
index bfe6f79e7f..327d2d91ca 100644
--- a/src/main/cpp/startup_options.h
+++ b/src/main/cpp/startup_options.h
@@ -192,6 +192,10 @@ class StartupOptions {
   // The capitalized name of this binary.
   const std::string product_name;
 
+  // If supplied, alternate location to write the blaze server's jvm's stdout.
+  // Otherwise a default path in the output base is used.
+  std::string server_jvm_out;
+
   // Blaze's output base.  Everything is relative to this.  See
   // the BlazeDirectories Java class for details.
   std::string output_base;
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 97ed798f53..c2290ca917 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
@@ -147,6 +147,25 @@ public class BlazeServerStartupOptions extends OptionsBase {
   )
   public PathFragment outputUserRoot;
 
+  /**
+   * Note: This option is only used by the C++ client, never by the Java server.
+   * It is included here to make sure that the option is documented in the help
+   * output, which is auto-generated by Java code.
+   */
+  @Option(
+    name = "server_jvm_out",
+    defaultValue = "null", // NOTE: purely decorative!  See class docstring.
+    category = "server startup",
+    documentationCategory = OptionDocumentationCategory.BAZEL_CLIENT_OPTIONS,
+    effectTags = {OptionEffectTag.AFFECTS_OUTPUTS, OptionEffectTag.LOSES_INCREMENTAL_STATE},
+    converter = OptionsUtils.PathFragmentConverter.class,
+    valueHelp = "<path>",
+    help =
+        "The location to write the server's JVM's output. If unset then defaults to a location "
+            + "in output_base."
+  )
+  public PathFragment serverJvmOut;
+
   @Option(
     name = "workspace_directory",
     defaultValue = "",