chore(deps): consolidate 12 dependency PRs and fix the nightly clippy break blocking them all - #55
Merged
Merged
Conversation
…dependency-roundup
…t' into chore/dependency-roundup
…dependency-roundup
…rminate-nix-action-digest' into chore/dependency-roundup
…digest' into chore/dependency-roundup
…' into chore/dependency-roundup
…2.x' into chore/dependency-roundup
…into chore/dependency-roundup # Conflicts: # flake.lock
…inError
clippy 0.1.100 (nightly) fires clippy::empty_enums on
`pub enum BuiltinError {}`. The lint is a member of the nursery
group, which the workspace denies, so the nightly leg of the CI
`check` matrix failed on every open branch regardless of content.
BuiltinError is uninhabited on purpose: its doc comment reserves the
slot for future builtins (read, wait) whose failure modes are richer
than an exit status, and CoreError::Builtin already wraps it. The
emptiness is a temporary property of an intentional design, not an
oversight, so the lint is wrong here. This mirrors the existing
#[allow(clippy::uninhabited_references)] on the Display impl
immediately below, which exists for the same reason.
Verified `cargo xtask check` exits 0 on both stable and nightly
(1.100.0-nightly, clippy 0.1.100).
The CI `check` matrix runs `cargo xtask check` on both stable and nightly, but devShells.default only carried the stable toolchain. A nightly-only lint break was therefore impossible to reproduce locally — which is how a single new clippy::nursery lint (clippy::empty_enums) held twelve dependency PRs red for weeks before anyone could diagnose it. The new shell deliberately omits extraDev and corePkgs. Those carry the stable rustc/cargo/clippy plus the pre-commit tooling, and combining them with the nightly toolchain would put two toolchains on PATH and trip the E0514 failure already documented on the default shell. Consequence: no pre-commit hooks in this shell — commit from the default shell, use this one only for `cargo xtask check` on nightly. CARGO_TARGET_DIR is set in the shellHook rather than as a static mkShell env var because it must be absolute. A relative value is resolved against each cargo invocation's cwd, and trybuild runs cargo from inside crates/fredshell-spec-macros; a relative target/nightly consequently produced a stray tree at crates/fredshell-spec-macros/target/nightly that the root-anchored /target/ gitignore does not cover, and cargo-machete then walked trybuild's generated manifests and failed the check with bogus unused-dependency findings. Verified: nixfmt/statix/deadnix clean, `nix flake check` passes, and `cargo xtask check` exits 0 in both shells with no stray target trees left behind.
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (4)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🔵 Needs a closer look
A required reference-document update remains unresolved, and the broad dependency and flake refresh warrants human review.
Pull request overview
Consolidates dependency updates, fixes the nightly Clippy failure, and adds a reproducible nightly Nix shell.
Changes:
- Allows the intentional empty
BuiltinErrorenum. - Adds an isolated nightly development shell.
- Updates Rust, Nix, GitHub Actions, and lockfiles.
File summaries
| File | Description |
|---|---|
flake.nix |
Adds the nightly development shell. |
flake.lock |
Refreshes pinned Nix inputs. |
crates/fredshell-core/src/lib.rs |
Allows the intentional empty enum. |
Cargo.toml |
Updates direct dependencies. |
Cargo.lock |
Refreshes dependency resolution. |
.github/workflows/ci.yml |
Updates pinned action revisions. |
Review details
Suppressed comments (1)
flake.nix:19
- This changes the authoritative
nixpkgs-referencerevision, buttests/spec/REFERENCE.mdstill declaresaec71e3...(and the old pin date). The repository's upgrade policy requires the machine-readable[reference]block to be updated with every flake pin bump; please update that document and re-record/inspect fixtures if the new Bash or coreutils versions differ.
nixpkgs-reference.url = "github:nixos/nixpkgs/88ae3822eb8aec31f12a4a1895cb064413511177";
- Files reviewed: 4/6 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.
fredclausen
added a commit
that referenced
this pull request
Sep 18, 2026
fix(spec): catch nixpkgs-reference pin drift, and repair the drift #55 introduced
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.
Consolidates all 12 open dependency PRs into one branch and fixes the single root
cause that was holding every one of them red.
Supersedes and closes: #43 #44 #45 #46 #47 #48 #49 #50 #51 #52 #53 #54
Root cause
All 12 PRs failed identically on
Check ubuntu-latest nightlyandCheck macos-latest nightly. None of them failed because of their own content:clippy::empty_enumsis a nursery lint, and the workspace setsnursery = { level = "deny" }. It began firing when nightly clippy picked thelint up, so every branch in the repo went red at once — including
main, whichstill has the same code. Stable was and is unaffected.
I reproduced this against today's nightly (
1.100.0-nightly, clippy0.1.100)and confirmed
empty_enumsis the only error; nothing else has accumulatedin the intervening weeks.
Changes
1.
fix:allowclippy::empty_enumsonBuiltinErrorOne line.
BuiltinErroris uninhabited deliberately — its doc comment reservesthe slot for future builtins (
read,wait) whose failure modes are richerthan an exit status, and
CoreError::Builtinalready wraps it. The emptiness isa temporary property of an intentional design, so the lint is wrong at this site.
This mirrors the pre-existing
#[allow(clippy::uninhabited_references)]on theDisplayimpl three lines below, which exists for exactly the same reason.2.
chore(flake):adddevShells.nightlyCI gates on nightly, but
devShells.defaultonly carried the stable toolchain —so a nightly-only break could not be reproduced locally at all. That gap is why
one lint sat undiagnosed across 12 PRs for weeks. I had to build a throwaway
nix shellto find it.The new shell deliberately omits
extraDevandcorePkgs: those carry thestable rustc/cargo/clippy plus pre-commit tooling, and mixing them with nightly
would put two toolchains on
PATHand trip the E0514 failure already documentedon the default shell. Trade-off is explicit in a comment — no pre-commit hooks in
the nightly shell, so commit from
defaultand usenightlyonly to runcargo xtask check.nix develop --impure .#nightly --command cargo xtask check3. The 12 dependency bumps
base640.23.1,clap4.6.6,syn3.0.5, plusCargo.lockmaintenancedeterminate-nix-action,dtolnay/rust-toolchain,Swatinem/rust-cache2.9.2,taiki-e/install-action2.87.12nixpkgs,nixpkgs-reference88ae382,precommit,rust-overlayNotes on the merge
#52,#53,#54(the threeupdate-*flake-input PRs) turned out to befully superseded —
renovate/lock-file-maintenancehad already advancedevery input past what they proposed. They are included in the history but
contribute nothing on top.
flake.lockbetweenlock-file-maintenanceandthe
update-*branches. Rather than hand-pick hunks across 12 conflictedblocks, I resolved it by regenerating the lock with
nix flake update, whichis the only resolution guaranteed internally consistent. All inputs are now at
current rather than at four different stale points.
Cargo.lockneeded one reconciliation the merges could not do themselves: astale
syn 3.0.3entry underwasm-bindgenresolved to3.0.5.Verification
cargo xtask check(fmt + clippy-D warnings+ machete + test + doc) runlocally in both toolchains:
pre-commit run --all-files— all 25 active hooks pass.nix flake check— all outputs evaluate, including both dev shells.Deliberately not done
CI posture is unchanged.
nursery+pedanticatdeny, on nightly, with-D warnings, as a required check, means clippy's release schedule can turnthe whole repo red with no code change — which is precisely what happened here.
Fixing this one lint does not prevent the next one. Making the nightly leg
advisory was considered and explicitly deferred; worth revisiting if this recurs.
Summary by CodeRabbit
Developer Experience
Maintenance