ci: build rust-msrv on the MSRV, not the pinned channel - #162
Merged
Conversation
The pinned dtolnay/rust-toolchain only runs `rustup default`, which rust-toolchain.toml outranks, so `toolchain: "1.88"` never governed the build: the job recompiled the pinned channel and an MSRV violation could land green. Export RUSTUP_TOOLCHAIN, which does outrank the toml — read from `rust-version` in Cargo.toml rather than repeated as a literal — and assert rustc's reported version before building, so a future regression fails loudly instead of silently re-testing the pin. `cargo +<toolchain>` and `rustup override` outrank the toml too; the env var wins on covering every later step, including rust-cache, whose key is hashed from the effective rustc and so stays honest. The other `toolchain:` inputs name the pinned channel anyway, so they are unaffected; `rust-beta` is not, and is left to its own change. Closes #137
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
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.
Closes #137.
rust-msrvwas not testing the MSRV. The pinned dtolnay/rust-toolchainaction only runs
rustup default <tc>— and even that iscontinue-on-error— whilerust-toolchain.tomloutranks the rustupdefault, so the job compiled with the pinned channel (1.98.0), not 1.88.
Since
rust-msrvis one of main's nine required contexts, an MSRVviolation could land green.
The job now derives the version from
Cargo.toml'srust-version,exports it as
RUSTUP_TOOLCHAIN, and asserts the effectiverustcbeforebuilding — so the literal
1.88is gone from CI and the old "keep in sync"comment is replaced by enforcement.
Proof, both directions
Fixed job, replayed step by step:
Same replay without the export — i.e. main as it stands:
A deliberate violation (
File::lock, stabilised in 1.89) fails the fixedjob and passed the old one:
The workspace genuinely compiles on 1.88 today,
--lockedincluded — noMSRV bump was needed or made.
Why
RUSTUP_TOOLCHAINcargo +1.88andrustup overridealso outrank the toolchain file. Theenv var wins on covering every later step, including
Swatinem/rust-cache,whose key is hashed from the effective rustc — with
cargo +1.88the keywould name 1.98.0 while the cached artifacts were 1.88.
Adversarial review before opening
A reviewer re-derived the mechanism, the empty-sed guard, the
version-comparison matrix and the cache-key reasoning independently, found
no vacuous-pass path, and returned NOT MERGE-SAFE on two confidently-false
prose statements: AGENTS.md asserted the
toolchain:inputs "all name thesame version" — which this very change falsifies — and three places claimed
RUSTUP_TOOLCHAINwas the only mechanism outranking the toolchain file,contradicting #137's own list of fixes. Both were verified against rustup
and corrected before this PR.
Related, deliberately not fixed here
rust-betais inert by the identical mechanism — it installs beta thencompiles the pin, so the early-warning job has been re-testing 1.98.0.
Tracked as #160; switching it on will surface real new-lint noise, which is
a judgement call rather than part of this fix.