docs(cargo-anvil): design generated-file consolidation - #173
martin-kolinek wants to merge 1 commit into
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: a9fc919b-99f7-4134-aad1-2116321b4e0c
There was a problem hiding this comment.
🟡 Changes recommended
Multiple critical and moderate design-contract issues remain unresolved.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This documentation-only PR defines consolidating Anvil-owned files under .anvil/, replacing the generated recipe tree with one composed Justfile, and clarifying setup, tooling, and migration behavior.
Changes:
- Links the consolidation proposal from the design README.
- Defines the target layout and portable recipe contracts.
- Documents installer, container, checksum, and upgrade behavior.
File summaries
| File | Summary and final review findings |
|---|---|
crates/cargo-anvil/docs/design/README.md |
Links the consolidation proposal and updates installation-policy wording. |
crates/cargo-anvil/docs/design/consolidation.md |
Defines the consolidated layout and implementation contracts. Findings: critical — undeclared {workspace-rust-version} placeholder (1 vote); container ignore/digest rules omit .anvil/anvil.just (2 votes); migration omits import and CI/action reference rewrites (1 vote). Moderate — undeclared --package-file contract (2 votes); unsupported concurrency and timeout responsibilities (1 vote); binary-only binstall conflicts with fallback policy (1 vote); checksum semantics do not match ordered rendering (1 vote); edited configuration migration is unspecified (1 vote); container installer-mode forwarding is unspecified (1 vote). Nit — clarify the hooks.ps1 trust-boundary exception (1 vote). |
Review details
Suppressed comments (7)
crates/cargo-anvil/docs/design/consolidation.md:255
- This adds concurrency bounds and per-command timeouts to cargo-each's responsibilities, but cargo-each's own design explicitly makes parallel scheduling a non-goal and its current execution contract has neither control (
crates/cargo-each/docs/design/README.md:73-82, 231-242). Record this as a coordinated cargo-each scope change with defined semantics, or remove it from this contract; otherwise the consolidation plan requires capabilities that the tool's design deliberately excludes.
- Expand package, target, and workspace-Rust-version placeholders.
- Spawn commands once, per package, or per target.
- Optionally bound concurrency and enforce per-command timeouts.
crates/cargo-anvil/docs/design/consolidation.md:87
- Moving the delta config does not automatically update its managed contents. The current
.delta.tomltrip-wire list still names legacy paths such as.delta.toml,.github/**,.pipelines/**, andjustfiles/**(.delta.toml:4-23); without replacing those with the relocated.anvil/**inputs, changes to the composed Justfile or moved backend/config files may not widen impact analysis and can leave checks under-scoped. Specify that pattern rewrite as part of this move.
- `.delta.toml` becomes `.anvil/config/delta.toml`; every invocation supplies
the path explicitly.
crates/cargo-anvil/docs/design/consolidation.md:169
- This target contract makes
binstallbinary-only and forbids source fallback, but the current installer contract explicitly falls back to exact-pincargo installafter a nonzero binstall result (crates/cargo-anvil/docs/design/local.md:301-307), while the goal above says to preserve the current setup policy. That is a behavior change rather than a consolidation detail; retain the fallback or explicitly revise the goal and the other setup/backend design documents.
- `binstall` is an explicit opt-in for environments that permit prebuilt
binaries. It passes `--disable-strategies compile` and does not fall back to
source. If no permitted binary strategy succeeds, setup fails and tells the
caller to retry with `installer=install`.
crates/cargo-anvil/docs/design/consolidation.md:315
- The new sections make catalog order observable in the rendered file, but the existing catalog checksum is explicitly order-independent because it sorts canonical artifact representations (
crates/cargo-anvil/src/catalog/builder.rs:67-81). Two catalogs with the same sections in different orders would therefore render different.anvil/anvil.justbytes but sharecatalog_checksum. Make checksum canonicalization order-sensitive for ordered sections (and test it), or define why this semantic difference is intentionally excluded.
An owned-file section is independently addressable in a catalog but does not
have an on-disk delimiter. Identity is `(path, id)`. Catalog order is rendering
order. At plan time, enabled sections with the same path are joined with one
canonical blank-line separator and passed to the existing owned-file planner as
one physical file.
crates/cargo-anvil/docs/design/consolidation.md:383
- Step 5 treats moved backend/config files as ordinary owned files, but
.delta.tomlandspellcheck.tomlare currently managed-region hosts: nonempty edits are refused, preserved, and remain tracked (docs/design/updates.md:298-307). Leaving an edited root file as an orphan while creating a fresh.anvil/config/...would make its repository settings invisible to the new recipes. Define how edited config content is migrated or retained, and cover it in the upgrade cases, rather than applying owned-file rules.
5. moves untouched backend/config files and preserves edited old files using
the normal owned-file rules;
crates/cargo-anvil/docs/design/consolidation.md:152
- This exception conflicts with the top-level security statement still in
docs/design/README.md:398-399and the current container contract, which explicitly dot-sourceshooks.ps1at runtime (docs/design/containers.md:545-550). Clarify here and in the top-level wording that.anvil/container/hooks.ps1is an intentional user-code trust-boundary exception; otherwise the generated-file contract promises both never executing user-edited files and direct execution of one.
The container driver is the exception that proves the boundary. Docker/Podman
selection, WSL path translation, linked-worktree mounts, image hashing, and
credential hooks are not workspace-package iteration. They remain contained
under `.anvil/container/` as directly invoked scripts unless the container
contract is simplified enough to express as a few Docker commands. They do not
crates/cargo-anvil/docs/design/consolidation.md:163
- The container build is a setup layer too, but the current Dockerfile setup hard-codes
just anvil-setup binstall(templates/anvil/container/Dockerfile.setup.region:48-52). This section says every setup layer must forward the caller-selected mode and that source-onlyinstallis required for internal environments; define how the container receives and forwardsinstaller, or explicitly exempt it. Otherwise the container path can violate the selected supply-chain policy.
`installer="install"|"binstall"` remains a caller-selected parameter at every
setup layer:
- `install` is the default and builds exclusively from source with
`cargo install --locked`. Internal environments use this path because
- Files reviewed: 2/2 changed files
- Comments generated: 4
- Review effort level: Lite
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| - The complete `justfiles/anvil/` tree becomes `.anvil/anvil.just`. | ||
| - `.anvil.lock` becomes `.anvil/manifest.toml`. |
| ```just | ||
| _anvil-install-workspace-toolchain: | ||
| cargo each --workspace --once -- rustup toolchain install {workspace-rust-version} --profile minimal |
| 3. composes `.anvil/anvil.just`; | ||
| 4. retires untouched files under `justfiles/anvil/` and keeps edited files as | ||
| ownership-transferred orphans; | ||
| 5. moves untouched backend/config files and preserves edited old files using | ||
| the normal owned-file rules; |
| cargo each --package-file target/anvil/impact/affected.packages --once -- \ | ||
| cargo clippy {packages} --all-targets --all-features --locked -- -D warnings |
|
Not yet relevant |
🤖 Defines the target contract for consolidating Anvil-owned files under .anvil/ and replacing the generated recipe tree with one delimiter-free composed Justfile. It also records portable Just setup semantics, tool responsibility boundaries, platform-constrained entrypoints, and upgrade behavior.\n\nPreparatory implementation PRs:\n- cargo-each: #176\n- cargo-coverage-gate: #179\n- cargo-aprz: #175\n- cargo-delta core: tekian/cargo-delta#32\n- cargo-delta ref/artifact flow: tekian/cargo-delta#33 (stacked on #32)\n\nThis PR changes no generated output or runtime behavior; the final cargo-anvil relocation follows after the tool contracts land.