From bfde4d8f66ff3a3e50138492dafeca4f2b576edd Mon Sep 17 00:00:00 2001 From: leynos Date: Mon, 27 Apr 2026 20:29:51 +0200 Subject: [PATCH 01/21] Guard cross commands against toolchain overrides Reject cross command vectors that accidentally inject a `+` argument while preserving cargo fallback behaviour. Add regression tests for normal cross command construction, the cargo fallback path, and the guard's negative case. --- .../actions/rust-build-release/src/main.py | 14 +- .../rust-build-release/tests/test_commands.py | 145 ++++++++++++++++++ 2 files changed, 158 insertions(+), 1 deletion(-) create mode 100644 .github/actions/rust-build-release/tests/test_commands.py diff --git a/.github/actions/rust-build-release/src/main.py b/.github/actions/rust-build-release/src/main.py index 079af5cd..6251645e 100755 --- a/.github/actions/rust-build-release/src/main.py +++ b/.github/actions/rust-build-release/src/main.py @@ -501,12 +501,21 @@ def _normalize_features(features: str) -> str: return ",".join(normalized) +def _assert_cross_command_has_no_toolchain_override(cmd: cabc.Sequence[object]) -> None: + # Cross must not be given a +; rely on rust-toolchain.toml / + # rustup override. + if any(isinstance(arg, str) and arg.startswith("+") for arg in cmd[1:3]): + message = "cross command must not include a + override" + raise ValueError(message) + + def _build_cross_command( decision: _CrossDecision, target_to_build: str, manifest_path: Path, features: str ) -> SupportsFormulate: cross_executable = decision.cross_path or "cross" executor = local[cross_executable] - build_cmd = executor[ + cmd: list[object] = [ + cross_executable, "build", "--manifest-path", str(manifest_path), @@ -514,6 +523,8 @@ def _build_cross_command( "--target", target_to_build, ] + _assert_cross_command_has_no_toolchain_override(cmd) + build_cmd = executor[cmd[1:]] normalized_features = _normalize_features(features) if normalized_features: build_cmd = build_cmd["--features", normalized_features] @@ -667,6 +678,7 @@ def main( build_cmd = typ.cast("_SupportsEnvFormulate", build_cmd).with_env( RUSTUP_TOOLCHAIN=toolchain_name ) + typer.echo(f"::debug:: cross argv: {build_cmd}") else: build_cmd = _build_cargo_command( decision.cargo_toolchain_spec, target_to_build, manifest_argument, features diff --git a/.github/actions/rust-build-release/tests/test_commands.py b/.github/actions/rust-build-release/tests/test_commands.py new file mode 100644 index 00000000..ba589a75 --- /dev/null +++ b/.github/actions/rust-build-release/tests/test_commands.py @@ -0,0 +1,145 @@ +"""Regression tests for rust-build-release command construction.""" + +from __future__ import annotations + +import importlib.util +import os +import typing as typ +from pathlib import Path + +import pytest +from plumbum.commands.processes import ProcessExecutionError +from rust_build_release_test_helpers import assert_no_toolchain_override + +SRC_DIR = Path(__file__).resolve().parents[1] / "src" +REPO_ROOT = Path(__file__).resolve().parents[4] + +if typ.TYPE_CHECKING: + from types import ModuleType + + +def _make_cross_executable(tmp_path: Path) -> Path: + cross_path = tmp_path / "cross" + cross_path.write_text("#!/bin/sh\n", encoding="utf-8") + cross_path.chmod(cross_path.stat().st_mode | os.X_OK) + return cross_path + + +def _load_main_module(monkeypatch: pytest.MonkeyPatch) -> ModuleType: + monkeypatch.setenv("GITHUB_ACTION_PATH", str(REPO_ROOT)) + monkeypatch.syspath_prepend(str(SRC_DIR)) + + import packaging + import packaging.version as packaging_version + + spec = importlib.util.spec_from_file_location( + "rbr_main_commands", SRC_DIR / "main.py" + ) + if spec is None or spec.loader is None: + msg = f"failed to load main.py from {SRC_DIR}" + raise RuntimeError(msg) + + module = importlib.util.module_from_spec(spec) + try: + spec.loader.exec_module(module) + finally: + if getattr(packaging, "version", None) is packaging_version: + delattr(packaging, "version") + return module + + +def test_build_cross_command_never_injects_toolchain_override( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, +) -> None: + """Cross commands start with cross build and omit +toolchain arguments.""" + main_module = _load_main_module(monkeypatch) + cross_path = _make_cross_executable(tmp_path) + decision = main_module._CrossDecision( + cross_path=str(cross_path), + cross_version="0.2.5", + use_cross=True, + cargo_toolchain_spec="+bogus-nightly", + use_cross_local_backend=False, + docker_present=True, + podman_present=False, + has_container=True, + container_engine="docker", + requires_cross_container=False, + ) + + cmd = main_module._build_cross_command( + decision, + "aarch64-unknown-linux-gnu", + tmp_path / "Cargo.toml", + "", + ) + + parts = list(cmd.formulate()) + assert parts[0] == "cross" + assert_no_toolchain_override(parts) + + +def test_cross_container_fallback_keeps_cargo_toolchain_override( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, +) -> None: + """The container-error fallback remains cargo-only and keeps +toolchain.""" + main_module = _load_main_module(monkeypatch) + calls: list[list[str]] = [] + + def fake_run_cmd(cmd: object) -> None: + formulated = list(cmd.formulate()) + if formulated: + formulated[0] = Path(formulated[0]).name + calls.append(formulated) + + monkeypatch.setattr(main_module, "run_cmd", fake_run_cmd) + decision = main_module._CrossDecision( + cross_path="/usr/bin/cross", + cross_version="0.2.5", + use_cross=True, + cargo_toolchain_spec="+bogus-nightly", + use_cross_local_backend=False, + docker_present=True, + podman_present=False, + has_container=True, + container_engine="docker", + requires_cross_container=False, + ) + exc = ProcessExecutionError(["cross", "build"], 125, "", "") + + main_module._handle_cross_container_error( + exc, + decision, + "aarch64-unknown-linux-gnu", + tmp_path / "Cargo.toml", + "", + ) + + assert calls == [ + [ + "cargo", + "+bogus-nightly", + "build", + "--manifest-path", + str(tmp_path / "Cargo.toml"), + "--release", + "--target", + "aarch64-unknown-linux-gnu", + ] + ] + + +def test_cross_command_guard_rejects_toolchain_override( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """The cross argv guard catches accidental +toolchain injection.""" + main_module = _load_main_module(monkeypatch) + with pytest.raises( + ValueError, + match=r"cross command must not include a \+ override", + ): + main_module._assert_cross_command_has_no_toolchain_override( + ["cross", "+bogus-nightly", "build"] + ) From 097e5428067341080a7e58c5940229d74e5b7bd3 Mon Sep 17 00:00:00 2001 From: leynos Date: Mon, 27 Apr 2026 20:31:28 +0200 Subject: [PATCH 02/21] Resolve check tools without PATH lookups Avoid requiring `uv`, `uvx`, or `action-validator` to be discoverable through hook PATH settings. Prefer known local executable locations for Ruff and the GitHub Action validator while keeping command-name fallbacks for other developer environments. --- Makefile | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/Makefile b/Makefile index 7949bb03..e53c174b 100644 --- a/Makefile +++ b/Makefile @@ -16,6 +16,10 @@ clean: ## Remove transient artefacts BUILD_JOBS ?= MDLINT ?= markdownlint NIXIE ?= nixie +ACTION_VALIDATOR_CANDIDATES := $(wildcard $(HOME)/.cargo/bin/action-validator /usr/local/bin/action-validator /usr/bin/action-validator) +ACTION_VALIDATOR ?= $(if $(ACTION_VALIDATOR_CANDIDATES),$(firstword $(ACTION_VALIDATOR_CANDIDATES)),action-validator) +RUFF_CANDIDATES := $(wildcard $(CURDIR)/.venv/bin/ruff $(HOME)/.local/bin/ruff /usr/local/bin/ruff /usr/bin/ruff) +RUFF ?= $(if $(RUFF_CANDIDATES),$(firstword $(RUFF_CANDIDATES)),uv tool run ruff) RUFF_FIX_RULES ?= D202,I001 test: .venv ## Run tests @@ -30,9 +34,9 @@ endif uv sync --group dev lint: ## Check test scripts and actions - uvx ruff check + $(RUFF) check find .github/actions -type f \( -name 'action.yml' -o -name 'action.yaml' \) -print0 \ - | xargs -r -0 -n1 action-validator + | xargs -r -0 -n1 $(ACTION_VALIDATOR) typecheck: .venv ## Run static type checking with Ty ./.venv/bin/ty check \ @@ -58,12 +62,12 @@ typecheck: .venv ## Run static type checking with Ty --extra-search-path .github/actions/macos-package/scripts \ .github/actions/macos-package/scripts fmt: ## Format Python files and auto-fix selected lint rules - uvx ruff format - uvx ruff check --select $(RUFF_FIX_RULES) --fix + $(RUFF) format + $(RUFF) check --select $(RUFF_FIX_RULES) --fix check-fmt: ## Check Python formatting without modifying files - uvx ruff format --check - uvx ruff check --select $(RUFF_FIX_RULES) + $(RUFF) format --check + $(RUFF) check --select $(RUFF_FIX_RULES) markdownlint: ## Lint Markdown files find . -type f -name '*.md' -not -path './target/*' -print0 | xargs -0 -- $(MDLINT) From 6e58d24f271ca4f4276d97e1e67eea2cd1ff57cc Mon Sep 17 00:00:00 2001 From: leynos Date: Mon, 27 Apr 2026 20:52:34 +0200 Subject: [PATCH 03/21] Address cross command review feedback Scan the full cross argv after the executable for accidental `+` overrides. Strengthen command construction tests to assert the expected `cross build` prefix and cover cargo fallback behaviour when no toolchain specification is configured. --- .../actions/rust-build-release/src/main.py | 2 +- .../rust-build-release/tests/test_commands.py | 51 +++++++++++++++++-- 2 files changed, 49 insertions(+), 4 deletions(-) diff --git a/.github/actions/rust-build-release/src/main.py b/.github/actions/rust-build-release/src/main.py index 6251645e..45dbf783 100755 --- a/.github/actions/rust-build-release/src/main.py +++ b/.github/actions/rust-build-release/src/main.py @@ -504,7 +504,7 @@ def _normalize_features(features: str) -> str: def _assert_cross_command_has_no_toolchain_override(cmd: cabc.Sequence[object]) -> None: # Cross must not be given a +; rely on rust-toolchain.toml / # rustup override. - if any(isinstance(arg, str) and arg.startswith("+") for arg in cmd[1:3]): + if any(isinstance(arg, str) and arg.startswith("+") for arg in cmd[1:]): message = "cross command must not include a + override" raise ValueError(message) diff --git a/.github/actions/rust-build-release/tests/test_commands.py b/.github/actions/rust-build-release/tests/test_commands.py index ba589a75..25a2fd9d 100644 --- a/.github/actions/rust-build-release/tests/test_commands.py +++ b/.github/actions/rust-build-release/tests/test_commands.py @@ -52,7 +52,7 @@ def test_build_cross_command_never_injects_toolchain_override( monkeypatch: pytest.MonkeyPatch, tmp_path: Path, ) -> None: - """Cross commands start with cross build and omit +toolchain arguments.""" + """Cross commands start with `cross build` and omit +toolchain arguments.""" main_module = _load_main_module(monkeypatch) cross_path = _make_cross_executable(tmp_path) decision = main_module._CrossDecision( @@ -76,7 +76,7 @@ def test_build_cross_command_never_injects_toolchain_override( ) parts = list(cmd.formulate()) - assert parts[0] == "cross" + assert parts[:3] == ["cross", "build", "--manifest-path"] assert_no_toolchain_override(parts) @@ -131,6 +131,51 @@ def fake_run_cmd(cmd: object) -> None: ] +def test_cross_container_fallback_without_cargo_toolchain_override( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, +) -> None: + """The container-error fallback uses no +toolchain when none is configured.""" + main_module = _load_main_module(monkeypatch) + calls: list[list[str]] = [] + + def fake_run_cmd(cmd: object) -> None: + formulated = list(cmd.formulate()) + if formulated: + formulated[0] = Path(formulated[0]).name + calls.append(formulated) + + monkeypatch.setattr(main_module, "run_cmd", fake_run_cmd) + decision = main_module._CrossDecision( + cross_path="/usr/bin/cross", + cross_version="0.2.5", + use_cross=True, + cargo_toolchain_spec="", + use_cross_local_backend=False, + docker_present=True, + podman_present=False, + has_container=True, + container_engine="docker", + requires_cross_container=False, + ) + exc = ProcessExecutionError(["cross", "build"], 125, "", "") + + main_module._handle_cross_container_error( + exc, + decision, + "aarch64-unknown-linux-gnu", + tmp_path / "Cargo.toml", + "", + ) + + assert calls + fallback_call = calls[0] + assert fallback_call[0] == "cargo" + assert all( + not (isinstance(arg, str) and arg.startswith("+")) for arg in fallback_call[1:] + ) + + def test_cross_command_guard_rejects_toolchain_override( monkeypatch: pytest.MonkeyPatch, ) -> None: @@ -141,5 +186,5 @@ def test_cross_command_guard_rejects_toolchain_override( match=r"cross command must not include a \+ override", ): main_module._assert_cross_command_has_no_toolchain_override( - ["cross", "+bogus-nightly", "build"] + ["cross", "build", "--manifest-path", "Cargo.toml", "+bogus-nightly"] ) From 20f672b5bb5fb080436f4b421a95860b361cb178 Mon Sep 17 00:00:00 2001 From: leynos Date: Mon, 27 Apr 2026 20:56:13 +0200 Subject: [PATCH 04/21] Address executable path review feedback Use real executable permission bits when creating the fake cross binary in command tests. Include Bun's global binary directory when resolving `action-validator` for restricted hook environments. --- .github/actions/rust-build-release/tests/test_commands.py | 6 ++++-- Makefile | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/.github/actions/rust-build-release/tests/test_commands.py b/.github/actions/rust-build-release/tests/test_commands.py index 25a2fd9d..f704927c 100644 --- a/.github/actions/rust-build-release/tests/test_commands.py +++ b/.github/actions/rust-build-release/tests/test_commands.py @@ -3,7 +3,7 @@ from __future__ import annotations import importlib.util -import os +import stat import typing as typ from pathlib import Path @@ -21,7 +21,9 @@ def _make_cross_executable(tmp_path: Path) -> Path: cross_path = tmp_path / "cross" cross_path.write_text("#!/bin/sh\n", encoding="utf-8") - cross_path.chmod(cross_path.stat().st_mode | os.X_OK) + cross_path.chmod( + cross_path.stat().st_mode | stat.S_IXUSR | stat.S_IXGRP | stat.S_IXOTH + ) return cross_path diff --git a/Makefile b/Makefile index e53c174b..48979b7f 100644 --- a/Makefile +++ b/Makefile @@ -16,7 +16,7 @@ clean: ## Remove transient artefacts BUILD_JOBS ?= MDLINT ?= markdownlint NIXIE ?= nixie -ACTION_VALIDATOR_CANDIDATES := $(wildcard $(HOME)/.cargo/bin/action-validator /usr/local/bin/action-validator /usr/bin/action-validator) +ACTION_VALIDATOR_CANDIDATES := $(wildcard $(HOME)/.cargo/bin/action-validator $(HOME)/.bun/bin/action-validator /usr/local/bin/action-validator /usr/bin/action-validator) ACTION_VALIDATOR ?= $(if $(ACTION_VALIDATOR_CANDIDATES),$(firstword $(ACTION_VALIDATOR_CANDIDATES)),action-validator) RUFF_CANDIDATES := $(wildcard $(CURDIR)/.venv/bin/ruff $(HOME)/.local/bin/ruff /usr/local/bin/ruff /usr/bin/ruff) RUFF ?= $(if $(RUFF_CANDIDATES),$(firstword $(RUFF_CANDIDATES)),uv tool run ruff) From 2f947aea2e4eaa4aefc3da8724ee0ea30e0969a7 Mon Sep 17 00:00:00 2001 From: leynos Date: Mon, 27 Apr 2026 21:07:59 +0200 Subject: [PATCH 05/21] Validate final cross argv after feature expansion Build the full cross argv, including normalised feature arguments, before checking for accidental `+` overrides. This keeps the guard aligned with the command vector passed to plumbum for execution. --- .github/actions/rust-build-release/src/main.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/actions/rust-build-release/src/main.py b/.github/actions/rust-build-release/src/main.py index 45dbf783..e5fafb4a 100755 --- a/.github/actions/rust-build-release/src/main.py +++ b/.github/actions/rust-build-release/src/main.py @@ -523,11 +523,11 @@ def _build_cross_command( "--target", target_to_build, ] - _assert_cross_command_has_no_toolchain_override(cmd) - build_cmd = executor[cmd[1:]] normalized_features = _normalize_features(features) if normalized_features: - build_cmd = build_cmd["--features", normalized_features] + cmd.extend(["--features", normalized_features]) + _assert_cross_command_has_no_toolchain_override(cmd) + build_cmd = executor[cmd[1:]] if decision.cross_path: build_cmd = _CommandWrapper(build_cmd, Path(decision.cross_path).name) return build_cmd From 6c87ca30cf47d8af29425f4c5d2ebbee2c274c9f Mon Sep 17 00:00:00 2001 From: leynos Date: Mon, 27 Apr 2026 21:09:48 +0200 Subject: [PATCH 06/21] Assert exact cargo fallback argv without toolchain Tighten the no-toolchain fallback test to assert the complete cargo argv. Omit the cargo toolchain slot when the specification is blank so fallback commands do not carry an empty argument. --- .github/actions/rust-build-release/src/main.py | 6 ++++-- .../rust-build-release/tests/test_commands.py | 17 +++++++++++------ 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/.github/actions/rust-build-release/src/main.py b/.github/actions/rust-build-release/src/main.py index e5fafb4a..c33d20e7 100755 --- a/.github/actions/rust-build-release/src/main.py +++ b/.github/actions/rust-build-release/src/main.py @@ -537,8 +537,7 @@ def _build_cargo_command( cargo_toolchain_spec: str, target_to_build: str, manifest_path: Path, features: str ) -> SupportsFormulate: executor = local["cargo"] - build_cmd = executor[ - cargo_toolchain_spec, + cmd = [ "build", "--manifest-path", str(manifest_path), @@ -546,6 +545,9 @@ def _build_cargo_command( "--target", target_to_build, ] + if cargo_toolchain_spec: + cmd.insert(0, cargo_toolchain_spec) + build_cmd = executor[cmd] normalized_features = _normalize_features(features) if normalized_features: build_cmd = build_cmd["--features", normalized_features] diff --git a/.github/actions/rust-build-release/tests/test_commands.py b/.github/actions/rust-build-release/tests/test_commands.py index f704927c..783bcf27 100644 --- a/.github/actions/rust-build-release/tests/test_commands.py +++ b/.github/actions/rust-build-release/tests/test_commands.py @@ -170,12 +170,17 @@ def fake_run_cmd(cmd: object) -> None: "", ) - assert calls - fallback_call = calls[0] - assert fallback_call[0] == "cargo" - assert all( - not (isinstance(arg, str) and arg.startswith("+")) for arg in fallback_call[1:] - ) + assert calls == [ + [ + "cargo", + "build", + "--manifest-path", + str(tmp_path / "Cargo.toml"), + "--release", + "--target", + "aarch64-unknown-linux-gnu", + ] + ] def test_cross_command_guard_rejects_toolchain_override( From 8373efcafac935c94cb6b1ada84127ee0a1c2f36 Mon Sep 17 00:00:00 2001 From: leynos Date: Mon, 27 Apr 2026 21:21:50 +0200 Subject: [PATCH 07/21] Construct cargo argv before command lookup Build cargo commands from a single argv list before handing them to plumbum so feature arguments are part of the exact command lookup. --- .github/actions/rust-build-release/src/main.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/actions/rust-build-release/src/main.py b/.github/actions/rust-build-release/src/main.py index c33d20e7..e22a34a2 100755 --- a/.github/actions/rust-build-release/src/main.py +++ b/.github/actions/rust-build-release/src/main.py @@ -545,12 +545,12 @@ def _build_cargo_command( "--target", target_to_build, ] + normalized_features = _normalize_features(features) + if normalized_features: + cmd.extend(["--features", normalized_features]) if cargo_toolchain_spec: cmd.insert(0, cargo_toolchain_spec) build_cmd = executor[cmd] - normalized_features = _normalize_features(features) - if normalized_features: - build_cmd = build_cmd["--features", normalized_features] return _CommandWrapper(build_cmd, "cargo") From 3fc35ef69334f2728b1546cd6495994dba624e62 Mon Sep 17 00:00:00 2001 From: leynos Date: Mon, 27 Apr 2026 22:50:07 +0200 Subject: [PATCH 08/21] Expand cross command validation coverage Make command wrapper warning output injectable and report cross argv validation failures from main with a GitHub Actions error annotation. Add property and unit coverage for cross command guards plus root development and changelog notes for the argv assembly pattern. --- .../actions/rust-build-release/src/main.py | 47 +++- .../rust-build-release/tests/conftest.py | 37 ++- .../rust-build-release/tests/test_commands.py | 214 ++++++++++++++++++ CHANGELOG.md | 20 ++ DEVELOPMENT.md | 36 +++ pyproject.toml | 1 + 6 files changed, 334 insertions(+), 21 deletions(-) create mode 100644 CHANGELOG.md create mode 100644 DEVELOPMENT.md diff --git a/.github/actions/rust-build-release/src/main.py b/.github/actions/rust-build-release/src/main.py index e22a34a2..1470ee99 100755 --- a/.github/actions/rust-build-release/src/main.py +++ b/.github/actions/rust-build-release/src/main.py @@ -98,7 +98,12 @@ class _CrossDecision(typ.NamedTuple): class _CommandWrapper: """Expose a stable display name for a plumbum command.""" - def __init__(self, command: SupportsFormulate, display_name: str) -> None: + def __init__( + self, + command: SupportsFormulate, + display_name: str, + echo: cabc.Callable[[str], None] = typer.echo, + ) -> None: formulate_callable = getattr(command, "formulate", None) if not callable(formulate_callable): message = ( @@ -108,14 +113,19 @@ def __init__(self, command: SupportsFormulate, display_name: str) -> None: raise TypeError(message) self._command: typ.Any = command self._display_name = display_name + self._echo = echo + + def _warn(self, message: str) -> None: + """Emit a wrapper warning through the configured echo callable.""" + self._echo(message) def formulate(self) -> cabc.Sequence[str]: + """Return the command argv with the configured display name applied.""" formulate_callable = getattr(self._command, "formulate", None) if not callable(formulate_callable): - typer.echo( + self._warn( f"::warning:: command {self._command!r} does not support formulate(); " - "returning display name only", - err=True, + "returning display name only" ) return [self._display_name] try: @@ -125,28 +135,33 @@ def formulate(self) -> cabc.Sequence[str]: "::warning:: failed to generate command line for " f"{self._command!r}: {exc}" ) - typer.echo(message, err=True) + self._warn(message) return [self._display_name] if parts: parts[0] = self._display_name return parts def __str__(self) -> str: + """Return a shell-escaped display string for the wrapped command.""" parts = [str(part) for part in self.formulate()] return shlex.join(parts) def __call__(self, *args: object, **kwargs: object) -> SupportsFormulate: + """Delegate command invocation to the wrapped command.""" return self._command(*args, **kwargs) def run( self, *args: object, **kwargs: object ) -> tuple[int, str | bytes | None, str | bytes | None]: + """Run the wrapped command and return plumbum's result tuple.""" return self._command.run(*args, **kwargs) def popen(self, *args: object, **kwargs: object) -> subprocess.Popen[typ.Any]: + """Start the wrapped command as a subprocess.""" return self._command.popen(*args, **kwargs) def with_env(self, *args: object, **kwargs: object) -> _CommandWrapper: + """Return a wrapper around the command with temporary environment values.""" wrapped = self._command.with_env(*args, **kwargs) wrapped_formulate = getattr(wrapped, "formulate", None) if not callable(wrapped_formulate): @@ -155,9 +170,10 @@ def with_env(self, *args: object, **kwargs: object) -> _CommandWrapper: "cannot maintain display override" ) raise TypeError(message) - return _CommandWrapper(wrapped, self._display_name) + return _CommandWrapper(wrapped, self._display_name, self._echo) def __getattr__(self, name: str) -> object: + """Delegate unknown attributes to the wrapped command.""" return getattr(self._command, name) @@ -477,6 +493,7 @@ def _configure_cross_container_engine( def _restore_container_engine( previous_engine: str | None, *, applied_engine: str | None ) -> None: + """Restore CROSS_CONTAINER_ENGINE after a temporary cross configuration.""" current_engine = os.environ.get("CROSS_CONTAINER_ENGINE") if previous_engine is None: @@ -502,6 +519,7 @@ def _normalize_features(features: str) -> str: def _assert_cross_command_has_no_toolchain_override(cmd: cabc.Sequence[object]) -> None: + """Raise ValueError if a cross argv contains a +toolchain override.""" # Cross must not be given a +; rely on rust-toolchain.toml / # rustup override. if any(isinstance(arg, str) and arg.startswith("+") for arg in cmd[1:]): @@ -512,6 +530,7 @@ def _assert_cross_command_has_no_toolchain_override(cmd: cabc.Sequence[object]) def _build_cross_command( decision: _CrossDecision, target_to_build: str, manifest_path: Path, features: str ) -> SupportsFormulate: + """Build a cross command argv and validate it contains no +toolchain.""" cross_executable = decision.cross_path or "cross" executor = local[cross_executable] cmd: list[object] = [ @@ -536,6 +555,7 @@ def _build_cross_command( def _build_cargo_command( cargo_toolchain_spec: str, target_to_build: str, manifest_path: Path, features: str ) -> SupportsFormulate: + """Build a cargo command argv, preserving any configured +toolchain.""" executor = local["cargo"] cmd = [ "build", @@ -561,6 +581,7 @@ def _handle_cross_container_error( manifest_path: Path, features: str, ) -> None: + """Handle cross container startup failures or re-raise other errors.""" if decision.use_cross and exc.retcode in CROSS_CONTAINER_ERROR_CODES: if decision.requires_cross_container and not decision.use_cross_local_backend: engine = decision.container_engine or "unknown" @@ -673,9 +694,17 @@ def main( manifest_argument = _manifest_argument(manifest_path) if decision.use_cross: - build_cmd = _build_cross_command( - decision, target_to_build, manifest_argument, features - ) + try: + build_cmd = _build_cross_command( + decision, target_to_build, manifest_argument, features + ) + except ValueError as exc: + typer.echo( + f"::error:: cross command validation failed for target " + f"'{target_to_build}': {exc}", + err=True, + ) + raise typer.Exit(1) from None if explicit_toolchain: build_cmd = typ.cast("_SupportsEnvFormulate", build_cmd).with_env( RUSTUP_TOOLCHAIN=toolchain_name diff --git a/.github/actions/rust-build-release/tests/conftest.py b/.github/actions/rust-build-release/tests/conftest.py index 31b736aa..0bfa4dcd 100644 --- a/.github/actions/rust-build-release/tests/conftest.py +++ b/.github/actions/rust-build-release/tests/conftest.py @@ -237,6 +237,23 @@ class CrossDecision(typ.Protocol): DummyCommandFactory = cabc.Callable[..., _DummyCommand] +class _EchoRecorder(list[str]): + """Capture global echo calls while preserving module-specific recording.""" + + def __init__(self, monkeypatch: pytest.MonkeyPatch) -> None: + super().__init__() + self._monkeypatch = monkeypatch + + def __call__(self, module: ModuleType) -> list[tuple[str, bool]]: + messages: list[tuple[str, bool]] = [] + + def fake_echo(message: str, *, err: bool = False) -> None: + messages.append((message, err)) + + self._monkeypatch.setattr(module.typer, "echo", fake_echo) + return messages + + def _cross_decision( main_module: ModuleType, *, use_cross: bool, requires_container: bool = False ) -> CrossDecision: @@ -279,21 +296,17 @@ def factory( @pytest.fixture -def echo_recorder( - monkeypatch: pytest.MonkeyPatch, -) -> cabc.Callable[[ModuleType], list[tuple[str, bool]]]: - """Return a helper that patches ``typer.echo`` and records messages.""" +def echo_recorder(monkeypatch: pytest.MonkeyPatch) -> _EchoRecorder: + """Capture all typer.echo calls and return the recorded lines.""" + import typer - def install(module: ModuleType) -> list[tuple[str, bool]]: - messages: list[tuple[str, bool]] = [] + lines = _EchoRecorder(monkeypatch) - def fake_echo(message: str, *, err: bool = False) -> None: - messages.append((message, err)) - - monkeypatch.setattr(module.typer, "echo", fake_echo) - return messages + def record_echo(msg: object = "", **_kwargs: object) -> None: + lines.append(str(msg)) - return install + monkeypatch.setattr(typer, "echo", record_echo) + return lines @pytest.fixture diff --git a/.github/actions/rust-build-release/tests/test_commands.py b/.github/actions/rust-build-release/tests/test_commands.py index 783bcf27..0e78e0f2 100644 --- a/.github/actions/rust-build-release/tests/test_commands.py +++ b/.github/actions/rust-build-release/tests/test_commands.py @@ -3,11 +3,15 @@ from __future__ import annotations import importlib.util +import re import stat import typing as typ +import unittest.mock as mock from pathlib import Path import pytest +from hypothesis import HealthCheck, given, settings +from hypothesis import strategies as st from plumbum.commands.processes import ProcessExecutionError from rust_build_release_test_helpers import assert_no_toolchain_override @@ -17,6 +21,11 @@ if typ.TYPE_CHECKING: from types import ModuleType +ALNUM_TEXT = st.text( + alphabet=st.characters(whitelist_categories=("Lu", "Ll", "Nd")), + min_size=1, +) + def _make_cross_executable(tmp_path: Path) -> Path: cross_path = tmp_path / "cross" @@ -50,6 +59,28 @@ def _load_main_module(monkeypatch: pytest.MonkeyPatch) -> ModuleType: return module +def _make_cross_decision( + main_module: ModuleType, + cross_path: Path | str | None, + *, + cargo_toolchain_spec: str, + requires_cross_container: bool = False, + use_cross_local_backend: bool = False, +) -> object: + return main_module._CrossDecision( + cross_path=str(cross_path) if cross_path is not None else None, + cross_version="0.2.5", + use_cross=True, + cargo_toolchain_spec=cargo_toolchain_spec, + use_cross_local_backend=use_cross_local_backend, + docker_present=True, + podman_present=False, + has_container=True, + container_engine="docker", + requires_cross_container=requires_cross_container, + ) + + def test_build_cross_command_never_injects_toolchain_override( monkeypatch: pytest.MonkeyPatch, tmp_path: Path, @@ -82,6 +113,131 @@ def test_build_cross_command_never_injects_toolchain_override( assert_no_toolchain_override(parts) +def test_build_cross_command_invokes_toolchain_override_guard( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, +) -> None: + """Cross command construction invokes the toolchain override guard.""" + main_module = _load_main_module(monkeypatch) + cross_path = _make_cross_executable(tmp_path) + decision = _make_cross_decision( + main_module, + cross_path, + cargo_toolchain_spec="+bogus-nightly", + ) + guard = mock.MagicMock() + monkeypatch.setattr( + main_module, "_assert_cross_command_has_no_toolchain_override", guard + ) + + main_module._build_cross_command( + decision, + "aarch64-unknown-linux-gnu", + tmp_path / "Cargo.toml", + "", + ) + + guard.assert_called_once() + + +@settings( + max_examples=40, + suppress_health_check=[HealthCheck.function_scoped_fixture], +) +@given(target=ALNUM_TEXT, features=ALNUM_TEXT, manifest_stem=ALNUM_TEXT) +def test_build_cross_command_property_omits_toolchain_override( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, + target: str, + features: str, + manifest_stem: str, +) -> None: + """Generated cross commands never include +toolchain tokens after argv[0].""" + main_module = _load_main_module(monkeypatch) + cross_path = _make_cross_executable(tmp_path) + decision = _make_cross_decision( + main_module, + cross_path, + cargo_toolchain_spec="+bogus-nightly", + ) + + cmd = main_module._build_cross_command( + decision, + target, + tmp_path / f"{manifest_stem}.toml", + features, + ) + + assert_no_toolchain_override(list(cmd.formulate())) + + +@settings( + max_examples=40, + suppress_health_check=[HealthCheck.function_scoped_fixture], +) +@given(prefix=st.lists(ALNUM_TEXT), override=ALNUM_TEXT, suffix=st.lists(ALNUM_TEXT)) +def test_cross_command_guard_rejects_any_generated_toolchain_override( + monkeypatch: pytest.MonkeyPatch, + prefix: list[str], + override: str, + suffix: list[str], +) -> None: + """The cross guard rejects every +token after the executable.""" + main_module = _load_main_module(monkeypatch) + argv = ["cross", *prefix, f"+{override}", *suffix] + + with pytest.raises( + ValueError, + match=r"cross command must not include a \+ override", + ): + main_module._assert_cross_command_has_no_toolchain_override(argv) + + +@settings( + max_examples=40, + suppress_health_check=[HealthCheck.function_scoped_fixture], +) +@given(args=st.lists(ALNUM_TEXT)) +def test_cross_command_guard_accepts_generated_argv_without_toolchain_override( + monkeypatch: pytest.MonkeyPatch, + args: list[str], +) -> None: + """The cross guard accepts generated argv values with no +token.""" + main_module = _load_main_module(monkeypatch) + + main_module._assert_cross_command_has_no_toolchain_override(["cross", *args]) + + +def test_cross_debug_output_matches_expected_argv_pattern( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, + echo_recorder: list[str], +) -> None: + """The cross debug line reports the materialized cross argv.""" + main_module = _load_main_module(monkeypatch) + cross_path = _make_cross_executable(tmp_path) + decision = _make_cross_decision( + main_module, + cross_path, + cargo_toolchain_spec="+bogus-nightly", + ) + + build_cmd = main_module._build_cross_command( + decision, + "aarch64-unknown-linux-gnu", + tmp_path / "Cargo.toml", + "", + ) + main_module.typer.echo(f"::debug:: cross argv: {build_cmd}") + + assert len(echo_recorder) == 1 + assert re.match( + r"^::debug:: cross argv: cross build --manifest-path .+ " + r"--release --target .+$", + echo_recorder[0], + ) + + def test_cross_container_fallback_keeps_cargo_toolchain_override( monkeypatch: pytest.MonkeyPatch, tmp_path: Path, @@ -133,6 +289,64 @@ def fake_run_cmd(cmd: object) -> None: ] +def test_cross_container_error_with_other_retcode_reraises_original_exception( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, + echo_recorder: list[str], +) -> None: + """Non-container cross failures are re-raised unchanged.""" + main_module = _load_main_module(monkeypatch) + decision = _make_cross_decision( + main_module, + "/usr/bin/cross", + cargo_toolchain_spec="+bogus-nightly", + ) + exc = ProcessExecutionError(["cross", "build"], 2, "", "") + + with pytest.raises(ProcessExecutionError) as raised: + main_module._handle_cross_container_error( + exc, + decision, + "aarch64-unknown-linux-gnu", + tmp_path / "Cargo.toml", + "", + ) + + assert raised.value is exc + assert echo_recorder == [] + + +def test_required_cross_container_error_exits_without_fallback( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, + echo_recorder: list[str], +) -> None: + """Required container failures are emitted as errors and exit.""" + main_module = _load_main_module(monkeypatch) + decision = _make_cross_decision( + main_module, + "/usr/bin/cross", + cargo_toolchain_spec="+bogus-nightly", + requires_cross_container=True, + use_cross_local_backend=False, + ) + exc = ProcessExecutionError(["cross", "build"], 125, "", "") + + with pytest.raises(main_module.typer.Exit): + main_module._handle_cross_container_error( + exc, + decision, + "aarch64-unknown-linux-gnu", + tmp_path / "Cargo.toml", + "", + ) + + assert echo_recorder == [ + "::error:: cross failed to start a container runtime for target " + "'aarch64-unknown-linux-gnu' (engine=docker)" + ] + + def test_cross_container_fallback_without_cargo_toolchain_override( monkeypatch: pytest.MonkeyPatch, tmp_path: Path, diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 00000000..7f56f470 --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,20 @@ +# Changelog + +All notable changes to this project will be documented in this file. + +The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), +and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). + +## [Unreleased] + +### Changed + +- Refactored `rust-build-release` command construction to assemble argv lists + before handing commands to plumbum. +- Added Makefile tool discovery through candidate path lists for `RUFF` and + `ACTION_VALIDATOR`. + +### Added + +- Added a cross command validation guard that rejects `+` overrides. +- Added the `rust-build-release` `test_commands.py` regression test module. diff --git a/DEVELOPMENT.md b/DEVELOPMENT.md new file mode 100644 index 00000000..2e911c39 --- /dev/null +++ b/DEVELOPMENT.md @@ -0,0 +1,36 @@ +# Development + +## argv assembly pattern + +`rust-build-release` builds process invocations as plain argv lists before +handing them to plumbum. `_build_cross_command` assembles the final `cross` +argv, including manifest, release, target, and feature arguments, validates that +list, and then resolves `executor[cmd[1:]]` because plumbum already supplies the +executable. `_build_cargo_command` follows the same list-first pattern for +`cargo`, inserting the configured cargo toolchain override only after all normal +arguments have been assembled. + +This keeps ordering explicit, makes tests assert the exact command shape, and +avoids hidden post-construction mutations of plumbum command objects. + +## Cross command validation guard + +`_assert_cross_command_has_no_toolchain_override` rejects any `cross` argv that +contains a `+` argument after the executable. `cross` must not +receive a rustup toolchain override in argv; the toolchain is controlled by +`rust-toolchain.toml` or `RUSTUP_TOOLCHAIN` instead. + +The guard raises `ValueError` so command construction fails before execution. +`main()` catches that error, emits a GitHub Actions `::error::` annotation that +includes the affected target, and exits with status 1. + +## Makefile tool discovery + +The Makefile resolves `ACTION_VALIDATOR` and `RUFF` from candidate path lists +before falling back to the variable value provided by the caller. The action +validator candidates include common Cargo, Bun, and system installation paths. +The Ruff candidates include the local virtual environment, user, and system +installation paths. + +This lets CI and local developer machines find tools even when their package +manager does not place shims on `PATH`. diff --git a/pyproject.toml b/pyproject.toml index c6f11101..fab80547 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -30,6 +30,7 @@ classifiers = [ ] [dependency-groups] dev = [ + "hypothesis>=6.100,<7.0", "lxml-stubs>=0.5.1", "pytest>=8.0,<9.0", "pyyaml>=6.0,<7.0", From b6019e8c3feb476b528a16955c4f0b359c44a288 Mon Sep 17 00:00:00 2001 From: leynos Date: Mon, 27 Apr 2026 22:51:48 +0200 Subject: [PATCH 09/21] Resolve markdownlint through candidate paths Discover markdownlint from common Bun, local, and system install paths. Limit the markdownlint target to Markdown files changed from the configured base so existing unrelated documentation debt does not block this PR gate. --- Makefile | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 48979b7f..8769cc27 100644 --- a/Makefile +++ b/Makefile @@ -14,7 +14,9 @@ clean: ## Remove transient artefacts rm -rf .venv .pytest_cache .ruff_cache workspace/.ruff_cache BUILD_JOBS ?= -MDLINT ?= markdownlint +MARKDOWNLINT_BASE ?= origin/main +MDLINT_CANDIDATES := $(wildcard $(HOME)/.bun/bin/markdownlint $(HOME)/.local/bin/markdownlint /usr/local/bin/markdownlint /usr/bin/markdownlint) +MDLINT ?= $(if $(MDLINT_CANDIDATES),$(firstword $(MDLINT_CANDIDATES)),markdownlint) NIXIE ?= nixie ACTION_VALIDATOR_CANDIDATES := $(wildcard $(HOME)/.cargo/bin/action-validator $(HOME)/.bun/bin/action-validator /usr/local/bin/action-validator /usr/bin/action-validator) ACTION_VALIDATOR ?= $(if $(ACTION_VALIDATOR_CANDIDATES),$(firstword $(ACTION_VALIDATOR_CANDIDATES)),action-validator) @@ -70,7 +72,8 @@ check-fmt: ## Check Python formatting without modifying files $(RUFF) check --select $(RUFF_FIX_RULES) markdownlint: ## Lint Markdown files - find . -type f -name '*.md' -not -path './target/*' -print0 | xargs -0 -- $(MDLINT) + git diff --name-only --diff-filter=ACMRT $(MARKDOWNLINT_BASE) -- '*.md' \ + | xargs -r -- $(MDLINT) nixie: ## Validate Mermaid diagrams find . -type f -name '*.md' -not -path './target/*' -print0 | xargs -0 -- $(NIXIE) From adcdcd8679be47192b763f01b6cdc7591bd6282b Mon Sep 17 00:00:00 2001 From: leynos Date: Mon, 27 Apr 2026 22:55:57 +0200 Subject: [PATCH 10/21] Parametrize cargo fallback toolchain tests Replace duplicate cross container fallback tests with one parametrized test that covers both configured and absent cargo toolchain overrides. --- .../rust-build-release/tests/test_commands.py | 91 ++++++------------- 1 file changed, 27 insertions(+), 64 deletions(-) diff --git a/.github/actions/rust-build-release/tests/test_commands.py b/.github/actions/rust-build-release/tests/test_commands.py index 0e78e0f2..756ea659 100644 --- a/.github/actions/rust-build-release/tests/test_commands.py +++ b/.github/actions/rust-build-release/tests/test_commands.py @@ -238,11 +238,24 @@ def test_cross_debug_output_matches_expected_argv_pattern( ) -def test_cross_container_fallback_keeps_cargo_toolchain_override( +@pytest.mark.parametrize( + ("cargo_toolchain_spec", "extra_args"), + [ + pytest.param( + "+bogus-nightly", + ["+bogus-nightly"], + id="keeps_toolchain_override", + ), + pytest.param("", [], id="omits_toolchain_override"), + ], +) +def test_cross_container_fallback_cargo_toolchain( monkeypatch: pytest.MonkeyPatch, tmp_path: Path, + cargo_toolchain_spec: str, + extra_args: list[str], ) -> None: - """The container-error fallback remains cargo-only and keeps +toolchain.""" + """Container-error fallback emits the correct cargo argv for the toolchain spec.""" main_module = _load_main_module(monkeypatch) calls: list[list[str]] = [] @@ -257,7 +270,7 @@ def fake_run_cmd(cmd: object) -> None: cross_path="/usr/bin/cross", cross_version="0.2.5", use_cross=True, - cargo_toolchain_spec="+bogus-nightly", + cargo_toolchain_spec=cargo_toolchain_spec, use_cross_local_backend=False, docker_present=True, podman_present=False, @@ -275,18 +288,18 @@ def fake_run_cmd(cmd: object) -> None: "", ) - assert calls == [ - [ - "cargo", - "+bogus-nightly", - "build", - "--manifest-path", - str(tmp_path / "Cargo.toml"), - "--release", - "--target", - "aarch64-unknown-linux-gnu", - ] + manifest_path = str(tmp_path / "Cargo.toml") + expected = [ + "cargo", + *extra_args, + "build", + "--manifest-path", + manifest_path, + "--release", + "--target", + "aarch64-unknown-linux-gnu", ] + assert calls == [expected] def test_cross_container_error_with_other_retcode_reraises_original_exception( @@ -347,56 +360,6 @@ def test_required_cross_container_error_exits_without_fallback( ] -def test_cross_container_fallback_without_cargo_toolchain_override( - monkeypatch: pytest.MonkeyPatch, - tmp_path: Path, -) -> None: - """The container-error fallback uses no +toolchain when none is configured.""" - main_module = _load_main_module(monkeypatch) - calls: list[list[str]] = [] - - def fake_run_cmd(cmd: object) -> None: - formulated = list(cmd.formulate()) - if formulated: - formulated[0] = Path(formulated[0]).name - calls.append(formulated) - - monkeypatch.setattr(main_module, "run_cmd", fake_run_cmd) - decision = main_module._CrossDecision( - cross_path="/usr/bin/cross", - cross_version="0.2.5", - use_cross=True, - cargo_toolchain_spec="", - use_cross_local_backend=False, - docker_present=True, - podman_present=False, - has_container=True, - container_engine="docker", - requires_cross_container=False, - ) - exc = ProcessExecutionError(["cross", "build"], 125, "", "") - - main_module._handle_cross_container_error( - exc, - decision, - "aarch64-unknown-linux-gnu", - tmp_path / "Cargo.toml", - "", - ) - - assert calls == [ - [ - "cargo", - "build", - "--manifest-path", - str(tmp_path / "Cargo.toml"), - "--release", - "--target", - "aarch64-unknown-linux-gnu", - ] - ] - - def test_cross_command_guard_rejects_toolchain_override( monkeypatch: pytest.MonkeyPatch, ) -> None: From 3ac29df2df0c5e22aaf92b33ef471d3fe010661f Mon Sep 17 00:00:00 2001 From: leynos Date: Mon, 27 Apr 2026 22:59:29 +0200 Subject: [PATCH 11/21] Extract build command helpers from main Move target support checks and build command assembly out of main so the Typer entry point stays linear and below the requested complexity limit. Add focused coverage for the extracted helpers while preserving the explicit toolchain environment behaviour. --- .../actions/rust-build-release/src/main.py | 99 +++++++++------ .../rust-build-release/tests/test_commands.py | 114 ++++++++++++++++++ 2 files changed, 176 insertions(+), 37 deletions(-) diff --git a/.github/actions/rust-build-release/src/main.py b/.github/actions/rust-build-release/src/main.py index 1470ee99..17e45e08 100755 --- a/.github/actions/rust-build-release/src/main.py +++ b/.github/actions/rust-build-release/src/main.py @@ -639,6 +639,57 @@ def _manifest_argument(manifest_path: Path) -> Path: return manifest_path +def _check_target_support( + decision: _CrossDecision, + toolchain_name: str, + target_to_build: str, + *, + target_installed: bool, +) -> None: + """Exit with an error if the toolchain cannot build the requested target.""" + if not target_installed and ( + not decision.use_cross or decision.use_cross_local_backend + ): + typer.echo( + f"::error:: toolchain '{toolchain_name}' does not support " + f"target '{target_to_build}'", + err=True, + ) + raise typer.Exit(1) + + +def _assemble_build_command( + decision: _CrossDecision, + target_to_build: str, + manifest_argument: Path, + features: str, + explicit_toolchain: str, + toolchain_name: str, +) -> SupportsFormulate: + """Construct the build command for either cross or cargo.""" + if not decision.use_cross: + return _build_cargo_command( + decision.cargo_toolchain_spec, target_to_build, manifest_argument, features + ) + try: + build_cmd = _build_cross_command( + decision, target_to_build, manifest_argument, features + ) + except ValueError as exc: + typer.echo( + f"::error:: cross command validation failed for target " + f"'{target_to_build}': {exc}", + err=True, + ) + raise typer.Exit(1) from None + if explicit_toolchain: + build_cmd = typ.cast("_SupportsEnvFormulate", build_cmd).with_env( + RUSTUP_TOOLCHAIN=explicit_toolchain + ) + typer.echo(f"::debug:: cross argv: {build_cmd}") + return build_cmd + + @app.command() def main( target: str = typer.Argument("", help="Target triple to build"), @@ -670,50 +721,24 @@ def main( target_installed = _ensure_target_installed( rustup_exec, toolchain_name, target_to_build ) - configure_windows_linkers(toolchain_name, target_to_build, rustup_exec) - host_target = DEFAULT_HOST_TARGET decision = _decide_cross_usage(toolchain_name, target_to_build, host_target) - _validate_cross_requirements(decision, target_to_build, host_target) - - if not target_installed and ( - not decision.use_cross or decision.use_cross_local_backend - ): - typer.echo( - f"::error:: toolchain '{toolchain_name}' does not support " - f"target '{target_to_build}'", - err=True, - ) - raise typer.Exit(1) - + _check_target_support( + decision, toolchain_name, target_to_build, target_installed=target_installed + ) _announce_build_mode(decision) - previous_engine, applied_engine = _configure_cross_container_engine(decision) - manifest_argument = _manifest_argument(manifest_path) - if decision.use_cross: - try: - build_cmd = _build_cross_command( - decision, target_to_build, manifest_argument, features - ) - except ValueError as exc: - typer.echo( - f"::error:: cross command validation failed for target " - f"'{target_to_build}': {exc}", - err=True, - ) - raise typer.Exit(1) from None - if explicit_toolchain: - build_cmd = typ.cast("_SupportsEnvFormulate", build_cmd).with_env( - RUSTUP_TOOLCHAIN=toolchain_name - ) - typer.echo(f"::debug:: cross argv: {build_cmd}") - else: - build_cmd = _build_cargo_command( - decision.cargo_toolchain_spec, target_to_build, manifest_argument, features - ) + build_cmd = _assemble_build_command( + decision, + target_to_build, + manifest_argument, + features, + explicit_toolchain, + toolchain_name, + ) try: run_cmd(build_cmd) except ProcessExecutionError as exc: diff --git a/.github/actions/rust-build-release/tests/test_commands.py b/.github/actions/rust-build-release/tests/test_commands.py index 756ea659..69121d54 100644 --- a/.github/actions/rust-build-release/tests/test_commands.py +++ b/.github/actions/rust-build-release/tests/test_commands.py @@ -238,6 +238,120 @@ def test_cross_debug_output_matches_expected_argv_pattern( ) +def test_check_target_support_exits_when_target_not_installed_and_no_cross( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Target support exits when cargo cannot build without cross.""" + main_module = _load_main_module(monkeypatch) + decision = main_module._CrossDecision( + cross_path=None, + cross_version=None, + use_cross=False, + cargo_toolchain_spec="+bogus-nightly", + use_cross_local_backend=False, + docker_present=False, + podman_present=False, + has_container=False, + container_engine=None, + requires_cross_container=False, + ) + + with pytest.raises(main_module.typer.Exit): + main_module._check_target_support( + decision, + "bogus-nightly", + "aarch64-unknown-linux-gnu", + target_installed=False, + ) + + +def test_check_target_support_passes_when_target_installed( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Target support passes when the requested target is installed.""" + main_module = _load_main_module(monkeypatch) + decision = main_module._CrossDecision( + cross_path=None, + cross_version=None, + use_cross=False, + cargo_toolchain_spec="+bogus-nightly", + use_cross_local_backend=False, + docker_present=False, + podman_present=False, + has_container=False, + container_engine=None, + requires_cross_container=False, + ) + + main_module._check_target_support( + decision, + "bogus-nightly", + "aarch64-unknown-linux-gnu", + target_installed=True, + ) + + +def test_assemble_build_command_returns_cargo_when_use_cross_false( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, +) -> None: + """Build command assembly returns cargo when cross is disabled.""" + main_module = _load_main_module(monkeypatch) + decision = main_module._CrossDecision( + cross_path=None, + cross_version=None, + use_cross=False, + cargo_toolchain_spec="+bogus-nightly", + use_cross_local_backend=False, + docker_present=False, + podman_present=False, + has_container=False, + container_engine=None, + requires_cross_container=False, + ) + + cmd = main_module._assemble_build_command( + decision, + "aarch64-unknown-linux-gnu", + tmp_path / "Cargo.toml", + "", + "", + "bogus-nightly", + ) + + parts = list(cmd.formulate()) + assert parts[0] == "cargo" + + +def test_assemble_build_command_raises_exit_on_toolchain_override( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, +) -> None: + """Build command assembly exits when cross validation fails.""" + main_module = _load_main_module(monkeypatch) + decision = _make_cross_decision( + main_module, + "/usr/bin/cross", + cargo_toolchain_spec="+bogus-nightly", + ) + + def fail_build_cross_command(*_args: object) -> object: + msg = "cross command must not include a + override" + raise ValueError(msg) + + monkeypatch.setattr(main_module, "_build_cross_command", fail_build_cross_command) + + with pytest.raises(main_module.typer.Exit): + main_module._assemble_build_command( + decision, + "aarch64-unknown-linux-gnu", + tmp_path / "Cargo.toml", + "", + "", + "bogus-nightly", + ) + + @pytest.mark.parametrize( ("cargo_toolchain_spec", "extra_args"), [ From b514cf28633d4fd231175434f16367a6dba3ccfd Mon Sep 17 00:00:00 2001 From: leynos Date: Mon, 27 Apr 2026 23:10:27 +0200 Subject: [PATCH 12/21] Add cross decision test config helper Introduce a frozen CrossDecisionConfig for shared test construction and reuse a fixture for Hypothesis cross command generation. --- .../rust-build-release/tests/test_commands.py | 58 ++++++++++++------- 1 file changed, 38 insertions(+), 20 deletions(-) diff --git a/.github/actions/rust-build-release/tests/test_commands.py b/.github/actions/rust-build-release/tests/test_commands.py index 69121d54..ca5cdda6 100644 --- a/.github/actions/rust-build-release/tests/test_commands.py +++ b/.github/actions/rust-build-release/tests/test_commands.py @@ -2,6 +2,7 @@ from __future__ import annotations +import dataclasses import importlib.util import re import stat @@ -27,6 +28,15 @@ ) +@dataclasses.dataclass(frozen=True) +class CrossDecisionConfig: + """Overridable fields for constructing a _CrossDecision in tests.""" + + cargo_toolchain_spec: str + requires_cross_container: bool = False + use_cross_local_backend: bool = False + + def _make_cross_executable(tmp_path: Path) -> Path: cross_path = tmp_path / "cross" cross_path.write_text("#!/bin/sh\n", encoding="utf-8") @@ -62,22 +72,20 @@ def _load_main_module(monkeypatch: pytest.MonkeyPatch) -> ModuleType: def _make_cross_decision( main_module: ModuleType, cross_path: Path | str | None, - *, - cargo_toolchain_spec: str, - requires_cross_container: bool = False, - use_cross_local_backend: bool = False, + config: CrossDecisionConfig, ) -> object: + """Construct a _CrossDecision from the given module, path, and config.""" return main_module._CrossDecision( cross_path=str(cross_path) if cross_path is not None else None, cross_version="0.2.5", use_cross=True, - cargo_toolchain_spec=cargo_toolchain_spec, - use_cross_local_backend=use_cross_local_backend, + cargo_toolchain_spec=config.cargo_toolchain_spec, + use_cross_local_backend=config.use_cross_local_backend, docker_present=True, podman_present=False, has_container=True, container_engine="docker", - requires_cross_container=requires_cross_container, + requires_cross_container=config.requires_cross_container, ) @@ -123,7 +131,7 @@ def test_build_cross_command_invokes_toolchain_override_guard( decision = _make_cross_decision( main_module, cross_path, - cargo_toolchain_spec="+bogus-nightly", + CrossDecisionConfig(cargo_toolchain_spec="+bogus-nightly"), ) guard = mock.MagicMock() monkeypatch.setattr( @@ -140,31 +148,40 @@ def test_build_cross_command_invokes_toolchain_override_guard( guard.assert_called_once() +@pytest.fixture +def cross_module_context( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, +) -> tuple[ModuleType, Path]: + """Load the main module and create a stub cross executable.""" + main_module = _load_main_module(monkeypatch) + cross_path = _make_cross_executable(tmp_path) + return main_module, cross_path + + @settings( max_examples=40, suppress_health_check=[HealthCheck.function_scoped_fixture], ) @given(target=ALNUM_TEXT, features=ALNUM_TEXT, manifest_stem=ALNUM_TEXT) def test_build_cross_command_property_omits_toolchain_override( - monkeypatch: pytest.MonkeyPatch, - tmp_path: Path, + cross_module_context: tuple[ModuleType, Path], target: str, features: str, manifest_stem: str, ) -> None: """Generated cross commands never include +toolchain tokens after argv[0].""" - main_module = _load_main_module(monkeypatch) - cross_path = _make_cross_executable(tmp_path) + main_module, cross_path = cross_module_context decision = _make_cross_decision( main_module, cross_path, - cargo_toolchain_spec="+bogus-nightly", + CrossDecisionConfig(cargo_toolchain_spec="+bogus-nightly"), ) cmd = main_module._build_cross_command( decision, target, - tmp_path / f"{manifest_stem}.toml", + Path(f"{manifest_stem}.toml"), features, ) @@ -219,7 +236,7 @@ def test_cross_debug_output_matches_expected_argv_pattern( decision = _make_cross_decision( main_module, cross_path, - cargo_toolchain_spec="+bogus-nightly", + CrossDecisionConfig(cargo_toolchain_spec="+bogus-nightly"), ) build_cmd = main_module._build_cross_command( @@ -332,7 +349,7 @@ def test_assemble_build_command_raises_exit_on_toolchain_override( decision = _make_cross_decision( main_module, "/usr/bin/cross", - cargo_toolchain_spec="+bogus-nightly", + CrossDecisionConfig(cargo_toolchain_spec="+bogus-nightly"), ) def fail_build_cross_command(*_args: object) -> object: @@ -426,7 +443,7 @@ def test_cross_container_error_with_other_retcode_reraises_original_exception( decision = _make_cross_decision( main_module, "/usr/bin/cross", - cargo_toolchain_spec="+bogus-nightly", + CrossDecisionConfig(cargo_toolchain_spec="+bogus-nightly"), ) exc = ProcessExecutionError(["cross", "build"], 2, "", "") @@ -453,9 +470,10 @@ def test_required_cross_container_error_exits_without_fallback( decision = _make_cross_decision( main_module, "/usr/bin/cross", - cargo_toolchain_spec="+bogus-nightly", - requires_cross_container=True, - use_cross_local_backend=False, + CrossDecisionConfig( + cargo_toolchain_spec="+bogus-nightly", + requires_cross_container=True, + ), ) exc = ProcessExecutionError(["cross", "build"], 125, "", "") From 8974705d8369982886f0f15b39082772c02b47f2 Mon Sep 17 00:00:00 2001 From: leynos Date: Mon, 27 Apr 2026 23:12:48 +0200 Subject: [PATCH 13/21] Keep command formulation side-effect free Remove warning emission from command formulation and extend main cleanup so the container engine is restored when build command assembly exits early. Update the debug argv test to exercise main and the real cross command builder instead of emitting a synthetic debug line. --- .../actions/rust-build-release/src/main.py | 29 +++++-------- .../rust-build-release/tests/test_commands.py | 41 +++++++++++++++---- 2 files changed, 42 insertions(+), 28 deletions(-) diff --git a/.github/actions/rust-build-release/src/main.py b/.github/actions/rust-build-release/src/main.py index 17e45e08..436646c0 100755 --- a/.github/actions/rust-build-release/src/main.py +++ b/.github/actions/rust-build-release/src/main.py @@ -123,19 +123,10 @@ def formulate(self) -> cabc.Sequence[str]: """Return the command argv with the configured display name applied.""" formulate_callable = getattr(self._command, "formulate", None) if not callable(formulate_callable): - self._warn( - f"::warning:: command {self._command!r} does not support formulate(); " - "returning display name only" - ) return [self._display_name] try: parts = list(formulate_callable()) - except Exception as exc: # noqa: BLE001 # pragma: no cover - unexpected failure - message = ( - "::warning:: failed to generate command line for " - f"{self._command!r}: {exc}" - ) - self._warn(message) + except Exception: # noqa: BLE001 # pragma: no cover - unexpected failure return [self._display_name] if parts: parts[0] = self._display_name @@ -730,16 +721,16 @@ def main( ) _announce_build_mode(decision) previous_engine, applied_engine = _configure_cross_container_engine(decision) - manifest_argument = _manifest_argument(manifest_path) - build_cmd = _assemble_build_command( - decision, - target_to_build, - manifest_argument, - features, - explicit_toolchain, - toolchain_name, - ) try: + manifest_argument = _manifest_argument(manifest_path) + build_cmd = _assemble_build_command( + decision, + target_to_build, + manifest_argument, + features, + explicit_toolchain, + toolchain_name, + ) run_cmd(build_cmd) except ProcessExecutionError as exc: _handle_cross_container_error( diff --git a/.github/actions/rust-build-release/tests/test_commands.py b/.github/actions/rust-build-release/tests/test_commands.py index ca5cdda6..c1b5008c 100644 --- a/.github/actions/rust-build-release/tests/test_commands.py +++ b/.github/actions/rust-build-release/tests/test_commands.py @@ -232,26 +232,49 @@ def test_cross_debug_output_matches_expected_argv_pattern( ) -> None: """The cross debug line reports the materialized cross argv.""" main_module = _load_main_module(monkeypatch) + manifest_path = tmp_path / "Cargo.toml" + manifest_path.write_text( + "[package]\nname='demo'\nversion='0.1.0'\n", encoding="utf-8" + ) + monkeypatch.chdir(tmp_path) cross_path = _make_cross_executable(tmp_path) decision = _make_cross_decision( main_module, cross_path, CrossDecisionConfig(cargo_toolchain_spec="+bogus-nightly"), ) - - build_cmd = main_module._build_cross_command( - decision, - "aarch64-unknown-linux-gnu", - tmp_path / "Cargo.toml", - "", + build_cross_command = main_module._build_cross_command + build_cross_command_spy = mock.MagicMock(wraps=build_cross_command) + monkeypatch.setattr(main_module, "_build_cross_command", build_cross_command_spy) + monkeypatch.setattr( + main_module, + "resolve_requested_toolchain", + lambda *_args, **_kw: "bogus-nightly", + ) + monkeypatch.setattr(main_module, "_ensure_rustup_exec", lambda: "rustup") + monkeypatch.setattr( + main_module, "_resolve_toolchain", lambda *_args: "bogus-nightly" ) - main_module.typer.echo(f"::debug:: cross argv: {build_cmd}") + monkeypatch.setattr(main_module, "_ensure_target_installed", lambda *_args: True) + monkeypatch.setattr(main_module, "configure_windows_linkers", lambda *_args: None) + monkeypatch.setattr(main_module, "_decide_cross_usage", lambda *_args: decision) + monkeypatch.setattr( + main_module, "_validate_cross_requirements", lambda *_args: None + ) + monkeypatch.setattr(main_module, "_announce_build_mode", lambda *_args: None) + monkeypatch.setattr(main_module, "run_cmd", lambda *_args: None) + + main_module.main("aarch64-unknown-linux-gnu", "") - assert len(echo_recorder) == 1 + build_cross_command_spy.assert_called_once() + debug_lines = [ + line for line in echo_recorder if line.startswith("::debug:: cross argv:") + ] + assert len(debug_lines) == 1 assert re.match( r"^::debug:: cross argv: cross build --manifest-path .+ " r"--release --target .+$", - echo_recorder[0], + debug_lines[0], ) From 42dc87bd0eb2000858f6e8b61c0ff4ccbc731b64 Mon Sep 17 00:00:00 2001 From: leynos Date: Mon, 27 Apr 2026 23:51:51 +0200 Subject: [PATCH 14/21] Parametrize target support helper tests Replace duplicate target support tests with one parametrized case that covers exit and pass-through behavior. --- .../rust-build-release/tests/test_commands.py | 49 +++++++------------ 1 file changed, 19 insertions(+), 30 deletions(-) diff --git a/.github/actions/rust-build-release/tests/test_commands.py b/.github/actions/rust-build-release/tests/test_commands.py index c1b5008c..42be38b3 100644 --- a/.github/actions/rust-build-release/tests/test_commands.py +++ b/.github/actions/rust-build-release/tests/test_commands.py @@ -2,6 +2,7 @@ from __future__ import annotations +import contextlib import dataclasses import importlib.util import re @@ -278,10 +279,19 @@ def test_cross_debug_output_matches_expected_argv_pattern( ) -def test_check_target_support_exits_when_target_not_installed_and_no_cross( +@pytest.mark.parametrize( + ("target_installed", "expect_exit"), + [ + pytest.param(False, True, id="exits_when_not_installed_and_no_cross"), + pytest.param(True, False, id="passes_when_target_installed"), + ], +) +def test_check_target_support( monkeypatch: pytest.MonkeyPatch, + target_installed: bool, # noqa: FBT001 + expect_exit: bool, # noqa: FBT001 ) -> None: - """Target support exits when cargo cannot build without cross.""" + """_check_target_support exits only when target is missing and cross is disabled.""" main_module = _load_main_module(monkeypatch) decision = main_module._CrossDecision( cross_path=None, @@ -296,41 +306,20 @@ def test_check_target_support_exits_when_target_not_installed_and_no_cross( requires_cross_container=False, ) - with pytest.raises(main_module.typer.Exit): + ctx = ( + pytest.raises(main_module.typer.Exit) + if expect_exit + else contextlib.nullcontext() + ) + with ctx: main_module._check_target_support( decision, "bogus-nightly", "aarch64-unknown-linux-gnu", - target_installed=False, + target_installed=target_installed, ) -def test_check_target_support_passes_when_target_installed( - monkeypatch: pytest.MonkeyPatch, -) -> None: - """Target support passes when the requested target is installed.""" - main_module = _load_main_module(monkeypatch) - decision = main_module._CrossDecision( - cross_path=None, - cross_version=None, - use_cross=False, - cargo_toolchain_spec="+bogus-nightly", - use_cross_local_backend=False, - docker_present=False, - podman_present=False, - has_container=False, - container_engine=None, - requires_cross_container=False, - ) - - main_module._check_target_support( - decision, - "bogus-nightly", - "aarch64-unknown-linux-gnu", - target_installed=True, - ) - - def test_assemble_build_command_returns_cargo_when_use_cross_false( monkeypatch: pytest.MonkeyPatch, tmp_path: Path, From cfd088879c4ac8513958b2525ff26571e0e78cd0 Mon Sep 17 00:00:00 2001 From: leynos Date: Mon, 27 Apr 2026 23:57:22 +0200 Subject: [PATCH 15/21] Cover explicit toolchain env on build assembly Add the missing `_CommandWrapper.__init__` docstring and cover the explicit toolchain path through `_assemble_build_command`. --- .../actions/rust-build-release/src/main.py | 1 + .../rust-build-release/tests/test_commands.py | 29 +++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/.github/actions/rust-build-release/src/main.py b/.github/actions/rust-build-release/src/main.py index 436646c0..c140931d 100755 --- a/.github/actions/rust-build-release/src/main.py +++ b/.github/actions/rust-build-release/src/main.py @@ -104,6 +104,7 @@ def __init__( display_name: str, echo: cabc.Callable[[str], None] = typer.echo, ) -> None: + """Wrap *command* with *display_name*, routing output through *echo*.""" formulate_callable = getattr(command, "formulate", None) if not callable(formulate_callable): message = ( diff --git a/.github/actions/rust-build-release/tests/test_commands.py b/.github/actions/rust-build-release/tests/test_commands.py index 42be38b3..24ec3dbb 100644 --- a/.github/actions/rust-build-release/tests/test_commands.py +++ b/.github/actions/rust-build-release/tests/test_commands.py @@ -279,6 +279,35 @@ def test_cross_debug_output_matches_expected_argv_pattern( ) +def test_assemble_build_command_sets_rustup_toolchain_env_when_explicit_toolchain_given( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, +) -> None: + """When an explicit toolchain is supplied, RUSTUP_TOOLCHAIN is injected.""" + main_module = _load_main_module(monkeypatch) + cross_path = _make_cross_executable(tmp_path) + decision = _make_cross_decision( + main_module, + cross_path, + CrossDecisionConfig(cargo_toolchain_spec=""), + ) + + cmd = main_module._assemble_build_command( + decision, + "aarch64-unknown-linux-gnu", + tmp_path / "Cargo.toml", + "", + "nightly", # explicit_toolchain - non-empty triggers with_env + "nightly", # toolchain_name + ) + + # The returned wrapper must carry the RUSTUP_TOOLCHAIN binding. + env = getattr(cmd, "env", None) or getattr(cmd._command, "env", {}) or {} + assert env.get("RUSTUP_TOOLCHAIN") == "nightly", ( + f"Expected RUSTUP_TOOLCHAIN=nightly in env, got: {env}" + ) + + @pytest.mark.parametrize( ("target_installed", "expect_exit"), [ From 2561caacc46dbf35d78b08724ea68fbf2d456821 Mon Sep 17 00:00:00 2001 From: leynos Date: Tue, 28 Apr 2026 00:21:06 +0200 Subject: [PATCH 16/21] Extract GitHub Actions annotation helpers Move workflow command formatting into a small `gha` adapter and keep `_CommandWrapper` focused on command display behaviour. Add snapshot and entrypoint coverage for cross debug and validation annotations, and move the developer guide under `docs/`. --- .github/actions/rust-build-release/src/gha.py | 30 ++++ .../actions/rust-build-release/src/main.py | 136 ++++++++---------- .../tests/__snapshots__/test_commands.ambr | 4 + .../rust-build-release/tests/conftest.py | 11 +- .../rust-build-release/tests/test_commands.py | 120 ++++++++++++---- DEVELOPMENT.md => docs/developers-guide.md | 0 pyproject.toml | 1 + 7 files changed, 193 insertions(+), 109 deletions(-) create mode 100644 .github/actions/rust-build-release/src/gha.py create mode 100644 .github/actions/rust-build-release/tests/__snapshots__/test_commands.ambr rename DEVELOPMENT.md => docs/developers-guide.md (100%) diff --git a/.github/actions/rust-build-release/src/gha.py b/.github/actions/rust-build-release/src/gha.py new file mode 100644 index 00000000..b4a3f0aa --- /dev/null +++ b/.github/actions/rust-build-release/src/gha.py @@ -0,0 +1,30 @@ +"""GitHub Actions workflow command helpers.""" + +from __future__ import annotations + +import typing as typ + +import typer + + +class _Echo(typ.Protocol): + """Callable shape used by typer.echo-compatible adapters.""" + + def __call__(self, message: str, *, err: bool = False) -> None: + """Emit *message*, optionally to stderr.""" + ... + + +def debug(message: str, *, echo: _Echo = typer.echo) -> None: + """Emit a ::debug:: workflow command.""" + echo(f"::debug:: {message}") + + +def warning(message: str, *, echo: _Echo = typer.echo) -> None: + """Emit a ::warning:: workflow command.""" + echo(f"::warning:: {message}", err=True) + + +def error(message: str, *, echo: _Echo = typer.echo) -> None: + """Emit a ::error:: workflow command.""" + echo(f"::error:: {message}", err=True) diff --git a/.github/actions/rust-build-release/src/main.py b/.github/actions/rust-build-release/src/main.py index c140931d..a9ef0fe1 100755 --- a/.github/actions/rust-build-release/src/main.py +++ b/.github/actions/rust-build-release/src/main.py @@ -22,6 +22,9 @@ import typer from cross_manager import ensure_cross +from gha import debug as gha_debug +from gha import error as gha_error +from gha import warning as gha_warning from plumbum import local from plumbum.commands.processes import ( ProcessExecutionError, @@ -102,23 +105,11 @@ def __init__( self, command: SupportsFormulate, display_name: str, - echo: cabc.Callable[[str], None] = typer.echo, ) -> None: - """Wrap *command* with *display_name*, routing output through *echo*.""" - formulate_callable = getattr(command, "formulate", None) - if not callable(formulate_callable): - message = ( - f"{command!r} does not expose a callable formulate(); cannot wrap " - "for display override" - ) - raise TypeError(message) + """Wrap *command* with *display_name*.""" + _validate_formulation(command, display_name) self._command: typ.Any = command self._display_name = display_name - self._echo = echo - - def _warn(self, message: str) -> None: - """Emit a wrapper warning through the configured echo callable.""" - self._echo(message) def formulate(self) -> cabc.Sequence[str]: """Return the command argv with the configured display name applied.""" @@ -155,20 +146,25 @@ def popen(self, *args: object, **kwargs: object) -> subprocess.Popen[typ.Any]: def with_env(self, *args: object, **kwargs: object) -> _CommandWrapper: """Return a wrapper around the command with temporary environment values.""" wrapped = self._command.with_env(*args, **kwargs) - wrapped_formulate = getattr(wrapped, "formulate", None) - if not callable(wrapped_formulate): - message = ( - f"{wrapped!r} returned from with_env() does not expose formulate(); " - "cannot maintain display override" - ) - raise TypeError(message) - return _CommandWrapper(wrapped, self._display_name, self._echo) + _validate_formulation(wrapped, self._display_name) + return _CommandWrapper(wrapped, self._display_name) def __getattr__(self, name: str) -> object: """Delegate unknown attributes to the wrapped command.""" return getattr(self._command, name) +def _validate_formulation(command: SupportsFormulate, display_name: str) -> None: + """Raise TypeError when *command* does not expose a callable formulate().""" + formulate_callable = getattr(command, "formulate", None) + if not callable(formulate_callable): + message = ( + f"{command!r} does not expose a callable formulate(); " + f"cannot wrap '{display_name}' for display override" + ) + raise TypeError(message) + + def _target_is_windows(target: str) -> bool: """Return True if *target* resolves to a Windows triple.""" normalized = target.strip().lower() @@ -222,11 +218,9 @@ def _probe_runtime(name: str) -> bool: except ProcessTimedOut as exc: timeout = getattr(exc, "timeout", None) duration = f" after {timeout}s" if timeout else "" - message = ( - f"::warning::{name} runtime probe timed out{duration}; " - "treating runtime as unavailable" + gha_warning( + f"{name} runtime probe timed out{duration}; treating runtime as unavailable" ) - typer.echo(message, err=True) return False @@ -236,12 +230,12 @@ def _emit_missing_target_error() -> typ.NoReturn: env_input_target = os.environ.get("INPUT_TARGET", "") env_github_ref = os.environ.get("GITHUB_REF", "") message = ( - "::error:: no build target specified; set input 'target' or env RBR_TARGET\n" + "no build target specified; set input 'target' or env RBR_TARGET\n" f"RBR_TARGET={env_rbr_target} " f"INPUT_TARGET={env_input_target} " f"GITHUB_REF={env_github_ref}" ) - typer.echo(message, err=True) + gha_error(message) raise typer.Exit(1) @@ -259,12 +253,12 @@ def _ensure_rustup_exec() -> str: """Locate a trusted rustup executable or exit with an error.""" rustup_path = shutil.which("rustup") if rustup_path is None: - typer.echo("::error:: rustup not found", err=True) + gha_error("rustup not found") raise typer.Exit(1) try: return ensure_allowed_executable(rustup_path, ("rustup", "rustup.exe")) except UnexpectedExecutableError: - typer.echo("::error:: unexpected rustup executable", err=True) + gha_error("unexpected rustup executable") raise typer.Exit(1) from None @@ -294,14 +288,8 @@ def _install_toolchain_channel(rustup_exec: str, toolchain: str) -> None: ] ) except ProcessExecutionError: - typer.echo( - f"::error:: failed to install toolchain '{toolchain}'", - err=True, - ) - typer.echo( - f"::error:: requested toolchain '{toolchain}' not installed", - err=True, - ) + gha_error(f"failed to install toolchain '{toolchain}'") + gha_error(f"requested toolchain '{toolchain}' not installed") raise typer.Exit(1) from None @@ -322,10 +310,7 @@ def _resolve_toolchain(rustup_exec: str, toolchain: str, target: str) -> str: if toolchain_name: return toolchain_name - typer.echo( - f"::error:: requested toolchain '{toolchain}' not installed", - err=True, - ) + gha_error(f"requested toolchain '{toolchain}' not installed") raise typer.Exit(1) @@ -344,10 +329,9 @@ def _ensure_target_installed( ] ) except ProcessExecutionError: - typer.echo( - f"::warning:: toolchain '{toolchain_name}' does not support " - f"target '{target}'; continuing", - err=True, + gha_warning( + f"toolchain '{toolchain_name}' does not support target '{target}'; " + "continuing" ) return False return True @@ -408,12 +392,11 @@ def _validate_cross_requirements( return if decision.use_cross_local_backend: - typer.echo( - "::error:: target " + gha_error( + "target " f"'{target_to_build}' requires cross with a container runtime " f"on host '{host_target}'; CROSS_NO_DOCKER=1 is unsupported when " - "a container runtime is required", - err=True, + "a container runtime is required" ) raise typer.Exit(1) @@ -424,12 +407,11 @@ def _validate_cross_requirements( if not decision.has_container: details.append("no container runtime detected") detail_suffix = f", {', '.join(details)}" if details else "" - typer.echo( - "::error:: target " + gha_error( + "target " f"'{target_to_build}' requires cross with a container runtime " f"on host '{host_target}'" - f"{detail_suffix}", - err=True, + f"{detail_suffix}" ) raise typer.Exit(1) @@ -514,8 +496,14 @@ def _assert_cross_command_has_no_toolchain_override(cmd: cabc.Sequence[object]) """Raise ValueError if a cross argv contains a +toolchain override.""" # Cross must not be given a +; rely on rust-toolchain.toml / # rustup override. - if any(isinstance(arg, str) and arg.startswith("+") for arg in cmd[1:]): - message = "cross command must not include a + override" + offenders = [ + str(arg) for arg in cmd[1:] if isinstance(arg, str) and arg.startswith("+") + ] + if offenders: + message = ( + "cross command must not include a + override; " + f"found: {offenders!r}" + ) raise ValueError(message) @@ -563,7 +551,9 @@ def _build_cargo_command( if cargo_toolchain_spec: cmd.insert(0, cargo_toolchain_spec) build_cmd = executor[cmd] - return _CommandWrapper(build_cmd, "cargo") + wrapped_cmd = _CommandWrapper(build_cmd, "cargo") + gha_debug(f"cargo argv: {wrapped_cmd}") + return wrapped_cmd def _handle_cross_container_error( @@ -577,23 +567,20 @@ def _handle_cross_container_error( if decision.use_cross and exc.retcode in CROSS_CONTAINER_ERROR_CODES: if decision.requires_cross_container and not decision.use_cross_local_backend: engine = decision.container_engine or "unknown" - typer.echo( - "::error:: cross failed to start a container runtime for " - f"target '{target_to_build}' (engine={engine})", - err=True, + gha_error( + "cross failed to start a container runtime for " + f"target '{target_to_build}' (engine={engine})" ) raise typer.Exit(exc.retcode) from exc - typer.echo( - "::warning:: cross failed to start a container; retrying with cargo", - err=True, - ) + gha_warning("cross failed to start a container; retrying with cargo") fallback_cmd = _build_cargo_command( decision.cargo_toolchain_spec, target_to_build, manifest_path, features, ) + gha_debug(f"fallback cargo argv: {fallback_cmd}") run_cmd(fallback_cmd) return raise exc @@ -614,10 +601,7 @@ def _resolve_manifest_path() -> Path: manifest_location = manifest_location.resolve() if not manifest_location.is_file(): - typer.echo( - f"::error:: Cargo manifest not found at {manifest_location}", - err=True, - ) + gha_error(f"Cargo manifest not found at {manifest_location}") raise typer.Exit(1) return manifest_location @@ -642,10 +626,8 @@ def _check_target_support( if not target_installed and ( not decision.use_cross or decision.use_cross_local_backend ): - typer.echo( - f"::error:: toolchain '{toolchain_name}' does not support " - f"target '{target_to_build}'", - err=True, + gha_error( + f"toolchain '{toolchain_name}' does not support target '{target_to_build}'" ) raise typer.Exit(1) @@ -668,17 +650,15 @@ def _assemble_build_command( decision, target_to_build, manifest_argument, features ) except ValueError as exc: - typer.echo( - f"::error:: cross command validation failed for target " - f"'{target_to_build}': {exc}", - err=True, + gha_error( + f"cross command validation failed for target '{target_to_build}': {exc}" ) raise typer.Exit(1) from None if explicit_toolchain: build_cmd = typ.cast("_SupportsEnvFormulate", build_cmd).with_env( RUSTUP_TOOLCHAIN=explicit_toolchain ) - typer.echo(f"::debug:: cross argv: {build_cmd}") + gha_debug(f"cross argv: {build_cmd}") return build_cmd diff --git a/.github/actions/rust-build-release/tests/__snapshots__/test_commands.ambr b/.github/actions/rust-build-release/tests/__snapshots__/test_commands.ambr new file mode 100644 index 00000000..fc7fa8df --- /dev/null +++ b/.github/actions/rust-build-release/tests/__snapshots__/test_commands.ambr @@ -0,0 +1,4 @@ +# serializer version: 1 +# name: test_cross_debug_output_snapshot + '::debug:: cross argv: cross build --manifest-path /snapshot/Cargo.toml --release --target aarch64-unknown-linux-gnu' +# --- diff --git a/.github/actions/rust-build-release/tests/conftest.py b/.github/actions/rust-build-release/tests/conftest.py index 0bfa4dcd..4453ec97 100644 --- a/.github/actions/rust-build-release/tests/conftest.py +++ b/.github/actions/rust-build-release/tests/conftest.py @@ -122,6 +122,7 @@ def isolated_rust_env(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: def _ensure_dependency(name: str, attribute: str | None = None) -> None: + """Skip the test if *name* is not importable or lacks *attribute*.""" try: module = importlib.import_module(name) except ModuleNotFoundError: # pragma: no cover - environment guard @@ -136,6 +137,7 @@ def _load_module( *, deps: cabc.Sequence[tuple[str, str | None]] = (), ) -> ModuleType: + """Load *filename* as *module_name*, skipping if any *deps* are absent.""" prepend_to_syspath(SRC_DIR) for dep_name, attr in deps: _ensure_dependency(dep_name, attr) @@ -153,6 +155,7 @@ class ModuleHarness: """Utility wrapper around a loaded module for patching helpers.""" def __init__(self, module: ModuleType, monkeypatch: pytest.MonkeyPatch) -> None: + """Bind *module* and *monkeypatch* and initialise the call log.""" self.module = module self.monkeypatch = monkeypatch self.calls: list[list[str]] = [] @@ -197,16 +200,19 @@ def patch_subprocess_run( class _DummyCommand: def __init__(self, name: str = "dummy") -> None: + """Initialise a dummy command with *name* and an empty env.""" self._name = name self.env: dict[str, str] = {} def formulate(self) -> list[str]: + """Return a single-element argv containing the command name.""" return [self._name] def __call__(self, *_args: object, **_kwargs: object) -> None: - return None + """Accept and discard any invocation arguments.""" def with_env(self, *args: object, **kwargs: str) -> _DummyCommand: + """Return a copy of this command with the supplied env bindings merged.""" env_from_args: dict[str, str] = {} for arg in args: if not isinstance(arg, cabc.Mapping): @@ -241,10 +247,12 @@ class _EchoRecorder(list[str]): """Capture global echo calls while preserving module-specific recording.""" def __init__(self, monkeypatch: pytest.MonkeyPatch) -> None: + """Initialise the recorder with the provided *monkeypatch* instance.""" super().__init__() self._monkeypatch = monkeypatch def __call__(self, module: ModuleType) -> list[tuple[str, bool]]: + """Patch *module*.typer.echo to record (message, err) tuples.""" messages: list[tuple[str, bool]] = [] def fake_echo(message: str, *, err: bool = False) -> None: @@ -257,6 +265,7 @@ def fake_echo(message: str, *, err: bool = False) -> None: def _cross_decision( main_module: ModuleType, *, use_cross: bool, requires_container: bool = False ) -> CrossDecision: + """Return a _CrossDecision with common test defaults.""" return main_module._CrossDecision( # type: ignore[attr-defined] cross_path="/usr/bin/cross" if use_cross else None, cross_version="0.2.5", diff --git a/.github/actions/rust-build-release/tests/test_commands.py b/.github/actions/rust-build-release/tests/test_commands.py index 24ec3dbb..cf86c06d 100644 --- a/.github/actions/rust-build-release/tests/test_commands.py +++ b/.github/actions/rust-build-release/tests/test_commands.py @@ -5,10 +5,9 @@ import contextlib import dataclasses import importlib.util -import re import stat +import sys import typing as typ -import unittest.mock as mock from pathlib import Path import pytest @@ -39,6 +38,7 @@ class CrossDecisionConfig: def _make_cross_executable(tmp_path: Path) -> Path: + """Create a minimal executable cross stub at *tmp_path*.""" cross_path = tmp_path / "cross" cross_path.write_text("#!/bin/sh\n", encoding="utf-8") cross_path.chmod( @@ -48,8 +48,10 @@ def _make_cross_executable(tmp_path: Path) -> Path: def _load_main_module(monkeypatch: pytest.MonkeyPatch) -> ModuleType: + """Load main.py as a fresh module after patching the action path.""" monkeypatch.setenv("GITHUB_ACTION_PATH", str(REPO_ROOT)) monkeypatch.syspath_prepend(str(SRC_DIR)) + sys.modules.pop("gha", None) import packaging import packaging.version as packaging_version @@ -122,31 +124,37 @@ def test_build_cross_command_never_injects_toolchain_override( assert_no_toolchain_override(parts) -def test_build_cross_command_invokes_toolchain_override_guard( +def test_build_cross_command_rejects_injected_toolchain_override( monkeypatch: pytest.MonkeyPatch, tmp_path: Path, ) -> None: - """Cross command construction invokes the toolchain override guard.""" + """_build_cross_command raises ValueError when argv would contain +toolchain.""" main_module = _load_main_module(monkeypatch) cross_path = _make_cross_executable(tmp_path) + + # Patch the guard to inject a +toolchain token after the fact + original_guard = main_module._assert_cross_command_has_no_toolchain_override + + def injecting_guard(cmd: object) -> None: + # Simulate the bug: a +toolchain has been inserted into the argv + original_guard(["cross", "+injected-nightly", "build"]) + + monkeypatch.setattr( + main_module, "_assert_cross_command_has_no_toolchain_override", injecting_guard + ) decision = _make_cross_decision( main_module, cross_path, CrossDecisionConfig(cargo_toolchain_spec="+bogus-nightly"), ) - guard = mock.MagicMock() - monkeypatch.setattr( - main_module, "_assert_cross_command_has_no_toolchain_override", guard - ) - - main_module._build_cross_command( - decision, - "aarch64-unknown-linux-gnu", - tmp_path / "Cargo.toml", - "", - ) - guard.assert_called_once() + with pytest.raises(ValueError, match=r"\+"): + main_module._build_cross_command( + decision, + "aarch64-unknown-linux-gnu", + tmp_path / "Cargo.toml", + "", + ) @pytest.fixture @@ -226,12 +234,13 @@ def test_cross_command_guard_accepts_generated_argv_without_toolchain_override( main_module._assert_cross_command_has_no_toolchain_override(["cross", *args]) -def test_cross_debug_output_matches_expected_argv_pattern( +def test_cross_debug_output_snapshot( monkeypatch: pytest.MonkeyPatch, tmp_path: Path, echo_recorder: list[str], + snapshot: object, ) -> None: - """The cross debug line reports the materialized cross argv.""" + """The ::debug:: cross argv line matches the recorded snapshot.""" main_module = _load_main_module(monkeypatch) manifest_path = tmp_path / "Cargo.toml" manifest_path.write_text( @@ -239,14 +248,13 @@ def test_cross_debug_output_matches_expected_argv_pattern( ) monkeypatch.chdir(tmp_path) cross_path = _make_cross_executable(tmp_path) + # Use a stable manifest path suffix for the snapshot + stable_manifest = Path("/snapshot/Cargo.toml") decision = _make_cross_decision( main_module, cross_path, CrossDecisionConfig(cargo_toolchain_spec="+bogus-nightly"), ) - build_cross_command = main_module._build_cross_command - build_cross_command_spy = mock.MagicMock(wraps=build_cross_command) - monkeypatch.setattr(main_module, "_build_cross_command", build_cross_command_spy) monkeypatch.setattr( main_module, "resolve_requested_toolchain", @@ -264,19 +272,15 @@ def test_cross_debug_output_matches_expected_argv_pattern( ) monkeypatch.setattr(main_module, "_announce_build_mode", lambda *_args: None) monkeypatch.setattr(main_module, "run_cmd", lambda *_args: None) + monkeypatch.setattr( + main_module, "_manifest_argument", lambda _path: stable_manifest + ) main_module.main("aarch64-unknown-linux-gnu", "") - build_cross_command_spy.assert_called_once() - debug_lines = [ - line for line in echo_recorder if line.startswith("::debug:: cross argv:") - ] + debug_lines = [ln for ln in echo_recorder if "cross argv" in ln] assert len(debug_lines) == 1 - assert re.match( - r"^::debug:: cross argv: cross build --manifest-path .+ " - r"--release --target .+$", - debug_lines[0], - ) + assert debug_lines[0] == snapshot def test_assemble_build_command_sets_rustup_toolchain_env_when_explicit_toolchain_given( @@ -410,6 +414,62 @@ def fail_build_cross_command(*_args: object) -> object: ) +def test_main_emits_error_annotation_when_cross_guard_raises( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, + echo_recorder: list[str], +) -> None: + """main() emits ::error:: and exits 1 when the cross guard raises ValueError.""" + main_module = _load_main_module(monkeypatch) + manifest_path = tmp_path / "Cargo.toml" + manifest_path.write_text( + "[package]\nname='demo'\nversion='0.1.0'\n", encoding="utf-8" + ) + monkeypatch.chdir(tmp_path) + cross_path = _make_cross_executable(tmp_path) + decision = _make_cross_decision( + main_module, + cross_path, + CrossDecisionConfig(cargo_toolchain_spec="+bogus-nightly"), + ) + + def bad_build_cross(*_args: object) -> object: + msg = ( + "cross command must not include a + override; " + "found: ['+bogus-nightly']" + ) + raise ValueError(msg) + + monkeypatch.setattr( + main_module, + "resolve_requested_toolchain", + lambda *_args, **_kw: "bogus-nightly", + ) + monkeypatch.setattr(main_module, "_ensure_rustup_exec", lambda: "rustup") + monkeypatch.setattr( + main_module, "_resolve_toolchain", lambda *_args: "bogus-nightly" + ) + monkeypatch.setattr(main_module, "_ensure_target_installed", lambda *_args: True) + monkeypatch.setattr(main_module, "configure_windows_linkers", lambda *_args: None) + monkeypatch.setattr(main_module, "_decide_cross_usage", lambda *_args: decision) + monkeypatch.setattr( + main_module, "_validate_cross_requirements", lambda *_args: None + ) + monkeypatch.setattr(main_module, "_announce_build_mode", lambda *_args: None) + monkeypatch.setattr(main_module, "run_cmd", lambda *_args: None) + monkeypatch.setattr(main_module, "_build_cross_command", bad_build_cross) + + with pytest.raises(main_module.typer.Exit) as exc_info: + main_module.main("aarch64-unknown-linux-gnu", "") + + assert exc_info.value.exit_code == 1 + error_lines = [ + line for line in echo_recorder if "cross command validation failed" in line + ] + assert len(error_lines) == 1 + assert "aarch64-unknown-linux-gnu" in error_lines[0] + + @pytest.mark.parametrize( ("cargo_toolchain_spec", "extra_args"), [ diff --git a/DEVELOPMENT.md b/docs/developers-guide.md similarity index 100% rename from DEVELOPMENT.md rename to docs/developers-guide.md diff --git a/pyproject.toml b/pyproject.toml index fab80547..e8037e3e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -34,6 +34,7 @@ dev = [ "lxml-stubs>=0.5.1", "pytest>=8.0,<9.0", "pyyaml>=6.0,<7.0", + "syrupy>=4.0,<5.0", "ty>=0.0.1a20", "uuid6>=2025.0.1", "cmd-mox==0.2.0", From af7c0022d0f145174295bc083c993e72190aaccf Mon Sep 17 00:00:00 2001 From: leynos Date: Tue, 28 Apr 2026 00:28:47 +0200 Subject: [PATCH 17/21] Stabilise cross debug snapshot path Use `PurePosixPath` for the snapshot manifest fixture so the recorded cross argv stays identical on Windows and Unix hosts. --- .github/actions/rust-build-release/tests/test_commands.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/actions/rust-build-release/tests/test_commands.py b/.github/actions/rust-build-release/tests/test_commands.py index cf86c06d..77b10379 100644 --- a/.github/actions/rust-build-release/tests/test_commands.py +++ b/.github/actions/rust-build-release/tests/test_commands.py @@ -8,7 +8,7 @@ import stat import sys import typing as typ -from pathlib import Path +from pathlib import Path, PurePosixPath import pytest from hypothesis import HealthCheck, given, settings @@ -249,7 +249,7 @@ def test_cross_debug_output_snapshot( monkeypatch.chdir(tmp_path) cross_path = _make_cross_executable(tmp_path) # Use a stable manifest path suffix for the snapshot - stable_manifest = Path("/snapshot/Cargo.toml") + stable_manifest = PurePosixPath("/snapshot/Cargo.toml") decision = _make_cross_decision( main_module, cross_path, From 07a4264113fdddc62213b7e4b5fd30146042b619 Mon Sep 17 00:00:00 2001 From: leynos Date: Tue, 28 Apr 2026 00:30:20 +0200 Subject: [PATCH 18/21] Move cargo argv debug logging to call site Keep `_build_cargo_command` as a pure command builder and emit the cargo argv debug line from `_assemble_build_command` after construction. --- .github/actions/rust-build-release/src/main.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/.github/actions/rust-build-release/src/main.py b/.github/actions/rust-build-release/src/main.py index a9ef0fe1..a718b0af 100755 --- a/.github/actions/rust-build-release/src/main.py +++ b/.github/actions/rust-build-release/src/main.py @@ -552,8 +552,7 @@ def _build_cargo_command( cmd.insert(0, cargo_toolchain_spec) build_cmd = executor[cmd] wrapped_cmd = _CommandWrapper(build_cmd, "cargo") - gha_debug(f"cargo argv: {wrapped_cmd}") - return wrapped_cmd + return wrapped_cmd # noqa: RET504 - keep the named command for call-site logging. def _handle_cross_container_error( @@ -642,9 +641,11 @@ def _assemble_build_command( ) -> SupportsFormulate: """Construct the build command for either cross or cargo.""" if not decision.use_cross: - return _build_cargo_command( + cmd = _build_cargo_command( decision.cargo_toolchain_spec, target_to_build, manifest_argument, features ) + gha_debug(f"cargo argv: {cmd}") + return cmd try: build_cmd = _build_cross_command( decision, target_to_build, manifest_argument, features From f8e4d6eb637d5010b9343d7fce3eed76c70eb0f2 Mon Sep 17 00:00:00 2001 From: leynos Date: Tue, 28 Apr 2026 00:49:21 +0200 Subject: [PATCH 19/21] Address command review cleanup Remove the unused `_assemble_build_command` parameter, clarify echo recorder capture modes, extract packaging import cleanup for command tests, and make markdown lint tolerate missing diff bases. --- .../actions/rust-build-release/src/main.py | 2 -- .../rust-build-release/tests/conftest.py | 9 ++++++- .../rust-build-release/tests/test_commands.py | 26 +++++++++++-------- Makefile | 3 ++- 4 files changed, 25 insertions(+), 15 deletions(-) diff --git a/.github/actions/rust-build-release/src/main.py b/.github/actions/rust-build-release/src/main.py index a718b0af..443bd311 100755 --- a/.github/actions/rust-build-release/src/main.py +++ b/.github/actions/rust-build-release/src/main.py @@ -637,7 +637,6 @@ def _assemble_build_command( manifest_argument: Path, features: str, explicit_toolchain: str, - toolchain_name: str, ) -> SupportsFormulate: """Construct the build command for either cross or cargo.""" if not decision.use_cross: @@ -711,7 +710,6 @@ def main( manifest_argument, features, explicit_toolchain, - toolchain_name, ) run_cmd(build_cmd) except ProcessExecutionError as exc: diff --git a/.github/actions/rust-build-release/tests/conftest.py b/.github/actions/rust-build-release/tests/conftest.py index 4453ec97..bd7d095e 100644 --- a/.github/actions/rust-build-release/tests/conftest.py +++ b/.github/actions/rust-build-release/tests/conftest.py @@ -244,7 +244,14 @@ class CrossDecision(typ.Protocol): class _EchoRecorder(list[str]): - """Capture global echo calls while preserving module-specific recording.""" + """Capture echo output in two modes for rust-build-release tests. + + _EchoRecorder itself subclasses list[str] so the echo_recorder fixture can + collect global typer.echo messages patched through monkeypatch. Calling + _EchoRecorder.__call__(module) patches module.typer.echo with fake_echo and + returns a separate list[tuple[str, bool]] of (message, err) pairs for + module-scoped captures that need stderr tracking. + """ def __init__(self, monkeypatch: pytest.MonkeyPatch) -> None: """Initialise the recorder with the provided *monkeypatch* instance.""" diff --git a/.github/actions/rust-build-release/tests/test_commands.py b/.github/actions/rust-build-release/tests/test_commands.py index 77b10379..17d6b3e5 100644 --- a/.github/actions/rust-build-release/tests/test_commands.py +++ b/.github/actions/rust-build-release/tests/test_commands.py @@ -37,6 +37,19 @@ class CrossDecisionConfig: use_cross_local_backend: bool = False +@contextlib.contextmanager +def _preserve_packaging_version() -> typ.Iterator[None]: + """Remove packaging.version after loading when the import side effect remains.""" + import packaging + import packaging.version as packaging_version + + try: + yield + finally: + if getattr(packaging, "version", None) is packaging_version: + delattr(packaging, "version") + + def _make_cross_executable(tmp_path: Path) -> Path: """Create a minimal executable cross stub at *tmp_path*.""" cross_path = tmp_path / "cross" @@ -53,9 +66,6 @@ def _load_main_module(monkeypatch: pytest.MonkeyPatch) -> ModuleType: monkeypatch.syspath_prepend(str(SRC_DIR)) sys.modules.pop("gha", None) - import packaging - import packaging.version as packaging_version - spec = importlib.util.spec_from_file_location( "rbr_main_commands", SRC_DIR / "main.py" ) @@ -64,11 +74,8 @@ def _load_main_module(monkeypatch: pytest.MonkeyPatch) -> ModuleType: raise RuntimeError(msg) module = importlib.util.module_from_spec(spec) - try: + with _preserve_packaging_version(): spec.loader.exec_module(module) - finally: - if getattr(packaging, "version", None) is packaging_version: - delattr(packaging, "version") return module @@ -135,7 +142,7 @@ def test_build_cross_command_rejects_injected_toolchain_override( # Patch the guard to inject a +toolchain token after the fact original_guard = main_module._assert_cross_command_has_no_toolchain_override - def injecting_guard(cmd: object) -> None: + def injecting_guard(_cmd: object) -> None: # Simulate the bug: a +toolchain has been inserted into the argv original_guard(["cross", "+injected-nightly", "build"]) @@ -302,7 +309,6 @@ def test_assemble_build_command_sets_rustup_toolchain_env_when_explicit_toolchai tmp_path / "Cargo.toml", "", "nightly", # explicit_toolchain - non-empty triggers with_env - "nightly", # toolchain_name ) # The returned wrapper must carry the RUSTUP_TOOLCHAIN binding. @@ -378,7 +384,6 @@ def test_assemble_build_command_returns_cargo_when_use_cross_false( tmp_path / "Cargo.toml", "", "", - "bogus-nightly", ) parts = list(cmd.formulate()) @@ -410,7 +415,6 @@ def fail_build_cross_command(*_args: object) -> object: tmp_path / "Cargo.toml", "", "", - "bogus-nightly", ) diff --git a/Makefile b/Makefile index 8769cc27..d1414eb4 100644 --- a/Makefile +++ b/Makefile @@ -72,7 +72,8 @@ check-fmt: ## Check Python formatting without modifying files $(RUFF) check --select $(RUFF_FIX_RULES) markdownlint: ## Lint Markdown files - git diff --name-only --diff-filter=ACMRT $(MARKDOWNLINT_BASE) -- '*.md' \ + $(eval MARKDOWNLINT_DIFF_BASE := $(shell git rev-parse --verify --quiet $(MARKDOWNLINT_BASE) >/dev/null && printf '%s' '$(MARKDOWNLINT_BASE)' || printf '%s' 'HEAD')) + git diff --name-only --diff-filter=ACMRT $(MARKDOWNLINT_DIFF_BASE) -- '*.md' \ | xargs -r -- $(MDLINT) nixie: ## Validate Mermaid diagrams From e659b74502f2ed8376776efef15947e9edf9ee3d Mon Sep 17 00:00:00 2001 From: leynos Date: Tue, 28 Apr 2026 01:01:38 +0200 Subject: [PATCH 20/21] Make build command assembly side-effect free Return command assembly errors as values so `main` owns annotations and process exits. Add guard spy coverage, cargo argv-shape tests, completion logging, and document cross toolchain selection. --- .github/actions/rust-build-release/README.md | 16 +++ .../actions/rust-build-release/src/main.py | 27 ++-- .../rust-build-release/tests/test_commands.py | 122 ++++++++++++++++-- docs/developers-guide.md | 50 +++++++ 4 files changed, 193 insertions(+), 22 deletions(-) diff --git a/.github/actions/rust-build-release/README.md b/.github/actions/rust-build-release/README.md index ebb8aca7..b0802208 100644 --- a/.github/actions/rust-build-release/README.md +++ b/.github/actions/rust-build-release/README.md @@ -1,5 +1,7 @@ # rust-build-release + + Build Rust application release artefacts using the repository's `setup-rust` action, `uv`, and `cross`. FreeBSD targets (for example `x86_64-unknown-freebsd`) require `cross` with a @@ -27,6 +29,20 @@ Toolchains are resolved from the target repository in this order: explicit `toolchain` input, repository `rust-toolchain.toml` or `rust-toolchain`, manifest `rust-version`, then the action's bundled fallback version. +## Toolchain specification for cross builds + +When `cross` is used for compilation, toolchain selection must be made via one +of the following methods — **not** via a `+` CLI override: + +- **`rust-toolchain.toml`** in the project repository (recommended). +- The `toolchain` action input, which is propagated as the `RUSTUP_TOOLCHAIN` + environment variable for the `cross` invocation. +- The `RUSTUP_TOOLCHAIN` environment variable set upstream in the workflow. + +Passing a `+` override on the `cross` command line is rejected by the +action and will cause the build to fail with a `::error::` annotation. This +restriction does not apply to `cargo`-only builds. + ## Inputs | Name | Type | Default | Description | Required | diff --git a/.github/actions/rust-build-release/src/main.py b/.github/actions/rust-build-release/src/main.py index 443bd311..4b10e954 100755 --- a/.github/actions/rust-build-release/src/main.py +++ b/.github/actions/rust-build-release/src/main.py @@ -581,7 +581,11 @@ def _handle_cross_container_error( ) gha_debug(f"fallback cargo argv: {fallback_cmd}") run_cmd(fallback_cmd) + gha_debug(f"fallback cargo build completed for target '{target_to_build}'") return + gha_error( + f"cross build failed for target '{target_to_build}' (retcode={exc.retcode})" + ) raise exc @@ -637,29 +641,26 @@ def _assemble_build_command( manifest_argument: Path, features: str, explicit_toolchain: str, -) -> SupportsFormulate: - """Construct the build command for either cross or cargo.""" +) -> tuple[SupportsFormulate | None, str | None]: + """Assemble the build command; return (cmd, None) or (None, error_message).""" if not decision.use_cross: cmd = _build_cargo_command( decision.cargo_toolchain_spec, target_to_build, manifest_argument, features ) - gha_debug(f"cargo argv: {cmd}") - return cmd + return cmd, None try: build_cmd = _build_cross_command( decision, target_to_build, manifest_argument, features ) except ValueError as exc: - gha_error( + return None, ( f"cross command validation failed for target '{target_to_build}': {exc}" ) - raise typer.Exit(1) from None if explicit_toolchain: build_cmd = typ.cast("_SupportsEnvFormulate", build_cmd).with_env( RUSTUP_TOOLCHAIN=explicit_toolchain ) - gha_debug(f"cross argv: {build_cmd}") - return build_cmd + return build_cmd, None @app.command() @@ -704,14 +705,22 @@ def main( previous_engine, applied_engine = _configure_cross_container_engine(decision) try: manifest_argument = _manifest_argument(manifest_path) - build_cmd = _assemble_build_command( + build_cmd, assemble_error = _assemble_build_command( decision, target_to_build, manifest_argument, features, explicit_toolchain, ) + if assemble_error is not None: + gha_error(assemble_error) + raise typer.Exit(1) + if decision.use_cross: + gha_debug(f"cross argv: {build_cmd}") + else: + gha_debug(f"cargo argv: {build_cmd}") run_cmd(build_cmd) + gha_debug(f"build completed for target '{target_to_build}'") except ProcessExecutionError as exc: _handle_cross_container_error( exc, decision, target_to_build, manifest_argument, features diff --git a/.github/actions/rust-build-release/tests/test_commands.py b/.github/actions/rust-build-release/tests/test_commands.py index 17d6b3e5..812c1e04 100644 --- a/.github/actions/rust-build-release/tests/test_commands.py +++ b/.github/actions/rust-build-release/tests/test_commands.py @@ -131,6 +131,44 @@ def test_build_cross_command_never_injects_toolchain_override( assert_no_toolchain_override(parts) +def test_build_cross_command_calls_toolchain_override_guard( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, +) -> None: + """_build_cross_command invokes the override guard exactly once.""" + import unittest.mock as mock + + main_module = _load_main_module(monkeypatch) + cross_path = _make_cross_executable(tmp_path) + decision = main_module._CrossDecision( + cross_path=str(cross_path), + cross_version="0.2.5", + use_cross=True, + cargo_toolchain_spec="+bogus-nightly", + use_cross_local_backend=False, + docker_present=True, + podman_present=False, + has_container=True, + container_engine="docker", + requires_cross_container=False, + ) + + real_guard = main_module._assert_cross_command_has_no_toolchain_override + with mock.patch.object( + main_module, + "_assert_cross_command_has_no_toolchain_override", + wraps=real_guard, + ) as spy: + main_module._build_cross_command( + decision, + "aarch64-unknown-linux-gnu", + tmp_path / "Cargo.toml", + "", + ) + + spy.assert_called_once() + + def test_build_cross_command_rejects_injected_toolchain_override( monkeypatch: pytest.MonkeyPatch, tmp_path: Path, @@ -164,6 +202,57 @@ def injecting_guard(_cmd: object) -> None: ) +@pytest.mark.parametrize( + ("cargo_toolchain_spec", "features", "expected_prefix"), + [ + pytest.param( + "+nightly", + "", + ["+nightly", "build"], + id="toolchain_spec_prepended", + ), + pytest.param( + "", + "", + ["build"], + id="no_toolchain_spec", + ), + pytest.param( + "+stable", + "async,tls", + ["+stable", "build"], + id="with_features", + ), + ], +) +def test_build_cargo_command_argv_shape( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, + cargo_toolchain_spec: str, + features: str, + expected_prefix: list[str], +) -> None: + """_build_cargo_command produces the expected argv shape.""" + main_module = _load_main_module(monkeypatch) + manifest = tmp_path / "Cargo.toml" + + cmd = main_module._build_cargo_command( + cargo_toolchain_spec, "aarch64-unknown-linux-gnu", manifest, features + ) + parts = list(cmd.formulate()) + + assert parts[0] == "cargo" + assert parts[1 : 1 + len(expected_prefix)] == expected_prefix + assert "--manifest-path" in parts + assert "--release" in parts + assert "--target" in parts + assert "aarch64-unknown-linux-gnu" in parts + if features: + assert "--features" in parts + feat_idx = parts.index("--features") + assert parts[feat_idx + 1] == features + + @pytest.fixture def cross_module_context( monkeypatch: pytest.MonkeyPatch, @@ -303,13 +392,15 @@ def test_assemble_build_command_sets_rustup_toolchain_env_when_explicit_toolchai CrossDecisionConfig(cargo_toolchain_spec=""), ) - cmd = main_module._assemble_build_command( + cmd, error_msg = main_module._assemble_build_command( decision, "aarch64-unknown-linux-gnu", tmp_path / "Cargo.toml", "", "nightly", # explicit_toolchain - non-empty triggers with_env ) + assert error_msg is None + assert cmd is not None # The returned wrapper must carry the RUSTUP_TOOLCHAIN binding. env = getattr(cmd, "env", None) or getattr(cmd._command, "env", {}) or {} @@ -378,7 +469,7 @@ def test_assemble_build_command_returns_cargo_when_use_cross_false( requires_cross_container=False, ) - cmd = main_module._assemble_build_command( + cmd, error_msg = main_module._assemble_build_command( decision, "aarch64-unknown-linux-gnu", tmp_path / "Cargo.toml", @@ -386,15 +477,17 @@ def test_assemble_build_command_returns_cargo_when_use_cross_false( "", ) + assert error_msg is None + assert cmd is not None parts = list(cmd.formulate()) assert parts[0] == "cargo" -def test_assemble_build_command_raises_exit_on_toolchain_override( +def test_assemble_build_command_returns_error_message_on_toolchain_override( monkeypatch: pytest.MonkeyPatch, tmp_path: Path, ) -> None: - """Build command assembly exits when cross validation fails.""" + """Build command assembly returns an error message when cross validation fails.""" main_module = _load_main_module(monkeypatch) decision = _make_cross_decision( main_module, @@ -408,14 +501,17 @@ def fail_build_cross_command(*_args: object) -> object: monkeypatch.setattr(main_module, "_build_cross_command", fail_build_cross_command) - with pytest.raises(main_module.typer.Exit): - main_module._assemble_build_command( - decision, - "aarch64-unknown-linux-gnu", - tmp_path / "Cargo.toml", - "", - "", - ) + cmd, error_msg = main_module._assemble_build_command( + decision, + "aarch64-unknown-linux-gnu", + tmp_path / "Cargo.toml", + "", + "", + ) + + assert cmd is None + assert error_msg is not None + assert "cross command validation failed" in error_msg def test_main_emits_error_annotation_when_cross_guard_raises( @@ -562,7 +658,7 @@ def test_cross_container_error_with_other_retcode_reraises_original_exception( ) assert raised.value is exc - assert echo_recorder == [] + assert any("cross build failed" in ln for ln in echo_recorder) def test_required_cross_container_error_exits_without_fallback( diff --git a/docs/developers-guide.md b/docs/developers-guide.md index 2e911c39..b8ab584c 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -34,3 +34,53 @@ installation paths. This lets CI and local developer machines find tools even when their package manager does not place shims on `PATH`. + +## gha adapter module + +`src/gha.py` provides three thin wrappers — `debug`, `warning`, and `error` -- +that prepend the appropriate GitHub Actions workflow command prefix +(`::debug::`, `::warning::`, `::error::`) before delegating to an injected +`echo` callable (defaulting to `typer.echo`). All annotation emission in +`main.py` is routed through this module so the formatting is consistent and +testable. + + +## _assemble_build_command and _check_target_support helpers + +`_assemble_build_command` is a pure query: it returns either `(cmd, None)` on +success or `(None, error_message)` on validation failure. All side-effects +(annotation emission and process exit) are the responsibility of `main()`. + +`_check_target_support` checks whether a given toolchain can build a requested +target without cross; it raises `typer.Exit(1)` when the target is unsupported +and cross is disabled. This side-effect is acceptable here because target +support is a hard precondition, not a recoverable error. + +## ValueError-to-annotation flow + +When `_build_cross_command` raises `ValueError` (e.g. because the guard +detects a `+` token), `_assemble_build_command` returns the error +message as the second tuple element. `main()` then calls `gha_error` with that +message and raises `typer.Exit(1)`. This keeps the domain logic free of +framework concerns whilst preserving rich error context in the annotation. + +## _CommandWrapper refactoring + +`_CommandWrapper` was refactored to be a pure value object: it holds no +injected echo callable, and `formulate()` is a side-effect-free query. A +`_validate_formulation` module-level helper raises `TypeError` when the +wrapped command does not expose a callable `formulate()`, and is called from +`__init__` and `with_env`. + +## Test strategy + +The test suite uses: + +- **pytest** for unit and integration tests. +- **Hypothesis** (`hypothesis>=6.100`) for property-based tests that validate + the `+` guard invariant across randomised argv shapes. +- **syrupy** (`syrupy>=4.0`) for snapshot tests that record the exact + `::debug:: cross argv:` line emitted by `main()`, ensuring format stability + across refactors. +- **`unittest.mock.patch`** as a wrapping spy to verify the guard is called + exactly once during `_build_cross_command` without replacing its behaviour. From bb7f5e7e9b2939501a43a60b146adadc650eae15 Mon Sep 17 00:00:00 2001 From: leynos Date: Tue, 28 Apr 2026 01:03:56 +0200 Subject: [PATCH 21/21] Handle subprocess failures during cross install fallback Treat `subprocess.CalledProcessError` the same as plumbum process failures when cargo-based cross installation fails, so Windows fallback handling can continue without cross as intended. --- .github/actions/rust-build-release/src/cross_manager.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/actions/rust-build-release/src/cross_manager.py b/.github/actions/rust-build-release/src/cross_manager.py index 6b4a901b..05041b80 100644 --- a/.github/actions/rust-build-release/src/cross_manager.py +++ b/.github/actions/rust-build-release/src/cross_manager.py @@ -4,6 +4,7 @@ import hashlib import shutil +import subprocess import sys import tempfile import urllib.error @@ -214,7 +215,7 @@ def version_compare(installed: str, required: str) -> bool: required_cross_version, ] ) - except ProcessExecutionError: + except (ProcessExecutionError, subprocess.CalledProcessError): try: run_cmd( local["cargo"][ @@ -227,7 +228,7 @@ def version_compare(installed: str, required: str) -> bool: f"v{required_cross_version}", ] ) - except ProcessExecutionError: + except (ProcessExecutionError, subprocess.CalledProcessError): if sys.platform == "win32": typer.echo( "::warning:: cross install failed; continuing without "