Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2868 +/- ##
==========================================
+ Coverage 76.40% 76.57% +0.17%
==========================================
Files 72 72
Lines 40250 40570 +320
==========================================
+ Hits 30752 31067 +315
- Misses 9498 9503 +5 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
These are assertions in tests, not production code. |
| /// The current format is 1 header byte for the version + bincode encoding | ||
| /// of the [`PreprocessorCacheEntry`] struct. | ||
| const FORMAT_VERSION: u8 = 1; | ||
| const FORMAT_VERSION: u8 = 2; |
There was a problem hiding this comment.
bumping the version makes read() return UnknownFormat on every entry already in a local cache, and c.rs propagates that with ? so the first build after upgrade fails. could we treat an unknown version as a miss?
There was a problem hiding this comment.
preprocessor_cache_entry_hash_key() hashes the version into the digest, so we won't hit it in the cache anyway.
I'll have unknown versions ignored, it makes sense.
There was a problem hiding this comment.
Added a new commit ignoring entries that cannot be deserialized.
| // listed share one preprocessor cache entry, so the include files it | ||
| // records have to be checked against this tree's copies and not against | ||
| // the ones that happened to write the entry. | ||
| let tree_root = compilation_tree_root(&absolute_input_path, &cwd, storage.basedirs())?; |
There was a problem hiding this comment.
could this move inside the use_preprocessor_cache_mode branch? it's only used there
| /// compiled now, which is the only tree whose headers say anything about | ||
| /// this compilation. Do not infer this from `path` being relative: the | ||
| /// preprocessor can name a file outside every basedir relatively too. | ||
| under_basedir: bool, |
There was a problem hiding this comment.
nobody will read 8 lines here :) please cut it to two
|
|
||
| - When a source file was compiled and its results were cached, a header file would have been included if it existed, but it did | ||
| not exist at the time. sccache does not know about such files, so it cannot invalidate the result if the header file later exists. | ||
| With `basedirs` this widens: a header that exists in one checkout and not in another, at a position the compiler searches first |
There was a problem hiding this comment.
the same explanation is now in README.md, docs/Caching.md, docs/Configuration.md and here. please keep one and link to it
96b6479 to
4f5c41e
Compare
|
v2:
|
A preprocessor cache entry that fails to read or deserialize, such as one truncated by a crash or written in another format, failed the whole compilation. Fall back to preprocessing instead; the miss path then overwrites the entry. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
With basedirs set, the preprocessor cache entry key no longer names one checkout, so every checkout listed shares an entry. The include files the entry records were absolute paths into whichever checkout wrote it, so the lookup validated that checkout's headers against themselves, always matched, and handed back its object cache key. A second checkout whose headers differ got the first one's object file. Record an include that lives under a basedir relative to it, and re-root it at lookup time at the tree being compiled: the basedir containing the input file, or the working directory if the input is outside all of them. Includes outside every basedir stay absolute. Bump FORMAT_VERSION; entries written before this cannot be read under the new rule. Fixes mozilla#2863. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The include list records only the files the preprocessor opened, not the directories it searched and found empty, so two checkouts that resolve an include to different files can still share a result. Saying that differing headers never share one claimed more than the code does. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
4f5c41e to
674fac6
Compare
|
v3: rebase past logical conflict and fix fallout |
3 participants
The basedirs feature strips the directory prefix from a cache entry, so two names
in different prefixes (common with worktrees) can intermatch.
Re-add the prefix to the key to avoid different worktrees from interfering with
each other.
Fixes #2863.