Skip to content

Commit

Permalink
Ignore read-only errors when updating the mtime of the install_base
Browse files Browse the repository at this point in the history
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
  • Loading branch information
rrbutani authored and copybara-github committed Dec 15, 2023
1 parent df91029 commit 7f782e3
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 1 deletion.
4 changes: 3 additions & 1 deletion src/main/cpp/blaze.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 6 additions & 0 deletions src/main/cpp/util/file_platform.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
14 changes: 14 additions & 0 deletions src/main/cpp/util/file_posix.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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_);
}
Expand Down
14 changes: 14 additions & 0 deletions src/main/cpp/util/file_windows.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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_);
}
Expand Down

0 comments on commit 7f782e3

Please sign in to comment.