Ignore read-only errors when updating the `mtime` of the `install_base` #20442

rrbutani · 2023-12-05T20:50:11Z

Note

Follow-up to #20373 (see here)

Currently if the --install_base path passed is not writable by the user invoking Bazel, the Bazel client crashes:

❯ 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:

bazel/src/main/cpp/blaze.cc

Lines 1010 to 1021 in a3c677d

    
               // 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; 
        
               } 
        
             } 
        
           }

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

cc: @tjgq

src/main/cpp/blaze.cc

rrbutani · 2023-12-05T20:52:03Z

src/main/cpp/blaze.cc

+      if (errno == EROFS || errno == EPERM) { raise_error = false; }
+#elif defined(_WIN32)
+      // On Windows, `SetToNow` is backed by `CreateFileW` + `SetFileTime`:
+      if (::GetLastError() == ERROR_ACCESS_DENIED) { raise_error = false; }


I am not sure whether this is correct; unfortunately I don't have access to a Windows machine at the moment.

CI seems to confirm that this does at least build on Windows.

tjgq

We try to keep the platform-dependent filesystem code in file_platform.h and its two implementations, file_posix.cc and file_windows.cc.

How about adding a new SetToNowIfPossible method there with the desired "permissions error counts as success" semantics?

Currently if the `--install_base` path passed is not writable by the user invoking Bazel, the Bazel client crashes[^1]: ```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 [^1]: Note that if you invoke Bazel again after this happens, it will successfully start up because the `install` symlink in the output base [that inhibits the `mtime` update/install base check] has already been [created]. [that inhibits the `mtime` update/install base check]: https://github.com/bazelbuild/bazel/blob/a3c677dfea2de636a719d50345a5a97af96fae60/src/main/cpp/blaze.cc#L983-L993 [created]: https://github.com/bazelbuild/bazel/blob/a3c677dfea2de636a719d50345a5a97af96fae60/src/main/cpp/blaze.cc#L1003 This commit updates the client to ignore such errors. See bazelbuild#20373 for context.

rrbutani · 2023-12-12T20:19:02Z

How about adding a new SetToNowIfPossible method there with the desired "permissions error counts as success" semantics?

@tjgq done; PTAL.

brentleyjones · 2023-12-15T16:55:21Z

@bazel-io flag

iancha1992 · 2023-12-15T17:58:33Z

@bazel-io fork 7.1.0

iancha1992 · 2023-12-15T17:59:01Z

@brentleyjones please let me know if you want this included with 6.5.0 as well.

brentleyjones · 2023-12-15T18:54:59Z

@iancha1992 If it applies nicely, it would be good to get in, yes.

iancha1992 · 2023-12-15T19:15:56Z

@bazel-io fork 6.5.0

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 bazelbuild#20373 for context. Closes bazelbuild#20442. PiperOrigin-RevId: 591266054 Change-Id: If53e7cad48cb62406f7883f72b413e4b5a0bb8e2

…tall_base` (#20568) 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. Commit 7f782e3 PiperOrigin-RevId: 591266054 Change-Id: If53e7cad48cb62406f7883f72b413e4b5a0bb8e2 Co-authored-by: Rahul Butani <[email protected]> Co-authored-by: Ian (Hee) Cha <[email protected]>

…tall_base` (#20648) 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. Commit 7f782e3 PiperOrigin-RevId: 591266054 Change-Id: If53e7cad48cb62406f7883f72b413e4b5a0bb8e2 Co-authored-by: Rahul Butani <[email protected]> Co-authored-by: Ian (Hee) Cha <[email protected]>

iancha1992 · 2024-01-17T00:26:39Z

The changes in this PR have been included in Bazel 6.5.0 RC1. Please test out the release candidate and report any issues as soon as possible. If you're using Bazelisk, you can point to the latest RC by setting USE_BAZEL_VERSION=last_rc.
Thanks!

iancha1992 · 2024-02-23T00:16:39Z

The changes in this PR have been included in Bazel 7.1.0 RC1. Please test out the release candidate and report any issues as soon as possible. If you're using Bazelisk, you can point to the latest RC by setting USE_BAZEL_VERSION=last_rc.
Thanks!

rrbutani commented Dec 5, 2023

View reviewed changes

rrbutani marked this pull request as ready for review December 5, 2023 21:05

github-actions bot added team-Rules-CPP Issues for C++ rules awaiting-review PR is awaiting review from an assigned reviewer labels Dec 5, 2023

rrbutani mentioned this pull request Dec 5, 2023

Add client option to disable updating the mtime of the install_base #20373

Closed

5 tasks

tjgq requested changes Dec 9, 2023

View reviewed changes

rrbutani force-pushed the fix/install-base-mtime-update-ignore-readonly-error branch from ed3d573 to da0afbb Compare December 12, 2023 19:14

rrbutani marked this pull request as draft December 12, 2023 19:15

rrbutani marked this pull request as ready for review December 12, 2023 20:17

rrbutani requested a review from tjgq December 12, 2023 22:57

tjgq approved these changes Dec 14, 2023

View reviewed changes

tjgq added awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally and removed awaiting-review PR is awaiting review from an assigned reviewer labels Dec 14, 2023

copybara-service bot closed this in 7f782e3 Dec 15, 2023

github-actions bot removed the awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally label Dec 15, 2023

bazel-io added the potential release blocker Flagged by community members using "@bazel-io flag". Should be added to a release blocker milestone label Dec 15, 2023

bazel-io removed the potential release blocker Flagged by community members using "@bazel-io flag". Should be added to a release blocker milestone label Dec 15, 2023

bazel-io mentioned this pull request Dec 15, 2023

[7.1.0] Ignore read-only errors when updating the mtime of the install_base #20565

Closed

bazel-io mentioned this pull request Dec 15, 2023

[6.5.0] Ignore read-only errors when updating the mtime of the install_base #20567

Closed

bazel-io mentioned this pull request Dec 15, 2023

[6.5.0] Ignore read-only errors when updating the mtime of the install_base #20568

Merged

rrbutani deleted the fix/install-base-mtime-update-ignore-readonly-error branch December 15, 2023 20:53

bazel-io mentioned this pull request Dec 21, 2023

[7.1.0] Ignore read-only errors when updating the mtime of the install_base #20648

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Ignore read-only errors when updating the `mtime` of the `install_base` #20442

Ignore read-only errors when updating the `mtime` of the `install_base` #20442

Uh oh!

rrbutani commented Dec 5, 2023

Uh oh!

Uh oh!

rrbutani Dec 5, 2023

Uh oh!

rrbutani Dec 5, 2023 •

edited

Loading

Uh oh!

tjgq left a comment

Uh oh!

rrbutani commented Dec 12, 2023

Uh oh!

brentleyjones commented Dec 15, 2023

Uh oh!

iancha1992 commented Dec 15, 2023

Uh oh!

iancha1992 commented Dec 15, 2023

Uh oh!

brentleyjones commented Dec 15, 2023

Uh oh!

iancha1992 commented Dec 15, 2023

Uh oh!

iancha1992 commented Jan 17, 2024

Uh oh!

iancha1992 commented Feb 23, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

	// 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;
	}
	}
	}

Ignore read-only errors when updating the mtime of the install_base #20442

Ignore read-only errors when updating the mtime of the install_base #20442

Uh oh!

Conversation

rrbutani commented Dec 5, 2023

Uh oh!

Uh oh!

rrbutani Dec 5, 2023

Choose a reason for hiding this comment

Uh oh!

rrbutani Dec 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tjgq left a comment

Choose a reason for hiding this comment

Uh oh!

rrbutani commented Dec 12, 2023

Uh oh!

brentleyjones commented Dec 15, 2023

Uh oh!

iancha1992 commented Dec 15, 2023

Uh oh!

iancha1992 commented Dec 15, 2023

Uh oh!

brentleyjones commented Dec 15, 2023

Uh oh!

iancha1992 commented Dec 15, 2023

Uh oh!

iancha1992 commented Jan 17, 2024

Uh oh!

iancha1992 commented Feb 23, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Ignore read-only errors when updating the `mtime` of the `install_base` #20442

Ignore read-only errors when updating the `mtime` of the `install_base` #20442

rrbutani Dec 5, 2023 •

edited

Loading