Uh oh!
There was an error while loading. Please reload this page.
ci: build on merges to master so the vcpkg cache is actually shared - #722
Merged
Conversation
The binary caching added in #721 was only half working. GitHub Actions lets a run read caches from its own ref and from the DEFAULT branch, and these workflows trigger only on pull_request and push:tags -- so nothing ever ran on master, no shared entry was ever seeded, and every PR missed, rebuilt ICU, and saved a private copy no other PR could read. Verified against the API after v3.8.9: all ten cache entries sat under refs/pull/720/merge and refs/pull/721/merge, none on refs/heads/master. Building on push to master seeds the entry that PR runs then hit. Cost is one extra build per merge, against ~10 min of ICU rebuild (Windows; ~4 min on Linux) per job on every PR run. It also means something finally verifies the merge result, which nothing did before. Also skip the cache steps on the ubuntu-20.04 matrix entry: it builds inside Docker and skips the non-Docker vcpkg step, so it was caching an empty directory (a 200-byte entry). The first master run after this merges is still a miss; the saving starts on the PR after that. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Uh oh!
There was an error while loading. Please reload this page.
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 freeto 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.
The binary caching I added in #721 was only half working, and I'd rather say so than leave it looking done.
What was wrong
GitHub Actions lets a run read caches from its own ref and from the default branch — nothing else. These workflows trigger only on
pull_requestandpush: tags, so nothing ever ran on master. No shared entry was ever seeded, so every PR missed, rebuilt ICU from source, and saved a private copy no other PR could read.Verified against the API after v3.8.9 — all ten entries under PR refs, none on
refs/heads/master:So #721 paid the cost of caching while delivering the benefit only within a single PR (second and later pushes). The
/MPhalf of that PR works fine and is unaffected — the Windows compile step went 3m40s → 1m41s and stays there.The fix
Build on push to master. That seeds the entry PR runs then hit.
The trade, stated plainly: one extra build per merge, against removing a ~10-minute ICU rebuild (Windows; ~4 min on Linux) from every job on every PR run. It also means something finally verifies the merge result, which nothing did before.
Also
Skips the cache steps on the
ubuntu-20.04matrix entry. That one builds inside Docker and skips the non-Docker vcpkg step, so it was caching an empty directory — the 200-byte entry in the listing above.Expectations
The first master run after this merges is still a miss. The saving starts on the PR after that, so judge it there rather than on the merge itself.
🤖 Generated with Claude Code