ci: save rust-cache only on main/staging pushes - #2609
Conversation
|
Note Gemini is unable to generate a review for this pull request due to the file types involved not being currently supported. |
The three clippy matrix entries (all-features, default, libsql-only) each had a distinct cache key, so each leg maintained its own multi-GB target/ in the GitHub Actions cache. GHA caps repo cache at 10 GB and evicts LRU, so these three near-duplicates crowd out other useful entries and forced each leg to re-warm after eviction. Switch to shared-key: clippy for the Linux matrix and shared-key: clippy-windows for the Windows matrix. Whichever leg finishes (and saves) first wins the slot; the other two restore from that cache on the next run. Because --all-features builds a superset of the artifacts needed by default / libsql-only, a shared cache is incrementally useful for every leg even when the saver wasn't all-features. This is independent of and complementary to #2609 (save-if gating); together they reduce total cache churn per clippy job to near-zero wall clock on PR runs after the first main/staging push refreshes the slot.
There was a problem hiding this comment.
Pull request overview
Adjusts GitHub Actions caching behavior to avoid spending minutes uploading Rust target/ caches on PR runs, while still keeping caches warm via long-lived branches.
Changes:
- Add
save-ifto allSwatinem/rust-cachesteps intest.ymlandcode_style.ymlto skip cache uploads except onmain/staging. - Add
pushtriggers forcode_style.ymlonmainandstagingto refresh caches outside PRs. - Gate
no-panicstopull_requestonly and update the roll-up job to treat it as allowed-to-be-skipped on pushes.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| .github/workflows/test.yml | Adds save-if to rust-cache steps so PR runs restore without uploading caches. |
| .github/workflows/code_style.yml | Adds push triggers + save-if on rust-cache; gates no-panics to PR-only and updates roll-up logic to allow skipped. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -101,11 +109,14 @@ jobs: | |||
| - uses: Swatinem/rust-cache@e18b497796c12c097a38f9edb9d0641fb99eee32 # v2 | |||
| with: | |||
| key: clippy-windows-${{ matrix.name }} | |||
There was a problem hiding this comment.
In clippy-windows, the rust-cache entry keyed as clippy-windows-${{ matrix.name }} will likely never get written after this change: the job only runs on pull_request (because github.base_ref is empty on push), and PR runs now have save-if false because github.ref is refs/pull/.... This means restores for this job will always be cold. Consider either (a) populating this cache on a push-triggered workflow/job, or (b) reusing an existing Windows cache key that is saved on pushes (e.g., align with windows-${{ matrix.name }} from test.yml) so clippy-windows can restore something that actually exists.
| key: clippy-windows-${{ matrix.name }} | |
| key: windows-${{ matrix.name }} |
There was a problem hiding this comment.
Good catch — fixed. clippy-windows now uses key: windows-${{ matrix.name }} (matches test.yml's windows-build), so it restores from a cache that test.yml's push-to-main runs actually populate. Kept the save-if on the clippy-windows step for defense but in practice it won't trigger (the job only runs on PRs).
| - uses: Swatinem/rust-cache@e18b497796c12c097a38f9edb9d0641fb99eee32 # v2 | ||
| with: | ||
| key: ${{ matrix.name }} | ||
| save-if: ${{ github.ref == 'refs/heads/main' || github.ref == 'refs/heads/staging' }} |
There was a problem hiding this comment.
save-if currently checks only github.ref. When this workflow is invoked via workflow_call (e.g., from scheduled/dispatch workflows) github.ref can still be refs/heads/main, which would allow cache uploads even though the event isn’t a push. If the intent is truly “save only on main/staging pushes”, gate additionally on github.event_name == 'push' (or otherwise ensure non-push callers don’t upload).
There was a problem hiding this comment.
Right — when staging-ci.yml calls test.yml via workflow_call from its hourly schedule, github.ref resolves to refs/heads/main and the save-if would fire. Updated all save-if guards in test.yml (and code_style.yml for consistency) to also check github.event_name == 'push' so only real pushes save.
* ci: share rust-cache across clippy matrix legs The three clippy matrix entries (all-features, default, libsql-only) each had a distinct cache key, so each leg maintained its own multi-GB target/ in the GitHub Actions cache. GHA caps repo cache at 10 GB and evicts LRU, so these three near-duplicates crowd out other useful entries and forced each leg to re-warm after eviction. Switch to shared-key: clippy for the Linux matrix and shared-key: clippy-windows for the Windows matrix. Whichever leg finishes (and saves) first wins the slot; the other two restore from that cache on the next run. Because --all-features builds a superset of the artifacts needed by default / libsql-only, a shared cache is incrementally useful for every leg even when the saver wasn't all-features. This is independent of and complementary to #2609 (save-if gating); together they reduce total cache churn per clippy job to near-zero wall clock on PR runs after the first main/staging push refreshes the slot. * ci(clippy): run only all-features on push Adds if: github.event_name == 'pull_request' || matrix.name == 'all-features' to the Linux clippy matrix so push events (main/staging) run only the all-features leg. This pins the saver for the new shared-key slot: --all-features builds a superset of the artifacts needed by default and libsql-only, so when those PR legs cache-restore they always start from the richest possible baseline rather than whichever leg happened to win a three-way race. PRs still run all three legs, so lint coverage is unchanged on the path that matters (before merge). Push events are only exercised after a PR has already passed, so the redundant two legs were just warming a cache anyway.
Swatinem/rust-cache was saving the multi-GB target/ directory at the end of every CI job — including PR jobs that will never be read from again by a different PR. The post-cache step was adding 5+ minutes of wall clock to each Clippy and Tests matrix leg, on top of the actual compile time. This change: - Adds save-if gating to every rust-cache step in code_style.yml and test.yml so the cache is only written when github.ref is refs/heads/main or refs/heads/staging. PR jobs still restore, they just skip the upload. - Adds push triggers on main and staging to code_style.yml so those pushes actually exercise each matrix leg and refresh the cache entry that PRs will restore from next. Without this, the cache would never be refreshed after the one-time manual warm-up. - Gates no-panics on pull_request events since it diffs against github.event.pull_request.base.sha, which is empty on push events. Expected impact on PR wall clock: ~5 minutes saved per Clippy / Tests matrix leg, no change to cache hit rate for subsequent PRs.
ac1e66b to
f15656c
Compare
Combines five reductions targeting the same root cause: clippy and cargo check were running the same compile work multiple times across matrix legs and platforms. - Drop `windows-build` from test.yml. Its `cargo check` matrix is fully subsumed by `clippy-windows`, which already type-checks `--all --benches --tests --examples` on the same configs. - Collapse PR-blocking clippy to `--all-features` only (Linux + Windows). Lint findings are almost never feature-gated, and the remaining configs (`default`, `libsql-only`) now run as `clippy-extra` / `clippy-windows-extra` jobs gated to `push` events on long-lived branches, where they verify feature combinations without slowing PRs. - Share rust-cache slot between clippy and tests via `shared-key: build` (Linux) and `shared-key: build-windows`. Clippy and `cargo test` produce overlapping artifacts for the same feature set, so a single slot serves both jobs and avoids duplicating multi-GB target/ caches. - Replace `cargo install cargo-component --locked || true` with a composite action backed by `taiki-e/install-action`. Drops a multi-minute source build to seconds and stops swallowing failures. Applied to test.yml, coverage.yml, and release.yml. - Add `paths-ignore` filters for top-level docs, LICENSE files, PNG assets, issue/PR templates, and `docs/**` (mintlify site). Conservative scope: any `.md` file actually `include_str!()`'d by the binary (workspace seeds, engine prompts, channel READMEs) remains unfiltered. Includes `save-if` guards on every rust-cache step so PR jobs are restore-only — complementary to #2609 and #2610. Gates `no-panics` to PR events (it diffs against the PR base SHA, unavailable on push) and teaches the `code-style` roll-up to accept `skipped` for the push-only / PR-only conditional jobs.
* ci: share rust-cache across clippy matrix legs The three clippy matrix entries (all-features, default, libsql-only) each had a distinct cache key, so each leg maintained its own multi-GB target/ in the GitHub Actions cache. GHA caps repo cache at 10 GB and evicts LRU, so these three near-duplicates crowd out other useful entries and forced each leg to re-warm after eviction. Switch to shared-key: clippy for the Linux matrix and shared-key: clippy-windows for the Windows matrix. Whichever leg finishes (and saves) first wins the slot; the other two restore from that cache on the next run. Because --all-features builds a superset of the artifacts needed by default / libsql-only, a shared cache is incrementally useful for every leg even when the saver wasn't all-features. This is independent of and complementary to nearai#2609 (save-if gating); together they reduce total cache churn per clippy job to near-zero wall clock on PR runs after the first main/staging push refreshes the slot. * ci(clippy): run only all-features on push Adds if: github.event_name == 'pull_request' || matrix.name == 'all-features' to the Linux clippy matrix so push events (main/staging) run only the all-features leg. This pins the saver for the new shared-key slot: --all-features builds a superset of the artifacts needed by default and libsql-only, so when those PR legs cache-restore they always start from the richest possible baseline rather than whichever leg happened to win a three-way race. PRs still run all three legs, so lint coverage is unchanged on the path that matters (before merge). Push events are only exercised after a PR has already passed, so the redundant two legs were just warming a cache anyway.
Swatinem/rust-cache was saving the multi-GB target/ directory at the end of every CI job — including PR jobs that will never be read from again by a different PR. The post-cache step was adding 5+ minutes of wall clock to each Clippy and Tests matrix leg, on top of the actual compile time. This change: - Adds save-if gating to every rust-cache step in code_style.yml and test.yml so the cache is only written when github.ref is refs/heads/main or refs/heads/staging. PR jobs still restore, they just skip the upload. - Adds push triggers on main and staging to code_style.yml so those pushes actually exercise each matrix leg and refresh the cache entry that PRs will restore from next. Without this, the cache would never be refreshed after the one-time manual warm-up. - Gates no-panics on pull_request events since it diffs against github.event.pull_request.base.sha, which is empty on push events. Expected impact on PR wall clock: ~5 minutes saved per Clippy / Tests matrix leg, no change to cache hit rate for subsequent PRs.
Summary
save-ifgating to everySwatinem/rust-cachestep incode_style.ymlandtest.ymlso the cache is only written whengithub.refisrefs/heads/mainorrefs/heads/staging. PR jobs still restore — they just skip the upload.mainandstagingtocode_style.ymlso those pushes actually exercise each matrix leg and refresh the cache that PRs will restore from. Without this, thesave-ifguard would mean the cache is never refreshed.no-panicsonpull_requestevents (it diffs againstgithub.event.pull_request.base.sha, which is empty on pushes) and teaches thecode-styleroll-up to accept "skipped" for it on push.Motivation
Right now on #2471's latest CI run, each Clippy matrix leg shows:
The actions cache save step is compressing and uploading a multi-GB `target/` directory on every PR job, even though PR jobs have unique cache keys and no other PR will read them. Only pushes to long-lived branches should refresh the cache.
Expected impact: ~5 min saved per Clippy / Tests matrix leg on PRs. No change to cache hit rate for subsequent PRs —
main/stagingpushes refresh the same keys PRs restore from.Test plan