Judge gradle-cache storing by new entry names, not net count - #134
Merged
Conversation
Gradle's periodic cleanup of entries unused for 7 days can remove more files from ~/.gradle/caches/build-cache-1 than a cold assemble adds, so the before/after count went negative on a lived-in machine and the check reported engaged-but-not-storing while 225 new entries had in fact been written. The check now snapshots entry basenames and asserts on the set difference, and reports concurrent cleanup in its evidence. Fixes #132
The cache directory also holds build-cache-1.lock and gc.properties; on a cold directory (CI) those alone satisfied a new-names check. An unreadable directory now throws instead of reading as all-new, and the removed count comes from the same name sets rather than mixing a recursive file count with a flat listing.
…toring A previous suite run on the same machine can leave every one of this fixture's task outputs in ~/.gradle/caches/build-cache-1; the cold workspace then loads FROM-CACHE and stores nothing, which is the cache working, not failing. The check now accepts either direction and names which one it saw.
A later check rebuilds wt1, so reading its log lazily would let a reordered check satisfy warm mode from the wrong build -- the same snapshot-timing class this branch already fixed for the name sets. The warm-mode pass string states outright that storing was not exercised.
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.
Description
The
gradle-cachecheck in the cache e2e judged storing by comparing the file count of~/.gradle/caches/build-cache-1before and after the cold assemble. Gradle's periodic cleanup (entries unused >7 days) can remove more entries than the build adds, so on a lived-in machine the delta goes negative and the check fails as "engaged-but-not-storing" while storing actually worked. Hit during the rc.5 pre-tag QA: net -1104 files, yet 225 entries were created during the window (proven by mtime). CI is unaffected for a simpler reason the review corrected: the e2e workflow restores no Gradle cache at all, so the directory starts empty there.Solution
Snapshot the cache's entry basenames before the cold assemble and assert that new names appear afterwards (set difference), which is immune to concurrent deletions. The evidence line now also reports how many old files cleanup removed in the same window, so the next reader of a passing-but-shrinking cache isn't puzzled.
Test plan
caches expo-androidrun on the same machine (populated, cleanup-prone gradle cache) that produced the false negative — result to be attached before marking ready.Fixes #132
Validation evidence (attached per review)
Two full
caches expo-androidruns on the machine that produced the false negative:0new entries and loaded FROM-CACHE instead -- correct Gradle behavior that the store-only assertion called a failure. The check now accepts either direction (store on a cold machine cache, load on a warm one) and names which it saw.gradle-cache PASS: --build-cache on the argv; warm machine cache reused by wt1 (89 FROM-CACHE); 89 FROM-CACHE task(s) in the second worktree.Review fixes also applied: entries are matched by the 32-hex name shape (
build-cache-1.lock/gc.propertiescan no longer satisfy the check on a cold directory -- the CI case), an unreadable directory now throws instead of reading as all-new, and the removed-count evidence derives from the same name sets instead of mixing a recursive count with a flat listing.