ci: run the checks inside the flake's development shell - #429
Merged
Conversation
The shell had drifted far from what CI actually builds: nixpkgs was still on release-22.11 and the toolchain was pinned to 1.65.0, three years and seventeen Rust releases behind the 1.82.0 the CI matrix has been gating on. Anyone using the shell was therefore developing against a compiler the project no longer supports. Bump nixpkgs to nixos-26.05, refresh fenix and flake-utils, and pin the toolchain to 1.82.0. That is not merely CI's claim: the workspace uses std::iter::repeat_n, stabilized in 1.82, so 1.82.0 is the true floor. Cargo.lock is v4, which independently rules out anything below 1.78. Rename devShell to devShells.default, the modern form, so the shell can be discovered and built by name. Add what CI needs to run entirely inside the shell: socat for the listener example test, and llvm-tools plus cargo-llvm-cov for the coverage report. rustfmt stays on nightly because rustfmt.toml uses nightly-only options.
With the v1-parsing feature enabled the or_else branch already yields the crate's own error type, so wrapping it back up as Ok(runtime_settings?) is a no-op that clippy rejects. Without the feature the types differ and the conversion is real, so plainly returning the value would not compile. map_err(Into::into) is correct either way: it performs the conversion when one is needed and resolves to the identity impl when it is not. Nightly clippy does not flag this today, which is why it went unnoticed; the MSRV clippy does.
CI is currently red for a reason that has nothing to do with the code: the actions-rs/* actions are archived and drag in actions/cache v2, which GitHub now fails automatically. They also duplicated the environment by hand, installing libarchive, protobuf and socat through apt and the toolchain through actions-rs/toolchain, so the versions CI used were never the versions the development shell provides. Use OSSystems/nix-actions instead and run every step inside the flake's shell, so contributors and CI build with the same pinned toolchain and the same native dependencies. The environment is now described in exactly one place, and a change to the shell is exercised by CI on the commit that makes it. A separate job runs nix flake check and builds the shell, so a broken flake is reported on its own rather than as a confusing failure inside the test job; it also warms the Nix store cache the other jobs restore. The MSRV/stable/nightly matrix collapses to a single job. The shell is pinned to the MSRV, so that job is the MSRV check. Coverage no longer needs a nightly cell because cargo-llvm-cov now comes from the shell, and clippy consequently runs at the MSRV, which is the compiler the code actually has to satisfy. Swatinem/rust-cache replaces the hand-rolled actions/cache plus the cargo-cache trimming step that existed to keep that cache from growing without bound. Note that this renames the check runs: the three "Test MSRV|stable|nightly - x86_64-unknown-linux-gnu" contexts become "Test - x86_64-unknown-linux-gnu" plus a new "Flake check", so branch protection needs updating to match.
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
1 participant
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.
CI is currently red on
masterfor a reason unrelated to the code: theactions-rs/*actions are archived and pull inactions/cachev2, which GitHub now fails automatically.Rather than patch around that, this switches CI to OSSystems/nix-actions and runs every step inside the flake's development shell, so contributors and CI use the same pinned toolchain and the same native dependencies. The environment is described in one place, and a change to the shell is exercised by CI on the commit that makes it.
What changed
flake:modernize the development shell and pin it to the real MSRVThe shell had drifted badly: nixpkgs was still on
release-22.11and the toolchain was pinned to 1.65.0, three years behind the 1.82.0 the CI matrix gates on. nixpkgs moves tonixos-26.05, fenix and flake-utils are refreshed, and the toolchain is pinned to 1.82.0.devShellbecomesdevShells.default, and the shell gainssocat(listener example test) plusllvm-toolsandcargo-llvm-cov(coverage). rustfmt stays on nightly becauserustfmt.tomluses nightly-only options.fix:runtime_settings: drop the needless question mark when parsingClippy now runs at the MSRV rather than nightly, and MSRV clippy flags
Ok(runtime_settings?).map_err(Into::into)is correct with thev1-parsingfeature both on and off; verified clippy-clean in both configurations.ci:run the checks inside the flake's development shellA separate job runs
nix flake checkand builds the shell, so a broken flake is reported on its own and the Nix store cache is warmed for the other jobs. The MSRV/stable/nightly matrix collapses to one job — the shell is pinned to the MSRV, so that job is the MSRV check, and coverage no longer needs a nightly cell.Swatinem/rust-cachereplaces the hand-rolledactions/cacheplus thecargo-cachetrimming step.MSRV is genuinely 1.82.0
Not just CI's claim. Probing 1.78 (the floor
Cargo.lockv4 imposes) fails onstd::iter::repeat_ninupdatehub/tests/common.rs, stabilized in 1.82.flake.nix's 1.65.0 was simply stale.Verification
Every step of both workflows was run locally inside
nix developbefore pushing:nix flake checkcargo check --locked --release --all --bins --examples --tests--all-featurescargo test --locked --release --all --all-featuressocatcargo fmt --all -- --checkcargo clippy --all-features --all --tests -- -D clippy::allcargo llvm-cov --all-features --workspace --lcovactionlintis clean on all workflow files.Before merging
This renames the check runs. The three
Test MSRV|stable|nightly - x86_64-unknown-linux-gnucontexts becomeTest - x86_64-unknown-linux-gnuplus a newFlake check, so branch protection needs updating to match.Not included:
ossystems/nix-actions/update-flake, the scheduledflake.lockbump. Happy to add it in a follow-up — it is what would have kept the flake from drifting three years behind in the first place.