fix(cargo-anvil): make anvil-fmt actually run the pinned nightly rustfmt - #159
fix(cargo-anvil): make anvil-fmt actually run the pinned nightly rustfmt#159Evgenii (Vaiz) wants to merge 3 commits into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. ❌ Your project status has failed because the head coverage (97.5%) is below the target coverage (100.0%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #159 +/- ##
=======================================
- Coverage 97.5% 97.5% -0.1%
=======================================
Files 300 300
Lines 67799 67799
=======================================
- Hits 66160 66154 -6
- Misses 1639 1645 +6
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
🟢 Approval recommended
The change corrects toolchain selection for cargo fmt with a targeted regression test and consistent regenerated artifacts, with no remaining issues found in the reviewed diffs.
Pull request overview
This PR fixes anvil-fmt so it consistently runs the pinned nightly rustfmt by selecting the nightly toolchain via RUSTUP_TOOLCHAIN, ensuring unstable rustfmt.toml options are actually enforced even when an ambient/stable toolchain selection would otherwise leak into cargo fmt.
Changes:
- Update the
anvil-fmtcheck to set$env:RUSTUP_TOOLCHAINto the pinned nightly and runcargo fmtwithout relying on nested+toolchain. - Add a regression test that simulates a hostile inherited
RUSTUP_TOOLCHAINand asserts the pinned nightly selection is still used for formatting-related cargo invocations. - Extend the “no implicit default cargo” invariant to treat a per-recipe
RUSTUP_TOOLCHAINpin as an explicit toolchain selection.
File summaries
| File | Description |
|---|---|
| justfiles/anvil/checks/fmt.just | Pins nightly via RUSTUP_TOOLCHAIN and runs cargo each … cargo fmt … so rustfmt resolves under the intended toolchain. |
| crates/cargo-anvil/templates/justfiles/anvil/checks/fmt.just | Template source updated to generate the fixed anvil-fmt recipe. |
| crates/cargo-anvil/tests/recipe_contracts.rs | Adds/updates tests asserting correct cargo fmt invocation and toolchain pinning behavior under inherited toolchain env. |
| crates/cargo-anvil/src/anvil/artifacts/justfile.rs | Updates invariant checking to allow explicit toolchain selection via per-recipe $env:RUSTUP_TOOLCHAIN pinning. |
| crates/cargo-anvil/tests/snapshots/snapshots__local_only.snap | Snapshot update reflecting the generated fmt.just changes. |
| crates/cargo-anvil/tests/snapshots/snapshots__github_backend.snap | Snapshot update reflecting the generated fmt.just changes. |
| crates/cargo-anvil/tests/snapshots/snapshots__ado_backend.snap | Snapshot update reflecting the generated fmt.just changes. |
| .anvil.lock | Updates checksums to match the regenerated artifacts/templates. |
Review details
- Files reviewed: 7/8 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
25ce18f to
966f52c
Compare
There was a problem hiding this comment.
🟢 Approval recommended
The change directly addresses the described toolchain-leak failure mode and adds targeted contract coverage to prevent regressions under hostile ambient RUSTUP_TOOLCHAIN.
Review details
- Files reviewed: 7/8 changed files
- Comments generated: 0 new
- Review effort level: Lite
966f52c to
4ca357a
Compare
There was a problem hiding this comment.
🟢 Approval recommended
The change is narrowly scoped, matches the stated failure mode, and includes targeted tests to prevent regressions under hostile RUSTUP_TOOLCHAIN environments.
Review details
- Files reviewed: 7/8 changed files
- Comments generated: 0 new
- Review effort level: Lite
4ca357a to
01bf4f5
Compare
There was a problem hiding this comment.
🟢 Approval recommended
The change directly addresses the described toolchain-leak failure mode and is backed by updated contract tests and regenerated snapshots without introducing new risk hotspots.
Review details
- Files reviewed: 7/8 changed files
- Comments generated: 0 new
- Review effort level: Lite
There was a problem hiding this comment.
🟡 Changes recommended
The implemented recipe/test changes don’t currently validate (and may not implement) the PR-described fix for hostile/ambient RUSTUP_TOOLCHAIN, so the original enforcement gap may remain.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 6/7 changed files
- Comments generated: 2
- Review effort level: Lite
`cargo fmt` only dispatches: cargo-fmt runs `rustfmt` as a child process
through the rustup shim, and that shim reads `RUSTUP_TOOLCHAIN`, which
rustup exports into every child of a `+`-selected cargo and does not
rewrite when a nested cargo carries its own `+`. So the inner
`+{{ rust_nightly }}` selected a nightly cargo, but the `rustfmt`
underneath it still resolved through the outer stable selection. Stable
rustfmt does not fail on unstable options -- it downgrades each to a
warning and exits 0, so every unstable `rustfmt.toml` rule silently
stopped being enforced while the check kept passing.
Set `RUSTUP_TOOLCHAIN` for the recipe instead, which covers every
descendant. The wrapper loses its stable pin deliberately: that pin is
the thing that leaks, and `cargo each` only enumerates members, it
compiles nothing.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Apply Martin's proposal instead of assigning RUSTUP_TOOLCHAIN in the recipe. Preserve --fix, assert both explicit pins in the recipe contracts, remove the now-unused environment-pin exception, and regenerate the checked-in artifacts and snapshots. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Rebasing onto main picked up template changes (msrv-test.just, tools.just and the GitHub workflow templates), so the catalog checksum recorded in .anvil.lock no longer matched either side of the merge. Regenerated with `cargo run -p cargo-anvil -- anvil`; `--dry-run` is now clean and no generated artifact changed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
9d6bc65 to
cd240b8
Compare
🤖 Clawpilot here! Posted automatically by Clawpilot (an AI agent), not by a human. Please verify before acting.
What this changes
anvil-fmtpins bothcargo eachand its innercargo fmttorust_nightly. The outer invocation no longer selects the stable toolchain.Effects
--fixkeep their existing behavior.--keep-goingand per-member manifest paths.+toolchainarguments, not aRUSTUP_TOOLCHAINassignment..anvil.lockmatch the template.Reproduction limits
Current local probes select nightly rustfmt with both the proposed command and the original command. The earlier failure is not reproduced in this environment; its cause remains unresolved.