Repository navigation
chore(anvil): adopt cargo-anvil 0.9.0 - #744
Merged
Merged
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #744 +/- ##
=======================================
Coverage 100.0% 100.0%
=======================================
Files 583 583
Lines 62930 62930
=======================================
Hits 62930 62930
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:
|
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The Anvil container Dockerfile currently COPYs a rust-toolchain.toml that is not present in this repository, which will break container builds.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Updates this repository’s generated cargo-anvil integration to the latest microsoft/ox-tools main catalog (commit 6615afcf), refreshing local Just recipes, container behavior, and the GitHub Actions Anvil PR workflow while keeping legacy checks for now.
Changes:
- Regenerate Anvil Just recipes (new dev build recipe; updated Miri, coverage, formatting, examples, doc-build, MSRV handling).
- Refresh Anvil container tagging/build inputs and contributor-facing instructions/skills.
- Consolidate PR + merge-queue validation into a single Anvil workflow caller and add a fan-in “Required Anvil checks” job.
File summaries
| File | Description |
|---|---|
| justfiles/anvil/versions.just | Bumps pinned tool versions (coverage-gate, heather) and keeps toolchain arg bootstrap logic. |
| justfiles/anvil/tools.just | Enhances stable/MSRV resolution and tool/component validation diagnostics. |
| justfiles/anvil/mod.just | Imports new generated dev build recipes into the Anvil module. |
| justfiles/anvil/dev/build.just | Adds anvil-build dev helper recipe plus setup/validate prereqs. |
| justfiles/anvil/container.just | Updates container tag hashing inputs (adds root MSRV into the tag stream) and env forwarding. |
| justfiles/anvil/checks/readme-check.just | Adds anvil-readme --fix wrapper for regeneration. |
| justfiles/anvil/checks/msrv-test.just | Prevents rustup auto-install during MSRV toolchain validation and improves error reporting. |
| justfiles/anvil/checks/miri.just | Reworks Miri execution to compile once and run test executables in parallel with profile variants. |
| justfiles/anvil/checks/miri-tree-borrows.just | Routes profile-specific Miri runs through the shared helper. |
| justfiles/anvil/checks/miri-strict-provenance.just | Routes profile-specific Miri runs through the shared helper. |
| justfiles/anvil/checks/miri-race-coverage.just | Routes profile-specific Miri runs through the shared helper. |
| justfiles/anvil/checks/llvm-cov.just | Adds Windows cmdline-limit workaround via response-file retry for lcov export. |
| justfiles/anvil/checks/fmt.just | Adds --fix support and switches to cargo-each driven formatting. |
| justfiles/anvil/checks/examples.just | Adds ability to run examples with timeouts and metadata-based filtering. |
| justfiles/anvil/checks/doc-build.just | Adds --open option for local doc viewing. |
| .github/workflows/anvil-pr.yml | Unifies PR + merge-group into one caller job with event-dependent inputs. |
| .github/workflows/anvil-pr-impl.yml | Adds required-checks fan-in job and updates PR-comment guard wording. |
| .github/skills/cargo-anvil-adoption/SKILL.md | Introduces generated adoption guidance skill content. |
| .github/instructions/cargo-anvil.instructions.md | Adds generated contributor instructions for Anvil verification flow. |
| .github/actions/anvil-setup/action.yml | Enforces minimum just version with PowerShell-based install/validation. |
| .delta.toml | Updates delta trip-wire patterns for Anvil-managed regions. |
| .anvil/container/Dockerfile.dockerignore | Admits root Cargo.toml into container build context for MSRV resolution. |
| .anvil/container/Dockerfile | Copies root manifest for MSRV/toolchain resolution during image setup and removes it afterward. |
| .anvil.lock | Updates catalog/tool lock metadata and checksums for regenerated files/regions. |
Review details
- Files reviewed: 23/24 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
martin-kolinek
changed the title
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The new required-checks fan-in job treats skipped dependencies as failures, which can incorrectly fail the required workflow context when a job family self-skips.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 24/25 changed files
- Comments generated: 2
- Review effort level: Lite
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
It introduces at least one concrete CI/impact-analysis correctness risk (constants.env removed from .delta.toml trip-wire patterns) plus a least-privilege concern in the unified merge_group caller permissions.
Review details
Suppressed comments (2)
Previously missed (1) — in code that hasn't changed since the last review.
.delta.toml:16
constants.envis a repo-wide build/CI input (the rootjustfilerequires it viadotenv-required, and multiple scripts/actions read it), but it was removed fromtrip_wire_patterns. Ifconstants.envchanges, cargo-delta impact analysis may under-scope and incorrectly skip checks/packages that are sensitive to toolchain/tool version changes.
Consider re-adding constants.env to trip_wire_patterns so any change conservatively marks the whole workspace affected.
.github/workflows/anvil-pr.yml:37
- This caller job now handles both
pull_requestandmerge_groupevents, but it unconditionally grants write scopes (statuses: writeandpull-requests: write). That means merge-queue validations run PR code with a write-capableGITHUB_TOKEN, even though the merge-group path doesn’t need to publish commit statuses or PR comments.
To keep least-privilege, consider restoring separate event-specific caller jobs (as before) or otherwise ensuring merge-group runs only with contents: read while preserving write scopes for same-repo pull_request runs.
statuses: write
# Write needed so the pr-fast job can upsert/clear the sticky PR
# comment carrying the cargo-semver-checks advisory (and any
# future advisory checks that emit target/anvil/comments/*).
pull-requests: write
- Files reviewed: 24/25 changed files
- Comments generated: 0 new
- Review effort level: Lite
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
.delta.toml dropping constants.env from trip-wire inputs can under-scope cargo-delta impact analysis even though constants.env is a workspace-wide CI/build input.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
.delta.toml:17
.delta.tomlno longer treatsconstants.envas a trip-wire, but this repo usesconstants.envas a workspace-wide build input (e.g., the rootjustfilerequires it viadotenv-path, and CI setup actions load toolchain/tool versions from it). Ifconstants.envchanges without being a trip-wire, cargo-delta impact scoping can under-scope checks even though toolchains/tools have changed for the whole workspace.
- Files reviewed: 24/25 changed files
- Comments generated: 0 new
- Review effort level: Lite
Evgenii (Vaiz)
approved these changes
Sep 10, 2026
martintmk
approved these changes
Sep 10, 2026
The generated container setup still expects this file, but the container is not used yet and will be fixed independently. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Evgenii (Vaiz)
approved these changes
Sep 10, 2026
martin-kolinek
enabled auto-merge (squash)
September 10, 2026 07:41
martintmk
approved these changes
Sep 10, 2026
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
The container/tagging recipes still hard-require a missing rust-toolchain.toml, and .delta.toml drops constants.env from trip-wire inputs despite repo-wide dependency on it.
Review details
Suppressed comments (4)
Previously missed (3) — in code that hasn't changed since the last review.
.delta.toml:16
- Removing
constants.envfromtrip_wire_patternscan under-scope impact analysis: the rootjustfilerequiresconstants.envfor repo-wide toolchain/tool version constants (set dotenv-path := "./constants.env"). Changes toconstants.envtherefore affect the entire workspace execution environment and should remain a conservative trip-wire input.
justfiles/anvil/container.just:174 anvil-container-tagstill treatsrust-toolchain.tomlas a declared input (it’s included in$inputsand the reparse-point guard), but the repository root currently has norust-toolchain*file. If the container path is intentionally toolchain-file-free for now, this recipe should not hard-require that file (otherwiseanvil-container-tagcan’t compute a tag from a clean checkout).
.anvil/container/Dockerfile.dockerignore:42- The container build context still admits
rust-toolchain.toml, but the repo root has norust-toolchain*file. If the container is meant to be buildable without a root toolchain selector (as noted in the PR description), droprust-toolchain.tomlfrom the Dockerfile-specific ignore allowlist.
.anvil/container/Dockerfile:108
- This container Dockerfile still
COPYsrust-toolchain.toml, but the repository root currently has norust-toolchain*file (so the container inputs are inconsistent with the checkout). Either add/restore a root toolchain file, or remove the dependency from the container build inputs (Dockerfile + ignore + tag recipe).
COPY rust-toolchain.toml ./
- Files reviewed: 23/24 changed files
- Comments generated: 0 new
- Review effort level: Lite
Record Anvil lint regions for packages added on main. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
The Anvil container Dockerfile still COPYs a root rust-toolchain.toml that does not exist in the repository, making container builds fail.
Review details
Suppressed comments (1)
.anvil/container/Dockerfile:110
.anvil/container/Dockerfilecopiesrust-toolchain.toml, but this PR does not add one and the repository currently has norust-toolchain*at the root. As a result, building the Anvil exec image will fail atCOPY rust-toolchain.toml.
Since the stable toolchain resolver already falls back to the root Cargo.toml rust-version when no toolchain file is present (see justfiles/anvil/versions.just _anvil_stable_toolchain_args), the container setup should not require copying a missing toolchain file. Remove the COPY rust-toolchain.toml line and update the adjacent comment so it no longer implies the file exists.
WORKDIR /opt/anvil
COPY justfiles ./justfiles
COPY rust-toolchain.toml ./
COPY Cargo.toml ./
RUN printf "import 'justfiles/anvil/mod.just'\n" > Justfile \
- Files reviewed: 23/24 changed files
- Comments generated: 0 new
- Review effort level: Lite
martintmk
approved these changes
Sep 10, 2026
This was referenced Sep 10, 2026
Zac Walker (ZacWalk)
added a commit
that referenced
this pull request
Sep 11, 2026
…#748) ## Root cause `OxidizerGitHub.PublishEachCommit` is red on `main`. The `Cargo test` step fails in the **release** profile on both Linux and Windows (dev passes), with one failing test out of ~7,700: ``` FAIL rallocator::telemetry snapshot_encodes_allocations_by_stack assertion `left == right` failed left: 1 right: 4 ``` The last green and the first red run used the same toolchain (`1.97.1-ms-20260901.3`), so this is a code regression, not a tooling migration: | Build | Commit | Result | | --- | --- | --- | | 20260910.1 | 70bd5aa (#744) | succeeded | | 20260910.2 | 8624f90 (#733) | **first failing build** | | 20260910.3 | 03e3dfe (#722) | failed, unrelated to the triggering commit | `snapshot_encodes_allocations_by_stack` allocates in a loop with a compile-time trip count and asserts that all four samples share one call stack. Call stacks are recorded as raw return addresses in `capture_platform_stack`, and the release profile fully unrolls that loop, so each sample allocates from its own call site. Instrumenting the test confirms all four allocations are recorded, but their stacks differ in exactly one frame - the return address inside the test function, stepping by a fixed instruction offset per iteration: ``` stack = [140702813012151, 140702812954602, 140702813025904, ...] stack = [140702813012151, 140702812954684, 140702813025904, ...] stack = [140702813012151, 140702812954766, 140702813025904, ...] stack = [140702813012151, 140702812954848, 140702813025904, ...] ``` The grouping filter therefore matches 1 event instead of 4. The allocations are not elided, they are split across four stack buckets. #733 reworked the allocation path and changed inlining enough to tip this loop into being unrolled; the test has never been protected against release codegen. This is the same class of fragility as #666 (`fix(rallocator): preserve allocation effects in release tests`), where LLVM elided unprotected test allocations. ## Fix Route every sample through a single `#[inline(never)]` helper and make the loop bound opaque, so all samples allocate from one instruction and record one shared call stack. `black_box` alone would not fix this, since the allocations do occur - the call site itself has to stay singular. ## Validation Against the exact CI toolchain (`ms-prod-1.97@llvm`, `1.97.1-ms-20260901.3`): - `cargo test --release --all-features -p rallocator --test telemetry` - fails on `main` before the change, 19/19 pass after it - `cargo test --all-features -p rallocator --test telemetry` (dev profile) - 19/19 pass - `cargo clippy --release --all-features -p rallocator --all-targets -- -D warnings` - clean - `cargo +nightly-2026-05-30 fmt -p rallocator -- --check` - clean - `just anvil-spellcheck` - clean Also reproduced and verified on 1.96.1; the failure does not reproduce on 1.95.0. Bug: https://o365exchange.visualstudio.com/O365%20Core/_workitems/edit/7866558 Failing build: https://dev.azure.com/O365Exchange/O365%20Core/_build/results?buildId=40955158&view=results First failing build: https://dev.azure.com/O365Exchange/O365%20Core/_build/results?buildId=40907883&view=results Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Zac Walker (ZacWalk)
added a commit
that referenced
this pull request
Sep 11, 2026
## Summary Prepare 58 coordinated crate releases, including the maintainer's macro-pinning and version-alignment commits. Initially rebased onto `8624f907` on main, including #744 and the merged #733 runtime-recording work. Subsequently merged main at `704eba82` in `b1b8dc17`, including #722 (compressors) and #745 (metabench support identifiers), without rewriting the PR history. All 79 pre-merge package versions are preserved; the new compressors package retains upstream `0.1.0`. No new release versions were chosen during the merge. - Release `thread_aware 0.12.0` and affected consumers for `thread_aware_core` adoption; retain `thread_aware_core 0.1.1` as a compatible addition. - Release `rest_over_grpc 0.2.1`, including the query-field nesting-depth fix from #729. - Preserve the observed family at `0.26.0`, fundle at `0.4.0`, and data-privacy facade/macros at `0.13.0`. - Preserve the later maintainer updates: exact macro pins; the ohno family at `0.5.2`; internity and macros at `0.2.1`; metabench and macros at `0.1.1`; multitude and macros at `0.10.0`; and templated-uri and macros at `0.6.0`. - Retain `data_privacy_core 0.1.2`, `plurality 0.2.2`, and `recoverable 0.2.0` without release bumps. - Preserve the reviewed release-note cleanup: remove CI-only and misattributed feature notes, restore preambles, normalize new categories, preserve historical entries, and describe #729/#730 once each. Retain the verified `http_path_template 0.2.0` publication date, 2026-07-22. - Address remaining feedback in `18de2ac3`: correct the uniflight `stampede` keyword and qualify optional dependency notes by their features in rest_over_grpc and seatbelt_http. - Reconcile registry lockfile resolution in `f3d71c7e`: restore seven `windows-sys` references and the related `tempfile`/`getrandom` reference to main, without changing any package versions. Relative to current main, this is release metadata and generated documentation, plus the bytesbuf rustdoc URL correction. The incoming implementation changes from #733, #722, and #745 come from main and were preserved. The root manifest conflict was resolved by retaining the release's fetch package versions and main's new flate2 dependency. ## Required Release Review The author confirmed on 2026-09-10 that `performables` and `seismograph_cli` must remain at upstream `0.1.0`; do not add release bumps for them. This is an intentional exception to the release planner's normal dependent cascade. Their initial-publication handling and dependency closure are specified below; keeping these versions does not mean that their publication can be skipped. The initial review compared 46 ordinary libraries against previous version-bump commits with `cargo semver-checks --all-features --release-type patch`. Manual evidence also covered the public `ThreadAware` identity migration, public Syn 2-to-3 helper types, the observed mutable-reference macro input restriction, and ohno's warning-policy caveat. The pre-merge SemVer report identifies 10 proc-macro packages and 26 ordinary consumers requiring manual compatibility review, with no reported under-increment. Comparing the pre-rebase head `826feb24` with `65cc1af8` shows no changes in the macro packages or `routerama_build`, so the earlier macro-source evidence survives that rebase. The subsequent merge brings in #745's metabench macro support-identifier fix unchanged from main; its unit, expansion, and compile-contract tests passed against the release versions. Prior evidence does not establish compatibility of the consumer changes introduced by #733 or replace the fresh merged-head report. Final release review must confirm the macro/consumer classifications, particularly the patch boundaries of `internity 0.2.1` and `arty_executor 0.1.2`, routerama parser/macro compatibility, ohno's generated-code warning policy, and the post-#733 public-type/runtime implications. The automated SemVer warning is informational; passing builds and resolved review threads do not replace this release signoff. ## Post-rebase Validation - Passed at `65cc1af8`: locked workspace `cargo check --all-features`. - Passed at `65cc1af8`: `just clippy` for the workspace, all features and targets. - Passed at `65cc1af8`: 811 selected tests across rallocator, thread-aware, REST-over-gRPC, internity, and metabench packages (43 binaries). - Passed at `65cc1af8`: pinned formatting checks across all 79 workspace packages, README generation, whitespace checks, and editor diagnostics. - Passed: comparison of surviving package versions against the exact pre-rebase head `826feb24`; all 72 match. Current exact/caret workspace dependency requirements match target versions. - Confirmed: the old `thread_heap_usage_includes_remote_allocations` test is absent after #733. No test was disabled or assertion weakened in this release branch. - The `65cc1af8` CI run completed with 58 successful checks and one skipped check. - Passed for the feedback fix `18de2ac3`: locked Cargo metadata verification, `just anvil-cargo-sort`, `just anvil-clippy`, `just anvil-fmt --fix`, `just anvil-readme --fix`, `just anvil-spellcheck`, whitespace checks, and editor diagnostics. Regeneration produced no additional source/README changes. The local spelling-tool prerequisite is now installed. - Passed for the lockfile fix `f3d71c7e`: full locked/offline Cargo metadata, `just anvil-clippy` (all targets/features), `just anvil-fmt --fix`, `just anvil-readme --fix`, `just anvil-spellcheck`, whitespace checks, and editor diagnostics. All 568 registry-package lock entries exactly match main; all 79 non-registry entries remain unchanged from `18de2ac3`. `ring` retains its required `windows-sys 0.52.0` dependency. - Fresh CI for the latest head must finish successfully; results from preceding commits are not a substitute. Before the rebase, selected runs passed 2,691 tests for release consumers and 969 tests including thread-aware and ohno. Those results are historical. ## Main-merge Validation For `b1b8dc17`, merging main at `704eba82`: - Passed: full locked/offline Cargo metadata for 80 workspace packages. All 79 prior manifest versions are unchanged; compressors remains `0.1.0`. - Passed: all 576 registry-package lock entries exactly match incoming main. The Windows dependency-resolution correction is preserved. - Passed: 553 unit/integration tests across compressors, metabench, metabench_macros, and metabench_macros_impl; none skipped. - Passed: 26 doctests across those packages, with seven existing ignored examples. - Passed: generated Anvil workspace Clippy (all targets/features), manifest sorting, formatting, README regeneration, spelling, whitespace checks, and editor diagnostics. - Regenerated the compressors README to link to the release's bytesbuf `0.10.0`; incoming compressors and metabench macro implementation code matches main. - Confirmed: the release selector still selects 58 bumped packages against current main. No release versions were added or changed. - GitHub reports the conflict resolved. Fresh CI and approval of the latest reviewable push are required; prior checks do not validate this merge. ## Publication This PR prepares releases only. No crates have been published or yanked as part of this work. Registry checks on 2026-09-10 found all seven new #733 packages unpublished: - `performables` - `seismograph` - `seismograph_protocol` - `seismograph_io` - `seismograph_cli` - `seismograph_runtime` - `seismograph_rallocator` Preserve their initial versions. The normal comparison against current main selects the 58 version-bumped packages and excludes these seven initial packages and the newly merged compressors `0.1.0`. The existing 58 releases require `seismograph_protocol`, `seismograph`, `performables`, and `seismograph_io`; shipping the CLI additionally requires the runtime/rallocator helpers. Publish prerequisites before their consumers in Cargo dependency order. The compressors registry/publication status was not checked in this merge update. Before merging, the publishing maintainer must confirm one of these paths: 1. Complete and verify the #733 initial publications, then publish the normal 58-package release set; or 2. Explicitly include the required initial packages in a catch-up publication together with the 58 bumped packages. After merging #722, the repository's `Get-PackagesWithVersionChanges` helper selects exactly 66 packages when compared against pre-#733 commit `70bd5aafadb9b11fc45d8977bd808066e574b8de`: the 58 bumped packages, the seven #733 packages, and compressors. Confirm actual registry availability, equivalent selection, and dependency ordering in the publishing pipeline before execution; the earlier 65-package count applied before compressors joined main. `.ado/pipelines/publish.yml` exposes `previousVersion` (default `HEAD^`) but delegates publishing to the external `ox-sdk` template. Its catch-up behavior has not been verified or executed here. The official pipeline triggers on main and defaults `executePublish` to true, so publication selection must be confirmed before merge, not deferred until after it. Do not merge until required checks, final release review, and the initial-publication plan are confirmed. Do not rerun the release driver on the already-bumped branch merely to include these initial packages; that would re-increment existing releases rather than preserve the approved version plan. ## Tooling Follow-up The release generator inserts sections ahead of preambles, combines curated headings with list-style categories, and uses tag-based history that can lag declared releases. The release artifacts have been repaired; generator behavior and regression tests remain separate follow-up work. --------- Co-authored-by: Vaiz <4908982+Vaiz@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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.
🤖 Updates the repository's cargo-anvil integration to the released
0.9.0catalog. Existing legacy checks remain intact for now.The released package has the same catalog checksum (
baa2a5e4...) as the previously adoptedmainbuild, so the 0.9.0 rerun changes only the recorded tool version in.anvil.lock; generated recipes and workflows are unchanged.Changes
0.9.0The generated container currently expects a root
rust-toolchain.toml, which this repository intentionally does not provide. The container path is not yet used and will be fixed independently rather than pinning normal repository commands to the MSRV.Local validation
just anvil-pr-fastjust anvil-pr-slow(same catalog checksum)cargo anvil --dry-runcargo metadata --no-deps --format-version 1 --lockedjust anvil-container-tag(before removing the temporary toolchain selector)