chore(ci): run the nightly-only checks again and raise the serde floor - #853
Open
thatssoheil wants to merge 1 commit into
Open
thatssoheil wants to merge 1 commit into
thatssoheil wants to merge 1 commit into
Conversation
ci/test-stable.sh gated its benchmark and minimal-versions checks on a RUST_VERSION environment variable that no workflow sets, so those two checks have not run in CI since tokio-rs#452. Gate on the toolchain actually in use instead: the nightly job switches the default toolchain before it runs the script, so `rustc --version` is the authority and cannot go stale. Running the check for the first time fails on the serde floor this crate declares: -Z minimal-versions resolves serde 1.0.60, whose alloc feature does not compile under it (E0432, unresolved imports alloc::BTreeMap, BTreeSet, BinaryHeap, LinkedList and VecDeque). 1.0.69 fails identically and 1.0.70 compiles, so the floor becomes 1.0.70. Verified locally with the nightly toolchain: cargo +nightly update -Z minimal-versions && cargo +nightly check --all-features cargo +nightly check --benches both succeed at floor 1.0.70 and fail at 1.0.69. The stable matrix is unaffected: cargo test, cargo test --all-features and cargo test --no-default-features --features serde all pass. Disclosure: AI-assisted (Hermes Agent, Nous Research); both claims above were re-checked against the real toolchains before this commit.
There was a problem hiding this comment.
🟢 Approval recommended
No unresolved blocking issues were identified.
Pull request overview
Updates nightly CI detection and raises the Serde minimum version so minimal-version checks pass.
Changes:
- Detect nightly via
rustc --version. - Raise the Serde floor from
1.0.60to1.0.70.
File summaries
| File | Description |
|---|---|
ci/test-stable.sh |
Re-enables nightly benchmark and minimal-version checks. |
Cargo.toml |
Updates the Serde dependency floor. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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.
Fixes #851.
ci/test-stable.shgated its benchmark check and its minimal-versions check onRUST_VERSION, and no workflow sets that variable -ci.ymlsetsRUSTFLAGS,RUST_BACKTRACEandnightly, none of themRUST_VERSION. So the block atci/test-stable.sh:14has never been entered since #452, in the nightly jobincluded.
This gates the block on the toolchain actually in use instead. The nightly job
runs
rustup update $nightly && rustup default $nightlybefore invoking thescript (
ci.yml), sorustc --versionreports a nightly version there and astable version in the
stablematrix. Nothing else changes for the stablejobs.
Enabling the gate exposes the second half of the issue: the check fails on the
serde floor the manifest declares.
-Z minimal-versionsresolves serde1.0.60, whose
allocfeature does not compile under it:So the floor moves to 1.0.70. That number is measured, not guessed - see below.
Both changes have to land together: enabling the gate alone turns the nightly
job red, and
publish_docsneedsnightly, so documentation would stoppublishing too.
Evidence
The gate is dead. Simulating the CI jobs with
rustc/cargoshims that logevery invocation,
ci/test-stable.shas it stands onmastermakes exactly twocargo calls under a nightly toolchain and never reaches the block; with this
change it makes six, including
cargo check --benches,cargo hack --remove-dev-deps --workspace,cargo update -Z minimal-versionsand
cargo check --all-features. Under a stable toolchain it still makes two.1.0.70 is the first working floor. In a scratch clone, with
cargo hack --remove-dev-deps --workspaceemulated (the crate does not havecargo-hackinstalled here) followed bycargo +nightly update -Z minimal-versions:cargo +nightly check --all-featurescargo +nightly check --benchesis rc=0 with the floor at 1.0.70.Stable is unaffected.
cargo test(248 passed),cargo test --all-featuresand
cargo test --no-default-features --features serdeall pass on the change.The manifest's
rust-versionis untouched, so theminrustjob is unaffected.One trade-off worth naming
If
rustcwere somehow unresolvable the condition would read as false and theblock would skip silently. That is the same shape of failure that produced this
issue, so it is worth a maintainer's eye; I kept it because by that point in the
script
cargohas already run successfully underset -e, and because this isthe form the issue's own comment proposes. Happy to switch to
rustup show active-toolchainor an explicitcommand -v rustcguard if youwould rather not rely on the version banner.
Disclosure
AI-assisted (Hermes Agent, Nous Research). Every claim above was re-verified
against the real toolchains before opening this; I can walk through any line.