Add client option to disable updating the mtime of the install_base

src/main/cpp/blaze.cc

@@ -476,6 +476,13 @@ static vector<string> GetServerExeArgs(const blaze_util::Path &jvm_path,
   } else {
     result.push_back("--client_debug=false");
   }
+  // Same as above; this option is only used by the client and doesn't
+  // necessitate a server restart:
+  if (startup_options.update_install_base_mtime) {
+    result.push_back("--update_install_base_mtime=true");
+  } else {
+    result.push_back("--update_install_base_mtime=false");
+  }
 
   // These flags are passed to the java process only for Blaze reporting
   // purposes; the real interpretation of the jvm flags occurs when we set up
@@ -858,6 +865,7 @@ static bool IsVolatileArg(const string &arg) {
   static const std::set<string> volatile_startup_options = {
       "--option_sources=", "--max_idle_secs=", "--connect_timeout_secs=",
       "--local_startup_timeout_secs=", "--client_debug=", "--preemptible=",
+      "--update_install_base_mtime=",
       // Internally, -XX:HeapDumpPath is set automatically via the user's TMPDIR
       // environment variable. Since that can change based on the shell, we
       // tolerate changes to it. Note that an explicit setting of
@@ -1009,13 +1017,15 @@ static void EnsureCorrectRunningVersion(const StartupOptions &startup_options,
 
     // Update the mtime of the install base so that cleanup tools can
     // find install bases that haven't been used for a long time
-    std::unique_ptr<blaze_util::IFileMtime> mtime(
-        blaze_util::CreateFileMtime());
-    if (!mtime->SetToNow(blaze_util::Path(startup_options.install_base))) {
-      string err = GetLastErrorString();
-      BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR)
-          << "failed to set timestamp on '" << startup_options.install_base
-          << "': " << err;
+    if (startup_options.update_install_base_mtime) {
+      std::unique_ptr<blaze_util::IFileMtime> mtime(
+          blaze_util::CreateFileMtime());
+      if (!mtime->SetToNow(blaze_util::Path(startup_options.install_base))) {
+        string err = GetLastErrorString();
+        BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR)
+            << "failed to set timestamp on '" << startup_options.install_base
+            << "': " << err;
+      }
     }
   }
 }

src/main/cpp/startup_options.cc

@@ -92,6 +92,7 @@ StartupOptions::StartupOptions(const string &product_name,
       expand_configs_in_place(true),
       digest_function(),
       idle_server_tasks(true),
+      update_install_base_mtime(true),
       original_startup_options_(std::vector<RcStartupFlag>()),
 #if defined(__APPLE__)
       macos_qos_class(QOS_CLASS_UNSPECIFIED),
@@ -143,6 +144,7 @@ StartupOptions::StartupOptions(const string &product_name,
   RegisterNullaryStartupFlag("shutdown_on_low_sys_mem",
                              &shutdown_on_low_sys_mem);
   RegisterNullaryStartupFlagNoRc("ignore_all_rc_files", &ignore_all_rc_files);
+  RegisterNullaryStartupFlag("update_install_base_mtime", &update_install_base_mtime);
   RegisterNullaryStartupFlag("unlimit_coredumps", &unlimit_coredumps);
   RegisterNullaryStartupFlag("watchfs", &watchfs);
   RegisterNullaryStartupFlag("write_command_log", &write_command_log);

src/main/cpp/startup_options.h

@@ -146,6 +146,10 @@ class StartupOptions {
   // Installation base for a specific release installation.
   std::string install_base;
 
+  // Whether to attempt to update the mtime of the provided `install_base`
+  // directory.
+  bool update_install_base_mtime;
+
   // The toplevel directory containing Blaze's output.  When Blaze is
   // run by a test, we use TEST_TMPDIR, simplifying the correct
   // hermetic invocation of Blaze from tests.

src/main/java/com/google/devtools/build/lib/runtime/BlazeServerStartupOptions.java

@@ -92,6 +92,16 @@ public String getTypeDescription() {
       help = "This launcher option is intended for use only by tests.")
   public PathFragment installBase;
 
+  @Option(
+      name = "update_install_base_mtime",
+      defaultValue = "true", // NOTE: only for documentation, value is set and used by the client.
+      documentationCategory = OptionDocumentationCategory.BAZEL_CLIENT_OPTIONS,
+      effectTags = {OptionEffectTag.BAZEL_INTERNAL_CONFIGURATION},
+      help =
+          "If set, the client will attempt to update the `mtime` of the `install_base` directory. "
+              + "Changing this option will not cause the server to restart.")
+  public boolean updateInstallBaseMtime;
+
   /*
    * The installation MD5 - a content hash of the blaze binary (includes the Blaze deploy JAR and
    * any other embedded binaries - anything that ends up in the install_base).

src/test/cpp/bazel_startup_options_test.cc

@@ -104,6 +104,7 @@ TEST_F(BazelStartupOptionsTest, ValidStartupFlags) {
   ExpectValidNullaryOption(options, "ignore_all_rc_files");
   ExpectValidNullaryOption(options, "shutdown_on_low_sys_mem");
   ExpectValidNullaryOption(options, "system_rc");
+  ExpectValidNullaryOption(options, "update_install_base_mtime");
   ExpectValidNullaryOption(options, "watchfs");
   ExpectValidNullaryOption(options, "workspace_rc");
   ExpectValidNullaryOption(options, "write_command_log");

src/test/shell/integration/client_test.sh

@@ -49,6 +49,57 @@ function test_client_debug_change_does_not_restart_server() {
   expect_not_log "WARNING.* Running B\\(azel\\|laze\\) server needs to be killed"
 }
 
+# Note: the install base check is elided if the `install` symlink in the output
+# base matches the given install base:
+# https://github.com/bazelbuild/bazel/blob/a3c677dfea2de636a719d50345a5a97af96fae60/src/main/cpp/blaze.cc#L991-L1001
+#
+# To force this check to re-run, we pass a new `output_base` path on every
+# invocation.
+function test_update_install_base_mtime_updates_mtime() {
+  local install_base="$TEST_TMPDIR/install"
+  local args=(
+    --batch --update_install_base_mtime
+    --install_base="${install_base}"
+  )
+
+  bazel "${args[@]}" --output_base="$TEST_TMPDIR/out/a" version >&$TEST_log || fail "'bazel version' failed"
+  local original_mtime=$(stat -c %Y "${install_base}")
+
+  sleep 1
+  bazel "${args[@]}" --output_base="$TEST_TMPDIR/out/b" version >&$TEST_log || fail "'bazel version' failed"
+  local new_mtime=$(stat -c %Y "${install_base}")
+
+  assert_not_equals "$original_mtime" "$new_mtime"
+}
+
+# Really we'd like to test that `bazel --noupdate_install_base_mtime ...` is
+# happy with a read-only install base but unfortunately it's tricky to make an
+# install base that's read-only without `root` (`chattr`, `chown`, `mount`, etc.
+# all need privileges).
+function test_no_update_install_base_mtime_does_not_update_mtime() {
+  local install_base="$TEST_TMPDIR/install"
+  local args=(
+    --batch --noupdate_install_base_mtime
+    --install_base="${install_base}"
+  )
+
+  bazel "${args[@]}" --output_base="$TEST_TMPDIR/out/c" version >&$TEST_log || fail "'bazel version' failed"
+  local original_mtime=$(stat -c %Y "${install_base}")
+
+  sleep 1
+  bazel "${args[@]}" --output_base="$TEST_TMPDIR/out/d" version >&$TEST_log || fail "'bazel version' failed"
+  local new_mtime=$(stat -c %Y "${install_base}")
+
+  assert_equals "$original_mtime" "$new_mtime"
+}
+
+function test_update_install_base_mtime_change_does_not_restart_server() {
+  local server_pid1=$(bazel --update_install_base_mtime info server_pid 2>$TEST_log)
+  local server_pid2=$(bazel --noupdate_install_base_mtime info server_pid 2>$TEST_log)
+  assert_equals "$server_pid1" "$server_pid2"
+  expect_not_log "WARNING.* Running B\\(azel\\|laze\\) server needs to be killed"
+}
+
 function test_server_restart_due_to_startup_options() {
   local server_pid1=$(bazel --write_command_log info server_pid 2>$TEST_log)
   local server_pid2=$(bazel --nowrite_command_log info server_pid 2>$TEST_log)