fix(msrv): declare the Rust floor and gate it against the dependency closure - #378
Open
avrabe wants to merge 1 commit into
Open
fix(msrv): declare the Rust floor and gate it against the dependency closure#378avrabe wants to merge 1 commit into
avrabe wants to merge 1 commit into
Conversation
…closure
spar has a minimum supported Rust version whether or not anyone writes it
down, because cargo enforces one. Nothing in the repo stated it:
[workspace.package] declared no rust-version and there is no
rust-toolchain.toml. So the effective floor was
max(rust-version) over the entire RESOLVED DEPENDENCY CLOSURE
an emergent property of Cargo.lock that moves on any `cargo update` with
no diff anywhere in this repo, discovered at build time by whichever
consumer happened to have the oldest toolchain.
Measured on the committed lock: 152 of 212 dependencies declare a
rust-version; the maximum is 1.89, from smol_str@0.3.6, one tier above
the 1.87.0 cluster (wit-bindgen, wasip3). A single transitive bump of
smol_str moves the number with nothing here changing.
This already caused a defect. In #364 the fixture-vm nixpkgs pin was
moved 24.05 -> 25.05 and recorded as "cannot compile -> can compile":
edition 2024 needs rustc >= 1.85, 25.05 ships 1.86.0, 1.86 >= 1.85, done.
Every step true, conclusion wrong:
error: rustc 1.86.0 is not supported by the following package:
smol_str@0.3.6 requires rustc 1.89
The edition floor is a NECESSARY condition mistaken for THE condition.
Worth naming precisely because it is not "forgot to check": the probe
genuinely ran and genuinely returned true. A one-variable check that
comes back green is the most convincing way to not verify something.
A bare rust-version line does not fix that — a declaration is just
another number that can go stale against the closure. So the declaration
is the SUBJECT of the check, not the answer to it.
- [workspace.package] rust-version = "1.89", inherited by all 23
members via `rust-version.workspace = true` (the same way they
already inherit version/edition/license/repository). Cargo.lock is
unchanged: this is a declaration, not a dependency change.
- tools/check_msrv.py re-derives the closure maximum from
`cargo metadata --locked` and fails if it exceeds the declared floor,
naming the package responsible.
It reads cargo metadata rather than parsing Cargo.toml because
`rust-version.workspace = true` means the effective value is computed by
cargo, not written in the member manifest. Metadata reports what cargo
actually enforces, and a member that failed to inherit simply has no
rust_version — so that guard falls out for free.
The asymmetry is deliberate: declared < closure fails (consumers cannot
build); declared > closure is reported, not failed, because spar's own
source may legitimately need more than its dependencies do and nothing
here can distinguish that from staleness.
Verified two-sidedly against the real workspace, not just fixtures, since
a gate that cannot fire is worse than none — it reads as coverage:
positive declared 1.89, committed lock -> PASS
negative declared 1.86 (the 25.05 rustc that broke #364) -> exit 1,
"the dependency closure requires rustc 1.89 ... raised by:
smol_str@0.3.6"
Nine self-tests, each recording the failure it proves is still caught.
Two are about the comparison itself: `1.89` and `1.89.0` must be the same
floor (crates.io mixes both spellings and this lockfile contains both),
and `1.100` must be ABOVE `1.99` — as strings it is below, so a
lexicographic compare would pass a floor that is actually too low. The
issue's sketch used `sort -V | tail -1`, which is correct but a GNU
extension; this does not depend on which sort is installed.
The self-test captures its fixtures' stderr instead of letting it
through. Six cases are supposed to fail and their messages are
`::error::` lines — GitHub Actions workflow commands, not plain text.
Emitted from a passing self-test they would stamp eight red annotations
onto the PR diff while the step exits 0, which is the same defect class
this gate exists to prevent.
Wired into rivet-validate rather than a code-gated job: the PR that
raises the real floor is a `cargo update` touching only Cargo.lock, which
is exactly the PR most likely to be filtered out by a path filter.
What this does NOT claim: it does not verify spar's own source compiles
on 1.89. That needs a job pinned to that toolchain. The claim is narrower
and exact — the declared floor is not below what the closure demands.
The channel-rustc half of #369 (compare nixpkgs' rustc against this
floor at nix EVALUATION time, turning a 14-minute build failure into a
seconds-long one) is deliberately NOT in this PR: it lands in
trace-fixtures.yml, which #364 is currently trying to get green for the
first time, and I cannot verify the eval expression locally. Follow-up
once #364 lands.
Refs #369
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Rivet verification gate✅ 20/20 passed
Filter: Failed artifacts(none) Updated automatically by |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
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.
Addresses #369 (the closure-vs-declared half; see Deliberately not in this PR below).
The gap
spar has a minimum supported Rust version whether or not anyone writes it down,
because cargo enforces one. Nothing in the repo stated it —
[workspace.package]declared no
rust-version, and there is norust-toolchain.toml. So theeffective floor was
an emergent property of
Cargo.lockthat moves on anycargo updatewith nodiff anywhere in this repo, discovered at build time by whichever consumer
happened to have the oldest toolchain.
Measured on the committed lock: 152 of 212 dependencies declare a
rust-version; the maximum is 1.89, fromsmol_str@0.3.6, one tier abovethe 1.87.0 cluster (
wit-bindgen,wasip3).Why a gate and not just a line
It already caused a defect. In #364 the fixture-vm nixpkgs pin moved 24.05 →
25.05, recorded as "cannot compile → can compile". The reasoning: edition 2024
needs rustc ≥ 1.85, 25.05 ships 1.86.0, 1.86 ≥ 1.85, done. Every step true.
Conclusion wrong:
The edition floor is a necessary condition mistaken for the condition. Worth
naming precisely because it isn't "forgot to check": the probe genuinely ran
and genuinely returned true. A one-variable check that comes back green is the
most convincing way to not verify something — it produces the feeling of
verification while silently scoping the claim to the one variable you thought of.
A bare
rust-versionline doesn't fix that; a declaration is just another numberthat can go stale. So the declaration is the subject of the check, not the
answer to it.
What landed
[workspace.package] rust-version = "1.89", inherited by all 23 members viarust-version.workspace = true— the same way they already inheritversion/edition/license/repository.Cargo.lockis unchanged: thisis a declaration, not a dependency change.
tools/check_msrv.pyre-derives the closure maximum fromcargo metadata --lockedand fails if it exceeds the declared floor, namingthe package responsible.
It reads
cargo metadatarather than parsingCargo.tomlbecauserust-version.workspace = truemeans the effective value is computed by cargo,not written in the member manifest. Metadata reports what cargo actually
enforces — and a member that failed to inherit simply has no
rust_version, sothat guard falls out for free.
The asymmetry is deliberate.
declared < closure→ fail (consumers cannotbuild).
declared > closure→ reported, not failed: spar's own source maylegitimately need more than its dependencies, and nothing here can tell that
apart from staleness. Failing would block the legitimate case.
Verified two-sidedly, against the real workspace
A gate that cannot fire is worse than none, because it reads as coverage. So the
negative case is run against the actual lockfile, not a fixture:
That is what #369 asked for: the negative case fails and names
smol_str@0.3.6.Nine self-tests
Each records the failure it proves is still caught. Two are about the comparison
itself:
1.89and1.89.0must be the same floor — crates.io mixes bothspellings and this very lockfile contains both. Treating them as different
would red the build over formatting.
1.100must be ABOVE1.99— as strings it is below, so a lexicographiccompare would pass a floor that is actually too low. The issue's sketch used
sort -V | tail -1, which is correct but a GNU extension; this doesn't dependon which
sortis installed.The self-test captures its fixtures' stderr rather than letting it through.
Six cases are supposed to fail, and their messages are
::error::lines —GitHub Actions workflow commands, not plain text. Emitted from a passing
self-test they would stamp eight red annotations onto the PR diff while the
step exits 0. That's the same defect class this gate exists to prevent, so it
would have been an unusually poor thing to ship here.
Wiring
Into
rivet-validate, not a code-gated job. The PR that raises the real floor isa
cargo updatetouching onlyCargo.lock— exactly the PR most likely to befiltered out by a path filter. Same reasoning already documented above the
human-scoped guardrail.
cargo metadataresolves, it does not compile, and thatjob already installs a toolchain.
What this does NOT claim
It does not verify that spar's own source compiles on 1.89. That needs a job
pinned to that toolchain. The claim here is narrower and exact: the declared
floor is not below what the dependency closure demands.
Deliberately not in this PR
The channel-rustc half of #369 — compare nixpkgs'
rustcagainst this floor atnix evaluation time, turning a 14-minute build failure into a seconds-long
one — is not here, for two reasons, the second being the real one:
nix evalexpression locally, and shipping an unverifiedprobe is precisely the failure this issue is about.
trace-fixtures.yml, which ci(fixture-vm): make the nightly actually run — lock, qcow2 attribute, git-dep hash (#362, #365) #364 is currently trying to getgreen for the first time in that workflow's history. Injecting a new
unverified step into that file now would confound the run in flight.
Follow-up once #364 lands. #369 stays open until then.
Conflict note
Touches
.github/workflows/ci.yml'srivet-validatejob, as does #377.Whichever merges second needs a rebase.
Scope
CI plumbing + manifest declarations, no rivet artifact — matching the
#353/#363/#364/#366/#372 precedent; this repo carries no
REQ-CI-*ids. Flaggedso the omission reads as a choice.
🤖 Generated with Claude Code