refactor(anvil): adopt cargo-each for scoped checks - #151
refactor(anvil): adopt cargo-each for scoped checks#151martin-kolinek wants to merge 18 commits into
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Codecov Report✅ All modified and coverable lines are covered by tests. ❌ Your project status has failed because the head coverage (97.6%) 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 #151 +/- ##
=====================================
Coverage 97.6% 97.6%
=====================================
Files 298 298
Lines 67348 67348
=====================================
Hits 65763 65763
Misses 1585 1585
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.
🟡 Changes recommended
The include-file projection format changed but the impact cache freshness key doesn’t encode a projection-format version, risking “cache hit” reuse of old-format include files that can break the new cargo each @selection call sites.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR refactors cargo-anvil’s impact-scoped check execution to use cargo-each as the package selection + execution boundary, shifting impact outputs from pre-built --package name@version ... strings / --skip into tokenized selector streams (--workspace, repeated --package + name, or --none).
Changes:
- Change impact cache projection to emit one selector token per line and update
_anvil-impact-includeconsumers to capture a splattable array. - Delegate many “ordinary” checks (clippy, docs, udeps, etc.) to
cargo each ... --once -- ...while preserving bespoke orchestration in specialized recipes. - Update tests, snapshots, docs, templates, and lockfile to reflect the new selector contract and
cargo-eachdependency.
File summaries
| File | Description |
|---|---|
| justfiles/anvil/impact.just | Emit tokenized cargo-each selectors (--none/--package name) and adjust include resolution. |
| justfiles/anvil/helpers.just | Document new selector-token capture pattern ($selection = @(& ... )). |
| justfiles/anvil/checks/udeps.just | Run udeps through cargo each @selection --once and add cargo-each prereqs. |
| justfiles/anvil/checks/semver-check.just | Consume selection token array and update empty-tier handling to --none. |
| justfiles/anvil/checks/mutants-diff.just | Delegate execution gating to cargo-each and add cargo-each prereqs. |
| justfiles/anvil/checks/msrv-test.just | Use cargo-each for affected selection and add cargo-each prereqs. |
| justfiles/anvil/checks/miri.just | Run miri through cargo-each selection. |
| justfiles/anvil/checks/miri-tree-borrows.just | Run tree-borrows miri through cargo-each selection. |
| justfiles/anvil/checks/miri-strict-provenance.just | Run strict-provenance miri through cargo-each selection. |
| justfiles/anvil/checks/miri-race-coverage.just | Run race-coverage miri through cargo-each selection. |
| justfiles/anvil/checks/loom.just | Update affected parsing to use selection tokens and --none. |
| justfiles/anvil/checks/llvm-cov.just | Update selection parsing to use bare names and --none. |
| justfiles/anvil/checks/license-headers.just | Gate workspace-wide tool via cargo-each selection. |
| justfiles/anvil/checks/fmt.just | Gate formatting via selection and delegate workspace iteration to cargo-each. |
| justfiles/anvil/checks/external-types.just | Replace manual metadata filtering/iteration with cargo-each --filter lib. |
| justfiles/anvil/checks/examples.just | Run build examples via cargo-each selection. |
| justfiles/anvil/checks/ensure-no-default-features.just | Gate tool via cargo-each selection and add cargo-each prereqs. |
| justfiles/anvil/checks/ensure-no-cyclic-deps.just | Gate tool via cargo-each selection and add cargo-each prereqs. |
| justfiles/anvil/checks/doc-test.just | Run doc tests via cargo-each selection. |
| justfiles/anvil/checks/doc-build.just | Run doc build via cargo-each selection. |
| justfiles/anvil/checks/clippy.just | Run clippy via cargo-each selection. |
| justfiles/anvil/checks/cargo-sort.just | Gate cargo-sort via cargo-each selection and add cargo-each prereqs. |
| justfiles/anvil/checks/cargo-hack.just | Run cargo-hack via cargo-each selection. |
| justfiles/anvil/checks/careful.just | Run cargo-careful via cargo-each selection and --none skipping. |
| justfiles/anvil/checks/bolero.just | Update affected selection parsing for bolero to use token array. |
| justfiles/anvil/checks/bench.just | Run bench via cargo-each selection. |
| crates/cargo-anvil/tests/snapshots/snapshots__local_only.snap | Update emitted justfile snapshot to the new selector-token contract. |
| crates/cargo-anvil/tests/recipe_contracts.rs | Update contracts for --none and tokenized selector output. |
| crates/cargo-anvil/tests/impact.rs | Update impact-behavior tests for --none and token-per-line include files. |
| crates/cargo-anvil/templates/justfiles/anvil/impact.just | Template: emit tokenized selectors and updated include logic. |
| crates/cargo-anvil/templates/justfiles/anvil/helpers.just | Template: document new selector-token capture pattern. |
| crates/cargo-anvil/templates/justfiles/anvil/checks/udeps.just | Template: udeps delegates to cargo-each and includes prereqs. |
| crates/cargo-anvil/templates/justfiles/anvil/checks/semver-check.just | Template: selection tokens + --none semantics. |
| crates/cargo-anvil/templates/justfiles/anvil/checks/mutants-diff.just | Template: cargo-each delegation + prereqs. |
| crates/cargo-anvil/templates/justfiles/anvil/checks/msrv-test.just | Template: cargo-each selection + prereqs. |
| crates/cargo-anvil/templates/justfiles/anvil/checks/miri.just | Template: cargo-each selection. |
| crates/cargo-anvil/templates/justfiles/anvil/checks/miri-tree-borrows.just | Template: cargo-each selection. |
| crates/cargo-anvil/templates/justfiles/anvil/checks/miri-strict-provenance.just | Template: cargo-each selection. |
| crates/cargo-anvil/templates/justfiles/anvil/checks/miri-race-coverage.just | Template: cargo-each selection. |
| crates/cargo-anvil/templates/justfiles/anvil/checks/loom.just | Template: token-array parsing and --none handling. |
| crates/cargo-anvil/templates/justfiles/anvil/checks/llvm-cov.just | Template: token-array parsing and --none handling. |
| crates/cargo-anvil/templates/justfiles/anvil/checks/license-headers.just | Template: gate via cargo-each selection + prereqs. |
| crates/cargo-anvil/templates/justfiles/anvil/checks/fmt.just | Template: gate + delegate workspace iteration to cargo-each. |
| crates/cargo-anvil/templates/justfiles/anvil/checks/external-types.just | Template: use cargo-each --filter lib instead of manual iteration. |
| crates/cargo-anvil/templates/justfiles/anvil/checks/examples.just | Template: cargo-each selection. |
| crates/cargo-anvil/templates/justfiles/anvil/checks/ensure-no-default-features.just | Template: gate via cargo-each selection + prereqs. |
| crates/cargo-anvil/templates/justfiles/anvil/checks/ensure-no-cyclic-deps.just | Template: gate via cargo-each selection + prereqs. |
| crates/cargo-anvil/templates/justfiles/anvil/checks/doc-test.just | Template: cargo-each selection. |
| crates/cargo-anvil/templates/justfiles/anvil/checks/doc-build.just | Template: cargo-each selection. |
| crates/cargo-anvil/templates/justfiles/anvil/checks/clippy.just | Template: cargo-each selection. |
| crates/cargo-anvil/templates/justfiles/anvil/checks/cargo-sort.just | Template: gate via cargo-each selection + prereqs. |
| crates/cargo-anvil/templates/justfiles/anvil/checks/cargo-hack.just | Template: cargo-each selection. |
| crates/cargo-anvil/templates/justfiles/anvil/checks/careful.just | Template: cargo-each selection + prereqs. |
| crates/cargo-anvil/templates/justfiles/anvil/checks/bolero.just | Template: token-array parsing updates. |
| crates/cargo-anvil/templates/justfiles/anvil/checks/bench.just | Template: cargo-each selection + prereqs. |
| crates/cargo-anvil/src/anvil/artifacts/justfile.rs | Update enforcement of “capture include tokens into $selection array” contract. |
| crates/cargo-anvil/docs/implementation.md | Document new “bare selector + cargo-each resolves versions” behavior. |
| crates/cargo-anvil/docs/design/local.md | Update design doc to reflect token-per-line include cache and cargo-each usage. |
| crates/cargo-anvil/docs/design/github.md | Update workflow design text from --skip to --none semantics. |
| crates/cargo-anvil/docs/design/checks.md | Update check scoping model to cargo-each selectors and --none. |
| crates/cargo-anvil/docs/design/ado.md | Update ADO design text from --skip to --none semantics. |
| .anvil.lock | Update catalog checksum and justfile checksums after regeneration. |
Review details
- Files reviewed: 63/64 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🔵 Needs a closer look
_anvil-impact-include should be backward-compatible with legacy cached include file formats to avoid post-upgrade local failures when an old impact cache is reused.
Review details
Suppressed comments (2)
justfiles/anvil/impact.just:624
- anvil-impact-include now returns token-per-line selectors, but it will also happily return legacy cached include.txt values as a single string token (e.g. "--skip" or "--package name@ver --package ..." from older Anvil versions). That can break local runs after an update if an existing target/anvil/impact cache is treated as fresh, because cargo-each will receive an unsupported selector token.
To make upgrades robust, consider parsing the cache file in a backward-compatible way: map legacy "--skip" -> "--none", and split legacy one-line "--package name@ver ..." into tokens while stripping any @version qualifiers.
crates/cargo-anvil/templates/justfiles/anvil/impact.just:624
- Same as the generated justfiles/anvil/impact.just: anvil-impact-include should be backward-compatible with legacy cached include.txt formats ("--skip" and single-line "--package name@ver ..."), otherwise local runs can break after upgrading if an old target/anvil/impact cache is considered fresh.
Consider parsing the cache file in a backward-compatible way (map "--skip" -> "--none", split legacy one-line lists into tokens, and strip any @version qualifiers).
- Files reviewed: 63/64 changed files
- Comments generated: 0 new
- Review effort level: Lite
There was a problem hiding this comment.
🔵 Needs a closer look
The Unix fake cargo shim in crates/cargo-anvil/tests/impact.rs doesn’t correctly handle cargo +<toolchain> install --list, which can break prereq-validation paths and make affected tests unreliable.
Review details
Suppressed comments (3)
Previously missed (3) — in code that hasn't changed since the last review.
crates/cargo-anvil/tests/impact.rs:933
- On Unix, the fake
cargoshim only special-casesinstall --listwhen those are the first two argv elements. In this repo_anvil_stable_toolchain_argscan prefix commands with a+<toolchain>argument, so calls likecargo +1.95 install --listwon’t hit this branch and can make prereq-validation tests flaky/incorrect.
crates/cargo-anvil/templates/justfiles/anvil/checks/llvm-cov.just:153 - The gate-scoping comment still claims $pkg contains version-qualified selectors ("--package X@ver ..."), but this PR changed impact projection to emit tokenized "--package" + bare package-name pairs (and cargo-each handles version-qualification). This comment is now misleading about what $pkg actually holds.
justfiles/anvil/checks/llvm-cov.just:153 - The gate-scoping comment still claims $pkg contains version-qualified selectors ("--package X@ver ..."), but this PR changed impact projection to emit tokenized "--package" + bare package-name pairs (and cargo-each handles version-qualification). This comment is now misleading about what $pkg actually holds.
- Files reviewed: 63/64 changed files
- Comments generated: 0 new
- Review effort level: Lite
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🔵 Needs a closer look
The updated local design doc example inlines _anvil-impact-include into cargo each without fail-fast exit-code handling, which can mislead users into running checks with an invalid/partial selection.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
crates/cargo-anvil/docs/design/local.md:536
- The example pipelines
_anvil-impact-includeinline intocargo each, but that loses the ability to fail fast when_anvil-impact-includeexits non-zero (e.g., impact-format ambiguity/metadata failures). The check recipes in this PR capture the selector array first and exit on$LASTEXITCODEbefore invoking cargo-each; the docs should mirror that safe pattern.
- Files reviewed: 63/64 changed files
- Comments generated: 0 new
- Review effort level: Lite
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> # Conflicts: # .anvil.lock
There was a problem hiding this comment.
🟡 Changes recommended
The updated design doc example in crates/cargo-anvil/docs/design/local.md diverges from the repository-enforced scoped-check contract (required $selection capture), risking misleading copy/paste guidance.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 63/64 changed files
- Comments generated: 1
- Review effort level: Lite
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🔵 Needs a closer look
It changes core impact-scoping and check execution behavior across many recipes/docs/tests, so a final human review is needed to validate end-to-end workflow expectations.
Review details
- Files reviewed: 63/64 changed files
- Comments generated: 0 new
- Review effort level: Lite
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> # Conflicts: # .anvil.lock # crates/cargo-anvil/templates/justfiles/anvil/checks/msrv-test.just # crates/cargo-anvil/tests/impact.rs # crates/cargo-anvil/tests/snapshots/snapshots__ado_backend.snap # crates/cargo-anvil/tests/snapshots/snapshots__github_backend.snap # crates/cargo-anvil/tests/snapshots/snapshots__local_only.snap # justfiles/anvil/checks/msrv-test.just
There was a problem hiding this comment.
🔵 Needs a closer look
It rewires a large number of CI-critical check recipes and their contracts around a new execution boundary, warranting final human verification of end-to-end behavior.
Review details
- Files reviewed: 65/66 changed files
- Comments generated: 0 new
- Review effort level: Lite
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟢 Approval recommended
The selector tokenization and cargo-each delegation are applied consistently across recipes/templates with corresponding docs/tests updated to preserve the scoping and skip contracts.
Review details
- Files reviewed: 65/66 changed files
- Comments generated: 0 new
- Review effort level: Lite
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟢 Approval recommended
The refactor consistently updates impact projection, selector consumption, and contract tests/docs to the new cargo-each token model, with only a small documentation wording nit noted.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
crates/cargo-anvil/templates/justfiles/anvil/impact.just:472
- This comment says “PowerShell captures a ready-to-splat array”, but callers still need the array-subexpression form (
@(& ...)) to guarantee an array even for single-token output. Tweaking the wording here would better match how the recipes actually capture the output above.
- Files reviewed: 65/66 changed files
- Comments generated: 0 new
- Review effort level: Lite
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> # Conflicts: # .anvil.lock
There was a problem hiding this comment.
🟡 Changes recommended
One updated doctest example (crates/cargo-gamma-lib/src/cfg/features.rs) applies cfg(feature="internals") only to imports while leaving the rest of the example body referencing internals, so the gating is incomplete and should be fixed.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 65/66 changed files
- Comments generated: 1
- Review effort level: Lite
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🔵 Needs a closer look
It rewires a large surface area of CI/local check orchestration and impact-scoping behavior, which warrants end-to-end human verification across workflows despite strong test/doc updates.
Review details
- Files reviewed: 65/66 changed files
- Comments generated: 0 new
- Review effort level: Lite
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🔵 Needs a closer look
It changes the core execution/scoping boundary used by many CI-critical checks (impact cache format + cargo-each delegation), warranting final human validation of workflow behavior.
Review details
- Files reviewed: 66/67 changed files
- Comments generated: 0 new
- Review effort level: Lite
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟢 Approval recommended
The refactor is consistently applied across templates/dogfood justfiles and is backed by updated contract + impact tests, with only a minor documentation alignment nit noted.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
crates/cargo-anvil/templates/justfiles/anvil/impact.just:476
- The
_anvil-impact-formatdocs say “one token per line so PowerShell captures a ready-to-splat array”, but the Usage example captures with$val = (& ... ). That yields a scalar string when the formatter returns a single token (--workspace/--none), and splatting a string is error-prone. Align the example with the required capture pattern used elsewhere (@(& ...)) so callers always get an array.
- Files reviewed: 66/67 changed files
- Comments generated: 0 new
- Review effort level: Lite
🤖 Uses cargo-each as the package-selection and execution boundary for cargo-anvil's impact-scoped checks.