Skip to content

fix(cargo-anvil): retry oversized Windows coverage exports - #166

Merged
martin-kolinek merged 19 commits into
mainfrom
fix/anvil-windows-coverage-sharding
Sep 8, 2026
Merged

fix(cargo-anvil): retry oversized Windows coverage exports#166
martin-kolinek merged 19 commits into
mainfrom
fix/anvil-windows-coverage-sharding

Conversation

@martin-kolinek

@martin-kolinek martin-kolinek commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator

🤖 Fixes cargo-anvil coverage reporting when a large Windows workspace causes llvm-cov export to exceed the process command-line limit.

  • Detects cargo-llvm-cov failures containing Windows OS error 206.
  • Parses the failed llvm-cov invocation and retries it through a UTF-8 response file.
  • Fails closed when the diagnostic is malformed, identifies another executable, or the retry fails.
  • Publishes the generated LCOV files directly from the ADO PR and scheduled pipelines instead of performing a second Cobertura export.

This change is intended for the cargo-anvil 0.9.0 release managed by #164; the package version and changelog remain owned by that release PR.

Validated with:

  • cargo test -p cargo-anvil --locked
  • cargo clippy -p cargo-anvil --all-targets --all-features -- -D warnings
  • just anvil-spellcheck
  • Native Windows Oxidizer just anvil-llvm-cov: 6,962 all-feature tests and 5,682 no-default-feature tests passed; the all-feature export hit OS error 206, retried through the response file, and passed all coverage gates.

Retry llvm-cov export through a response file when Windows rejects the cargo-llvm-cov command with OS error 206. Publish the resulting LCOV files directly in ADO and bump cargo-anvil to 0.8.1.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI lite review requested due to automatic review settings September 7, 2026 17:57

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The template updates are not reflected in the repo’s generated justfiles/anvil/checks/llvm-cov.just, so just anvil-llvm-cov in this repository will still run the old behavior unless the generated outputs are regenerated and committed.

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

Pull request overview

This PR updates cargo-anvil’s llvm-cov check to handle Windows command-line-length failures (OS error 206) by retrying the underlying llvm-cov export via an LLVM response file, and switches ADO coverage publishing to use the produced LCOV files directly.

Changes:

  • Add a PowerShell helper to detect (os error 206) from cargo llvm-cov report, parse the failed llvm-cov export invocation, and retry with a response file.
  • Publish coverage to ADO from lcov-*.info rather than generating/publishing Cobertura exports.
  • Update tests/snapshots and design docs to reflect the new behavior; bump cargo-anvil version to 0.8.1.
File summaries
File Description
crates/cargo-anvil/templates/justfiles/anvil/checks/llvm-cov.just Implements the Windows 206 retry via response file and removes Cobertura reporting.
crates/cargo-anvil/tests/recipe_contracts.rs Adds a Windows-only contract test exercising the response-file retry path.
crates/cargo-anvil/tests/snapshots/snapshots__local_only.snap Updates rendered recipe snapshot to include the new helper + LCOV-only flow.
crates/cargo-anvil/tests/snapshots/snapshots__github_backend.snap Same snapshot update for the GitHub backend rendering.
crates/cargo-anvil/tests/snapshots/snapshots__ado_backend.snap Updates ADO backend snapshot (both recipe + pipeline wiring changes).
crates/cargo-anvil/templates/ado/pr-stages.yml Switches ADO PR stages coverage publish glob from Cobertura XML to LCOV info.
crates/cargo-anvil/templates/ado/scheduled-stages.yml Switches scheduled pipeline coverage publish glob from Cobertura XML to LCOV info.
crates/cargo-anvil/src/anvil/artifacts/ado.rs Updates template-contract assertions to require LCOV publish wiring and forbid Cobertura.
crates/cargo-anvil/docs/design/checks.md Documents the Windows 206 retry behavior and LCOV-only outputs.
crates/cargo-anvil/docs/design/ado.md Updates ADO publishing guidance to use LCOV directly.
crates/cargo-anvil/README.md Refreshes generated links/metadata for the 0.8.1 version reference.
crates/cargo-anvil/Cargo.toml Bumps crate version to 0.8.1.
Cargo.lock Updates the workspace lockfile for the 0.8.1 bump.
Review details
  • Files reviewed: 12/13 changed files
  • Comments generated: 1
  • Review effort level: Lite

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

Comment thread crates/cargo-anvil/templates/justfiles/anvil/checks/llvm-cov.just
Regenerate the in-tree Anvil manifest and llvm-cov recipe from cargo-anvil 0.8.1.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings September 7, 2026 18:03

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The new Invoke-AnvilLcovReport implementation buffers and replays cargo llvm-cov report output, which removes streaming logs and can create “idle” periods in CI; streaming while capturing (e.g., Tee-Object) would avoid that operational risk.

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

Review details
  • Files reviewed: 13/15 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread crates/cargo-anvil/templates/justfiles/anvil/checks/llvm-cov.just Outdated
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings September 7, 2026 18:11
Tee cargo-llvm-cov output to the console while retaining it for the Windows error 206 response-file fallback.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The Windows response-file retry path can leave an empty/partial LCOV file on failure due to stdout redirection, which may be unintentionally published downstream.

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

Review details
  • Files reviewed: 13/15 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread crates/cargo-anvil/templates/justfiles/anvil/checks/llvm-cov.just Outdated
Copilot AI review requested due to automatic review settings September 7, 2026 18:15
Delete partial LCOV output when the Windows response-file retry fails, and cover cleanup of reports and temporary files in the recipe contract.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The new PowerShell helper temporarily changes $ErrorActionPreference without try/finally restoration in two places, which can leak error-handling state on terminating errors.

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

Review details

Suppressed comments (1)

crates/cargo-anvil/templates/justfiles/anvil/checks/llvm-cov.just:162

  • The retry path also toggles $ErrorActionPreference without a try/finally. If invoking llvm-cov export throws a terminating error (e.g., resolution issues), $ErrorActionPreference may not be restored, which can change later error semantics in the recipe.
            $previousErrorActionPreference = $ErrorActionPreference
            $ErrorActionPreference = 'Continue'
            & $llvmCov export "@$responsePath" > $OutputPath 2> $retryErrorPath
            $retryExitCode = $LASTEXITCODE
            $ErrorActionPreference = $previousErrorActionPreference
  • Files reviewed: 13/15 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread crates/cargo-anvil/templates/justfiles/anvil/checks/llvm-cov.just Outdated
Copilot AI review requested due to automatic review settings September 7, 2026 18:20

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The changes are cohesive, include targeted Windows-specific contract coverage for the retry behavior, and consistently update the templates, generated outputs, and documentation to match the new LCOV-only publishing flow.

Review details
  • Files reviewed: 13/15 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Restore ErrorActionPreference in finally blocks around both native coverage invocations.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings September 7, 2026 18:28

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

The Windows 206 retry path uses a single-line parse for the extracted llvm-cov export command that can fail if the diagnostic ever wraps the command across lines, which would silently disable the intended fallback.

Review details

Suppressed comments (1)

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

crates/cargo-anvil/templates/justfiles/anvil/checks/llvm-cov.just:137

  • $failedProcess uses a single-line regex ((?s)) so the captured command may legally contain newlines, but the follow-up $command regex is not single-line. If cargo-llvm-cov ever line-wraps the huge llvm-cov export ... invocation (plausible when hitting Windows error 206), the match will fail and the response-file retry will never run. Consider making the $command match single-line as well so arguments can include newlines/whitespace wrapping while still enforcing the ... export <args> shape.
  • Files reviewed: 13/15 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Allow newlines in cargo-llvm-cov diagnostic arguments and exercise a wrapped object argument in the Windows retry contract.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings September 7, 2026 18:40

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

The new PowerShell helper can return an unintended $null exit code on early/terminating errors due to an uninitialized $reportExitCode, which should be made deterministic.

Review details

Suppressed comments (1)

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

crates/cargo-anvil/templates/justfiles/anvil/checks/llvm-cov.just:116

  • $reportExitCode is only assigned after the cargo llvm-cov report pipeline runs. If a terminating error occurs before that assignment, $reportExitCode stays $null, and the function can end up returning $null (which the caller later compares / exits with), potentially masking the failure as success or exiting with an unintended code. Initializing the variable up-front makes failure behavior deterministic.
  • Files reviewed: 13/15 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Default both native coverage invocation results to failure so exceptional paths cannot yield a null success-like result.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings September 7, 2026 18:47
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings September 8, 2026 09:46
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The changes are well-scoped, include explicit Windows-focused contract coverage for the new retry path, and keep templates/docs/snapshots consistent with the updated LCOV-only pipeline behavior.

Review details
  • Files reviewed: 13/15 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings September 8, 2026 10:54

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The new Windows response-file helper has a duplicated $responsePath assignment that should be removed and then propagated via regeneration so templates and rendered outputs remain consistent.

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

Review details
  • Files reviewed: 13/15 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread crates/cargo-anvil/templates/justfiles/anvil/checks/llvm-cov.just Outdated
Comment thread crates/cargo-anvil/Cargo.toml Outdated
@martin-kolinek
martin-kolinek enabled auto-merge (squash) September 8, 2026 12:05
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings September 8, 2026 14:08
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

There’s a confirmed copy/paste bug in the new response-file retry helper (duplicate $responsePath assignment) that should be corrected and then propagated to the generated recipe/snapshots.

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

Review details
  • Files reviewed: 13/15 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread crates/cargo-anvil/templates/justfiles/anvil/checks/llvm-cov.just Outdated
Copilot AI review requested due to automatic review settings September 8, 2026 14:12

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

It changes both the Windows coverage execution path and the ADO publishing format, so it warrants a final human verification of CI/pipeline behavior.

Review details
  • Files reviewed: 13/15 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings September 8, 2026 14:26

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The change is narrowly scoped, includes targeted Windows contract coverage for the retry path, and updates templates/snapshots/docs consistently with the new LCOV-only publishing flow.

Review details
  • Files reviewed: 11/12 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

@martin-kolinek
martin-kolinek merged commit 6615afc into main Sep 8, 2026
29 checks passed
@martin-kolinek
martin-kolinek deleted the fix/anvil-windows-coverage-sharding branch September 8, 2026 15:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants