Skip to content

Cache workspace member compilation during Clippy#20459

Draft
zaniebot wants to merge 5 commits into
astral-sh:mainfrom
zaniebot:zb/nightly-clippy-workspace-cache
Draft

Cache workspace member compilation during Clippy#20459
zaniebot wants to merge 5 commits into
astral-sh:mainfrom
zaniebot:zb/nightly-clippy-workspace-cache

Conversation

@zaniebot

@zaniebot zaniebot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Clippy currently restores only third-party Rust dependencies, so every run recompiles the uv workspace before linting. Install the pinned nightly toolchain before cache restoration on Linux and Windows, retain workspace-member artifacts under a new cache generation, and use checksum freshness so restored artifacts remain reusable despite changed mtimes.

Pinned nightly also exposes several actionable diagnostics that stable Clippy does not: fix the unbalanced documentation backtick, return ready futures for non-awaiting trait methods, remove redundant else blocks, and box the large error variants. The lint scope and -D warnings policy remain unchanged.

Five warm-cache samples on each production runner show lower Clippy and overall job times (Linux, Windows). The table shows stable → optimized means and the change with a 95% confidence interval. Cold compilation and cache saves have one observation per side.

Metric Linux (ubuntu-latest) Windows (namespace-profile-windows-2022-x86-64-4)
Cold Clippy 180s → 172s 142s → 135s
Warm Clippy 66.4s → 39.2s, −41.0% [−47.1%, −34.2%] 66.4s → 57.6s, −13.3% [−22.4%, −3.1%]
Warm job 99.4s → 70.0s, −29.6% [−35.8%, −22.7%] 70.6s → 61.4s, −13.0% [−21.0%, −4.3%]
Compressed cache 403 → 431 MiB 334 → 360 MiB
Cache save 8s → 9s 9s → 8s
Cache restore 9.6s → 7.0s, −27.1% [−48.9%, +4.1%] 9.2s → 9.8s, +6.5% [−9.6%, +25.5%]

The workspace cache adds only about 25–28 MiB compressed. Warm Clippy saves about 27s on Linux and 9s on Windows, while cache-save and restore changes are small or inconclusive.

@zanieb

zanieb commented Jul 15, 2026

Copy link
Copy Markdown
Member

We might want to stay on stable here, especially since it's not a bottleneck, but I want to see the timings.

Nightly lints are nice to check sometimes too.

@Gankra

Gankra commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

It's always super awkward and annoying when local dev and CI diverge on what version of clippy to use. At least historically it's been common for stable and nightly to incompatibly disagree on a clippy lint (e.g. if you fix one, the other then breaks, making it impossible to satisfy both at once).

So I wouldn't ship this unless we're willing to make nightly be our pin in rust-toolchain.toml (and I don't think we want to be living that life...?).

Comment on lines +112 to +127
- name: "Install Rust toolchain"
run: rustup toolchain install nightly-2026-07-13 --profile minimal --component clippy
- uses: Swatinem/rust-cache@c19371144df3bb44fab255c43d04cbc2ab54d1c4 # v2.9.1
env:
RUSTUP_TOOLCHAIN: nightly-2026-07-13
with:
prefix-key: v1-rust-workspace
cache-workspace-crates: "true"
save-if: ${{ inputs.save-rust-cache == 'true' }}
- name: "Check uv_build dependencies"
uses: EmbarkStudios/cargo-deny-action@bb137d7af7e4fb67e5f82a49c4fce4fad40782fe # v2.0.20
with:
command: check bans
manifest-path: crates/uv-build/Cargo.toml
- name: "Install Rust toolchain"
run: rustup component add clippy
- name: "Clippy"
run: cargo clippy --workspace --all-targets --all-features --locked -- -D warnings
run: cargo +nightly-2026-07-13 clippy --workspace --all-targets --all-features --locked -Z checksum-freshness -- -D warnings

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we use + and set RUSTUP_TOOLCHAIN? Isn't only one of those needed?

Also, unsure if we have a new enough rustup, but rustup toolchain install has an --override flag to make the installed toolchain used within this directory. An upcoming version adds a --default so it isn't directory based.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The environment variable is only set in the Swatinem/rust-cache action where we can't control the command

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants