From 7f782e3c1a1c4a21beea19c2cedb614be8316e67 Mon Sep 17 00:00:00 2001
From: Rahul Butani <rrbutani@users.noreply.github.com>
Date: Fri, 15 Dec 2023 08:44:54 -0800
Subject: [PATCH] Ignore read-only errors when updating the `mtime` of the
 `install_base`
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Currently if the `--install_base` path passed is not writable by the user invoking Bazel, the Bazel client crashes:
```console
❯ bazel --install_base=/some/read/only/path version
FATAL: failed to set timestamp on '/some/read/only/path': (error: 30): Read-only file system
```

This happens because the Bazel client (unconditionally) attempts to update the `mtime` of this path:
https://github.com/bazelbuild/bazel/blob/a3c677dfea2de636a719d50345a5a97af96fae60/src/main/cpp/blaze.cc#L1010-L1021

This commit updates the client to ignore such errors. See #20373 for context.

Closes #20442.

PiperOrigin-RevId: 591266054
Change-Id: If53e7cad48cb62406f7883f72b413e4b5a0bb8e2
---
 src/main/cpp/blaze.cc             |  4 +++-
 src/main/cpp/util/file_platform.h |  6 ++++++
 src/main/cpp/util/file_posix.cc   | 14 ++++++++++++++
 src/main/cpp/util/file_windows.cc | 14 ++++++++++++++
 4 files changed, 37 insertions(+), 1 deletion(-)

diff --git a/src/main/cpp/blaze.cc b/src/main/cpp/blaze.cc
index 6a764b15a52082..128104d4ecfec7 100644
--- a/src/main/cpp/blaze.cc
+++ b/src/main/cpp/blaze.cc
@@ -1011,7 +1011,9 @@ static void EnsureCorrectRunningVersion(const StartupOptions &startup_options,
     // 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))) {
+    // Ignore permissions errors (i.e. if the install base is not writable):
+    if (!mtime->SetToNowIfPossible(
+            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
diff --git a/src/main/cpp/util/file_platform.h b/src/main/cpp/util/file_platform.h
index a0860dea7d0c5b..4157f01f8fd058 100644
--- a/src/main/cpp/util/file_platform.h
+++ b/src/main/cpp/util/file_platform.h
@@ -48,6 +48,12 @@ class IFileMtime {
   // Returns true if the mtime was changed successfully.
   virtual bool SetToNow(const Path &path) = 0;
 
+  // Attempt to set the mtime of file under `path` to the current time.
+  //
+  // Returns true if the mtime was changed successfully OR if setting the mtime
+  // failed due to permissions errors.
+  virtual bool SetToNowIfPossible(const Path &path) = 0;
+
   // Sets the mtime of file under `path` to the distant future.
   // "Distant future" should be on the order of some years into the future, like
   // a decade.
diff --git a/src/main/cpp/util/file_posix.cc b/src/main/cpp/util/file_posix.cc
index 8b1d541660c6ea..ffe97880471475 100644
--- a/src/main/cpp/util/file_posix.cc
+++ b/src/main/cpp/util/file_posix.cc
@@ -453,6 +453,7 @@ class PosixFileMtime : public IFileMtime {
 
   bool IsUntampered(const Path &path) override;
   bool SetToNow(const Path &path) override;
+  bool SetToNowIfPossible(const Path &path) override;
   bool SetToDistantFuture(const Path &path) override;
 
  private:
@@ -486,6 +487,19 @@ bool PosixFileMtime::SetToNow(const Path &path) {
   return Set(path, times);
 }
 
+bool PosixFileMtime::SetToNowIfPossible(const Path &path) {
+  bool okay = this->SetToNow(path);
+  if (!okay) {
+    // `SetToNow`/`Set` are backed by `utime(2)` which can return `EROFS` and
+    // `EPERM` when there's a permissions issue:
+    if (errno == EROFS || errno == EPERM) {
+      okay = true;
+    }
+  }
+
+  return okay;
+}
+
 bool PosixFileMtime::SetToDistantFuture(const Path &path) {
   return Set(path, distant_future_);
 }
diff --git a/src/main/cpp/util/file_windows.cc b/src/main/cpp/util/file_windows.cc
index 33eba1f40122c4..2cb41a4617a105 100644
--- a/src/main/cpp/util/file_windows.cc
+++ b/src/main/cpp/util/file_windows.cc
@@ -115,6 +115,7 @@ class WindowsFileMtime : public IFileMtime {
 
   bool IsUntampered(const Path& path) override;
   bool SetToNow(const Path& path) override;
+  bool SetToNowIfPossible(const Path& path) override;
   bool SetToDistantFuture(const Path& path) override;
 
  private:
@@ -179,6 +180,19 @@ bool WindowsFileMtime::SetToNow(const Path& path) {
   return Set(path, GetNow());
 }
 
+bool WindowsFileMtime::SetToNowIfPossible(const Path& path) {
+  bool okay = this->SetToNow(path);
+  if (!okay) {
+    // `SetToNow` is backed by `CreateFileW` + `SetFileTime`; the former can
+    // return `ERROR_ACCESS_DENIED` if there's a permissions issue:
+    if (GetLastError() == ERROR_ACCESS_DENIED) {
+      okay = true;
+    }
+  }
+
+  return okay;
+}
+
 bool WindowsFileMtime::SetToDistantFuture(const Path& path) {
   return Set(path, distant_future_);
 }