Fix stale direct-mode cache hits when a source file's own body changes - #52
Merged
Conversation
RecordManifest built a direct-mode manifest purely from the compiler's include notes (/showIncludes) or a GNU depfile. Neither ever names the primary translation unit itself: /showIncludes only emits notes for #include targets, and a depfile rule's own target is excluded from its dependency list. So a manifest never got an entry that changed when the .cpp file's own body was edited without touching any header, and ValidateManifest kept validating a stale object indefinitely -- served with a fake compile success, surviving daemon/service restarts since the manifest and object live in the persisted disk tier. Fix: RecordManifest now appends the TU's own source path to the list it hands to BuildManifest, alongside the headers. BuildManifest already canonicalizes, hashes, sorts, and dedupes every path it is given, so this makes the TU's own content hash a checked manifest entry with no format or schema change. Fixes #51 Fixes #49 Signed-off-by: Yaraslau Tamashevich <y.tamashevich@lastrada.net>
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
1 participant
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.
fastcache-cc's direct mode records a manifest of everything a translation
unit's compile depends on, then re-validates that manifest on the next
compile by re-hashing each recorded entry. That entry list was built purely
from the compiler's include notes (
/showIncludes) or a GNU depfile —neither of which ever names the primary translation unit itself:
/showIncludesonly emits notes for#includetargets, and a depfilerule's own target is excluded from its dependency list.
As a result, editing a
.cppfile's own body — without touching any headerit includes — left every recorded hash unchanged, so
ValidateManifestkept reporting the manifest as valid and the launcher replayed the
previously cached object forever, reporting fake compile success. Because
the manifest and object live in the persisted disk cache tier, this
survived daemon/service restarts.
Changes
RecordManifestnow appends the TU's own source path to the list handedto
BuildManifest, alongside the headers reported by the compiler.BuildManifestalready canonicalizes, hashes, sorts, and dedupes everypath it receives, so this makes the TU's own content hash a checked
manifest entry with no format or schema change.
/showIncludesscenario:builds a manifest from headers only, edits the source file's own body,
and asserts
ValidateManifestnow correctly invalidates it.Fixes #51
Fixes #49