mozilla / sccache

Sccache is a ccache-like tool. It is used as a compiler wrapper and avoids compilation when possible. Sccache has the capability to utilize caching in remote storage environments, including various cloud storage options, or alternatively, in local storage.
Apache License 2.0
5.85k stars 552 forks source link

Add generic read-only storage mode for a storage that fails the write check #2091

Closed jkoritzinsky closed 9 months ago

jkoritzinsky commented 9 months ago

Today, sccache can detect that a storage medium is read-only or read-write even if this capability was not specified by the config. However, it doesn't do anything with this information other than log it for diagnostics.

This PR changes sccache to use this information to avoid writing to a read-only storage. This is useful for cases where one might want to use sccache in a read-only configuration in some cases (e.g. public PR builds) while only updating the cache from a separate pipeline (that has write access) without using a separate configuration file.

Additionally, this change reduces the number of "access denied" errors that would be logged on the storage side when sccache detects that it only has read-only access (instead of continuing to attempt to write to the storage).

Fixes #1853

codecov-commenter commented 9 months ago

Codecov Report

Attention: 69 lines in your changes are missing coverage. Please review.

Comparison is base (ffe3070) 30.77% compared to head (6d906b9) 30.76%. Report is 4 commits behind head on main.

Files Patch % Lines
src/cache/readonly.rs 31.48% 7 Missing and 30 partials :warning:
tests/sccache_cargo.rs 21.62% 3 Missing and 26 partials :warning:
src/server.rs 40.00% 0 Missing and 3 partials :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #2091 +/- ## ========================================== - Coverage 30.77% 30.76% -0.02% ========================================== Files 52 53 +1 Lines 19913 20037 +124 Branches 9630 9726 +96 ========================================== + Hits 6129 6164 +35 + Misses 7959 7950 -9 - Partials 5825 5923 +98 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

sylvestre commented 9 months ago

Could you please add a test and fix the clippy warning? Thanks

jkoritzinsky commented 9 months ago

I've fixed the clippy warning and added a few unit tests.

sylvestre commented 9 months ago

Sorry, I was talking about an integration test like https://github.com/mozilla/sccache/blob/ffe3070f77ef3301c8ff718316e4ab017ec83042/tests/sccache_cargo.rs#L318 thanks

jkoritzinsky commented 9 months ago

@sylvestre I've added a test that validates (through the sccache log) that in a scenario where the underlying cache is detected as read-only, the ReadOnlyStorage handles the write requests and the requests never go to the underlying storage.

sylvestre commented 9 months ago

thanks!