diff --git a/.github/workflows/build-and-package.yml b/.github/workflows/build-and-package.yml index edce86f35..a26537fe6 100644 --- a/.github/workflows/build-and-package.yml +++ b/.github/workflows/build-and-package.yml @@ -67,11 +67,6 @@ jobs: BIN_NAME: ${{ inputs['bin-name'] }} VERSION: ${{ inputs.version }} MAN_ARCH: ${{ inputs['package-arch'] != '' && inputs['package-arch'] || 'unknown' }} - # Pre-set RUSTFLAGS so the setup-rust-toolchain step nested inside - # rust-build-release does not export its "-D warnings" default, which - # would shadow .cargo/config.toml and strip -Zpolonius=next (see - # docs/adr-006-adopt-polonius-nightly-toolchain.md). - RUSTFLAGS: -Zpolonius=next steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 @@ -99,6 +94,9 @@ jobs: bin-name: ${{ env.BIN_NAME }} project-dir: . manifest-path: Cargo.toml + # Preserve the Polonius requirement through nested toolchain setup + # (see docs/adr-006-adopt-polonius-nightly-toolchain.md). + rustflags: -Zpolonius=next skip-man-page-discovery: 'true' - name: Generate release help diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 38018b3e7..e700f0232 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -17,10 +17,6 @@ jobs: # docs/adr-006-adopt-polonius-nightly-toolchain.md), so CI builds with # the dated nightly pinned in rust-toolchain.toml. NETSUKE_RUST_TOOLCHAIN: nightly-2026-06-25 - # Pre-set RUSTFLAGS so setup-rust-toolchain's "-D warnings" default does - # not shadow .cargo/config.toml and strip -Zpolonius=next; tools such as - # cargo-llvm-cov append their own flags to this value. - RUSTFLAGS: -D warnings -Zpolonius=next WHITAKER_INSTALLER_VERSION: '0.2.7' # Single source of truth for the cargo-nextest pin. `make test` runs the # non-doctest suite through nextest, so the job installs it up front. @@ -53,6 +49,8 @@ jobs: with: toolchain: ${{ env.NETSUKE_RUST_TOOLCHAIN }} components: rustfmt, clippy + # Preserve warnings-as-errors and Polonius through toolchain setup. + rustflags: -D warnings -Zpolonius=next - name: Install cargo-nextest uses: taiki-e/install-action@18b1216eba7f8039b0f8d131d5473787f0edce68 # v2.85.3 with: diff --git a/.github/workflows/coverage-main.yml b/.github/workflows/coverage-main.yml index ac8ad60f9..e5314fb36 100644 --- a/.github/workflows/coverage-main.yml +++ b/.github/workflows/coverage-main.yml @@ -19,10 +19,6 @@ jobs: env: CARGO_TERM_COLOR: always BUILD_PROFILE: debug - # Pre-set RUSTFLAGS so setup-rust-toolchain's "-D warnings" default does - # not shadow .cargo/config.toml and strip -Zpolonius=next; cargo-llvm-cov - # appends its instrumentation flags to this value. - RUSTFLAGS: -D warnings -Zpolonius=next steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: @@ -34,6 +30,9 @@ jobs: # docs/adr-006-adopt-polonius-nightly-toolchain.md). toolchain: nightly-2026-06-25 components: rustfmt, clippy + # Preserve warnings-as-errors and Polonius through toolchain setup; + # cargo-llvm-cov appends its instrumentation flags to this value. + rustflags: -D warnings -Zpolonius=next - name: Test and Measure Coverage uses: leynos/shared-actions/.github/actions/generate-coverage@2f90d1041ea108148be0620e3bbcc1fa80ac03e4 with: diff --git a/.github/workflows/netsukefile-test.yml b/.github/workflows/netsukefile-test.yml index 439340c80..4ec983608 100644 --- a/.github/workflows/netsukefile-test.yml +++ b/.github/workflows/netsukefile-test.yml @@ -15,9 +15,6 @@ jobs: # Match rust-toolchain.toml: the tree needs -Zpolonius=next (see # docs/adr-006-adopt-polonius-nightly-toolchain.md). NETSUKE_RUST_TOOLCHAIN: nightly-2026-06-25 - # Pre-set RUSTFLAGS so setup-rust-toolchain's "-D warnings" default does - # not shadow .cargo/config.toml and strip -Zpolonius=next. - RUSTFLAGS: -Zpolonius=next steps: - name: Checkout repository uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 @@ -27,6 +24,8 @@ jobs: uses: leynos/shared-actions/.github/actions/setup-rust@2f90d1041ea108148be0620e3bbcc1fa80ac03e4 with: toolchain: ${{ env.NETSUKE_RUST_TOOLCHAIN }} + # Preserve the Polonius requirement through toolchain setup. + rustflags: -Zpolonius=next - name: Show rustc version run: | rustup show diff --git a/docs/developers-guide.md b/docs/developers-guide.md index 9ed87131b..14fa10136 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -259,6 +259,61 @@ double lookups, unconditional key clones, or id indirection, and do not pad new code with defensive clones that only NLL required. When a borrow-centric form fails to compile, consult the migration notes before restructuring. +### Polonius CI shared-action contract + +GitHub Actions jobs do not read `.cargo/config.toml` for every build they +launch, and the shared Rust setup actions export their own `RUSTFLAGS`. The +Polonius flags therefore travel as *action inputs*, not as job environment +variables: each affected workflow passes them through the relevant shared +action's `with.rustflags` input, and none of them may set a job-level +`env.RUSTFLAGS`. A job-level override would win over the action's exported +value and silently drop the flag, so the tree would fail to borrow-check with a +confusing `E0499` rather than an obvious configuration error. + +Four workflows carry the contract: + +| Workflow | Job | Shared action | `with.rustflags` | +| --- | --- | --- | --- | +| [`ci.yml`](../.github/workflows/ci.yml) | `build-test` | `setup-rust` | `-D warnings -Zpolonius=next` | +| [`coverage-main.yml`](../.github/workflows/coverage-main.yml) | `coverage-upload` | `setup-rust` | `-D warnings -Zpolonius=next` | +| [`netsukefile-test.yml`](../.github/workflows/netsukefile-test.yml) | `netsukefile` | `setup-rust` | `-Zpolonius=next` | +| [`build-and-package.yml`](../.github/workflows/build-and-package.yml) | `build` | `rust-build-release` | `-Zpolonius=next` | + +CI and coverage add `-D warnings` because those jobs gate on a warning-free +build; the Netsukefile and packaging jobs carry the Polonius flag alone, so a +new upstream warning cannot break a release build. The coverage action's +`cargo-llvm-cov` invocation inherits the flags `setup-rust` exports and appends +its own instrumentation flags. + +`NETSUKE_RUST_TOOLCHAIN` follows a separate rule. CI and Netsukefile pin it to +the channel in `rust-toolchain.toml` so those jobs provision the dated nightly +explicitly; coverage and packaging must leave it unset, because they select +their toolchain through the action's own `toolchain` input and a second, +independently edited pin would let the two disagree. + +[`tests/polonius_toolchain_contract.rs`](../tests/polonius_toolchain_contract.rs) +enforces all four callers. For each one it asserts: + +- the job uses the expected shared-action reference — path *and* pinned + revision (see "Workflow pins and Dependabot" below for why the exact + revision is asserted here); +- the `with.rustflags` value matches the table above in full, not merely that + it contains `-Zpolonius=next`, so a dropped `-D warnings` is caught too; +- the job declares no `env.RUSTFLAGS`; +- the `NETSUKE_RUST_TOOLCHAIN` policy above — pinned to the + `rust-toolchain.toml` channel for CI and Netsukefile, absent for coverage and + packaging. + +Run it with: + +```bash +cargo nextest run --test polonius_toolchain_contract +``` + +Keep this section and the [Polonius migration notes](polonius.md) in step: both +describe the same contract, and the notes list every remaining harness that +must propagate the flag. + ## Quality gates Run these commands before finalizing any change: @@ -431,10 +486,17 @@ the test fails until a human edits the pinned constant to match. That defeats the purpose of automated dependency updates and turns a routine bump into a manual chore. -Contract tests may still verify the *shape* of a reusable-workflow caller. They -must not verify the specific SHA value. - -- Do assert the workflow references the correct reusable workflow path. +The default, therefore, is shape-only: contract tests verify the *shape* of a +shared-action caller and not the specific SHA value. This covers both forms of +call into `shared-actions` — a step that `uses:` a composite action with a +`with:` block, and a job that `uses:` a reusable workflow. The one sanctioned +departure is a caller whose behaviour depends on a feature the shared action +gained at a known revision; the Polonius exception below is the only current +instance. The bullets that follow state the default; they do not apply to a +caller covered by that exception. + +- Do assert the caller references the correct shared-action or + reusable-workflow path. - Do assert the ref is pinned to a full 40-character commit SHA, not a mutable branch such as `main` or `rolling`. - Do assert the expected `on:` triggers, least-privilege `permissions:`, and @@ -453,9 +515,33 @@ def test_uses_pinned_full_sha(caller_step): assert SHA_RE.match(ref), f"expected a 40-hex commit SHA, got {ref!r}" ``` -If a workflow's behaviour genuinely depends on a feature only present from a -particular commit onwards, express that as a comment or a changelog note, not -as a test assertion on the SHA string. +The policy above governs callers whose behaviour does not depend on a specific +shared-action revision: the caller would keep working across any upstream bump, +so pinning the SHA in a test buys nothing and costs a manual edit per bump. +`tests/workflow_contracts/mutation_testing_test.py` is the canonical example. + +#### Exception: the Polonius shared-action contract + +The four workflows described under [Polonius CI shared-action +contract](#polonius-ci-shared-action-contract) do depend on a specific +revision. The `rustflags` input they rely on was introduced at a known commit +in `leynos/shared-actions`. A revision that predates it does not fail the run — +an unrecognized `with:` key on a composite action is a warning, not an error — +it simply never exports the flag, so the build fails later as a borrow-check +error rather than as a configuration error. + +`tests/polonius_toolchain_contract.rs` therefore asserts each of those +workflows' exact shared-action path *and* pinned revision, held in the +`SETUP_RUST_ACTION` and `RUST_BUILD_RELEASE_ACTION` constants. A Dependabot +bump of these four references is expected to fail the test until someone +updates the constants, and that failure is the point: it forces a human to +confirm the new revision still implements the `rustflags` input contract before +the bump lands. Restrict this exception to callers with a genuine +revision-level dependency; everywhere else, the shape-only policy applies. + +If a workflow's behaviour does not depend on a feature from a particular commit +onwards, do not assert its SHA — express any advisory note as a comment or a +changelog entry instead. ## Mutation-testing workflow contract tests diff --git a/docs/polonius.md b/docs/polonius.md index d33c40c5e..6e31ac552 100644 --- a/docs/polonius.md +++ b/docs/polonius.md @@ -89,8 +89,8 @@ Scanner suspects that turned out not to be NLL residue: The plumbing itself is contract-tested: `tests/polonius_toolchain_contract.rs` pins the dated-nightly channel, the `.cargo/config.toml` `build.rustflags` entry, the `POLONIUS_FLAGS` default and every RUSTFLAGS-setting Makefile -recipe, and the `RUSTFLAGS` and toolchain presets in the CI, Netsukefile, -coverage, and packaging workflows. +recipe, and the shared-action `with.rustflags` and toolchain inputs in the CI, +Netsukefile, coverage, and packaging workflows. ## Harness consequences @@ -106,14 +106,17 @@ flag or avoid compiling the crate: - **Kani** and **Whitaker** run under their own toolchains but read the workspace `.cargo/config.toml` or the Makefile `RUSTFLAGS`, so they borrow-check with `-Zpolonius=next` and need no special handling. -- **CI setup actions**: `actions-rust-lang/setup-rust-toolchain` exports - `RUSTFLAGS="-D warnings"` into the job environment when the variable is - unset, which shadows `.cargo/config.toml` for every later step. The workflows - therefore pre-set `RUSTFLAGS` (including `-Zpolonius=next`) at job level — - the action defers to an existing value — and the Makefile recipes append - `POLONIUS_FLAGS` to any ambient `RUSTFLAGS` as a second line of defence. - `cargo-llvm-cov` appends its instrumentation flags to the ambient value, so - coverage inherits the flag from the job environment. +- **CI setup actions**: the shared `setup-rust` and `rust-build-release` + actions receive the Polonius flags through their `with.rustflags` inputs; + workflows must not set a job-level `env.RUSTFLAGS`. CI and coverage pass + `-D warnings -Zpolonius=next`, while Netsukefile tests and packaging pass + `-Zpolonius=next`. The coverage action's `cargo-llvm-cov` invocation inherits + the flags exported by `setup-rust` and appends its instrumentation flags. + Makefile recipes still append `POLONIUS_FLAGS` when they set ambient + `RUSTFLAGS`. The per-workflow values, the `NETSUKE_RUST_TOOLCHAIN` policy and + the reason the contract test pins each action's exact revision are set out in + the developer guide under [Polonius CI shared-action + contract](developers-guide.md#polonius-ci-shared-action-contract). - **Registry installs**: the crates.io package excludes `rust-toolchain.toml` and `.cargo/config.toml`, and registry builds run outside the checkout, so `cargo install netsuke-build` must select the pinned diff --git a/tests/polonius_toolchain_contract.rs b/tests/polonius_toolchain_contract.rs index c2e7734d5..545be1330 100644 --- a/tests/polonius_toolchain_contract.rs +++ b/tests/polonius_toolchain_contract.rs @@ -4,8 +4,11 @@ //! pinned in `rust-toolchain.toml` (see ADR-006 and docs/polonius.md). An //! inherited `RUSTFLAGS` environment variable overrides the //! `.cargo/config.toml` `build.rustflags` table, so Makefile recipes that -//! compile borrow-checked targets and workflows that preset it must re-state -//! the flag. These tests fail when any layer drops the required policy. +//! compile borrow-checked targets must restate the flag, while workflows must +//! pass it through the shared action's `with.rustflags` input and reject a +//! job-level `env.RUSTFLAGS` override. These tests fail when any layer drops +//! the required policy, so a regression cannot reach CI as a confusing +//! borrow-check error. #[path = "support/makefile.rs"] mod makefile; @@ -19,6 +22,53 @@ use toml::Value as TomlValue; const POLONIUS_FLAG: &str = "-Zpolonius=next"; const POLONIUS_VAR: &str = "$(POLONIUS_FLAGS)"; +const SETUP_RUST_ACTION: &str = concat!( + "leynos/shared-actions/.github/actions/setup-rust@", + "2f90d1041ea108148be0620e3bbcc1fa80ac03e4" +); +const RUST_BUILD_RELEASE_ACTION: &str = concat!( + "leynos/shared-actions/.github/actions/rust-build-release@", + "2f90d1041ea108148be0620e3bbcc1fa80ac03e4" +); +const WARNINGS_POLONIUS_RUSTFLAGS: &str = "-D warnings -Zpolonius=next"; + +/// Describes one workflow's shared-action and toolchain contract. +struct WorkflowExpectation { + path: &'static str, + job: &'static str, + action: &'static str, + rustflags: &'static str, + pins_toolchain_env: bool, +} + +const CI_WORKFLOW: WorkflowExpectation = WorkflowExpectation { + path: ".github/workflows/ci.yml", + job: "build-test", + action: SETUP_RUST_ACTION, + rustflags: WARNINGS_POLONIUS_RUSTFLAGS, + pins_toolchain_env: true, +}; +const NETSUKEFILE_WORKFLOW: WorkflowExpectation = WorkflowExpectation { + path: ".github/workflows/netsukefile-test.yml", + job: "netsukefile", + action: SETUP_RUST_ACTION, + rustflags: POLONIUS_FLAG, + pins_toolchain_env: true, +}; +const COVERAGE_WORKFLOW: WorkflowExpectation = WorkflowExpectation { + path: ".github/workflows/coverage-main.yml", + job: "coverage-upload", + action: SETUP_RUST_ACTION, + rustflags: WARNINGS_POLONIUS_RUSTFLAGS, + pins_toolchain_env: false, +}; +const PACKAGING_WORKFLOW: WorkflowExpectation = WorkflowExpectation { + path: ".github/workflows/build-and-package.yml", + job: "build", + action: RUST_BUILD_RELEASE_ACTION, + rustflags: POLONIUS_FLAG, + pins_toolchain_env: false, +}; /// Returns the dated nightly channel pinned in `rust-toolchain.toml`. /// @@ -86,6 +136,61 @@ fn makefile_declares_the_polonius_flags_variable() -> Result<()> { Ok(()) } +#[rstest] +#[case::ci(CI_WORKFLOW)] +#[case::netsukefile(NETSUKEFILE_WORKFLOW)] +#[case::coverage(COVERAGE_WORKFLOW)] +#[case::packaging(PACKAGING_WORKFLOW)] +fn workflows_pass_polonius_rustflags_to_shared_actions( + #[case] expectation: WorkflowExpectation, +) -> Result<()> { + let WorkflowExpectation { + path, + job, + action: expected_action, + rustflags: expected_rustflags, + pins_toolchain_env, + } = expectation; + let workflow: YamlValue = serde_yaml::from_str(&read_repo_file(Utf8Path::new(path))?) + .with_context(|| format!("parse {path}"))?; + ensure!( + yaml_str(&workflow, &["jobs", job, "env", "RUSTFLAGS"]).is_none(), + "{path} job {job} should pass RUSTFLAGS through the shared action input" + ); + let steps = workflow + .get("jobs") + .and_then(|jobs| jobs.get(job)) + .and_then(|job_value| job_value.get("steps")) + .and_then(YamlValue::as_sequence) + .with_context(|| format!("{path} job {job} should declare steps"))?; + let shared_action = steps + .iter() + .find(|step| yaml_str(step, &["uses"]) == Some(expected_action)) + .with_context(|| format!("{path} job {job} should use {expected_action}"))?; + let rustflags = yaml_str(shared_action, &["with", "rustflags"]) + .with_context(|| format!("{path} {expected_action} should pass rustflags"))?; + ensure!( + rustflags == expected_rustflags, + "{path} {expected_action} passes {rustflags:?}, expected {expected_rustflags:?}" + ); + let toolchain_env = yaml_str(&workflow, &["jobs", job, "env", "NETSUKE_RUST_TOOLCHAIN"]); + if pins_toolchain_env { + let expected = pinned_toolchain()?; + let toolchain = toolchain_env + .with_context(|| format!("{path} job {job} should pin NETSUKE_RUST_TOOLCHAIN"))?; + ensure!( + toolchain == expected, + "{path} job {job} pins {toolchain:?}, but rust-toolchain.toml pins {expected:?}" + ); + } else { + ensure!( + toolchain_env.is_none(), + "{path} job {job} should not override NETSUKE_RUST_TOOLCHAIN, found {toolchain_env:?}" + ); + } + Ok(()) +} + #[rstest] #[case::test_nextest("test-nextest", true)] #[case::doctest("doctest", true)] @@ -118,36 +223,6 @@ fn rustflags_setting_recipes_apply_polonius_policy( Ok(()) } -#[rstest] -#[case::ci(".github/workflows/ci.yml", "build-test", true)] -#[case::netsukefile(".github/workflows/netsukefile-test.yml", "netsukefile", true)] -#[case::coverage(".github/workflows/coverage-main.yml", "coverage-upload", false)] -#[case::packaging(".github/workflows/build-and-package.yml", "build", false)] -fn workflows_preset_polonius_rustflags( - #[case] path: &str, - #[case] job: &str, - #[case] pins_toolchain_env: bool, -) -> Result<()> { - let workflow: YamlValue = serde_yaml::from_str(&read_repo_file(Utf8Path::new(path))?) - .with_context(|| format!("parse {path}"))?; - let rustflags = yaml_str(&workflow, &["jobs", job, "env", "RUSTFLAGS"]) - .with_context(|| format!("{path} job {job} should preset RUSTFLAGS"))?; - ensure!( - rustflags.contains(POLONIUS_FLAG), - "{path} job {job} presets RUSTFLAGS without {POLONIUS_FLAG}: {rustflags:?}" - ); - if pins_toolchain_env { - let expected = pinned_toolchain()?; - let toolchain = yaml_str(&workflow, &["jobs", job, "env", "NETSUKE_RUST_TOOLCHAIN"]) - .with_context(|| format!("{path} job {job} should pin NETSUKE_RUST_TOOLCHAIN"))?; - ensure!( - toolchain == expected, - "{path} job {job} pins {toolchain:?}, but rust-toolchain.toml pins {expected:?}" - ); - } - Ok(()) -} - #[test] fn coverage_workflow_setup_matches_the_pinned_toolchain() -> Result<()> { let path = ".github/workflows/coverage-main.yml";