Add SCCACHE_ALLOW_INCREMENTAL to keep caching the non-incremental parts of a build - #2810
Open
PHPCraftdream wants to merge 1 commit into
Open
PHPCraftdream wants to merge 1 commit into
PHPCraftdream wants to merge 1 commit into
Conversation
sylvestre
reviewed
Aug 18, 2026
Comment on lines
1582
to
1591
| // CARGO_INCREMENTAL/CARGO_BUILD_INCREMENTAL only affect rustc output through | ||
| // the `-C incremental=` argument cargo derives from them, which is already | ||
| // hashed as part of the argument list (and marks the invocation non-cacheable | ||
| // anyway). Hashing the env var too would needlessly fork the cache between | ||
| // incremental-enabled and incremental-disabled builds of identical deps. | ||
| // No CACHE_VERSION bump: this only merges previously-distinct keys whose | ||
| // entries are output-identical (the env var itself does not change rustc | ||
| // output, and `env!("CARGO_INCREMENTAL")` readers are still tracked through | ||
| // dep-info env-deps), so existing entries stay valid under the merged key. | ||
| if var == "CARGO_MAKEFLAGS" |
Collaborator
There was a problem hiding this comment.
do we need such a long comment?
Author
There was a problem hiding this comment.
Trimmed to two lines, matching the one-line-per-variable style of the list above it.
sylvestre
reviewed
Aug 18, 2026
Comment on lines
+894
to
+919
| // Incrementally compiled crates cannot be cached, so by default | ||
| // sccache refuses to run at all once cargo enables incremental | ||
| // compilation, rather than silently returning a useless cache. | ||
| // | ||
| // That refusal is all-or-nothing, while incremental compilation is | ||
| // a property of an individual invocation: cargo passes | ||
| // `-C incremental=` only to workspace and path crates, so the | ||
| // registry dependencies of the very same build remain perfectly | ||
| // cacheable. `SCCACHE_ALLOW_INCREMENTAL` opts into that split. | ||
| // | ||
| // Nothing is left unguarded when it is set. compiler/rust.rs has | ||
| // recognised `-C incremental=` since 2018 and maps it to | ||
| // `CompilerArguments::CannotCache`, which runs the real compiler | ||
| // with the original command line and skips caching just that one | ||
| // request -- the same path already taken by `crate-type`, | ||
| // `multiple input files` and every other non-cacheable reason, | ||
| // and still reported as such by `--show-stats`. So the | ||
| // incremental crates compile incrementally and uncached, while | ||
| // everything else is cached exactly as before. | ||
| // | ||
| // Compared against "1" rather than merely tested for presence (as | ||
| // SCCACHE_IGNORE_SERVER_IO_ERROR above already does), so that an | ||
| // opt-in configured globally -- a user environment variable, or | ||
| // cargo's `[env]` table, which is injected into every wrapper | ||
| // process cargo spawns -- can still be turned back off for a single | ||
| // build with SCCACHE_ALLOW_INCREMENTAL=0. |
Collaborator
There was a problem hiding this comment.
can you please update your agent to not write long comments? thanks
Author
There was a problem hiding this comment.
Point taken. Cut to four lines here and trimmed the rest of the PR the same way: 70 lines of added comment removed, 25 kept, code unchanged. The reasoning now lives in the PR description instead.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2810 +/- ##
==========================================
+ Coverage 73.14% 73.20% +0.05%
==========================================
Files 72 72
Lines 37615 37672 +57
==========================================
+ Hits 27512 27576 +64
+ Misses 10103 10096 -7 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
PHPCraftdream
force-pushed
the
allow-incremental-opt-in
branch
from
August 18, 2026 14:29
6fe395d to
d07cd8e
Compare
Collaborator
|
similar feedback about comment 0 |
Author
|
Fair. Cut it down to the essentials. |
sylvestre
reviewed
Sep 29, 2026
| let mut normal_mappings = HashMap::new(); | ||
| let mut verbatim_mappings = HashMap::new(); | ||
| for (_dist_path, local_path) in self.dist_to_local_path.iter() { | ||
| for local_path in self.dist_to_local_path.values() { |
Collaborator
There was a problem hiding this comment.
unrelated, could you please move it to a separate pr? thanks
…ompiles sccache exits with an error when CARGO_INCREMENTAL=1 (mozilla#1596, mozilla#1767), although cargo passes `-C incremental=` only to workspace members and path dependencies, so a build's registry dependencies stay cacheable. With SCCACHE_ALLOW_INCREMENTAL=1 the refusal is skipped and the incremental invocations take the existing CannotCache("incremental") path. The default refusal is unchanged. CARGO_INCREMENTAL/CARGO_BUILD_INCREMENTAL are no longer hashed into the Rust cache key: they affect output only via `-C incremental=` (never cached) or `env!` (hashed via dep-info). CACHE_VERSION is not bumped; entries cached with CARGO_INCREMENTAL set (e.g. to 0) miss once. Tests: the default refusal still fires; with the flag, the registry dep hits an entry written by a CARGO_INCREMENTAL=0 build while the workspace crate compiles incrementally; the key ignores CARGO_INCREMENTAL in the environment but not as a dep-info env-dep. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
PHPCraftdream
force-pushed
the
allow-incremental-opt-in
branch
from
September 29, 2026 21:43
d07cd8e to
a4a745c
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds an opt-in
SCCACHE_ALLOW_INCREMENTAL=1. The default refusal onCARGO_INCREMENTAL=1is unchanged.cargo passes
-C incremental=only to workspace members and path dependencies, so the registry dependencies of an incremental build are still cacheable. With the flag set, the incremental invocations take the existingCannotCache("incremental")path and the rest of the build is cached.CARGO_INCREMENTAL/CARGO_BUILD_INCREMENTALare no longer part of the Rust cache key: they affect output only via-C incremental=(never cached) orenv!(still hashed via dep-info).CACHE_VERSIONis not bumped, since no key can now map to a different output. This does change keys for anyone who setsCARGO_INCREMENTALtoday (e.g.=0in CI): one cold run after upgrading, after which incremental and non-incremental builds share dependency entries. Happy to split this part out if you'd rather not take it now.The unrelated clippy fix moved to #2877.