fix(cargo-anvil): let the container image resolve the declared MSRV by wukchung · Pull Request #155 · microsoft/ox-tools

martinhavelka (wukchung) · 2026-09-03T11:27:26Z

🤖 Authored by an AI agent. Please verify before acting.

Problem

cargo-anvil 0.6.0 added the MSRV check. Its setup recipe anvil-msrv-test-setup is reached by just anvil-setup through the PR tier (anvil-setup -> anvil-full-setup -> anvil-pr-setup -> anvil-pr-slow-setup -> anvil-pr-msrv-setup), and it resolves the toolchain by scanning the root Cargo.toml for rust-version.

The container image copies justfiles and rust-toolchain.toml and nothing else, so the scan found no manifest and threw:

anvil: Cargo.toml not found at repository root '/opt/anvil'
error: recipe `_anvil-resolve-stable` failed with exit code 1
error: recipe `anvil-msrv-test-setup` failed with exit code 1

This is not a degraded image, it is no image, so every anvil-container recipe is unusable after a cold build. Existing images do not shield anyone: the tag digests justfiles/anvil/, and 0.6.0 adds checks/msrv-test.just and groups/pr-msrv.just, which renames the image and forces exactly the rebuild that fails.

The fix

Admit the root Cargo.toml to the build context and copy it to /opt/anvil.

That directory is already the root the recipes resolve against: it is justfile_directory(), and it holds justfiles/ and rust-toolchain.toml. Copying the manifest alongside them completes it for the one question the setup asks, and the resolver reads it there exactly as it does on a developer's machine.

tools.just therefore needs no container-specific behaviour. The only change to it is a new root-msrv action, which the tag needs; Get-RootMsrv, Get-MsrvSelection, Assert-WorkspaceMsrvCompatibility and the manifest scanner are untouched.

The workspace members the manifest names stay out of the context: they are a checkout, and the image is not one. The one path that would need them, workspace MSRV validation, returns early whenever a root toolchain file selects the compiler. That makes the toolchain file a precondition of this design rather than a convenience, and a contract test now pins it, because the natural fix for repositories that have no such file would otherwise put that branch back in reach of a manifest with no members.

The tag hashes the value, not the file

The image installs the toolchain the MSRV names, so raising it changes what the image contains and must rename it. anvil-container-tag resolves the declared value through root-msrv and frames it into the digest as its own length-prefixed record.

Hashing the manifest instead would be a correctness problem rather than only a cost one: a tag must change when the contents change and not otherwise. The root manifest is the busiest file in a workspace while rust-version barely moves. In microsoft/oxidizer the root manifest changed in 99 commits over six months while rust-version changed once, so hashing the file would rename the image, and oblige a publisher to rebuild and republish it, ~99 times for an image whose contents were identical every time.

Alternative rejected

Pass the value as ARG ANVIL_ROOT_MSRV. This is what the first revision of this PR did, and it works, but the channel is fragile in a way the composed Dockerfile makes likely. The setup region is a documented replacement point, so a repository substituting its own region keeps receiving the build argument while silently losing the declaration that receives it. The resulting failure is failed to install MSRV toolchain '1.92', which sends the reader hunting a toolchain that is not the problem. It also teaches the most general file in the tree, tools.just, about a context that only exists inside an image, and every future build-time fact would need the same plumbing again. Dropping the COPY by comparison fails with Cargo.toml not found at repository root, which names the missing thing.

Validation

scripts/test-anvil-dogfood.ps1 -Engine docker -Tier anvil-pr-fast -Clean, a cold build on Docker 29.7.1: 24/25 checks passed in 56 minutes, with anvil-pr-fast passing inside the image in 32:35.

The single failure is every pinned tool reports its version successfully, which is pre-existing and unrelated: the script probes cargo subcommands as bare binaries (cargo-hack --version), which they reject by design, and it fails identically on main for 8 of the 21 pinned tools.

Also green in that run: the regenerate precondition, all 21 tools present and executing, image reuse across runs, and the full identity suite including "editing tools.just renames the image" and "dropping a setup dependency renames the image".

Unit and integration: 521 tests pass, including 30 recipe contracts and all three backend snapshots. cargo fmt --check and cargo clippy --all-targets -- -D warnings are clean, and regeneration is idempotent.

WI: https://o365exchange.visualstudio.com/O365%20Core/_workitems/edit/7834621

`anvil-msrv-test-setup` resolves the MSRV by scanning the root `Cargo.toml`, and `just anvil-setup` reaches it through the PR tier. The container image is built with no repository source in its context by design, so that scan threw and the install layer failed. No image was produced, which left every `anvil-container` recipe unusable after a cold build. The MSRV is the one version anvil installs that is declared in the source rather than pinned in `versions.just`, and the image already carries the repository's other toolchain declaration, `rust-toolchain.toml`. Close that asymmetry by making the MSRV an image input: `anvil-container-tag` resolves the declared value through a new `_anvil-resolve-stable root-msrv` action and hashes it, the build passes it as `ARG ANVIL_ROOT_MSRV`, and the resolver reads it only when no root manifest is present, so it can never shadow a real declaration. The value is carried rather than the manifest holding it. Copying `Cargo.toml` into the context would rename the image on every dependency edit, obliging a publisher to rebuild and republish for changes that cannot alter a byte the image contains, while `rust-version` moves perhaps once. A repository that declares no MSRV sends `none`, which is an answer; an unset variable is not, so a build that drops the argument stops rather than producing an image silently missing a toolchain it claims to install. Validated with a cold image build on Docker 29.7.1: the build completes and `rustup toolchain list` in the resulting image reports the declared MSRV alongside the pinned stable and nightlies. Contract tests cover manifest precedence over the override, the `none` declaration, the refusal when nothing is declared, and that the tag follows the MSRV while ignoring unrelated manifest edits. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 80f12d44-5a83-4650-9806-d3e247175858

…ainer-msrv # Conflicts: # .anvil.lock

Codecov Comments Bot (codecov-commenter) · 2026-09-03T11:39:53Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.5%. Comparing base (f3c65bb) to head (966fa83).

❌ 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    #155   +/-   ##
=====================================
  Coverage   97.5%   97.5%           
=====================================
  Files        299     299           
  Lines      67766   67766           
=====================================
+ Hits       66127   66128    +1     
+ Misses      1639    1638    -1

Flag	Coverage Δ
linux	`97.5% <ø> (?)`
linux-arm	`97.5% <ø> (?)`
windows	`97.7% <ø> (?)`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

…ainer-msrv # Conflicts: # .anvil.lock

…old it The setup region installs the toolchain named by the repository's declared MSRV, which lives in the root `Cargo.toml` rather than in `versions.just`. That value reached the build as `ARG ANVIL_ROOT_MSRV`, resolved on the host and passed as a build argument, with the resolver reading the variable whenever it found no manifest. That channel is fragile in a way the composed Dockerfile makes likely. The setup region is a documented replacement point, and the `ARG` sat inside it, so a repository substituting its own region kept receiving the build argument while silently losing the declaration that receives it. The resulting failure is `failed to install MSRV toolchain '1.92'`, which sends the reader hunting a toolchain that is not the problem. Admit the root manifest to the build context and copy it to `/opt/anvil` instead. That directory is already the root the recipes resolve against: it is `justfile_directory()`, and it holds `justfiles/` and the toolchain pin. Copying the manifest alongside them completes it for the one question the setup asks, and the resolver reads it there exactly as it does on a developer's machine. `tools.just` therefore returns to what it was, minus the new `root-msrv` action the tag still needs. The variable, its `none` sentinel, its error message and the `--build-arg` all go away, and with them a general resolver's knowledge of containers. Dropping the `COPY` now fails with `Cargo.toml not found at repository root`, which names the missing thing. The workspace members the manifest lists stay out of the context: they are a checkout, and the image is not one. The one path that would need them, workspace MSRV validation, returns early whenever a root toolchain file selects the compiler, which this image requires and copies. The tag continues to hash the resolved value rather than the file. The manifest is the busiest file in a workspace while `rust-version` moves perhaps once in a repository's life, so hashing it would rename the image, and oblige a publisher to rebuild and republish, for a stream of edits that cannot alter a byte the image contains. Contracts follow the mechanism: the context admits the manifest, the setup region copies it, and no build argument survives anywhere. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2963084a-ef08-4cf7-adc6-e78e61556a84

The prose ran about four lines per line of code, well past what the change needs. Keep the reasons a reader cannot recover from the code -- why the value is hashed rather than the manifest, why the members stay out of the context, why the declared value rather than the mapped one -- and drop the restatement around them. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2963084a-ef08-4cf7-adc6-e78e61556a84

…n rests on Workspace MSRV validation reads every member manifest, and the build context carries none. It stays out of reach only because a root toolchain file selects the compiler and makes the resolver return early -- which holds today by accident, since the unconditional COPY means a repository without one cannot build an image at all. Making that COPY conditional is a reasonable fix for those repositories, and it would silently put the branch back in reach of a partial workspace. State the dependency so that change has to confront this one. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2963084a-ef08-4cf7-adc6-e78e61556a84

Copilot

🟡 Changes recommended

The container image now includes Cargo.toml but the tag identity does not hash the manifest bytes, which can allow tag reuse while the final image contents differ.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR fixes cargo-anvil container image builds that started failing after the MSRV check was introduced, by ensuring the container build context contains the root Cargo.toml needed to resolve the declared MSRV and by incorporating the declared MSRV value into the container image tag identity.

Changes:

Add a root-msrv action to _anvil-resolve-stable so the declared MSRV can be resolved as a total value (<version> or none) for tagging.
Update container tagging to hash the resolved root MSRV value (not the whole manifest) and update the container build inputs to copy the root Cargo.toml.
Add recipe contract tests and update snapshots/docs to lock in the intended container/MSRV behavior.

File summaries

File	Description
justfiles/anvil/tools.just	Adds `_anvil-resolve-stable root-msrv` action to report declared MSRV (or `none`).
crates/cargo-anvil/templates/justfiles/anvil/tools.just	Same `root-msrv` action change in the template source.
justfiles/anvil/container.just	Hashes resolved root MSRV value into the container image tag digest.
crates/cargo-anvil/templates/justfiles/anvil/container.just	Template source equivalent of the container tag MSRV hashing change.
crates/cargo-anvil/templates/anvil/container/Dockerfile.setup.region	Copies `Cargo.toml` into `/opt/anvil` during container setup.
crates/cargo-anvil/templates/anvil/container/Dockerfile.dockerignore	Admits `Cargo.toml` into the build context.
.anvil/container/Dockerfile	Local repo Dockerfile now copies `Cargo.toml` during setup.
.anvil/container/Dockerfile.dockerignore	Local repo dockerignore now admits `Cargo.toml`.
crates/cargo-anvil/tests/recipe_contracts.rs	Adds contract tests for `root-msrv` and container context/tag invariants; adds stubbing to keep container-tag tests focused.
crates/cargo-anvil/tests/snapshots/snapshots__local_only.snap	Updates snapshot expectations for Dockerfile/dockerignore/container tag changes.
crates/cargo-anvil/tests/snapshots/snapshots__github_backend.snap	Same snapshot updates for GitHub backend.
crates/cargo-anvil/tests/snapshots/snapshots__ado_backend.snap	Same snapshot updates for ADO backend.
crates/cargo-anvil/docs/implementation.md	Documents the new `root-msrv` action and why it must be total for tagging.
crates/cargo-anvil/docs/design/containers.md	Updates container design to include MSRV-based tagging and the `Cargo.toml` context requirement.
.anvil.lock	Updates checksums for the changed Anvil-managed inputs/regions.

Review details

Files reviewed: 14/15 changed files
Comments generated: 1
Review effort level: Lite

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

martinhavelka (wukchung)

🤖 Review of the MSRV/container change. Verified locally: template and generated copies are byte-identical, the three .anvil.lock checksums match recomputed SHA-256, the msrv digest record is unambiguously framed and always first, and the root-msrv branch runs before any rustup or network side effect. Two findings below, neither in the mechanism itself.

The tag hashes the declared MSRV rather than the manifest, so an edit that leaves rust-version alone computes the same tag. With the manifest left in place that tag named two different filesystems, and a reused or published image carried a Cargo.toml matching no checkout. Deleting it in the same RUN that reads it makes the identity exact again, and restores the design note's claim that those edits cannot alter a byte the image contains. Three statements in containers.md that still described the old three-path context are corrected alongside it, including the replacement rule a downstream catalog reads verbatim. Also drops an env_remove for ANVIL_ROOT_MSRV: nothing on this branch reads that variable, so the call could not affect what a fixture resolves and its comment described a mechanism that no longer exists. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2963084a-ef08-4cf7-adc6-e78e61556a84

Copilot

🟢 Approval recommended

The change set is consistent across templates + generated outputs, includes updated design/docs, and adds targeted contract coverage for the new MSRV/tag behavior.

Review details

Files reviewed: 14/15 changed files
Comments generated: 0 new
Review effort level: Lite

…ainer-msrv # Conflicts: # .anvil.lock

Copilot

🟡 Changes recommended

The new contract tests should be tightened so the MSRV resolver stub validates the requested action and the root-msrv test explicitly covers the [workspace.package] precedence path used by this repo.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Files reviewed: 14/15 changed files
Comments generated: 2
Review effort level: Lite

The container fixtures' stub answered any action, so a tag recipe that asked for 'msrv' -- which returns the mapped toolchain rather than the declared version -- would have digested the wrong value with every test still green. It now rejects anything but 'root-msrv', which also proves the real caller asks for it. The scanner's precedence half was untested: the fixture default declares only [package], while a workspace root declares both and workspace.package wins. That is the shape the image resolves against, so cover it directly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2963084a-ef08-4cf7-adc6-e78e61556a84

Copilot

🟢 Approval recommended

The fix is narrowly scoped, aligns with the PR description, and the only feedback is a minor test robustness improvement.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

crates/cargo-anvil/tests/recipe_contracts.rs:532

This assertion can be tripped by a future documentation/comment change in Dockerfile.dockerignore (e.g., mentioning !crates in a comment), even if the effective ignore patterns are unchanged. Consider filtering out comment lines before checking for disallowed include patterns so the test only asserts on the actual dockerignore rules.

Files reviewed: 14/15 changed files
Comments generated: 0 new
Review effort level: Lite

martinhavelka (wukchung) and others added 2 commits September 3, 2026 13:26

Merge remote-tracking branch 'origin/main' into u/mhavelka/anvil-cont…

64bcbb8

…ainer-msrv # Conflicts: # .anvil.lock

martinhavelka (wukchung) and others added 4 commits September 4, 2026 11:21

Merge remote-tracking branch 'origin/main' into u/mhavelka/anvil-cont…

a03afce

…ainer-msrv # Conflicts: # .anvil.lock

martinhavelka (wukchung) changed the title ~~fix(cargo-anvil): supply the declared MSRV to the container image build~~ fix(cargo-anvil): let the container image resolve the declared MSRV Sep 4, 2026

martinhavelka (wukchung) marked this pull request as ready for review September 4, 2026 10:36

Copilot AI lite review requested due to automatic review settings September 4, 2026 10:37

Copilot started reviewing on behalf of martinhavelka (wukchung) September 4, 2026 10:37 View session

Copilot AI reviewed Sep 4, 2026

View reviewed changes

Comment thread crates/cargo-anvil/templates/anvil/container/Dockerfile.setup.region

martinhavelka (wukchung) commented Sep 4, 2026

View reviewed changes

Comment thread crates/cargo-anvil/docs/design/containers.md

Comment thread crates/cargo-anvil/tests/recipe_contracts.rs Outdated

martinhavelka (wukchung) mentioned this pull request Sep 4, 2026

fix(cargo-anvil): let a repository with no root toolchain file build the container image #161

Draft

Copilot AI review requested due to automatic review settings September 4, 2026 11:40

Copilot started reviewing on behalf of martinhavelka (wukchung) September 4, 2026 11:41 View session

Copilot AI reviewed Sep 4, 2026

View reviewed changes

Evgenii (Vaiz) approved these changes Sep 4, 2026

View reviewed changes

martinhavelka (wukchung) enabled auto-merge (squash) September 4, 2026 13:21

Merge remote-tracking branch 'origin/main' into u/mhavelka/anvil-cont…

97f27a8

…ainer-msrv # Conflicts: # .anvil.lock

Copilot AI review requested due to automatic review settings September 4, 2026 13:52

Copilot started reviewing on behalf of martinhavelka (wukchung) September 4, 2026 13:52 View session

Copilot AI reviewed Sep 4, 2026

View reviewed changes

Comment thread crates/cargo-anvil/tests/recipe_contracts.rs

Comment thread crates/cargo-anvil/tests/recipe_contracts.rs

Evgeny Fedorov (evgenyfedorov2) approved these changes Sep 4, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings September 4, 2026 14:17

Copilot started reviewing on behalf of martinhavelka (wukchung) September 4, 2026 14:18 View session

Evgenii (Vaiz) approved these changes Sep 4, 2026

View reviewed changes

Copilot AI reviewed Sep 4, 2026

View reviewed changes

martinhavelka (wukchung) merged commit 245b029 into main Sep 4, 2026
50 checks passed

martinhavelka (wukchung) deleted the u/mhavelka/anvil-container-msrv branch September 4, 2026 15:26

Navigation Menu

Uh oh!

fix(cargo-anvil): let the container image resolve the declared MSRV - #155

fix(cargo-anvil): let the container image resolve the declared MSRV#155
martinhavelka (wukchung) merged 9 commits into
mainfrom
u/mhavelka/anvil-container-msrv

martinhavelka (wukchung) commented Sep 3, 2026 •

edited

Loading

Uh oh!

Codecov Comments Bot (codecov-commenter) commented Sep 3, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

martinhavelka (wukchung) left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Conversation

martinhavelka (wukchung) commented Sep 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

The fix

The tag hashes the value, not the file

Alternative rejected

Validation

Uh oh!

Codecov Comments Bot (codecov-commenter) commented Sep 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

martinhavelka (wukchung) left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Suppressed comments (1)

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

martinhavelka (wukchung) commented Sep 3, 2026 •

edited

Loading

Codecov Comments Bot (codecov-commenter) commented Sep 3, 2026 •

edited

Loading