fix(cargo-anvil): remove Bash from ADO steps - #160
Conversation
3aacc30 to
c3f5e4d
Compare
There was a problem hiding this comment.
🔵 Needs a closer look
It changes cross-platform CI execution semantics in generated ADO pipelines (Bash→pwsh), which merits a final human review/validation in an ADO environment.
Pull request overview
This PR updates cargo-anvil’s generated Azure DevOps (ADO) pipeline steps to run via PowerShell (pwsh) instead of Bash, ensuring the impact-mode (ANVIL_IMPACT) is passed via step environment, and refreshes the crate artifacts/docs for the 0.7.1 patch release.
Changes:
- Switch ADO templates from
bash:topwsh:and moveANVIL_IMPACTto the stepenv:mapping for group steps. - Add/adjust tests and snapshots to enforce “no Bash steps” in emitted ADO artifacts and reflect the new templates.
- Update versioned release outputs (README links, changelog entry, version bump, lockfiles,
.anvil.lock).
File summaries
| File | Description |
|---|---|
| crates/cargo-anvil/tests/snapshots/snapshots__ado_backend.snap | Snapshot updates reflecting pwsh ADO steps and env-based impact mode. |
| crates/cargo-anvil/templates/ado/steps/setup.yml | Replace Bash-based Just bootstrap and setup invocation with pwsh. |
| crates/cargo-anvil/templates/ado/steps/impact.yml | Run impact install/compute steps via pwsh rather than Bash. |
| crates/cargo-anvil/templates/ado/steps/group.yml | Run group just anvil-<group> via pwsh and thread ANVIL_IMPACT via env:. |
| crates/cargo-anvil/src/anvil/artifacts/ado.rs | Update assertions for pwsh templates and add a test preventing Bash steps in ADO artifacts. |
| crates/cargo-anvil/README.md | Regenerated README content reflecting the 0.7.1 docs.rs links/dependency info. |
| crates/cargo-anvil/docs/design/ado.md | Design doc updates describing pwsh usage and env-based ANVIL_IMPACT wiring. |
| crates/cargo-anvil/CHANGELOG.md | Add 0.7.1 release entry (and Unreleased header). |
| crates/cargo-anvil/Cargo.toml | Bump crate version to 0.7.1. |
| Cargo.lock | Lockfile update reflecting cargo-anvil 0.7.1. |
| .anvil.lock | Update anvil tool version and catalog checksum for 0.7.1. |
Review details
- Files reviewed: 6/6 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Use PowerShell for generated setup, impact, and group steps so Windows ADO agents do not require Git Bash. Preserve impact selection through the step environment and add a regression guard over emitted ADO artifacts. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ad83c731-5326-43e7-84fe-3aef46ef031f
c3f5e4d to
551dc68
Compare
There was a problem hiding this comment.
🟢 Approval recommended
The changes are coherent across templates, tests, snapshots, and design documentation, and no objective issues were identified in the updated artifacts.
Review details
- Files reviewed: 9/11 changed files
- Comments generated: 0 new
- Review effort level: Lite
There was a problem hiding this comment.
🟡 Changes recommended
The new pwsh: just … steps don’t consistently propagate native-command failures via $LASTEXITCODE, which can make pipeline steps succeed even when just fails.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 9/11 changed files
- Comments generated: 3
- Review effort level: Lite
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #160 +/- ##
=========================================
+ Coverage 97.5% 100.0% +2.4%
=========================================
Files 299 24 -275
Lines 67766 2072 -65694
=========================================
- Hits 66126 2072 -64054
+ Misses 1640 0 -1640
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:
|
| - bash: | | ||
| if ! command -v just >/dev/null 2>&1 ; then | ||
| - pwsh: | | ||
| $ErrorActionPreference = 'Stop' |
There was a problem hiding this comment.
In addition to $ErrorActionPreference = 'Stop' we should have the "treat exit code as error" flag (I forgot its name, NativeErrorPreference or something) set in every one of our pwsh snippets. We should also standardize that pattern somewhere (agents.md?).
There was a problem hiding this comment.
[copilot speaking] Agreed that a standardized PowerShell preamble may be useful repository-wide. It is not required for these steps, however: ADO's pwsh: task propagates the final native command's $LASTEXITCODE by default. Applying that convention to every PowerShell snippet would be broader than this Bash-removal change, so I suggest handling it separately.
|
[copilot speaking] End-to-end ADO validation will be performed downstream in O365 Core/ox-tools after it adopts cargo-anvil 0.7.1. That pipeline exercises these generated templates on the relevant 1ES Windows agents where Bash may be absent. This PR changes the interpreter used by the ADO command steps; it does not change pipeline topology or recipe selection. |
Use PowerShell for generated setup, impact, and group steps so Windows ADO agents do not require Git Bash. Preserve impact selection through the step environment and add a regression guard over emitted ADO artifacts. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ad83c731-5326-43e7-84fe-3aef46ef031f
551dc68 to
127ac64
Compare
|
[copilot speaking] The cargo-anvil release commit has been removed from this PR. Since this branch was created, #154 merged a breaking cargo-anvil change, so the next release must be 0.8.0 rather than 0.7.1. This PR is now limited to the ADO Bash-removal change; the combined release will be prepared separately after merge. |
There was a problem hiding this comment.
🟢 Approval recommended
The Bash-to-PowerShell migration is consistently applied across templates, validated by updated snapshots and a new “no Bash steps” regression test (only a minor doc wording nit remains).
Review details
- Files reviewed: 6/6 changed files
- Comments generated: 1
- Review effort level: Lite
There was a problem hiding this comment.
🟢 Approval recommended
The changes consistently replace Bash steps with pwsh across templates, docs, and snapshots, and include a targeted regression test to prevent Bash reintroduction.
Review details
- Files reviewed: 6/7 changed files
- Comments generated: 0 new
- Review effort level: Lite
There was a problem hiding this comment.
🟢 Approval recommended
Feedback is limited to minor documentation wording consistency; the behavioral/template changes and regression test coverage look coherent.
Review details
Suppressed comments (2)
crates/cargo-anvil/docs/design/ado.md:613
- This section still uses shell-style
ANVIL_IMPACT=consumenotation even though the templates now setANVIL_IMPACTvia the YAMLenv:mapping. Rewording to “setANVIL_IMPACTtoconsume…” avoids implying=syntax in ADO YAML.
3. **The group step template** runs `just anvil-<group>` with an impact mode fixed **by
group class at emit time** (never probed from a file). PR groups — which always download the
artifact — set `ANVIL_IMPACT=consume` in the step environment. In consume mode `anvil-impact` is a pure
no-op — it trusts the downloaded cache verbatim and **neither snapshots nor
crates/cargo-anvil/docs/design/ado.md:619
- Similarly here,
ANVIL_IMPACT=offuses shell-assignment notation inside prose while the emitted ADO YAML usesenv:mappings (ANVIL_IMPACT: "off"). Reword to describe the value rather than using=.
4. **Scheduled stages download nothing** and always validate the full workspace, so the
group step sets `ANVIL_IMPACT=off` in the step environment. Like the PR `consume`, this is fixed by group
class at emit time and is **not** derived from `target/anvil/impact/impact.state`: the
- Files reviewed: 6/7 changed files
- Comments generated: 0 new
- Review effort level: Lite
Use PowerShell for generated setup, impact, and group steps so Windows ADO agents do not require Git Bash. Preserve impact selection through the step environment and add a regression guard over emitted ADO artifacts. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ad83c731-5326-43e7-84fe-3aef46ef031f
dd38cd0 to
81c3624
Compare
There was a problem hiding this comment.
🟢 Approval recommended
The template changes are consistently applied across emitted artifacts, with updated snapshots/docs and added tests guarding against Bash reintroduction.
Review details
- Files reviewed: 6/7 changed files
- Comments generated: 0 new
- Review effort level: Lite
Use PowerShell for generated setup, impact, and group steps so Windows ADO agents do not require Git Bash. Preserve impact selection through the step environment and add a regression guard over emitted ADO artifacts. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ad83c731-5326-43e7-84fe-3aef46ef031f
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ad83c731-5326-43e7-84fe-3aef46ef031f
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ad83c731-5326-43e7-84fe-3aef46ef031f
81c3624 to
2e4841f
Compare
Summary
ANVIL_IMPACTthrough the group step environment and prevent Bash steps from returningValidation
cargo test -p cargo-anvil anvil::artifacts::ado::tests --lib --quietcargo test -p cargo-anvil --test snapshots ado_backend --quiet