From c5426e9c103252f7ad768cb7f2ea0054c2b980bb Mon Sep 17 00:00:00 2001 From: Payton McIntosh Date: Sun, 6 Sep 2026 01:02:01 +0100 Subject: [PATCH 01/11] Install cargo-llvm-cov 0.9.0 from the tool manifest cargo 1.100 nightlies (from 2026-08-22) enable the new build-dir layout, which places test executables under debug/build///out. cargo-llvm-cov 0.6.24 searched debug/deps and failed with "failed to collect object files" after every test had passed; statelet has reported exactly that since its toolchain moved to nightly-2026-08-23, and every repository reaches it on its next weekly toolchain bump. cargo-llvm-cov 0.9.0 reads the new layout (verified locally on statelet: 0.6.24 fails, 0.9.0 produces lcov on the same tree). Replace the cargo-binstall of 0.6.24 in generate-coverage and ratchet-coverage with a manifest-driven installer: it resolves the entry with the install-tool resolver, downloads the release archive, verifies its SHA-256 against .github/tool-manifest.toml (each digest recomputed from an independent download; upstream publishes no sidecars), extracts only the named member, and reuses an installed binary that already reports the pinned version. Tests hold the pinned version to the manifest for every supported runner, exercise download bounding, digest mismatch, missing member and reuse. The now-unused Ensure cargo-binstall step and its tests are removed, and cargo-binstall leaves the Cargo cache paths. --- .../actions/generate-coverage/CHANGELOG.md | 13 + .github/actions/generate-coverage/README.md | 10 +- .github/actions/generate-coverage/action.yml | 69 ---- .../scripts/install_cargo_llvm_cov.py | 377 ++++++++++++++++-- .../test_generate_coverage_cache_provider.py | 1 - .../tests/test_install_cargo_llvm_cov.py | 258 ++++++++++++ .../generate-coverage/tests/test_scripts.py | 280 ------------- .github/actions/ratchet-coverage/CHANGELOG.md | 4 + .github/actions/ratchet-coverage/README.md | 14 +- .../scripts/install_cargo_llvm_cov.py | 376 +++++++++++++++-- 10 files changed, 994 insertions(+), 408 deletions(-) create mode 100644 .github/actions/generate-coverage/tests/test_install_cargo_llvm_cov.py diff --git a/.github/actions/generate-coverage/CHANGELOG.md b/.github/actions/generate-coverage/CHANGELOG.md index 74e875c56..3ea0c66d3 100644 --- a/.github/actions/generate-coverage/CHANGELOG.md +++ b/.github/actions/generate-coverage/CHANGELOG.md @@ -2,6 +2,19 @@ ## Unreleased +- Install `cargo-llvm-cov` from the tool manifest at 0.9.0, replacing the + `cargo-binstall` of 0.6.24. cargo 1.100 nightlies (from 2026-08-22) use + Cargo's new build-dir layout, which places test executables under + `debug/build///out`; 0.6.24 searched `debug/deps` and failed + with `failed to collect object files` after every test had passed, which is + what statelet's coverage lane has reported since its toolchain moved to + nightly-2026-08-23. 0.9.0 reads the new layout. The installer resolves the + manifest entry with the `install-tool` resolver, downloads the release + archive, verifies its SHA-256 against the manifest, extracts only the named + member, and reuses an installed binary that already reports the pinned + version. The `Ensure cargo-binstall` step is removed, as nothing in this + action invokes `cargo binstall` any more, and `~/.cargo/bin/cargo-binstall` + leaves the Cargo cache paths. - Refuse a `publish-baseline` that is neither `auto` nor `always`, before the action restores anything, rather than treating an unrecognized value as `auto`. diff --git a/.github/actions/generate-coverage/README.md b/.github/actions/generate-coverage/README.md index ee802e634..a8e73c8f6 100644 --- a/.github/actions/generate-coverage/README.md +++ b/.github/actions/generate-coverage/README.md @@ -10,10 +10,12 @@ manifest, set `cargo-manifest` to point to a nested `Cargo.toml`. It installs the project dependencies plus `slipcover`, `pytest`, and `coverage` automatically via `uv` into an isolated throwaway virtual environment (`.venv-coverage`) before running the tests, so no system-level Python installs -are required. When Rust coverage is required, `cargo-llvm-cov` is installed via -a pinned `cargo-binstall`. `cargo-nextest` is downloaded directly from its +are required. When Rust coverage is required, `cargo-llvm-cov` is installed from +the repository's tool manifest (`.github/tool-manifest.toml`): the entry names +the release archive and its SHA-256 digest per target, and the installer +extracts only the named member. `cargo-nextest` is downloaded directly from its pinned official release; both the archive and extracted binary have fixed -SHA-256 digests, and no Cargo source-build fallback exists. If both +SHA-256 digests. Neither has a Cargo source-build fallback. If both configuration files are present, coverage is run for each language and the Cobertura reports are merged using `uvx merge-cobertura`. @@ -203,7 +205,7 @@ With the default `cache-provider: github`, setup-uv retains its historical automatic policy: its GitHub cache is enabled on GitHub-hosted runners and disabled on self-hosted runners. The action also caches Cargo artefacts and Python dependencies with `actions/cache`. The Cargo cache covers the -`cargo-binstall`, `cargo-llvm-cov`, and `cargo-nextest` binaries, the Cargo +`cargo-llvm-cov` and `cargo-nextest` binaries, the Cargo registry, and the Cargo Git index. It no longer archives the `target` tree. Coverage builds an instrumented `target/llvm-cov-target` tree, whereas lint and diff --git a/.github/actions/generate-coverage/action.yml b/.github/actions/generate-coverage/action.yml index 85c995d8c..6906c084d 100644 --- a/.github/actions/generate-coverage/action.yml +++ b/.github/actions/generate-coverage/action.yml @@ -212,7 +212,6 @@ runs: uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 with: path: | - ~/.cargo/bin/cargo-binstall ~/.cargo/bin/cargo-llvm-cov ~/.cargo/bin/cargo-nextest ~/.cargo/registry @@ -220,74 +219,6 @@ runs: key: ${{ runner.os }}-llvmcov-${{ hashFiles('**/Cargo.lock') }} restore-keys: | ${{ runner.os }}-llvmcov- - - name: Ensure cargo-binstall - if: steps.detect.outputs.lang == 'rust' || steps.detect.outputs.lang == 'mixed' - run: | - set -euo pipefail - # NOTE: BINSTALL_SHA256 pins and validates only the installer script - # (install-from-binstall-release.sh), not the cargo-binstall binary that - # the script subsequently downloads and executes. - # - # Keep BINSTALL_VERSION and BINSTALL_SHA256 in sync; update both together. - # To refresh the installer checksum: curl -fsSL "$INSTALLER_URL" | shasum -a 256 | awk '{print $1}' - # - # If a stronger supply-chain posture is required (similar to cargo-nextest), - # extend this step to also verify the checksum of the downloaded cargo-binstall - # artefact before invoking it, assuming such checksums are available upstream. - # Export so the child installer shell inherits it; the installer reads - # BINSTALL_VERSION from the environment to pick the pinned release and - # otherwise silently falls back to releases/latest. - export BINSTALL_VERSION="v1.19.1" - binstall_ver="${BINSTALL_VERSION#v}" - # Match the pinned version as a whole, space-delimited token so a - # look-alike such as 1.19.10 does not satisfy a 1.19.1 pin. The `-V` - # output may be a bare version ("1.19.1") or " ". - binstall_version_matches() { - case " $1 " in - *" ${binstall_ver} "*) return 0 ;; - *) return 1 ;; - esac - } - if command -v cargo-binstall >/dev/null 2>&1; then - existing_version="$(cargo-binstall -V)" - if binstall_version_matches "$existing_version"; then - echo "cargo-binstall already installed: $existing_version" - exit 0 - fi - echo "cargo-binstall version mismatch: expected $binstall_ver, found $existing_version; reinstalling pinned version" >&2 - fi - BINSTALL_SHA256="d3a93702160e0ec03e2a4e996855db1f01adee801fb84a43add24e0877ef8eae" - INSTALLER_URL="https://raw.githubusercontent.com/cargo-bins/cargo-binstall/${BINSTALL_VERSION}/install-from-binstall-release.sh" - INSTALLER_PATH="$(mktemp)" - trap 'rm -f "$INSTALLER_PATH"' EXIT - curl -fsSL --proto '=https' --tlsv1.2 "$INSTALLER_URL" -o "$INSTALLER_PATH" - if [ ! -s "$INSTALLER_PATH" ]; then - echo "cargo-binstall installer download failed or empty" >&2 - exit 1 - fi - if command -v sha256sum >/dev/null 2>&1; then - ACTUAL_SHA256="$(sha256sum "$INSTALLER_PATH" | awk '{print $1}')" - else - ACTUAL_SHA256="$(shasum -a 256 "$INSTALLER_PATH" | awk '{print $1}')" - fi - if [ "$ACTUAL_SHA256" != "$BINSTALL_SHA256" ]; then - echo "cargo-binstall install script checksum mismatch: $ACTUAL_SHA256" >&2 - exit 1 - fi - bash "$INSTALLER_PATH" - cargo_home_bin="${CARGO_HOME:-$HOME/.cargo}/bin" - if [ -n "${GITHUB_PATH:-}" ]; then - echo "$cargo_home_bin" >> "$GITHUB_PATH" - fi - cargo_binstall="$cargo_home_bin/cargo-binstall" - installed_version="$("$cargo_binstall" -V)" - if ! binstall_version_matches "$installed_version"; then - echo "cargo-binstall version verification failed: expected $binstall_ver" >&2 - printf '%s\n' "$installed_version" >&2 - exit 1 - fi - echo "cargo-binstall $installed_version verified" - shell: bash - name: Install cargo-llvm-cov if: steps.detect.outputs.lang == 'rust' || steps.detect.outputs.lang == 'mixed' run: uv run --script "${{ github.action_path }}/scripts/install_cargo_llvm_cov.py" diff --git a/.github/actions/generate-coverage/scripts/install_cargo_llvm_cov.py b/.github/actions/generate-coverage/scripts/install_cargo_llvm_cov.py index 9bb25f633..79c3c4136 100755 --- a/.github/actions/generate-coverage/scripts/install_cargo_llvm_cov.py +++ b/.github/actions/generate-coverage/scripts/install_cargo_llvm_cov.py @@ -3,43 +3,374 @@ # requires-python = ">=3.12" # dependencies = ["plumbum", "typer"] # /// -"""Install cargo-llvm-cov via cargo-binstall.""" +"""Install cargo-llvm-cov from the repository's tool manifest. + +The manifest (``.github/tool-manifest.toml``) is the one place a tool's +version, release URL and digest are pinned, so this script resolves the entry +for the runner with the same resolver the ``install-tool`` action uses, then +downloads the archive, verifies its SHA-256, extracts only the named member and +installs it into ``CARGO_HOME/bin``. It never invokes Cargo, so a missing +prebuilt archive is a hard error rather than a source-build fallback. + +An installed binary at the pinned version is reused rather than replaced, which +is what makes the archive cache and a second call cheap. +""" from __future__ import annotations +import hashlib +import importlib.util +import logging +import os +import platform +import shutil +import tarfile +import tempfile +import time +import tomllib +import typing as typ +import urllib.error +import urllib.request +import zipfile +from pathlib import Path + import typer -from cmd_utils_loader import run_cmd -from plumbum.cmd import cargo -from plumbum.commands.processes import ProcessExecutionError +from plumbum import local +from plumbum.commands.processes import CommandNotFound, ProcessExecutionError + +if typ.TYPE_CHECKING: + from types import ModuleType + +logger = logging.getLogger(__name__) +logging.basicConfig(level=logging.DEBUG, format="%(levelname)s %(name)s %(message)s") + +#: The version this action installs. It must name an entry in the manifest; +#: the resolver refuses a version that is not listed rather than floating. +CARGO_LLVM_COV_VERSION = "0.9.0" + +TOOL_NAME = "cargo-llvm-cov" + +#: Where the manifest and the shared resolver live relative to this script: +#: ``/.github/actions//scripts/``. +_GITHUB_DIR = Path(__file__).resolve().parents[3] +MANIFEST_PATH = _GITHUB_DIR / "tool-manifest.toml" +RESOLVER_PATH = _GITHUB_DIR / "actions" / "install-tool" / "scripts" / "resolve_tool.py" + +# A cargo-llvm-cov release archive is under 2 MB; 200 MB bounds the disk a +# redirected endpoint could consume before the digest check rejects it. +_MAX_ARCHIVE_BYTES = 200 * 1024 * 1024 + +#: ``platform`` names to the ``runner.os`` / ``runner.arch`` vocabulary the +#: resolver reads, for when the script runs outside a GitHub Actions job. +_SYSTEMS = {"Linux": "Linux", "Darwin": "macOS", "Windows": "Windows"} +_MACHINES = {"x86_64": "X64", "amd64": "X64", "arm64": "ARM64", "aarch64": "ARM64"} + + +class ResolvedTool(typ.NamedTuple): + """The manifest entry selected for this runner.""" + + triple: str + url: str + sha256: str + member: str + extension: str + binary: str + version_args: tuple[str, ...] + expected_version: str + + @property + def filename(self) -> str: + """Return the archive's file name.""" + return self.url.rsplit("/", 1)[-1] + -# Keep CARGO_LLVM_COV_VERSION in sync with security audits; update as needed. -CARGO_LLVM_COV_VERSION = "0.6.24" +def emit_metric(line: str) -> None: + """Print one bounded metric line and append it to the job summary, if set.""" + typer.echo(f"metric {line}") + summary_path = os.environ.get("GITHUB_STEP_SUMMARY") + if not summary_path: + return + with Path(summary_path).open("a", encoding="utf-8") as handle: + handle.write(f"metric {line}\n") -def install_cargo_llvm_cov() -> None: - """Install cargo-llvm-cov using cargo-binstall.""" +def _load_resolver() -> ModuleType: + """Import the install-tool resolver from its own script directory.""" + spec = importlib.util.spec_from_file_location("resolve_tool", RESOLVER_PATH) + if spec is None or spec.loader is None: + typer.echo(f"cannot load the tool resolver at {RESOLVER_PATH}", err=True) + raise typer.Exit(1) + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +def runner_description() -> tuple[str, str]: + """Return the runner OS and architecture as GitHub Actions names them. + + ``RUNNER_OS`` and ``RUNNER_ARCH`` are authoritative inside a job; outside + one they are derived from ``platform`` so the script can be run locally. + """ + runner_os = os.environ.get("RUNNER_OS") or _SYSTEMS.get(platform.system(), "") + runner_arch = os.environ.get("RUNNER_ARCH") or _MACHINES.get( + platform.machine().lower(), "" + ) + return runner_os, runner_arch + + +def resolve_tool( + version: str = CARGO_LLVM_COV_VERSION, + *, + manifest_path: Path = MANIFEST_PATH, + runner: tuple[str, str] | None = None, +) -> ResolvedTool: + """Resolve the manifest entry for ``version`` on this runner or fail.""" + resolver = _load_resolver() + try: + with manifest_path.open("rb") as handle: + manifest = tomllib.load(handle) + except (OSError, tomllib.TOMLDecodeError) as exc: + emit_metric("cargo-llvm-cov.resolve=manifest-unreadable") + typer.echo(f"could not read the tool manifest {manifest_path}: {exc}", err=True) + raise typer.Exit(1) from exc + runner_os, runner_arch = runner or runner_description() + fields = resolver.resolve( + manifest, TOOL_NAME, version, resolver.Runner(runner_os, runner_arch) + ) + if fields.get("status") != "ok": + emit_metric(f"cargo-llvm-cov.resolve={fields.get('error_kind')}") + typer.echo(str(fields.get("error_message")), err=True) + raise typer.Exit(1) + emit_metric("cargo-llvm-cov.resolve=ok") + return ResolvedTool( + triple=str(fields["triple"]), + url=str(fields["url"]), + sha256=str(fields["sha256"]), + member=str(fields["member"]), + extension=str(fields["extension"]), + binary=str(fields["binary"]), + version_args=tuple(str(fields["version_args"]).split()), + expected_version=str(fields["expected_version"]), + ) + + +def _sha256_path(path: Path) -> str: + """Compute the SHA-256 digest for ``path``.""" + hasher = hashlib.sha256() + with path.open("rb") as handle: + for chunk in iter(lambda: handle.read(8192), b""): + hasher.update(chunk) + return hasher.hexdigest() + + +def cargo_bin() -> Path: + """Return the Cargo binary directory honoured by the caller.""" + cargo_home = Path(os.environ.get("CARGO_HOME", Path.home() / ".cargo")) + return cargo_home / "bin" + + +def reported_version(binary: Path, version_args: tuple[str, ...]) -> str | None: + """Return the version line ``binary`` reports, or None if it cannot run.""" + if not version_args: + return None try: - cmd = cargo[ - "binstall", - "cargo-llvm-cov", - "--version", - CARGO_LLVM_COV_VERSION, - "--no-confirm", - "--force", - ] - run_cmd(cmd) - typer.echo("cargo-llvm-cov installed successfully") - except ProcessExecutionError as exc: + output = local[str(binary)][list(version_args)](timeout=60) + except (OSError, CommandNotFound, ProcessExecutionError): + return None + text = str(output).strip() + return text.splitlines()[0] if text else "" + + +def installed_at_pinned_version(destination: Path, tool: ResolvedTool) -> bool: + """Whether ``destination`` already holds the binary at the pinned version.""" + if not destination.is_file(): + return False + reported = reported_version(destination, tool.version_args) + return reported is not None and reported.startswith(tool.expected_version) + + +class _ArchiveTooLargeError(Exception): + """Raised when a downloaded archive exceeds ``_MAX_ARCHIVE_BYTES``.""" + + def __init__(self, bytes_read: int) -> None: + super().__init__( + f"archive exceeded {_MAX_ARCHIVE_BYTES} bytes (read {bytes_read})" + ) + self.bytes_read = bytes_read + + +def _copy_bounded( + source: typ.IO[bytes], + destination: typ.IO[bytes], + max_bytes: int, + *, + chunk_size: int = 1024 * 1024, +) -> int: + """Copy ``source`` to ``destination`` in chunks, up to ``max_bytes``.""" + total = 0 + while True: + chunk = source.read(chunk_size) + if not chunk: + return total + total += len(chunk) + if total > max_bytes: + raise _ArchiveTooLargeError(total) + destination.write(chunk) + + +def download_archive(tool: ResolvedTool, destination: Path) -> None: + """Download the pinned release archive to ``destination``.""" + # The URL comes from the manifest, whose contract test holds every URL to + # https on a release host, so non-HTTPS schemes cannot reach this boundary. + request = urllib.request.Request( # noqa: S310 + tool.url, headers={"User-Agent": "generate-coverage"} + ) + logger.info( + "event=llvm-cov.download.start archive=%s url=%s", tool.filename, tool.url + ) + started = time.monotonic() + try: + with ( + urllib.request.urlopen(request, timeout=60) as response, # noqa: S310 + destination.open("wb") as output, + ): + bytes_written = _copy_bounded(response, output, _MAX_ARCHIVE_BYTES) + except _ArchiveTooLargeError as exc: + destination.unlink(missing_ok=True) + duration = time.monotonic() - started + emit_metric( + f"cargo-llvm-cov.download=failed duration_seconds={duration:.3f} bytes=0" + ) + typer.echo( + f"cargo-llvm-cov release archive exceeded {_MAX_ARCHIVE_BYTES} bytes " + "and was discarded", + err=True, + ) + raise typer.Exit(1) from exc + except (OSError, urllib.error.URLError) as exc: + duration = time.monotonic() - started + emit_metric( + f"cargo-llvm-cov.download=failed duration_seconds={duration:.3f} bytes=0" + ) + typer.echo(f"cargo-llvm-cov release download failed: {exc}", err=True) + raise typer.Exit(1) from exc + duration = time.monotonic() - started + logger.info( + "event=llvm-cov.download.finish archive=%s outcome=ok " + "duration_seconds=%.3f bytes=%d", + tool.filename, + duration, + bytes_written, + ) + emit_metric( + f"cargo-llvm-cov.download=ok duration_seconds={duration:.3f} " + f"bytes={bytes_written}" + ) + + +def verify_archive(archive: Path, tool: ResolvedTool) -> None: + """Fail unless the downloaded archive matches the manifest digest.""" + actual = _sha256_path(archive) + if actual != tool.sha256: + logger.error( + "event=llvm-cov.archive.verify archive=%s outcome=mismatch " + "expected=%s actual=%s", + tool.filename, + tool.sha256, + actual, + ) + emit_metric("cargo-llvm-cov.archive-digest=mismatch") + typer.echo("cargo-llvm-cov release archive checksum mismatch", err=True) + raise typer.Exit(1) + emit_metric("cargo-llvm-cov.archive-digest=ok") + + +def _copy_member(source: typ.IO[bytes], destination: Path) -> None: + """Copy one archive member stream to ``destination`` and close the stream.""" + with source, destination.open("wb") as output: + shutil.copyfileobj(source, output) + + +def extract_member(archive: Path, tool: ResolvedTool, destination: Path) -> None: + """Extract exactly the manifest's ``member`` from a verified archive.""" + if tool.extension == "zip": + with zipfile.ZipFile(archive) as package: + if tool.member not in package.namelist(): + message = f"{tool.member} missing from {tool.filename}" + raise ValueError(message) + _copy_member(package.open(tool.member), destination) + return + with tarfile.open(archive, "r:gz") as package: + try: + member = package.getmember(tool.member) + except KeyError as exc: + message = f"{tool.member} missing from {tool.filename}" + raise ValueError(message) from exc + source = package.extractfile(member) + if source is None: + message = f"{tool.member} is not a file in {tool.filename}" + raise ValueError(message) + _copy_member(source, destination) + + +def install( + tool: ResolvedTool, + destination: Path, + *, + fetch: typ.Callable[[ResolvedTool, Path], None] = download_archive, +) -> None: + """Download, verify and install the binary. + + ``destination`` is left intact when any step before the final move fails. + """ + destination.parent.mkdir(parents=True, exist_ok=True) + with tempfile.TemporaryDirectory(prefix="cargo-llvm-cov-") as workdir: + archive = Path(workdir) / tool.filename + fetch(tool, archive) + verify_archive(archive, tool) + staged = Path(workdir) / tool.binary + try: + extract_member(archive, tool, staged) + except (ValueError, tarfile.TarError, zipfile.BadZipFile) as exc: + emit_metric("cargo-llvm-cov.install=failed") + typer.echo(f"cargo-llvm-cov archive extraction failed: {exc}", err=True) + raise typer.Exit(1) from exc + staged.chmod(0o755) + shutil.move(str(staged), str(destination)) + reported = reported_version(destination, tool.version_args) + if reported is None or not reported.startswith(tool.expected_version): + emit_metric("cargo-llvm-cov.install=version-mismatch") typer.echo( - f"cargo binstall failed with code {exc.retcode}: {exc.stderr}", + f"installed cargo-llvm-cov reports {reported!r}, expected " + f"{tool.expected_version!r}", err=True, ) - raise typer.Exit(code=exc.retcode or 1) from exc + raise typer.Exit(1) + emit_metric("cargo-llvm-cov.install=ok") + + +def export_path(directory: Path) -> None: + """Add ``directory`` to the job's PATH for later steps, if in a job.""" + github_path = os.environ.get("GITHUB_PATH") + if not github_path: + return + with Path(github_path).open("a", encoding="utf-8") as handle: + handle.write(f"{directory}\n") def main() -> None: - """Install cargo-llvm-cov via cargo-binstall.""" - install_cargo_llvm_cov() + """Install cargo-llvm-cov at the pinned version from the tool manifest.""" + tool = resolve_tool() + destination = cargo_bin() / tool.binary + if installed_at_pinned_version(destination, tool): + emit_metric("cargo-llvm-cov.install=reused") + typer.echo(f"cargo-llvm-cov {CARGO_LLVM_COV_VERSION} already installed") + else: + install(tool, destination) + typer.echo( + f"cargo-llvm-cov {CARGO_LLVM_COV_VERSION} installed to {destination}" + ) + export_path(destination.parent) if __name__ == "__main__": diff --git a/.github/actions/generate-coverage/tests/test_generate_coverage_cache_provider.py b/.github/actions/generate-coverage/tests/test_generate_coverage_cache_provider.py index d7744382c..392f8509f 100644 --- a/.github/actions/generate-coverage/tests/test_generate_coverage_cache_provider.py +++ b/.github/actions/generate-coverage/tests/test_generate_coverage_cache_provider.py @@ -135,7 +135,6 @@ def test_cargo_cache_archives_binaries_registry_and_git_index_only() -> None: ] assert paths == [ - "~/.cargo/bin/cargo-binstall", "~/.cargo/bin/cargo-llvm-cov", "~/.cargo/bin/cargo-nextest", "~/.cargo/registry", diff --git a/.github/actions/generate-coverage/tests/test_install_cargo_llvm_cov.py b/.github/actions/generate-coverage/tests/test_install_cargo_llvm_cov.py new file mode 100644 index 000000000..a27bfe2f6 --- /dev/null +++ b/.github/actions/generate-coverage/tests/test_install_cargo_llvm_cov.py @@ -0,0 +1,258 @@ +"""Verify the manifest-driven cargo-llvm-cov installer. + +The installer resolves its entry from ``.github/tool-manifest.toml`` with the +``install-tool`` resolver, so these tests hold the pinned version to the +manifest for every runner the resolver knows, exercise download, digest +verification, extraction and installation against a local archive, and check +that an installed binary at the pinned version is reused rather than replaced. +""" + +from __future__ import annotations + +import hashlib +import io +import tarfile +import typing as typ +import zipfile + +import pytest +from _coverage_test_support import _exit_code, _load_module + +if typ.TYPE_CHECKING: + from pathlib import Path + from types import ModuleType + +RUNNERS = { + "linux-x64": ("Linux", "X64"), + "linux-arm64": ("Linux", "ARM64"), + "macos-x64": ("macOS", "X64"), + "macos-arm64": ("macOS", "ARM64"), + "windows-x64": ("Windows", "X64"), +} + + +@pytest.fixture +def install_llvm_cov_module(monkeypatch: pytest.MonkeyPatch) -> ModuleType: + """Return a freshly loaded installer with job-level side effects disabled.""" + monkeypatch.delenv("GITHUB_STEP_SUMMARY", raising=False) + monkeypatch.delenv("GITHUB_PATH", raising=False) + monkeypatch.delenv("RUNNER_OS", raising=False) + monkeypatch.delenv("RUNNER_ARCH", raising=False) + return _load_module(monkeypatch, "install_cargo_llvm_cov") + + +@pytest.mark.parametrize("runner", list(RUNNERS.values()), ids=list(RUNNERS)) +def test_pinned_version_resolves_from_the_manifest_for_every_runner( + install_llvm_cov_module: ModuleType, runner: tuple[str, str] +) -> None: + """The version the script pins is in the manifest for each supported runner.""" + tool = install_llvm_cov_module.resolve_tool(runner=runner) + + version = install_llvm_cov_module.CARGO_LLVM_COV_VERSION + assert f"/v{version}/" in tool.url, tool.url + assert tool.expected_version == f"cargo-llvm-cov {version}" + assert tool.version_args == ("llvm-cov", "--version") + assert len(tool.sha256) == 64 + assert tool.binary.endswith(".exe") == (runner[0] == "Windows") + + +def test_manifest_pin_is_the_layout_aware_release( + install_llvm_cov_module: ModuleType, +) -> None: + """The pin is at least 0.9.0, the first release reading Cargo's new layout. + + cargo 1.100 nightlies place test executables under + ``debug/build///out`` and 0.6.24 searched ``debug/deps``, + failing with "not found object files" after every test passed. + """ + major, minor, _patch = ( + int(part) for part in install_llvm_cov_module.CARGO_LLVM_COV_VERSION.split(".") + ) + assert (major, minor) >= (0, 9) + + +def test_unknown_version_is_refused_rather_than_floated( + install_llvm_cov_module: ModuleType, +) -> None: + """A version the manifest does not list fails resolution.""" + with pytest.raises(BaseException) as excinfo: # noqa: PT011 - typer.Exit + install_llvm_cov_module.resolve_tool("0.0.1", runner=RUNNERS["linux-x64"]) + + assert _exit_code(excinfo.value) == 1 + + +def _tarball_with(member: str, payload: bytes) -> bytes: + buffer = io.BytesIO() + with tarfile.open(fileobj=buffer, mode="w:gz") as package: + info = tarfile.TarInfo(member) + info.size = len(payload) + info.mode = 0o755 + package.addfile(info, io.BytesIO(payload)) + return buffer.getvalue() + + +def _zip_with(member: str, payload: bytes) -> bytes: + buffer = io.BytesIO() + with zipfile.ZipFile(buffer, mode="w") as package: + package.writestr(member, payload) + return buffer.getvalue() + + +_FAKE_BINARY = b"#!/bin/sh\necho 'cargo-llvm-cov 0.9.0'\n" + + +def _fake_tool( + module: ModuleType, archive: bytes, *, extension: str, member: str +) -> object: + url = ( + "https://github.com/taiki-e/cargo-llvm-cov/releases/download/v0.9.0/" + f"cargo-llvm-cov-x86_64-unknown-linux-gnu.{extension}" + ) + return module.ResolvedTool( + triple="x86_64-unknown-linux-gnu", + url=url, + sha256=hashlib.sha256(archive).hexdigest(), + member=member, + extension=extension, + binary="cargo-llvm-cov", + version_args=("llvm-cov", "--version"), + expected_version="cargo-llvm-cov 0.9.0", + ) + + +def _write_fetch(archive: bytes) -> typ.Callable[[object, Path], None]: + def fetch(_tool: object, destination: Path) -> None: + destination.write_bytes(archive) + + return fetch + + +@pytest.mark.skipif( + not hasattr(__import__("os"), "fork"), reason="fake binary is a shell script" +) +@pytest.mark.parametrize("extension", ["tar.gz", "zip"], ids=["tarball", "zip"]) +def test_install_extracts_the_manifest_member_and_verifies_it( + install_llvm_cov_module: ModuleType, tmp_path: Path, extension: str +) -> None: + """A digest-verified archive installs exactly its member and reports the version.""" + archive = ( + _tarball_with("cargo-llvm-cov", _FAKE_BINARY) + if extension == "tar.gz" + else _zip_with("cargo-llvm-cov", _FAKE_BINARY) + ) + tool = _fake_tool( + install_llvm_cov_module, archive, extension=extension, member="cargo-llvm-cov" + ) + destination = tmp_path / "bin" / "cargo-llvm-cov" + + install_llvm_cov_module.install(tool, destination, fetch=_write_fetch(archive)) + + assert destination.read_bytes() == _FAKE_BINARY + assert destination.stat().st_mode & 0o111 + assert install_llvm_cov_module.installed_at_pinned_version(destination, tool) + + +def test_digest_mismatch_fails_and_preserves_the_existing_binary( + install_llvm_cov_module: ModuleType, tmp_path: Path +) -> None: + """A tampered archive is rejected before anything touches the destination.""" + archive = _tarball_with("cargo-llvm-cov", _FAKE_BINARY) + tool = _fake_tool( + install_llvm_cov_module, archive, extension="tar.gz", member="cargo-llvm-cov" + )._replace(sha256="0" * 64) + destination = tmp_path / "bin" / "cargo-llvm-cov" + destination.parent.mkdir() + destination.write_bytes(b"previous") + + with pytest.raises(BaseException) as excinfo: # noqa: PT011 - typer.Exit + install_llvm_cov_module.install(tool, destination, fetch=_write_fetch(archive)) + + assert _exit_code(excinfo.value) == 1 + assert destination.read_bytes() == b"previous" + + +def test_missing_member_fails_and_preserves_the_existing_binary( + install_llvm_cov_module: ModuleType, tmp_path: Path +) -> None: + """An archive without the manifest's member is an error, not a guess.""" + archive = _tarball_with("some-other-binary", _FAKE_BINARY) + tool = _fake_tool( + install_llvm_cov_module, archive, extension="tar.gz", member="cargo-llvm-cov" + ) + destination = tmp_path / "bin" / "cargo-llvm-cov" + destination.parent.mkdir() + destination.write_bytes(b"previous") + + with pytest.raises(BaseException) as excinfo: # noqa: PT011 - typer.Exit + install_llvm_cov_module.install(tool, destination, fetch=_write_fetch(archive)) + + assert _exit_code(excinfo.value) == 1 + assert destination.read_bytes() == b"previous" + + +def test_oversized_download_is_discarded( + install_llvm_cov_module: ModuleType, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """A response beyond the byte cap is cut off and removed.""" + monkeypatch.setattr(install_llvm_cov_module, "_MAX_ARCHIVE_BYTES", 16) + + class _Response(io.BytesIO): + def __enter__(self) -> _Response: + return self + + def __exit__(self, *_args: object) -> None: + self.close() + + monkeypatch.setattr( + install_llvm_cov_module.urllib.request, + "urlopen", + lambda *_a, **_k: _Response(b"x" * 64), + ) + tool = _fake_tool( + install_llvm_cov_module, b"", extension="tar.gz", member="cargo-llvm-cov" + ) + destination = tmp_path / tool.filename + + with pytest.raises(BaseException) as excinfo: # noqa: PT011 - typer.Exit + install_llvm_cov_module.download_archive(tool, destination) + + assert _exit_code(excinfo.value) == 1 + assert not destination.exists() + + +def test_main_reuses_an_installed_binary_at_the_pinned_version( + install_llvm_cov_module: ModuleType, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """A binary already reporting the pinned version is kept and exported to PATH.""" + cargo_home = tmp_path / "cargo" + monkeypatch.setenv("CARGO_HOME", str(cargo_home)) + github_path = tmp_path / "github_path" + monkeypatch.setenv("GITHUB_PATH", str(github_path)) + monkeypatch.setenv("RUNNER_OS", "Linux") + monkeypatch.setenv("RUNNER_ARCH", "X64") + binary = cargo_home / "bin" / "cargo-llvm-cov" + binary.parent.mkdir(parents=True) + binary.write_bytes(_FAKE_BINARY) + binary.chmod(0o755) + + def fail_install(*_args: object, **_kwargs: object) -> None: + message = "install must not run for a reused binary" + raise AssertionError(message) + + monkeypatch.setattr(install_llvm_cov_module, "install", fail_install) + + install_llvm_cov_module.main() + + assert github_path.read_text(encoding="utf-8").strip() == str(binary.parent) + + +def test_summary_metrics_are_bounded_lines( + install_llvm_cov_module: ModuleType, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Each metric is one ``metric key=value`` line appended to the summary.""" + summary = tmp_path / "summary.md" + monkeypatch.setenv("GITHUB_STEP_SUMMARY", str(summary)) + + install_llvm_cov_module.emit_metric("cargo-llvm-cov.install=ok") + + assert summary.read_text(encoding="utf-8") == "metric cargo-llvm-cov.install=ok\n" diff --git a/.github/actions/generate-coverage/tests/test_scripts.py b/.github/actions/generate-coverage/tests/test_scripts.py index 49acd5568..f692cee3f 100644 --- a/.github/actions/generate-coverage/tests/test_scripts.py +++ b/.github/actions/generate-coverage/tests/test_scripts.py @@ -2749,286 +2749,6 @@ def _generate_coverage_step(step_name: str) -> dict[str, object]: return step -def test_generate_coverage_ensures_binstall_before_llvm_cov() -> None: - """cargo-binstall must exist before cargo-llvm-cov invokes cargo binstall.""" - steps = _generate_coverage_steps() - step_names = [step.get("name") for step in steps] - - assert step_names.index("Ensure cargo-binstall") < step_names.index( - "Install cargo-llvm-cov" - ) - - -def test_generate_coverage_binstall_is_not_nextest_only() -> None: - """cargo-llvm-cov also needs cargo-binstall when nextest is disabled.""" - step = _generate_coverage_step("Ensure cargo-binstall") - condition = step.get("if") - - assert isinstance(condition, str) - assert "steps.detect.outputs.lang == 'rust'" in condition - assert "steps.detect.outputs.lang == 'mixed'" in condition - assert "use-cargo-nextest" not in condition - - -def _ensure_binstall_script() -> str: - """Return the shell body for the cargo-binstall installation step.""" - run_script = _generate_coverage_step("Ensure cargo-binstall").get("run") - - assert isinstance(run_script, str) - return run_script - - -def _write_executable(path: Path, content: str) -> None: - """Write an executable test double.""" - path.write_text(content, encoding="utf-8") - path.chmod(0o755) - - -@dataclasses.dataclass(frozen=True) -class _BinstallScriptResult: - """Capture the outcome of running the Ensure cargo-binstall shell body.""" - - returncode: int - stdout: str - stderr: str - - -def _run_ensure_binstall_script(tmp_path: Path) -> _BinstallScriptResult: - """Execute the Ensure cargo-binstall shell body in an isolated PATH.""" - env = { - **os.environ, - "CARGO_HOME": str(tmp_path / "cargo-home"), - "GITHUB_PATH": str(tmp_path / "github-path"), - "HOME": str(tmp_path / "home"), - "PATH": f"{tmp_path / 'bin'}{os.pathsep}/usr/bin{os.pathsep}/bin", - } - command = local["/bin/bash"]["-c", _ensure_binstall_script()] - result = run_plumbum_command(command, method="run", env=env) - return _BinstallScriptResult( - returncode=result.returncode, - stdout=result.stdout, - stderr=result.stderr, - ) - - -def _write_fake_binstall_installer( - tmp_path: Path, - *, - installed_version: str = "1.19.1", -) -> None: - """Write fake curl, sha256sum, and bash commands for installer-path tests.""" - bin_dir = tmp_path / "bin" - bin_dir.mkdir(exist_ok=True) - install_log = tmp_path / "installer.log" - version_log = tmp_path / "binstall-version.log" - checksum = "d3a93702160e0ec03e2a4e996855db1f01adee801fb84a43add24e0877ef8eae" - - _write_executable( - bin_dir / "curl", - """#!/bin/sh -output="" -while [ "$#" -gt 0 ]; do - if [ "$1" = "-o" ]; then - shift - output="$1" - fi - shift -done -if [ -z "$output" ]; then - exit 2 -fi -printf '%s\\n' "fake installer" > "$output" -""", - ) - _write_executable( - bin_dir / "sha256sum", - f"""#!/bin/sh -printf '%s %s\\n' "{checksum}" "$1" -""", - ) - _write_executable( - bin_dir / "bash", - f"""#!/bin/sh -printf '%s\\n' "$*" >> "{install_log}" -printf '%s\\n' "${{BINSTALL_VERSION:-UNSET}}" >> "{version_log}" -mkdir -p "$CARGO_HOME/bin" -cat > "$CARGO_HOME/bin/cargo-binstall" <<'ENDOFINSTALL' -#!/bin/sh -printf '%s\\n' "cargo-binstall {installed_version}" -ENDOFINSTALL -chmod +x "$CARGO_HOME/bin/cargo-binstall" -""", - ) - - -def _write_existing_cargo_binstall(tmp_path: Path, version: str) -> None: - """Write an existing cargo-binstall test double into PATH.""" - bin_dir = tmp_path / "bin" - bin_dir.mkdir(exist_ok=True) - calls_log = tmp_path / "existing-binstall.log" - _write_executable( - bin_dir / "cargo-binstall", - f"""#!/bin/sh -printf '%s\\n' "$*" >> "{calls_log}" -printf '%s\\n' "cargo-binstall {version}" -""", - ) - - -@dataclasses.dataclass(frozen=True) -class _BinstallVersionCase: - """Describe one existing-cargo-binstall version-comparison outcome.""" - - existing_version: str - arrange_pinned_installer: bool - ran_pinned_installer: bool - expected_message: str - expected_stream: str - - -@pytest.mark.parametrize( - "case", - [ - pytest.param( - _BinstallVersionCase( - "1.19.1", - arrange_pinned_installer=False, - ran_pinned_installer=False, - expected_message=( - "cargo-binstall already installed: cargo-binstall 1.19.1" - ), - expected_stream="stdout", - ), - id="fast-path-verified-version", - ), - pytest.param( - _BinstallVersionCase( - "1.15.0", - arrange_pinned_installer=True, - ran_pinned_installer=True, - expected_message=( - "version mismatch: expected 1.19.1, found cargo-binstall 1.15.0" - ), - expected_stream="stderr", - ), - id="mismatch-installs-pinned-version", - ), - pytest.param( - _BinstallVersionCase( - "1.19.10", - arrange_pinned_installer=True, - ran_pinned_installer=True, - expected_message=( - "version mismatch: expected 1.19.1, found cargo-binstall 1.19.10" - ), - expected_stream="stderr", - ), - id="longer-version-look-alike-is-rejected", - ), - ], -) -def test_generate_coverage_binstall_version_comparison_outcomes( - case: _BinstallVersionCase, - tmp_path: Path, -) -> None: - """Existing cargo-binstall versions are compared exactly, not by substring. - - A verified existing binary is reused without installing; anything else, - including a longer version string that merely starts with the pin (a - ``1.19.10`` look-alike for ``1.19.1``), falls through to the pinned - installer. - """ - _write_existing_cargo_binstall(tmp_path, case.existing_version) - if case.arrange_pinned_installer: - _write_fake_binstall_installer(tmp_path) - else: - _write_executable( - tmp_path / "bin" / "curl", - """#!/bin/sh -echo "curl should not run for a verified cargo-binstall" >&2 -exit 99 -""", - ) - - result = _run_ensure_binstall_script(tmp_path) - - assert result.returncode == 0, result.stderr - haystack = result.stdout if case.expected_stream == "stdout" else result.stderr - assert case.expected_message in haystack - if case.ran_pinned_installer: - assert (tmp_path / "installer.log").read_text(encoding="utf-8") - assert "cargo-binstall cargo-binstall 1.19.1 verified" in result.stdout - else: - assert (tmp_path / "existing-binstall.log").read_text( - encoding="utf-8" - ) == "-V\n" - - -def test_generate_coverage_binstall_install_verifies_installed_version( - tmp_path: Path, -) -> None: - """The install path fails when the installed binary has the wrong version.""" - _write_fake_binstall_installer(tmp_path, installed_version="1.15.0") - - result = _run_ensure_binstall_script(tmp_path) - - assert result.returncode == 1 - assert "cargo-binstall version verification failed: expected 1.19.1" in ( - result.stderr - ) - - -def test_generate_coverage_binstall_exports_pinned_version_to_installer( - tmp_path: Path, -) -> None: - """The pinned version is exported so the child installer inherits it.""" - _write_fake_binstall_installer(tmp_path) - - result = _run_ensure_binstall_script(tmp_path) - - assert result.returncode == 0, result.stderr - version_seen = (tmp_path / "binstall-version.log").read_text(encoding="utf-8") - # Without `export`, the installer subshell sees BINSTALL_VERSION unset and - # would silently fall back to releases/latest. - assert version_seen.strip() == "v1.19.1" - - -def test_generate_coverage_binstall_appends_cargo_bin_to_github_path( - tmp_path: Path, -) -> None: - """A successful install appends the Cargo bin directory to GITHUB_PATH.""" - _write_fake_binstall_installer(tmp_path) - - result = _run_ensure_binstall_script(tmp_path) - - assert result.returncode == 0, result.stderr - github_path = (tmp_path / "github-path").read_text(encoding="utf-8") - expected_bin = str(tmp_path / "cargo-home" / "bin") - assert expected_bin in github_path.splitlines() - - -def test_generate_coverage_binstall_checksum_mismatch_aborts( - tmp_path: Path, -) -> None: - """A bad installer checksum aborts before the installer script runs.""" - _write_fake_binstall_installer(tmp_path) - # Override sha256sum to report a non-matching digest. - _write_executable( - tmp_path / "bin" / "sha256sum", - """#!/bin/sh -z16=0000000000000000 -printf '%s %s\\n' "$z16$z16$z16$z16" "$1" -""", - ) - - result = _run_ensure_binstall_script(tmp_path) - - assert result.returncode == 1 - assert "install script checksum mismatch" in result.stderr - # The installer script must not run when the checksum does not match. - assert not (tmp_path / "installer.log").exists() - - def _python_step_env_contract() -> dict[str, str]: """Return the env contract for the Python coverage step.""" steps = _generate_coverage_steps() diff --git a/.github/actions/ratchet-coverage/CHANGELOG.md b/.github/actions/ratchet-coverage/CHANGELOG.md index 9e515bbe9..9ad0e7f76 100644 --- a/.github/actions/ratchet-coverage/CHANGELOG.md +++ b/.github/actions/ratchet-coverage/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +- Install `cargo-llvm-cov` from the tool manifest at 0.9.0 instead of + `cargo-binstall` at 0.6.24, for the same reason as `generate-coverage`: + cargo 1.100 nightlies use Cargo's new build-dir layout, which 0.6.24 cannot + read and 0.9.0 can. The installer is the same manifest-driven script. - Refuse a `publish-baseline` that is neither `auto` nor `always`, before the action restores anything. A condition can only decide whether a step runs, so a typo would otherwise read as `auto` and stop publication silently. diff --git a/.github/actions/ratchet-coverage/README.md b/.github/actions/ratchet-coverage/README.md index f5ce77d91..6e056c445 100644 --- a/.github/actions/ratchet-coverage/README.md +++ b/.github/actions/ratchet-coverage/README.md @@ -34,13 +34,13 @@ the same across platforms. The action restores the previous coverage baseline using [actions/cache](https://github.com/actions/cache) and installs `cargo-llvm-cov` -if necessary. After running the coverage command, it compares the new -percentage with the stored baseline. Both values are rounded to two decimals -before comparison to avoid failures from floating‑point noise. The job fails if -coverage drops. On success the baseline file is updated, and on a push to -`refs/heads/main` it is saved back to the cache for future runs. A -`workflow_dispatch`, and a push to any other branch, update the file for the -run and publish nothing. +from the repository's tool manifest if necessary. After running the coverage +command, it compares the new percentage with the stored baseline. Both values +are rounded to two decimals before comparison to avoid failures from +floating‑point noise. The job fails if coverage drops. On success the baseline +file is updated, and on a push to `refs/heads/main` it is saved back to the +cache for future runs. A `workflow_dispatch`, and a push to any other branch, +update the file for the run and publish nothing. ## Caching diff --git a/.github/actions/ratchet-coverage/scripts/install_cargo_llvm_cov.py b/.github/actions/ratchet-coverage/scripts/install_cargo_llvm_cov.py index c38dcb6bc..79c3c4136 100755 --- a/.github/actions/ratchet-coverage/scripts/install_cargo_llvm_cov.py +++ b/.github/actions/ratchet-coverage/scripts/install_cargo_llvm_cov.py @@ -3,46 +3,374 @@ # requires-python = ">=3.12" # dependencies = ["plumbum", "typer"] # /// -"""Install cargo-llvm-cov via cargo-binstall.""" +"""Install cargo-llvm-cov from the repository's tool manifest. + +The manifest (``.github/tool-manifest.toml``) is the one place a tool's +version, release URL and digest are pinned, so this script resolves the entry +for the runner with the same resolver the ``install-tool`` action uses, then +downloads the archive, verifies its SHA-256, extracts only the named member and +installs it into ``CARGO_HOME/bin``. It never invokes Cargo, so a missing +prebuilt archive is a hard error rather than a source-build fallback. + +An installed binary at the pinned version is reused rather than replaced, which +is what makes the archive cache and a second call cheap. +""" from __future__ import annotations +import hashlib +import importlib.util +import logging +import os +import platform +import shutil +import tarfile +import tempfile +import time +import tomllib +import typing as typ +import urllib.error +import urllib.request +import zipfile +from pathlib import Path + import typer -from plumbum.cmd import cargo -from plumbum.commands.processes import ProcessExecutionError +from plumbum import local +from plumbum.commands.processes import CommandNotFound, ProcessExecutionError + +if typ.TYPE_CHECKING: + from types import ModuleType + +logger = logging.getLogger(__name__) +logging.basicConfig(level=logging.DEBUG, format="%(levelname)s %(name)s %(message)s") + +#: The version this action installs. It must name an entry in the manifest; +#: the resolver refuses a version that is not listed rather than floating. +CARGO_LLVM_COV_VERSION = "0.9.0" + +TOOL_NAME = "cargo-llvm-cov" + +#: Where the manifest and the shared resolver live relative to this script: +#: ``/.github/actions//scripts/``. +_GITHUB_DIR = Path(__file__).resolve().parents[3] +MANIFEST_PATH = _GITHUB_DIR / "tool-manifest.toml" +RESOLVER_PATH = _GITHUB_DIR / "actions" / "install-tool" / "scripts" / "resolve_tool.py" + +# A cargo-llvm-cov release archive is under 2 MB; 200 MB bounds the disk a +# redirected endpoint could consume before the digest check rejects it. +_MAX_ARCHIVE_BYTES = 200 * 1024 * 1024 + +#: ``platform`` names to the ``runner.os`` / ``runner.arch`` vocabulary the +#: resolver reads, for when the script runs outside a GitHub Actions job. +_SYSTEMS = {"Linux": "Linux", "Darwin": "macOS", "Windows": "Windows"} +_MACHINES = {"x86_64": "X64", "amd64": "X64", "arm64": "ARM64", "aarch64": "ARM64"} + + +class ResolvedTool(typ.NamedTuple): + """The manifest entry selected for this runner.""" + + triple: str + url: str + sha256: str + member: str + extension: str + binary: str + version_args: tuple[str, ...] + expected_version: str + + @property + def filename(self) -> str: + """Return the archive's file name.""" + return self.url.rsplit("/", 1)[-1] -from cmd_utils_importer import import_cmd_utils -run_cmd = import_cmd_utils().run_cmd +def emit_metric(line: str) -> None: + """Print one bounded metric line and append it to the job summary, if set.""" + typer.echo(f"metric {line}") + summary_path = os.environ.get("GITHUB_STEP_SUMMARY") + if not summary_path: + return + with Path(summary_path).open("a", encoding="utf-8") as handle: + handle.write(f"metric {line}\n") -# Keep CARGO_LLVM_COV_VERSION in sync with security audits; update as needed. -CARGO_LLVM_COV_VERSION = "0.6.24" +def _load_resolver() -> ModuleType: + """Import the install-tool resolver from its own script directory.""" + spec = importlib.util.spec_from_file_location("resolve_tool", RESOLVER_PATH) + if spec is None or spec.loader is None: + typer.echo(f"cannot load the tool resolver at {RESOLVER_PATH}", err=True) + raise typer.Exit(1) + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module -def install_cargo_llvm_cov() -> None: - """Install cargo-llvm-cov using cargo-binstall.""" + +def runner_description() -> tuple[str, str]: + """Return the runner OS and architecture as GitHub Actions names them. + + ``RUNNER_OS`` and ``RUNNER_ARCH`` are authoritative inside a job; outside + one they are derived from ``platform`` so the script can be run locally. + """ + runner_os = os.environ.get("RUNNER_OS") or _SYSTEMS.get(platform.system(), "") + runner_arch = os.environ.get("RUNNER_ARCH") or _MACHINES.get( + platform.machine().lower(), "" + ) + return runner_os, runner_arch + + +def resolve_tool( + version: str = CARGO_LLVM_COV_VERSION, + *, + manifest_path: Path = MANIFEST_PATH, + runner: tuple[str, str] | None = None, +) -> ResolvedTool: + """Resolve the manifest entry for ``version`` on this runner or fail.""" + resolver = _load_resolver() + try: + with manifest_path.open("rb") as handle: + manifest = tomllib.load(handle) + except (OSError, tomllib.TOMLDecodeError) as exc: + emit_metric("cargo-llvm-cov.resolve=manifest-unreadable") + typer.echo(f"could not read the tool manifest {manifest_path}: {exc}", err=True) + raise typer.Exit(1) from exc + runner_os, runner_arch = runner or runner_description() + fields = resolver.resolve( + manifest, TOOL_NAME, version, resolver.Runner(runner_os, runner_arch) + ) + if fields.get("status") != "ok": + emit_metric(f"cargo-llvm-cov.resolve={fields.get('error_kind')}") + typer.echo(str(fields.get("error_message")), err=True) + raise typer.Exit(1) + emit_metric("cargo-llvm-cov.resolve=ok") + return ResolvedTool( + triple=str(fields["triple"]), + url=str(fields["url"]), + sha256=str(fields["sha256"]), + member=str(fields["member"]), + extension=str(fields["extension"]), + binary=str(fields["binary"]), + version_args=tuple(str(fields["version_args"]).split()), + expected_version=str(fields["expected_version"]), + ) + + +def _sha256_path(path: Path) -> str: + """Compute the SHA-256 digest for ``path``.""" + hasher = hashlib.sha256() + with path.open("rb") as handle: + for chunk in iter(lambda: handle.read(8192), b""): + hasher.update(chunk) + return hasher.hexdigest() + + +def cargo_bin() -> Path: + """Return the Cargo binary directory honoured by the caller.""" + cargo_home = Path(os.environ.get("CARGO_HOME", Path.home() / ".cargo")) + return cargo_home / "bin" + + +def reported_version(binary: Path, version_args: tuple[str, ...]) -> str | None: + """Return the version line ``binary`` reports, or None if it cannot run.""" + if not version_args: + return None try: - cmd = cargo[ - "binstall", - "cargo-llvm-cov", - "--version", - CARGO_LLVM_COV_VERSION, - "--no-confirm", - "--force", - ] - run_cmd(cmd) - typer.echo("cargo-llvm-cov installed successfully") - except ProcessExecutionError as exc: + output = local[str(binary)][list(version_args)](timeout=60) + except (OSError, CommandNotFound, ProcessExecutionError): + return None + text = str(output).strip() + return text.splitlines()[0] if text else "" + + +def installed_at_pinned_version(destination: Path, tool: ResolvedTool) -> bool: + """Whether ``destination`` already holds the binary at the pinned version.""" + if not destination.is_file(): + return False + reported = reported_version(destination, tool.version_args) + return reported is not None and reported.startswith(tool.expected_version) + + +class _ArchiveTooLargeError(Exception): + """Raised when a downloaded archive exceeds ``_MAX_ARCHIVE_BYTES``.""" + + def __init__(self, bytes_read: int) -> None: + super().__init__( + f"archive exceeded {_MAX_ARCHIVE_BYTES} bytes (read {bytes_read})" + ) + self.bytes_read = bytes_read + + +def _copy_bounded( + source: typ.IO[bytes], + destination: typ.IO[bytes], + max_bytes: int, + *, + chunk_size: int = 1024 * 1024, +) -> int: + """Copy ``source`` to ``destination`` in chunks, up to ``max_bytes``.""" + total = 0 + while True: + chunk = source.read(chunk_size) + if not chunk: + return total + total += len(chunk) + if total > max_bytes: + raise _ArchiveTooLargeError(total) + destination.write(chunk) + + +def download_archive(tool: ResolvedTool, destination: Path) -> None: + """Download the pinned release archive to ``destination``.""" + # The URL comes from the manifest, whose contract test holds every URL to + # https on a release host, so non-HTTPS schemes cannot reach this boundary. + request = urllib.request.Request( # noqa: S310 + tool.url, headers={"User-Agent": "generate-coverage"} + ) + logger.info( + "event=llvm-cov.download.start archive=%s url=%s", tool.filename, tool.url + ) + started = time.monotonic() + try: + with ( + urllib.request.urlopen(request, timeout=60) as response, # noqa: S310 + destination.open("wb") as output, + ): + bytes_written = _copy_bounded(response, output, _MAX_ARCHIVE_BYTES) + except _ArchiveTooLargeError as exc: + destination.unlink(missing_ok=True) + duration = time.monotonic() - started + emit_metric( + f"cargo-llvm-cov.download=failed duration_seconds={duration:.3f} bytes=0" + ) typer.echo( - f"cargo binstall failed with code {exc.retcode}: {exc.stderr}", + f"cargo-llvm-cov release archive exceeded {_MAX_ARCHIVE_BYTES} bytes " + "and was discarded", err=True, ) - raise typer.Exit(code=exc.retcode or 1) from exc + raise typer.Exit(1) from exc + except (OSError, urllib.error.URLError) as exc: + duration = time.monotonic() - started + emit_metric( + f"cargo-llvm-cov.download=failed duration_seconds={duration:.3f} bytes=0" + ) + typer.echo(f"cargo-llvm-cov release download failed: {exc}", err=True) + raise typer.Exit(1) from exc + duration = time.monotonic() - started + logger.info( + "event=llvm-cov.download.finish archive=%s outcome=ok " + "duration_seconds=%.3f bytes=%d", + tool.filename, + duration, + bytes_written, + ) + emit_metric( + f"cargo-llvm-cov.download=ok duration_seconds={duration:.3f} " + f"bytes={bytes_written}" + ) + + +def verify_archive(archive: Path, tool: ResolvedTool) -> None: + """Fail unless the downloaded archive matches the manifest digest.""" + actual = _sha256_path(archive) + if actual != tool.sha256: + logger.error( + "event=llvm-cov.archive.verify archive=%s outcome=mismatch " + "expected=%s actual=%s", + tool.filename, + tool.sha256, + actual, + ) + emit_metric("cargo-llvm-cov.archive-digest=mismatch") + typer.echo("cargo-llvm-cov release archive checksum mismatch", err=True) + raise typer.Exit(1) + emit_metric("cargo-llvm-cov.archive-digest=ok") + + +def _copy_member(source: typ.IO[bytes], destination: Path) -> None: + """Copy one archive member stream to ``destination`` and close the stream.""" + with source, destination.open("wb") as output: + shutil.copyfileobj(source, output) + + +def extract_member(archive: Path, tool: ResolvedTool, destination: Path) -> None: + """Extract exactly the manifest's ``member`` from a verified archive.""" + if tool.extension == "zip": + with zipfile.ZipFile(archive) as package: + if tool.member not in package.namelist(): + message = f"{tool.member} missing from {tool.filename}" + raise ValueError(message) + _copy_member(package.open(tool.member), destination) + return + with tarfile.open(archive, "r:gz") as package: + try: + member = package.getmember(tool.member) + except KeyError as exc: + message = f"{tool.member} missing from {tool.filename}" + raise ValueError(message) from exc + source = package.extractfile(member) + if source is None: + message = f"{tool.member} is not a file in {tool.filename}" + raise ValueError(message) + _copy_member(source, destination) + + +def install( + tool: ResolvedTool, + destination: Path, + *, + fetch: typ.Callable[[ResolvedTool, Path], None] = download_archive, +) -> None: + """Download, verify and install the binary. + + ``destination`` is left intact when any step before the final move fails. + """ + destination.parent.mkdir(parents=True, exist_ok=True) + with tempfile.TemporaryDirectory(prefix="cargo-llvm-cov-") as workdir: + archive = Path(workdir) / tool.filename + fetch(tool, archive) + verify_archive(archive, tool) + staged = Path(workdir) / tool.binary + try: + extract_member(archive, tool, staged) + except (ValueError, tarfile.TarError, zipfile.BadZipFile) as exc: + emit_metric("cargo-llvm-cov.install=failed") + typer.echo(f"cargo-llvm-cov archive extraction failed: {exc}", err=True) + raise typer.Exit(1) from exc + staged.chmod(0o755) + shutil.move(str(staged), str(destination)) + reported = reported_version(destination, tool.version_args) + if reported is None or not reported.startswith(tool.expected_version): + emit_metric("cargo-llvm-cov.install=version-mismatch") + typer.echo( + f"installed cargo-llvm-cov reports {reported!r}, expected " + f"{tool.expected_version!r}", + err=True, + ) + raise typer.Exit(1) + emit_metric("cargo-llvm-cov.install=ok") + + +def export_path(directory: Path) -> None: + """Add ``directory`` to the job's PATH for later steps, if in a job.""" + github_path = os.environ.get("GITHUB_PATH") + if not github_path: + return + with Path(github_path).open("a", encoding="utf-8") as handle: + handle.write(f"{directory}\n") def main() -> None: - """Install cargo-llvm-cov via cargo-binstall.""" - install_cargo_llvm_cov() + """Install cargo-llvm-cov at the pinned version from the tool manifest.""" + tool = resolve_tool() + destination = cargo_bin() / tool.binary + if installed_at_pinned_version(destination, tool): + emit_metric("cargo-llvm-cov.install=reused") + typer.echo(f"cargo-llvm-cov {CARGO_LLVM_COV_VERSION} already installed") + else: + install(tool, destination) + typer.echo( + f"cargo-llvm-cov {CARGO_LLVM_COV_VERSION} installed to {destination}" + ) + export_path(destination.parent) if __name__ == "__main__": From 6d2861e491ea011baa06cc5237d13ce285e45c14 Mon Sep 17 00:00:00 2001 From: Payton McIntosh Date: Sun, 6 Sep 2026 01:05:09 +0100 Subject: [PATCH 02/11] Fold the two rejected-archive tests into one parametrised case CodeScene flagged the digest-mismatch and missing-member tests as duplicated code; they differ only in how the archive is unusable. --- .../tests/test_install_cargo_llvm_cov.py | 45 ++++++++++--------- 1 file changed, 25 insertions(+), 20 deletions(-) diff --git a/.github/actions/generate-coverage/tests/test_install_cargo_llvm_cov.py b/.github/actions/generate-coverage/tests/test_install_cargo_llvm_cov.py index a27bfe2f6..56880f3d2 100644 --- a/.github/actions/generate-coverage/tests/test_install_cargo_llvm_cov.py +++ b/.github/actions/generate-coverage/tests/test_install_cargo_llvm_cov.py @@ -9,6 +9,7 @@ from __future__ import annotations +import dataclasses import hashlib import io import tarfile @@ -152,33 +153,37 @@ def test_install_extracts_the_manifest_member_and_verifies_it( assert install_llvm_cov_module.installed_at_pinned_version(destination, tool) -def test_digest_mismatch_fails_and_preserves_the_existing_binary( - install_llvm_cov_module: ModuleType, tmp_path: Path -) -> None: - """A tampered archive is rejected before anything touches the destination.""" - archive = _tarball_with("cargo-llvm-cov", _FAKE_BINARY) - tool = _fake_tool( - install_llvm_cov_module, archive, extension="tar.gz", member="cargo-llvm-cov" - )._replace(sha256="0" * 64) - destination = tmp_path / "bin" / "cargo-llvm-cov" - destination.parent.mkdir() - destination.write_bytes(b"previous") +@dataclasses.dataclass(frozen=True) +class _RejectedArchive: + """One way a downloaded archive can be unusable.""" - with pytest.raises(BaseException) as excinfo: # noqa: PT011 - typer.Exit - install_llvm_cov_module.install(tool, destination, fetch=_write_fetch(archive)) - - assert _exit_code(excinfo.value) == 1 - assert destination.read_bytes() == b"previous" + member: str + tamper_digest: bool -def test_missing_member_fails_and_preserves_the_existing_binary( - install_llvm_cov_module: ModuleType, tmp_path: Path +@pytest.mark.parametrize( + "case", + [ + pytest.param( + _RejectedArchive(member="cargo-llvm-cov", tamper_digest=True), + id="digest-mismatch", + ), + pytest.param( + _RejectedArchive(member="some-other-binary", tamper_digest=False), + id="missing-member", + ), + ], +) +def test_rejected_archive_fails_and_preserves_the_existing_binary( + install_llvm_cov_module: ModuleType, tmp_path: Path, case: _RejectedArchive ) -> None: - """An archive without the manifest's member is an error, not a guess.""" - archive = _tarball_with("some-other-binary", _FAKE_BINARY) + """A tampered or malformed archive is an error that leaves the binary alone.""" + archive = _tarball_with(case.member, _FAKE_BINARY) tool = _fake_tool( install_llvm_cov_module, archive, extension="tar.gz", member="cargo-llvm-cov" ) + if case.tamper_digest: + tool = tool._replace(sha256="0" * 64) destination = tmp_path / "bin" / "cargo-llvm-cov" destination.parent.mkdir() destination.write_bytes(b"previous") From fa81f19e17fe7e0054baa25767217988d98c64d0 Mon Sep 17 00:00:00 2001 From: Payton McIntosh Date: Sun, 6 Sep 2026 02:07:11 +0100 Subject: [PATCH 03/11] Make resolution pure, the version match exact and the publish atomic Review round on #470: resolve_tool no longer emits metrics and raises a typed ToolResolutionError that main turns into the bounded resolve metric; the installed-version check requires the exact expected line rather than a prefix, with a Hypothesis property holding that only that line counts as installed; the extracted binary is staged in a temporary directory beside the destination and published with a rename, so a concurrent reader cannot observe a partial executable even when TMPDIR and CARGO_HOME sit on different filesystems. Tests now drive main end to end against a temporary manifest served over a local HTTP server, cover a binary reporting another version on both the install and the reuse path, and the developers' and users' guides describe the manifest-driven installer in place of the removed cargo-binstall pinning. --- .../actions/generate-coverage/CHANGELOG.md | 6 +- .../scripts/install_cargo_llvm_cov.py | 72 +++-- .../tests/test_install_cargo_llvm_cov.py | 293 +++++++++++++++--- .../scripts/install_cargo_llvm_cov.py | 72 +++-- docs/developers-guide.md | 88 +++--- docs/users-guide.md | 14 +- 6 files changed, 416 insertions(+), 129 deletions(-) diff --git a/.github/actions/generate-coverage/CHANGELOG.md b/.github/actions/generate-coverage/CHANGELOG.md index 3ea0c66d3..fea6f2663 100644 --- a/.github/actions/generate-coverage/CHANGELOG.md +++ b/.github/actions/generate-coverage/CHANGELOG.md @@ -11,8 +11,10 @@ nightly-2026-08-23. 0.9.0 reads the new layout. The installer resolves the manifest entry with the `install-tool` resolver, downloads the release archive, verifies its SHA-256 against the manifest, extracts only the named - member, and reuses an installed binary that already reports the pinned - version. The `Ensure cargo-binstall` step is removed, as nothing in this + member, stages it beside the destination and publishes it with a rename so + no reader sees a partial executable, and reuses an installed binary that + already reports exactly the pinned version. The `Ensure cargo-binstall` step + is removed, as nothing in this action invokes `cargo binstall` any more, and `~/.cargo/bin/cargo-binstall` leaves the Cargo cache paths. - Refuse a `publish-baseline` that is neither `auto` nor `always`, before the diff --git a/.github/actions/generate-coverage/scripts/install_cargo_llvm_cov.py b/.github/actions/generate-coverage/scripts/install_cargo_llvm_cov.py index 79c3c4136..b6a1251b8 100755 --- a/.github/actions/generate-coverage/scripts/install_cargo_llvm_cov.py +++ b/.github/actions/generate-coverage/scripts/install_cargo_llvm_cov.py @@ -50,6 +50,9 @@ TOOL_NAME = "cargo-llvm-cov" +#: The resolver reports every other failure kind; this one is ours. +MANIFEST_UNREADABLE = "manifest-unreadable" + #: Where the manifest and the shared resolver live relative to this script: #: ``/.github/actions//scripts/``. _GITHUB_DIR = Path(__file__).resolve().parents[3] @@ -118,30 +121,50 @@ def runner_description() -> tuple[str, str]: return runner_os, runner_arch +class ToolResolutionError(Exception): + """The manifest offers no usable entry for this tool, version and runner. + + ``kind`` is one of the resolver's closed set of reasons, so the caller can + publish it as a bounded metric without inspecting the message. + """ + + def __init__(self, kind: str, message: str) -> None: + super().__init__(message) + self.kind = kind + + +def load_manifest(manifest_path: Path = MANIFEST_PATH) -> dict[str, object]: + """Read the tool manifest, or raise ``ToolResolutionError``.""" + try: + with manifest_path.open("rb") as handle: + return tomllib.load(handle) + except (OSError, tomllib.TOMLDecodeError) as exc: + message = f"could not read the tool manifest {manifest_path}: {exc}" + raise ToolResolutionError(MANIFEST_UNREADABLE, message) from exc + + def resolve_tool( version: str = CARGO_LLVM_COV_VERSION, *, - manifest_path: Path = MANIFEST_PATH, + manifest: dict[str, object] | None = None, runner: tuple[str, str] | None = None, ) -> ResolvedTool: - """Resolve the manifest entry for ``version`` on this runner or fail.""" + """Return the manifest entry for ``version`` on ``runner``. + + A query with no side effects: it reads the manifest (or the one passed + in) and either returns the entry or raises ``ToolResolutionError``. + """ resolver = _load_resolver() - try: - with manifest_path.open("rb") as handle: - manifest = tomllib.load(handle) - except (OSError, tomllib.TOMLDecodeError) as exc: - emit_metric("cargo-llvm-cov.resolve=manifest-unreadable") - typer.echo(f"could not read the tool manifest {manifest_path}: {exc}", err=True) - raise typer.Exit(1) from exc + if manifest is None: + manifest = load_manifest() runner_os, runner_arch = runner or runner_description() fields = resolver.resolve( manifest, TOOL_NAME, version, resolver.Runner(runner_os, runner_arch) ) if fields.get("status") != "ok": - emit_metric(f"cargo-llvm-cov.resolve={fields.get('error_kind')}") - typer.echo(str(fields.get("error_message")), err=True) - raise typer.Exit(1) - emit_metric("cargo-llvm-cov.resolve=ok") + kind = str(fields.get("error_kind")) + message = str(fields.get("error_message")) + raise ToolResolutionError(kind, message) return ResolvedTool( triple=str(fields["triple"]), url=str(fields["url"]), @@ -185,8 +208,7 @@ def installed_at_pinned_version(destination: Path, tool: ResolvedTool) -> bool: """Whether ``destination`` already holds the binary at the pinned version.""" if not destination.is_file(): return False - reported = reported_version(destination, tool.version_args) - return reported is not None and reported.startswith(tool.expected_version) + return reported_version(destination, tool.version_args) == tool.expected_version class _ArchiveTooLargeError(Exception): @@ -324,7 +346,13 @@ def install( ``destination`` is left intact when any step before the final move fails. """ destination.parent.mkdir(parents=True, exist_ok=True) - with tempfile.TemporaryDirectory(prefix="cargo-llvm-cov-") as workdir: + # Staged in the destination's own directory so the final publish is a + # rename on one filesystem: a temporary directory elsewhere would turn + # the move into copy-then-delete, during which a concurrent reader could + # open a half-written executable. + with tempfile.TemporaryDirectory( + prefix=".cargo-llvm-cov-staging-", dir=destination.parent + ) as workdir: archive = Path(workdir) / tool.filename fetch(tool, archive) verify_archive(archive, tool) @@ -336,9 +364,9 @@ def install( typer.echo(f"cargo-llvm-cov archive extraction failed: {exc}", err=True) raise typer.Exit(1) from exc staged.chmod(0o755) - shutil.move(str(staged), str(destination)) + staged.replace(destination) reported = reported_version(destination, tool.version_args) - if reported is None or not reported.startswith(tool.expected_version): + if reported != tool.expected_version: emit_metric("cargo-llvm-cov.install=version-mismatch") typer.echo( f"installed cargo-llvm-cov reports {reported!r}, expected " @@ -360,7 +388,13 @@ def export_path(directory: Path) -> None: def main() -> None: """Install cargo-llvm-cov at the pinned version from the tool manifest.""" - tool = resolve_tool() + try: + tool = resolve_tool() + except ToolResolutionError as exc: + emit_metric(f"cargo-llvm-cov.resolve={exc.kind}") + typer.echo(str(exc), err=True) + raise typer.Exit(1) from exc + emit_metric("cargo-llvm-cov.resolve=ok") destination = cargo_bin() / tool.binary if installed_at_pinned_version(destination, tool): emit_metric("cargo-llvm-cov.install=reused") diff --git a/.github/actions/generate-coverage/tests/test_install_cargo_llvm_cov.py b/.github/actions/generate-coverage/tests/test_install_cargo_llvm_cov.py index 56880f3d2..d2cca6384 100644 --- a/.github/actions/generate-coverage/tests/test_install_cargo_llvm_cov.py +++ b/.github/actions/generate-coverage/tests/test_install_cargo_llvm_cov.py @@ -3,26 +3,36 @@ The installer resolves its entry from ``.github/tool-manifest.toml`` with the ``install-tool`` resolver, so these tests hold the pinned version to the manifest for every runner the resolver knows, exercise download, digest -verification, extraction and installation against a local archive, and check -that an installed binary at the pinned version is reused rather than replaced. +verification, extraction and installation against local archives, drive the +whole entry point across a real HTTP boundary, and check that only a binary +reporting exactly the pinned version is reused. """ from __future__ import annotations import dataclasses +import functools import hashlib +import http.server +import importlib.util import io import tarfile +import threading import typing as typ import zipfile +from pathlib import Path import pytest +import typer from _coverage_test_support import _exit_code, _load_module +from hypothesis import given +from hypothesis import strategies as st if typ.TYPE_CHECKING: - from pathlib import Path from types import ModuleType + from install_cargo_llvm_cov import ResolvedTool + RUNNERS = { "linux-x64": ("Linux", "X64"), "linux-arm64": ("Linux", "ARM64"), @@ -31,6 +41,9 @@ "windows-x64": ("Windows", "X64"), } +_FAKE_BINARY = b"#!/bin/sh\necho 'cargo-llvm-cov 0.9.0'\n" +_WRONG_VERSION_BINARY = b"#!/bin/sh\necho 'cargo-llvm-cov 0.6.24'\n" + @pytest.fixture def install_llvm_cov_module(monkeypatch: pytest.MonkeyPatch) -> ModuleType: @@ -75,11 +88,21 @@ def test_manifest_pin_is_the_layout_aware_release( def test_unknown_version_is_refused_rather_than_floated( install_llvm_cov_module: ModuleType, ) -> None: - """A version the manifest does not list fails resolution.""" - with pytest.raises(BaseException) as excinfo: # noqa: PT011 - typer.Exit + """A version the manifest does not list raises a typed resolution error.""" + with pytest.raises(install_llvm_cov_module.ToolResolutionError) as excinfo: install_llvm_cov_module.resolve_tool("0.0.1", runner=RUNNERS["linux-x64"]) - assert _exit_code(excinfo.value) == 1 + assert excinfo.value.kind == "unknown-version" + + +def test_unreadable_manifest_is_a_typed_error( + install_llvm_cov_module: ModuleType, tmp_path: Path +) -> None: + """A missing manifest is reported by kind, not as a stack trace.""" + with pytest.raises(install_llvm_cov_module.ToolResolutionError) as excinfo: + install_llvm_cov_module.load_manifest(tmp_path / "absent.toml") + + assert excinfo.value.kind == install_llvm_cov_module.MANIFEST_UNREADABLE def _tarball_with(member: str, payload: bytes) -> bytes: @@ -99,25 +122,31 @@ def _zip_with(member: str, payload: bytes) -> bytes: return buffer.getvalue() -_FAKE_BINARY = b"#!/bin/sh\necho 'cargo-llvm-cov 0.9.0'\n" - - def _fake_tool( - module: ModuleType, archive: bytes, *, extension: str, member: str -) -> object: - url = ( - "https://github.com/taiki-e/cargo-llvm-cov/releases/download/v0.9.0/" - f"cargo-llvm-cov-x86_64-unknown-linux-gnu.{extension}" - ) - return module.ResolvedTool( - triple="x86_64-unknown-linux-gnu", - url=url, - sha256=hashlib.sha256(archive).hexdigest(), - member=member, - extension=extension, - binary="cargo-llvm-cov", - version_args=("llvm-cov", "--version"), - expected_version="cargo-llvm-cov 0.9.0", + module: ModuleType, + archive: bytes, + *, + extension: str, + member: str, + url: str | None = None, +) -> ResolvedTool: + if url is None: + url = ( + "https://github.com/taiki-e/cargo-llvm-cov/releases/download/v0.9.0/" + f"cargo-llvm-cov-x86_64-unknown-linux-gnu.{extension}" + ) + return typ.cast( + "ResolvedTool", + module.ResolvedTool( + triple="x86_64-unknown-linux-gnu", + url=url, + sha256=hashlib.sha256(archive).hexdigest(), + member=member, + extension=extension, + binary="cargo-llvm-cov", + version_args=("llvm-cov", "--version"), + expected_version="cargo-llvm-cov 0.9.0", + ), ) @@ -128,9 +157,6 @@ def fetch(_tool: object, destination: Path) -> None: return fetch -@pytest.mark.skipif( - not hasattr(__import__("os"), "fork"), reason="fake binary is a shell script" -) @pytest.mark.parametrize("extension", ["tar.gz", "zip"], ids=["tarball", "zip"]) def test_install_extracts_the_manifest_member_and_verifies_it( install_llvm_cov_module: ModuleType, tmp_path: Path, extension: str @@ -151,6 +177,9 @@ def test_install_extracts_the_manifest_member_and_verifies_it( assert destination.read_bytes() == _FAKE_BINARY assert destination.stat().st_mode & 0o111 assert install_llvm_cov_module.installed_at_pinned_version(destination, tool) + assert [p.name for p in destination.parent.iterdir()] == ["cargo-llvm-cov"], ( + "the staging directory must not outlive the install" + ) @dataclasses.dataclass(frozen=True) @@ -188,13 +217,73 @@ def test_rejected_archive_fails_and_preserves_the_existing_binary( destination.parent.mkdir() destination.write_bytes(b"previous") - with pytest.raises(BaseException) as excinfo: # noqa: PT011 - typer.Exit + with pytest.raises(typer.Exit) as excinfo: install_llvm_cov_module.install(tool, destination, fetch=_write_fetch(archive)) assert _exit_code(excinfo.value) == 1 assert destination.read_bytes() == b"previous" +def test_installed_binary_reporting_another_version_fails_the_install( + install_llvm_cov_module: ModuleType, tmp_path: Path +) -> None: + """A verified archive whose binary reports the wrong version is still a failure.""" + archive = _tarball_with("cargo-llvm-cov", _WRONG_VERSION_BINARY) + tool = _fake_tool( + install_llvm_cov_module, archive, extension="tar.gz", member="cargo-llvm-cov" + ) + destination = tmp_path / "bin" / "cargo-llvm-cov" + + with pytest.raises(typer.Exit) as excinfo: + install_llvm_cov_module.install(tool, destination, fetch=_write_fetch(archive)) + + assert _exit_code(excinfo.value) == 1 + + +def _installer_module() -> ModuleType: + """Load the installer without pytest fixtures, for the property test. + + Hypothesis runs the test body many times per call and function-scoped + fixtures would be shared across those examples, so the module is loaded + directly here. + """ + script = ( + Path(__file__).resolve().parents[1] / "scripts" / "install_cargo_llvm_cov.py" + ) + spec = importlib.util.spec_from_file_location( + "install_cargo_llvm_cov_property", script + ) + assert spec is not None + assert spec.loader is not None + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +@given(reported=st.one_of(st.none(), st.text(max_size=40))) +def test_only_the_exact_expected_version_counts_as_installed( + reported: str | None, +) -> None: + """``installed_at_pinned_version`` accepts the exact version line and nothing else. + + A prefix match would accept ``cargo-llvm-cov 0.9.0-rc1`` or + ``cargo-llvm-cov 0.9.01``; a substring match would accept a longer line + that merely mentions the version. + """ + module = _installer_module() + tool = _fake_tool(module, b"", extension="tar.gz", member="cargo-llvm-cov") + + class _Present: + def is_file(self) -> bool: + return True + + module.reported_version = lambda _binary, _args: reported + + outcome = module.installed_at_pinned_version(typ.cast("Path", _Present()), tool) + + assert outcome == (reported == "cargo-llvm-cov 0.9.0") + + def test_oversized_download_is_discarded( install_llvm_cov_module: ModuleType, tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: @@ -218,24 +307,33 @@ def __exit__(self, *_args: object) -> None: ) destination = tmp_path / tool.filename - with pytest.raises(BaseException) as excinfo: # noqa: PT011 - typer.Exit + with pytest.raises(typer.Exit) as excinfo: install_llvm_cov_module.download_archive(tool, destination) assert _exit_code(excinfo.value) == 1 assert not destination.exists() -def test_main_reuses_an_installed_binary_at_the_pinned_version( - install_llvm_cov_module: ModuleType, tmp_path: Path, monkeypatch: pytest.MonkeyPatch -) -> None: - """A binary already reporting the pinned version is kept and exported to PATH.""" +def _job_environment( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> tuple[Path, Path, Path]: + """Point CARGO_HOME, GITHUB_PATH and GITHUB_STEP_SUMMARY into ``tmp_path``.""" cargo_home = tmp_path / "cargo" - monkeypatch.setenv("CARGO_HOME", str(cargo_home)) github_path = tmp_path / "github_path" + summary = tmp_path / "summary.md" + monkeypatch.setenv("CARGO_HOME", str(cargo_home)) monkeypatch.setenv("GITHUB_PATH", str(github_path)) + monkeypatch.setenv("GITHUB_STEP_SUMMARY", str(summary)) monkeypatch.setenv("RUNNER_OS", "Linux") monkeypatch.setenv("RUNNER_ARCH", "X64") - binary = cargo_home / "bin" / "cargo-llvm-cov" + return cargo_home / "bin" / "cargo-llvm-cov", github_path, summary + + +def test_main_reuses_an_installed_binary_at_the_pinned_version( + install_llvm_cov_module: ModuleType, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """A binary already reporting the pinned version is kept and exported to PATH.""" + binary, github_path, summary = _job_environment(monkeypatch, tmp_path) binary.parent.mkdir(parents=True) binary.write_bytes(_FAKE_BINARY) binary.chmod(0o755) @@ -249,15 +347,128 @@ def fail_install(*_args: object, **_kwargs: object) -> None: install_llvm_cov_module.main() assert github_path.read_text(encoding="utf-8").strip() == str(binary.parent) + assert "metric cargo-llvm-cov.install=reused" in summary.read_text(encoding="utf-8") -def test_summary_metrics_are_bounded_lines( +def test_main_replaces_an_installed_binary_at_another_version( install_llvm_cov_module: ModuleType, tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: - """Each metric is one ``metric key=value`` line appended to the summary.""" - summary = tmp_path / "summary.md" - monkeypatch.setenv("GITHUB_STEP_SUMMARY", str(summary)) + """A binary reporting a different version triggers a fresh install.""" + binary, _github_path, _summary = _job_environment(monkeypatch, tmp_path) + binary.parent.mkdir(parents=True) + binary.write_bytes(_WRONG_VERSION_BINARY) + binary.chmod(0o755) + calls: list[Path] = [] + + monkeypatch.setattr( + install_llvm_cov_module, + "install", + lambda _tool, destination, **_kwargs: calls.append(destination), + ) + + install_llvm_cov_module.main() + + assert calls == [binary] + - install_llvm_cov_module.emit_metric("cargo-llvm-cov.install=ok") +class _ArchiveHandler(http.server.BaseHTTPRequestHandler): + """Serve one archive at one path; anything else is a 404.""" + + archive: typ.ClassVar[bytes] = b"" + path_served: typ.ClassVar[str] = "" + + def do_GET(self) -> None: + if self.path != self.path_served: + self.send_error(404) + return + self.send_response(200) + self.send_header("Content-Length", str(len(self.archive))) + self.end_headers() + self.wfile.write(self.archive) + + def log_message(self, *_args: object) -> None: + return + + +@pytest.fixture +def archive_server() -> typ.Iterator[typ.Callable[[bytes, str], str]]: + """Start a local HTTP server and return ``serve(archive, path) -> url``.""" + server = http.server.ThreadingHTTPServer(("127.0.0.1", 0), _ArchiveHandler) + thread = threading.Thread(target=server.serve_forever, daemon=True) + thread.start() - assert summary.read_text(encoding="utf-8") == "metric cargo-llvm-cov.install=ok\n" + def serve(archive: bytes, path: str) -> str: + _ArchiveHandler.archive = archive + _ArchiveHandler.path_served = path + return f"http://127.0.0.1:{server.server_port}{path}" + + yield serve + server.shutdown() + server.server_close() + + +def test_entry_point_installs_from_a_manifest_over_http( + install_llvm_cov_module: ModuleType, + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + archive_server: typ.Callable[[bytes, str], str], +) -> None: + """``main`` resolves, downloads, verifies, installs and exports, end to end. + + The manifest is a temporary one whose entry points at a local HTTP + server, so the real download path runs without leaving the machine. + """ + binary, github_path, summary = _job_environment(monkeypatch, tmp_path) + archive = _tarball_with("cargo-llvm-cov", _FAKE_BINARY) + url = archive_server( + archive, "/v0.9.0/cargo-llvm-cov-x86_64-unknown-linux-gnu.tar.gz" + ) + manifest = tmp_path / "tool-manifest.toml" + manifest.write_text( + "schema = 1\n\n" + "[[tool]]\n" + 'name = "cargo-llvm-cov"\n' + f'version = "{install_llvm_cov_module.CARGO_LLVM_COV_VERSION}"\n' + 'binary = "cargo-llvm-cov"\n' + 'version-args = ["llvm-cov", "--version"]\n\n' + " [[tool.target]]\n" + ' triple = "x86_64-unknown-linux-gnu"\n' + f' url = "{url}"\n' + f' sha256 = "{hashlib.sha256(archive).hexdigest()}"\n' + ' member = "cargo-llvm-cov"\n' + ' sidecar-verified = "absent"\n', + encoding="utf-8", + ) + monkeypatch.setattr(install_llvm_cov_module, "MANIFEST_PATH", manifest) + monkeypatch.setattr( + install_llvm_cov_module, + "load_manifest", + functools.partial(install_llvm_cov_module.load_manifest, manifest), + ) + + install_llvm_cov_module.main() + + assert binary.read_bytes() == _FAKE_BINARY + assert binary.stat().st_mode & 0o111 + assert github_path.read_text(encoding="utf-8").strip() == str(binary.parent) + metrics = summary.read_text(encoding="utf-8") + assert "metric cargo-llvm-cov.resolve=ok" in metrics + assert "metric cargo-llvm-cov.download=ok" in metrics + assert "metric cargo-llvm-cov.archive-digest=ok" in metrics + assert "metric cargo-llvm-cov.install=ok" in metrics + + +def test_entry_point_reports_a_resolution_failure_by_kind( + install_llvm_cov_module: ModuleType, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """An unsupported runner exits 1 with a bounded resolve metric.""" + _binary, _github_path, summary = _job_environment(monkeypatch, tmp_path) + monkeypatch.setenv("RUNNER_OS", "Plan9") + + with pytest.raises(typer.Exit) as excinfo: + install_llvm_cov_module.main() + + assert _exit_code(excinfo.value) == 1 + assert "metric cargo-llvm-cov.resolve=unsupported-runner" in summary.read_text( + encoding="utf-8" + ) diff --git a/.github/actions/ratchet-coverage/scripts/install_cargo_llvm_cov.py b/.github/actions/ratchet-coverage/scripts/install_cargo_llvm_cov.py index 79c3c4136..b6a1251b8 100755 --- a/.github/actions/ratchet-coverage/scripts/install_cargo_llvm_cov.py +++ b/.github/actions/ratchet-coverage/scripts/install_cargo_llvm_cov.py @@ -50,6 +50,9 @@ TOOL_NAME = "cargo-llvm-cov" +#: The resolver reports every other failure kind; this one is ours. +MANIFEST_UNREADABLE = "manifest-unreadable" + #: Where the manifest and the shared resolver live relative to this script: #: ``/.github/actions//scripts/``. _GITHUB_DIR = Path(__file__).resolve().parents[3] @@ -118,30 +121,50 @@ def runner_description() -> tuple[str, str]: return runner_os, runner_arch +class ToolResolutionError(Exception): + """The manifest offers no usable entry for this tool, version and runner. + + ``kind`` is one of the resolver's closed set of reasons, so the caller can + publish it as a bounded metric without inspecting the message. + """ + + def __init__(self, kind: str, message: str) -> None: + super().__init__(message) + self.kind = kind + + +def load_manifest(manifest_path: Path = MANIFEST_PATH) -> dict[str, object]: + """Read the tool manifest, or raise ``ToolResolutionError``.""" + try: + with manifest_path.open("rb") as handle: + return tomllib.load(handle) + except (OSError, tomllib.TOMLDecodeError) as exc: + message = f"could not read the tool manifest {manifest_path}: {exc}" + raise ToolResolutionError(MANIFEST_UNREADABLE, message) from exc + + def resolve_tool( version: str = CARGO_LLVM_COV_VERSION, *, - manifest_path: Path = MANIFEST_PATH, + manifest: dict[str, object] | None = None, runner: tuple[str, str] | None = None, ) -> ResolvedTool: - """Resolve the manifest entry for ``version`` on this runner or fail.""" + """Return the manifest entry for ``version`` on ``runner``. + + A query with no side effects: it reads the manifest (or the one passed + in) and either returns the entry or raises ``ToolResolutionError``. + """ resolver = _load_resolver() - try: - with manifest_path.open("rb") as handle: - manifest = tomllib.load(handle) - except (OSError, tomllib.TOMLDecodeError) as exc: - emit_metric("cargo-llvm-cov.resolve=manifest-unreadable") - typer.echo(f"could not read the tool manifest {manifest_path}: {exc}", err=True) - raise typer.Exit(1) from exc + if manifest is None: + manifest = load_manifest() runner_os, runner_arch = runner or runner_description() fields = resolver.resolve( manifest, TOOL_NAME, version, resolver.Runner(runner_os, runner_arch) ) if fields.get("status") != "ok": - emit_metric(f"cargo-llvm-cov.resolve={fields.get('error_kind')}") - typer.echo(str(fields.get("error_message")), err=True) - raise typer.Exit(1) - emit_metric("cargo-llvm-cov.resolve=ok") + kind = str(fields.get("error_kind")) + message = str(fields.get("error_message")) + raise ToolResolutionError(kind, message) return ResolvedTool( triple=str(fields["triple"]), url=str(fields["url"]), @@ -185,8 +208,7 @@ def installed_at_pinned_version(destination: Path, tool: ResolvedTool) -> bool: """Whether ``destination`` already holds the binary at the pinned version.""" if not destination.is_file(): return False - reported = reported_version(destination, tool.version_args) - return reported is not None and reported.startswith(tool.expected_version) + return reported_version(destination, tool.version_args) == tool.expected_version class _ArchiveTooLargeError(Exception): @@ -324,7 +346,13 @@ def install( ``destination`` is left intact when any step before the final move fails. """ destination.parent.mkdir(parents=True, exist_ok=True) - with tempfile.TemporaryDirectory(prefix="cargo-llvm-cov-") as workdir: + # Staged in the destination's own directory so the final publish is a + # rename on one filesystem: a temporary directory elsewhere would turn + # the move into copy-then-delete, during which a concurrent reader could + # open a half-written executable. + with tempfile.TemporaryDirectory( + prefix=".cargo-llvm-cov-staging-", dir=destination.parent + ) as workdir: archive = Path(workdir) / tool.filename fetch(tool, archive) verify_archive(archive, tool) @@ -336,9 +364,9 @@ def install( typer.echo(f"cargo-llvm-cov archive extraction failed: {exc}", err=True) raise typer.Exit(1) from exc staged.chmod(0o755) - shutil.move(str(staged), str(destination)) + staged.replace(destination) reported = reported_version(destination, tool.version_args) - if reported is None or not reported.startswith(tool.expected_version): + if reported != tool.expected_version: emit_metric("cargo-llvm-cov.install=version-mismatch") typer.echo( f"installed cargo-llvm-cov reports {reported!r}, expected " @@ -360,7 +388,13 @@ def export_path(directory: Path) -> None: def main() -> None: """Install cargo-llvm-cov at the pinned version from the tool manifest.""" - tool = resolve_tool() + try: + tool = resolve_tool() + except ToolResolutionError as exc: + emit_metric(f"cargo-llvm-cov.resolve={exc.kind}") + typer.echo(str(exc), err=True) + raise typer.Exit(1) from exc + emit_metric("cargo-llvm-cov.resolve=ok") destination = cargo_bin() / tool.binary if installed_at_pinned_version(destination, tool): emit_metric("cargo-llvm-cov.install=reused") diff --git a/docs/developers-guide.md b/docs/developers-guide.md index c0b8861d1..c32731350 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -916,53 +916,47 @@ environment arrives as an argument rather than through `os.environ`. `_required_env` and `_env_bool` in `common.py` accept the same optional mapping for that reason. -## `generate-coverage` cargo-binstall Pinning - -`generate-coverage` provisions its own `cargo-binstall` in the "Ensure -cargo-binstall" step before installing `cargo-llvm-cov`. It follows the same -pinning discipline as `setup-rust`: `BINSTALL_VERSION` and the installer-script -`BINSTALL_SHA256` are a pair and must be updated together. - -The step is idempotent and verifies the version on both paths: - -- **Fast path** — if `cargo-binstall` is already on `PATH`, its `-V` output is - matched against the pinned version. On a match the step reuses the binary and - exits without any network access; on a mismatch it logs the discrepancy and - falls through to a pinned reinstall. -- **Install path** — the checksum-pinned installer script is downloaded and its - SHA-256 verified before execution, and the freshly installed binary's version - is re-checked so a wrong installed version fails the step. - -Both paths are exercised by behavioural tests in -`.github/actions/generate-coverage/tests/test_scripts.py`, which execute the -extracted step body against fake `cargo-binstall` binaries and installers -rather than asserting on the step's source text. - -### CARGO_HOME resolution and PATH handling - -The "Ensure cargo-binstall" step derives the active Cargo bin directory at -runtime: - -```bash -cargo_home_bin="${CARGO_HOME:-$HOME/.cargo}/bin" -``` - -This respects any custom `CARGO_HOME` set by the caller. The resolved path is -used for three purposes: - -1. **GITHUB_PATH** – when `GITHUB_PATH` is set, the resolved bin directory is - appended so that subsequent workflow steps see the binary on their `PATH`. -2. **Current-step PATH** – the bin directory is prepended to the *current* - shell's `PATH` (guarded by a `case ":$PATH:"` check to avoid duplication) so - that in-step commands can also find the binary. -3. **Absolute-path verification** – `cargo-binstall` is invoked via its - resolved absolute path (`"$cargo_binstall"`) rather than as an unqualified - command, ensuring that verification succeeds even when the bin directory has - not yet been propagated to the shell's `PATH` by other means. - -Keep `cargo_home_bin` resolution and the `BINSTALL_VERSION` pin in sync: both -must reflect the same intended installation location and version whenever the -pin is updated. +## `generate-coverage` and `ratchet-coverage` cargo-llvm-cov installation + +Both coverage actions install `cargo-llvm-cov` with the same script, +`scripts/install_cargo_llvm_cov.py`, from the repository's tool manifest +(`.github/tool-manifest.toml`). `CARGO_LLVM_COV_VERSION` in the script names +the manifest entry; the resolver refuses a version the manifest does not list, +so bumping the tool means adding the manifest entry first (every digest from an +independent download, as the manifest's header prescribes) and moving the +constant second. + +The script has one pure step and one effectful one: + +- **Resolution** (`load_manifest`, `resolve_tool`) reads the manifest and + selects the archive for the runner with the `install-tool` resolver + (`.github/actions/install-tool/scripts/resolve_tool.py`), using `RUNNER_OS` + and `RUNNER_ARCH` inside a job and `platform` outside one. It returns a + `ResolvedTool` or raises `ToolResolutionError` carrying the resolver's + bounded failure kind; it publishes nothing. +- **Installation** (`install`) downloads the archive with a 200 MB cap, + verifies its SHA-256 against the manifest, extracts exactly the manifest's + `member`, stages it in a temporary directory beside the destination and + publishes it with a rename, so a concurrent reader never sees a partially + written executable, then checks the binary reports exactly + `expected_version`. A binary already reporting that version is reused. + +`main` is the command boundary: it turns a `ToolResolutionError` into an exit +status and publishes the `cargo-llvm-cov.resolve`, `.download`, +`.archive-digest` and `.install` metrics, each over a closed set of values, to +the log and the job summary. It appends `CARGO_HOME/bin` to `GITHUB_PATH` so +later steps find the binary. + +The behavioural tests live in +`.github/actions/generate-coverage/tests/test_install_cargo_llvm_cov.py`. They +hold the pinned version to the manifest for every runner the resolver knows, +drive `main` end to end against a temporary manifest whose entry points at a +local HTTP server, and cover digest mismatch, a missing member, an oversized +download, a binary reporting another version, and the reuse path. A Hypothesis +property holds that only the exact expected version line counts as installed. + +`generate-coverage` no longer provisions `cargo-binstall`: nothing in either +action invokes `cargo binstall`. ## `generate-coverage` cargo-nextest installation diff --git a/docs/users-guide.md b/docs/users-guide.md index 487f71e26..f4e152f42 100644 --- a/docs/users-guide.md +++ b/docs/users-guide.md @@ -580,7 +580,19 @@ failure is also annotated with `::error`. ## `generate-coverage` action The `generate-coverage` composite action runs `cargo llvm-cov` for Rust -projects and, by default, drives it through `cargo nextest`. The optional +projects and, by default, drives it through `cargo nextest`. + +`cargo-llvm-cov` itself is installed from the repository's tool manifest +(`.github/tool-manifest.toml`), at 0.9.0. The installer selects the archive for +the runner's operating system and architecture (x86_64 and aarch64 Linux, +x86_64 and aarch64 macOS, x86_64 Windows), verifies its SHA-256 digest against +the manifest, extracts only the named executable into `CARGO_HOME/bin`, and +reuses an installed binary that already reports the pinned version. There is +no `cargo binstall` or `cargo install` fallback. 0.9.0 is the first release +that reads Cargo's new build-dir layout, which cargo 1.100 nightlies enable by +default; the previous 0.6.24 searched `target/llvm-cov-target/debug/deps` and +failed with `failed to collect object files` on those toolchains after every +test had passed. The same installer serves `ratchet-coverage`. The optional `use-cargo-nextest` input defaults to `true`; set it to `false` to run `cargo llvm-cov` directly instead of through `cargo nextest`. From b344bcbeda8a3e239efa67c7615b6adcd3348968 Mon Sep 17 00:00:00 2001 From: Payton McIntosh Date: Sun, 6 Sep 2026 02:30:45 +0100 Subject: [PATCH 04/11] Validate the manifest schema and trim the test helper The installers call the resolver directly, so they now fail closed on a manifest schema other than the one the resolver reads, as the install-tool action does, with a typed unsupported-schema kind and a test. The test helper _fake_tool loses an unused url parameter (CodeScene: five arguments where four is the ceiling). --- .../scripts/install_cargo_llvm_cov.py | 10 +++++++ .../tests/test_install_cargo_llvm_cov.py | 30 ++++++++++++------- .../scripts/install_cargo_llvm_cov.py | 10 +++++++ 3 files changed, 39 insertions(+), 11 deletions(-) diff --git a/.github/actions/generate-coverage/scripts/install_cargo_llvm_cov.py b/.github/actions/generate-coverage/scripts/install_cargo_llvm_cov.py index b6a1251b8..d7fc6f917 100755 --- a/.github/actions/generate-coverage/scripts/install_cargo_llvm_cov.py +++ b/.github/actions/generate-coverage/scripts/install_cargo_llvm_cov.py @@ -157,6 +157,16 @@ def resolve_tool( resolver = _load_resolver() if manifest is None: manifest = load_manifest() + schema = manifest.get("schema") + if schema != resolver.SCHEMA: + # The generic install-tool action fails closed on a schema it does + # not read; calling the resolver directly must not skip that check, + # or a later layout with plausible old fields would resolve wrongly. + message = ( + f"the tool manifest declares schema {schema!r}; this installer " + f"reads schema {resolver.SCHEMA}" + ) + raise ToolResolutionError(resolver.UNSUPPORTED_SCHEMA, message) runner_os, runner_arch = runner or runner_description() fields = resolver.resolve( manifest, TOOL_NAME, version, resolver.Runner(runner_os, runner_arch) diff --git a/.github/actions/generate-coverage/tests/test_install_cargo_llvm_cov.py b/.github/actions/generate-coverage/tests/test_install_cargo_llvm_cov.py index d2cca6384..e85792c62 100644 --- a/.github/actions/generate-coverage/tests/test_install_cargo_llvm_cov.py +++ b/.github/actions/generate-coverage/tests/test_install_cargo_llvm_cov.py @@ -95,6 +95,20 @@ def test_unknown_version_is_refused_rather_than_floated( assert excinfo.value.kind == "unknown-version" +def test_manifest_with_another_schema_is_refused( + install_llvm_cov_module: ModuleType, +) -> None: + """A manifest schema this installer does not read fails closed, by kind.""" + manifest = {"schema": 2, "tool": []} + + with pytest.raises(install_llvm_cov_module.ToolResolutionError) as excinfo: + install_llvm_cov_module.resolve_tool( + manifest=manifest, runner=RUNNERS["linux-x64"] + ) + + assert excinfo.value.kind == "unsupported-schema" + + def test_unreadable_manifest_is_a_typed_error( install_llvm_cov_module: ModuleType, tmp_path: Path ) -> None: @@ -123,18 +137,12 @@ def _zip_with(member: str, payload: bytes) -> bytes: def _fake_tool( - module: ModuleType, - archive: bytes, - *, - extension: str, - member: str, - url: str | None = None, + module: ModuleType, archive: bytes, *, extension: str, member: str ) -> ResolvedTool: - if url is None: - url = ( - "https://github.com/taiki-e/cargo-llvm-cov/releases/download/v0.9.0/" - f"cargo-llvm-cov-x86_64-unknown-linux-gnu.{extension}" - ) + url = ( + "https://github.com/taiki-e/cargo-llvm-cov/releases/download/v0.9.0/" + f"cargo-llvm-cov-x86_64-unknown-linux-gnu.{extension}" + ) return typ.cast( "ResolvedTool", module.ResolvedTool( diff --git a/.github/actions/ratchet-coverage/scripts/install_cargo_llvm_cov.py b/.github/actions/ratchet-coverage/scripts/install_cargo_llvm_cov.py index b6a1251b8..d7fc6f917 100755 --- a/.github/actions/ratchet-coverage/scripts/install_cargo_llvm_cov.py +++ b/.github/actions/ratchet-coverage/scripts/install_cargo_llvm_cov.py @@ -157,6 +157,16 @@ def resolve_tool( resolver = _load_resolver() if manifest is None: manifest = load_manifest() + schema = manifest.get("schema") + if schema != resolver.SCHEMA: + # The generic install-tool action fails closed on a schema it does + # not read; calling the resolver directly must not skip that check, + # or a later layout with plausible old fields would resolve wrongly. + message = ( + f"the tool manifest declares schema {schema!r}; this installer " + f"reads schema {resolver.SCHEMA}" + ) + raise ToolResolutionError(resolver.UNSUPPORTED_SCHEMA, message) runner_os, runner_arch = runner or runner_description() fields = resolver.resolve( manifest, TOOL_NAME, version, resolver.Runner(runner_os, runner_arch) From 0ce893b67f3bd26ed5462d462840a1b19ae4e0a8 Mon Sep 17 00:00:00 2001 From: Payton McIntosh Date: Sun, 6 Sep 2026 02:32:41 +0100 Subject: [PATCH 05/11] Probe the staged binary before publishing it A checksum-valid archive whose binary reports another version used to be published and only then rejected, replacing whatever was installed. The version probe now runs on the staged file, so a mismatch leaves the destination untouched; the test asserts the previous binary survives. --- .../scripts/install_cargo_llvm_cov.py | 21 +++++++++++-------- .../tests/test_install_cargo_llvm_cov.py | 6 +++++- .../scripts/install_cargo_llvm_cov.py | 21 +++++++++++-------- 3 files changed, 29 insertions(+), 19 deletions(-) diff --git a/.github/actions/generate-coverage/scripts/install_cargo_llvm_cov.py b/.github/actions/generate-coverage/scripts/install_cargo_llvm_cov.py index d7fc6f917..498ac2d8d 100755 --- a/.github/actions/generate-coverage/scripts/install_cargo_llvm_cov.py +++ b/.github/actions/generate-coverage/scripts/install_cargo_llvm_cov.py @@ -374,16 +374,19 @@ def install( typer.echo(f"cargo-llvm-cov archive extraction failed: {exc}", err=True) raise typer.Exit(1) from exc staged.chmod(0o755) + # Probe the staged binary, not the published one: a checksum-valid + # archive whose binary reports another version must leave whatever + # was installed before untouched. + reported = reported_version(staged, tool.version_args) + if reported != tool.expected_version: + emit_metric("cargo-llvm-cov.install=version-mismatch") + typer.echo( + f"extracted cargo-llvm-cov reports {reported!r}, expected " + f"{tool.expected_version!r}", + err=True, + ) + raise typer.Exit(1) staged.replace(destination) - reported = reported_version(destination, tool.version_args) - if reported != tool.expected_version: - emit_metric("cargo-llvm-cov.install=version-mismatch") - typer.echo( - f"installed cargo-llvm-cov reports {reported!r}, expected " - f"{tool.expected_version!r}", - err=True, - ) - raise typer.Exit(1) emit_metric("cargo-llvm-cov.install=ok") diff --git a/.github/actions/generate-coverage/tests/test_install_cargo_llvm_cov.py b/.github/actions/generate-coverage/tests/test_install_cargo_llvm_cov.py index e85792c62..247c3e84b 100644 --- a/.github/actions/generate-coverage/tests/test_install_cargo_llvm_cov.py +++ b/.github/actions/generate-coverage/tests/test_install_cargo_llvm_cov.py @@ -235,17 +235,21 @@ def test_rejected_archive_fails_and_preserves_the_existing_binary( def test_installed_binary_reporting_another_version_fails_the_install( install_llvm_cov_module: ModuleType, tmp_path: Path ) -> None: - """A verified archive whose binary reports the wrong version is still a failure.""" + """A verified archive whose binary reports the wrong version is not published.""" archive = _tarball_with("cargo-llvm-cov", _WRONG_VERSION_BINARY) tool = _fake_tool( install_llvm_cov_module, archive, extension="tar.gz", member="cargo-llvm-cov" ) destination = tmp_path / "bin" / "cargo-llvm-cov" + destination.parent.mkdir() + destination.write_bytes(b"previous") with pytest.raises(typer.Exit) as excinfo: install_llvm_cov_module.install(tool, destination, fetch=_write_fetch(archive)) assert _exit_code(excinfo.value) == 1 + assert destination.read_bytes() == b"previous" + assert [p.name for p in destination.parent.iterdir()] == ["cargo-llvm-cov"] def _installer_module() -> ModuleType: diff --git a/.github/actions/ratchet-coverage/scripts/install_cargo_llvm_cov.py b/.github/actions/ratchet-coverage/scripts/install_cargo_llvm_cov.py index d7fc6f917..498ac2d8d 100755 --- a/.github/actions/ratchet-coverage/scripts/install_cargo_llvm_cov.py +++ b/.github/actions/ratchet-coverage/scripts/install_cargo_llvm_cov.py @@ -374,16 +374,19 @@ def install( typer.echo(f"cargo-llvm-cov archive extraction failed: {exc}", err=True) raise typer.Exit(1) from exc staged.chmod(0o755) + # Probe the staged binary, not the published one: a checksum-valid + # archive whose binary reports another version must leave whatever + # was installed before untouched. + reported = reported_version(staged, tool.version_args) + if reported != tool.expected_version: + emit_metric("cargo-llvm-cov.install=version-mismatch") + typer.echo( + f"extracted cargo-llvm-cov reports {reported!r}, expected " + f"{tool.expected_version!r}", + err=True, + ) + raise typer.Exit(1) staged.replace(destination) - reported = reported_version(destination, tool.version_args) - if reported != tool.expected_version: - emit_metric("cargo-llvm-cov.install=version-mismatch") - typer.echo( - f"installed cargo-llvm-cov reports {reported!r}, expected " - f"{tool.expected_version!r}", - err=True, - ) - raise typer.Exit(1) emit_metric("cargo-llvm-cov.install=ok") From de9fd09aeea8536e5aaaa6bdd5013a24766b1f73 Mon Sep 17 00:00:00 2001 From: Payton McIntosh Date: Sun, 6 Sep 2026 03:03:37 +0100 Subject: [PATCH 06/11] Make the version probe a value and the reuse decision pure Review round three on #470. probe_version is now the one place a process is spawned to read a version and returns a VersionProbe over absent, unrunnable and reported; installed_at_pinned_version takes that value and the expected version and is pure; main publishes the probe outcome as a bounded cargo-llvm-cov.probe metric before deciding reuse. The Hypothesis property ranges over probe states as well as version strings, a parametrised test exercises each probe outcome against a real file, every test helper has a docstring, and the generate-coverage design document gains a dated decision superseding the cargo-binstall one and its addendum. --- .../scripts/install_cargo_llvm_cov.py | 61 ++++++++++++---- .../tests/test_install_cargo_llvm_cov.py | 69 +++++++++++++++---- .../scripts/install_cargo_llvm_cov.py | 61 ++++++++++++---- docs/developers-guide.md | 7 +- docs/generate-coverage-design.md | 28 ++++++-- 5 files changed, 176 insertions(+), 50 deletions(-) diff --git a/.github/actions/generate-coverage/scripts/install_cargo_llvm_cov.py b/.github/actions/generate-coverage/scripts/install_cargo_llvm_cov.py index 498ac2d8d..a5a2d80e9 100755 --- a/.github/actions/generate-coverage/scripts/install_cargo_llvm_cov.py +++ b/.github/actions/generate-coverage/scripts/install_cargo_llvm_cov.py @@ -202,23 +202,52 @@ def cargo_bin() -> Path: return cargo_home / "bin" -def reported_version(binary: Path, version_args: tuple[str, ...]) -> str | None: - """Return the version line ``binary`` reports, or None if it cannot run.""" +#: What probing an installed binary can find. Closed, so the command +#: boundary can publish it as a bounded metric. +PROBE_ABSENT = "absent" +PROBE_UNRUNNABLE = "unrunnable" +PROBE_REPORTED = "reported" + + +class VersionProbe(typ.NamedTuple): + """The outcome of asking a binary for its version. + + ``version`` is the first line the binary printed when ``state`` is + ``reported``, and ``None`` otherwise. + """ + + state: str + version: str | None + + def metric_state(self, expected_version: str) -> str: + """Return the bounded state this probe publishes against a pin.""" + if self.state != PROBE_REPORTED: + return self.state + return "pinned" if self.version == expected_version else "other-version" + + +def probe_version(binary: Path, version_args: tuple[str, ...]) -> VersionProbe: + """Run ``binary`` with ``version_args`` and report what happened. + + The only place a process is spawned to read a version. Every outcome is + a value: a missing file, a binary that cannot run or exits non-zero, and + a reported version line are all distinct states rather than exceptions. + """ + if not binary.is_file(): + return VersionProbe(PROBE_ABSENT, None) if not version_args: - return None + return VersionProbe(PROBE_UNRUNNABLE, None) try: output = local[str(binary)][list(version_args)](timeout=60) except (OSError, CommandNotFound, ProcessExecutionError): - return None + return VersionProbe(PROBE_UNRUNNABLE, None) text = str(output).strip() - return text.splitlines()[0] if text else "" + return VersionProbe(PROBE_REPORTED, text.splitlines()[0] if text else "") -def installed_at_pinned_version(destination: Path, tool: ResolvedTool) -> bool: - """Whether ``destination`` already holds the binary at the pinned version.""" - if not destination.is_file(): - return False - return reported_version(destination, tool.version_args) == tool.expected_version +def installed_at_pinned_version(probe: VersionProbe, expected_version: str) -> bool: + """Whether a probe shows exactly the pinned version. Pure.""" + return probe.state == PROBE_REPORTED and probe.version == expected_version class _ArchiveTooLargeError(Exception): @@ -377,12 +406,12 @@ def install( # Probe the staged binary, not the published one: a checksum-valid # archive whose binary reports another version must leave whatever # was installed before untouched. - reported = reported_version(staged, tool.version_args) - if reported != tool.expected_version: + probe = probe_version(staged, tool.version_args) + if not installed_at_pinned_version(probe, tool.expected_version): emit_metric("cargo-llvm-cov.install=version-mismatch") typer.echo( - f"extracted cargo-llvm-cov reports {reported!r}, expected " - f"{tool.expected_version!r}", + f"extracted cargo-llvm-cov {probe.state}, reports " + f"{probe.version!r}, expected {tool.expected_version!r}", err=True, ) raise typer.Exit(1) @@ -409,7 +438,9 @@ def main() -> None: raise typer.Exit(1) from exc emit_metric("cargo-llvm-cov.resolve=ok") destination = cargo_bin() / tool.binary - if installed_at_pinned_version(destination, tool): + probe = probe_version(destination, tool.version_args) + emit_metric(f"cargo-llvm-cov.probe={probe.metric_state(tool.expected_version)}") + if installed_at_pinned_version(probe, tool.expected_version): emit_metric("cargo-llvm-cov.install=reused") typer.echo(f"cargo-llvm-cov {CARGO_LLVM_COV_VERSION} already installed") else: diff --git a/.github/actions/generate-coverage/tests/test_install_cargo_llvm_cov.py b/.github/actions/generate-coverage/tests/test_install_cargo_llvm_cov.py index 247c3e84b..930539160 100644 --- a/.github/actions/generate-coverage/tests/test_install_cargo_llvm_cov.py +++ b/.github/actions/generate-coverage/tests/test_install_cargo_llvm_cov.py @@ -120,6 +120,7 @@ def test_unreadable_manifest_is_a_typed_error( def _tarball_with(member: str, payload: bytes) -> bytes: + """Return a gzip tarball holding ``payload`` at ``member``, mode 0o755.""" buffer = io.BytesIO() with tarfile.open(fileobj=buffer, mode="w:gz") as package: info = tarfile.TarInfo(member) @@ -130,6 +131,7 @@ def _tarball_with(member: str, payload: bytes) -> bytes: def _zip_with(member: str, payload: bytes) -> bytes: + """Return a zip archive holding ``payload`` at ``member``.""" buffer = io.BytesIO() with zipfile.ZipFile(buffer, mode="w") as package: package.writestr(member, payload) @@ -139,6 +141,7 @@ def _zip_with(member: str, payload: bytes) -> bytes: def _fake_tool( module: ModuleType, archive: bytes, *, extension: str, member: str ) -> ResolvedTool: + """Return a ``ResolvedTool`` whose digest matches ``archive``.""" url = ( "https://github.com/taiki-e/cargo-llvm-cov/releases/download/v0.9.0/" f"cargo-llvm-cov-x86_64-unknown-linux-gnu.{extension}" @@ -159,7 +162,10 @@ def _fake_tool( def _write_fetch(archive: bytes) -> typ.Callable[[object, Path], None]: + """Return a ``fetch`` stand-in that writes ``archive`` instead of downloading.""" + def fetch(_tool: object, destination: Path) -> None: + """Write the canned archive to ``destination``.""" destination.write_bytes(archive) return fetch @@ -184,7 +190,10 @@ def test_install_extracts_the_manifest_member_and_verifies_it( assert destination.read_bytes() == _FAKE_BINARY assert destination.stat().st_mode & 0o111 - assert install_llvm_cov_module.installed_at_pinned_version(destination, tool) + probe = install_llvm_cov_module.probe_version(destination, tool.version_args) + assert install_llvm_cov_module.installed_at_pinned_version( + probe, tool.expected_version + ) assert [p.name for p in destination.parent.iterdir()] == ["cargo-llvm-cov"], ( "the staging directory must not outlive the install" ) @@ -272,28 +281,55 @@ def _installer_module() -> ModuleType: return module -@given(reported=st.one_of(st.none(), st.text(max_size=40))) -def test_only_the_exact_expected_version_counts_as_installed( - reported: str | None, +_PROBE_STATES = st.sampled_from(["absent", "unrunnable", "reported"]) + + +@given(state=_PROBE_STATES, version=st.one_of(st.none(), st.text(max_size=40))) +def test_only_a_reported_exact_version_counts_as_installed( + state: str, version: str | None ) -> None: - """``installed_at_pinned_version`` accepts the exact version line and nothing else. + """``installed_at_pinned_version`` accepts one probe outcome and nothing else. A prefix match would accept ``cargo-llvm-cov 0.9.0-rc1`` or ``cargo-llvm-cov 0.9.01``; a substring match would accept a longer line - that merely mentions the version. + that merely mentions the version; and an absent or unrunnable binary must + never count, whatever ``version`` says. """ module = _installer_module() - tool = _fake_tool(module, b"", extension="tar.gz", member="cargo-llvm-cov") + probe = module.VersionProbe(state, version) - class _Present: - def is_file(self) -> bool: - return True + outcome = module.installed_at_pinned_version(probe, "cargo-llvm-cov 0.9.0") - module.reported_version = lambda _binary, _args: reported + assert outcome == (state == "reported" and version == "cargo-llvm-cov 0.9.0") + assert probe.metric_state("cargo-llvm-cov 0.9.0") == ( + state if state != "reported" else ("pinned" if outcome else "other-version") + ) - outcome = module.installed_at_pinned_version(typ.cast("Path", _Present()), tool) - assert outcome == (reported == "cargo-llvm-cov 0.9.0") +@pytest.mark.parametrize( + ("binary", "expected"), + [ + pytest.param(None, ("absent", None), id="absent"), + pytest.param(b"not executable", ("unrunnable", None), id="unrunnable"), + pytest.param(_FAKE_BINARY, ("reported", "cargo-llvm-cov 0.9.0"), id="reported"), + ], +) +def test_probe_version_reports_each_outcome_as_a_value( + install_llvm_cov_module: ModuleType, + tmp_path: Path, + binary: bytes | None, + expected: tuple[str, str | None], +) -> None: + """A missing, unrunnable and reporting binary are three distinct probe states.""" + path = tmp_path / "cargo-llvm-cov" + if binary is not None: + path.write_bytes(binary) + if binary == _FAKE_BINARY: + path.chmod(0o755) + + probe = install_llvm_cov_module.probe_version(path, ("llvm-cov", "--version")) + + assert tuple(probe) == expected def test_oversized_download_is_discarded( @@ -303,10 +339,14 @@ def test_oversized_download_is_discarded( monkeypatch.setattr(install_llvm_cov_module, "_MAX_ARCHIVE_BYTES", 16) class _Response(io.BytesIO): + """A urlopen response that streams more bytes than the cap allows.""" + def __enter__(self) -> _Response: + """Enter the response context.""" return self def __exit__(self, *_args: object) -> None: + """Close the response.""" self.close() monkeypatch.setattr( @@ -351,6 +391,7 @@ def test_main_reuses_an_installed_binary_at_the_pinned_version( binary.chmod(0o755) def fail_install(*_args: object, **_kwargs: object) -> None: + """Fail the test if the installer tries to install.""" message = "install must not run for a reused binary" raise AssertionError(message) @@ -399,6 +440,7 @@ def do_GET(self) -> None: self.wfile.write(self.archive) def log_message(self, *_args: object) -> None: + """Keep the server quiet during the test.""" return @@ -410,6 +452,7 @@ def archive_server() -> typ.Iterator[typ.Callable[[bytes, str], str]]: thread.start() def serve(archive: bytes, path: str) -> str: + """Publish ``archive`` at ``path`` and return its URL.""" _ArchiveHandler.archive = archive _ArchiveHandler.path_served = path return f"http://127.0.0.1:{server.server_port}{path}" diff --git a/.github/actions/ratchet-coverage/scripts/install_cargo_llvm_cov.py b/.github/actions/ratchet-coverage/scripts/install_cargo_llvm_cov.py index 498ac2d8d..a5a2d80e9 100755 --- a/.github/actions/ratchet-coverage/scripts/install_cargo_llvm_cov.py +++ b/.github/actions/ratchet-coverage/scripts/install_cargo_llvm_cov.py @@ -202,23 +202,52 @@ def cargo_bin() -> Path: return cargo_home / "bin" -def reported_version(binary: Path, version_args: tuple[str, ...]) -> str | None: - """Return the version line ``binary`` reports, or None if it cannot run.""" +#: What probing an installed binary can find. Closed, so the command +#: boundary can publish it as a bounded metric. +PROBE_ABSENT = "absent" +PROBE_UNRUNNABLE = "unrunnable" +PROBE_REPORTED = "reported" + + +class VersionProbe(typ.NamedTuple): + """The outcome of asking a binary for its version. + + ``version`` is the first line the binary printed when ``state`` is + ``reported``, and ``None`` otherwise. + """ + + state: str + version: str | None + + def metric_state(self, expected_version: str) -> str: + """Return the bounded state this probe publishes against a pin.""" + if self.state != PROBE_REPORTED: + return self.state + return "pinned" if self.version == expected_version else "other-version" + + +def probe_version(binary: Path, version_args: tuple[str, ...]) -> VersionProbe: + """Run ``binary`` with ``version_args`` and report what happened. + + The only place a process is spawned to read a version. Every outcome is + a value: a missing file, a binary that cannot run or exits non-zero, and + a reported version line are all distinct states rather than exceptions. + """ + if not binary.is_file(): + return VersionProbe(PROBE_ABSENT, None) if not version_args: - return None + return VersionProbe(PROBE_UNRUNNABLE, None) try: output = local[str(binary)][list(version_args)](timeout=60) except (OSError, CommandNotFound, ProcessExecutionError): - return None + return VersionProbe(PROBE_UNRUNNABLE, None) text = str(output).strip() - return text.splitlines()[0] if text else "" + return VersionProbe(PROBE_REPORTED, text.splitlines()[0] if text else "") -def installed_at_pinned_version(destination: Path, tool: ResolvedTool) -> bool: - """Whether ``destination`` already holds the binary at the pinned version.""" - if not destination.is_file(): - return False - return reported_version(destination, tool.version_args) == tool.expected_version +def installed_at_pinned_version(probe: VersionProbe, expected_version: str) -> bool: + """Whether a probe shows exactly the pinned version. Pure.""" + return probe.state == PROBE_REPORTED and probe.version == expected_version class _ArchiveTooLargeError(Exception): @@ -377,12 +406,12 @@ def install( # Probe the staged binary, not the published one: a checksum-valid # archive whose binary reports another version must leave whatever # was installed before untouched. - reported = reported_version(staged, tool.version_args) - if reported != tool.expected_version: + probe = probe_version(staged, tool.version_args) + if not installed_at_pinned_version(probe, tool.expected_version): emit_metric("cargo-llvm-cov.install=version-mismatch") typer.echo( - f"extracted cargo-llvm-cov reports {reported!r}, expected " - f"{tool.expected_version!r}", + f"extracted cargo-llvm-cov {probe.state}, reports " + f"{probe.version!r}, expected {tool.expected_version!r}", err=True, ) raise typer.Exit(1) @@ -409,7 +438,9 @@ def main() -> None: raise typer.Exit(1) from exc emit_metric("cargo-llvm-cov.resolve=ok") destination = cargo_bin() / tool.binary - if installed_at_pinned_version(destination, tool): + probe = probe_version(destination, tool.version_args) + emit_metric(f"cargo-llvm-cov.probe={probe.metric_state(tool.expected_version)}") + if installed_at_pinned_version(probe, tool.expected_version): emit_metric("cargo-llvm-cov.install=reused") typer.echo(f"cargo-llvm-cov {CARGO_LLVM_COV_VERSION} already installed") else: diff --git a/docs/developers-guide.md b/docs/developers-guide.md index c32731350..6c644704d 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -938,8 +938,11 @@ The script has one pure step and one effectful one: verifies its SHA-256 against the manifest, extracts exactly the manifest's `member`, stages it in a temporary directory beside the destination and publishes it with a rename, so a concurrent reader never sees a partially - written executable, then checks the binary reports exactly - `expected_version`. A binary already reporting that version is reused. + written executable. Before publishing it probes the staged binary + (`probe_version`, the one place a process is spawned to read a version, + returning a `VersionProbe` value over `absent`, `unrunnable` and `reported`) + and the pure `installed_at_pinned_version` accepts only a reported line equal + to `expected_version`. The same probe on the destination decides reuse. `main` is the command boundary: it turns a `ToolResolutionError` into an exit status and publishes the `cargo-llvm-cov.resolve`, `.download`, diff --git a/docs/generate-coverage-design.md b/docs/generate-coverage-design.md index 7400531fb..45a4efee4 100644 --- a/docs/generate-coverage-design.md +++ b/docs/generate-coverage-design.md @@ -59,6 +59,23 @@ action and the evolution of its supporting scripts. relying on an unpinned or stale binary already present on the runner. Both the fast (reuse) and install paths are covered by behavioural tests that execute the extracted step body against fake binaries and installers. +- *2026-09-06* — `cargo-llvm-cov` is installed from the repository tool + manifest (`.github/tool-manifest.toml`) at 0.9.0, and the "Ensure + cargo-binstall" step is gone: nothing in the action shells out to + `cargo binstall` any more, so the 2026-07-04 decision above is superseded. + The trigger was cargo 1.100 nightlies enabling Cargo's new build-dir layout, + which puts test executables under `debug/build///out`; + cargo-llvm-cov 0.6.24 searched `debug/deps` and failed with `failed to + collect object files` after every test passed, and 0.9.0 reads the new + layout. `install_cargo_llvm_cov.py` (shared byte for byte with + `ratchet-coverage`) resolves the manifest entry through the `install-tool` + resolver, refusing any other manifest schema, selects the archive for the + runner's OS and architecture, downloads it with a 200 MB cap, verifies its + SHA-256 against the manifest, extracts only the named member, probes the + staged binary and publishes it with a rename beside the destination only + when it reports exactly the pinned version. Resolution and the version + probe are values (`ToolResolutionError`, `VersionProbe`) and the command + boundary in `main` publishes every outcome as a bounded metric. - *2026-09-03* — The ratchet baseline cache moved from the full `actions/cache` action to the `actions/cache/restore` and `actions/cache/save` sub-actions at one pinned revision. The full action registers a post-job save of its own, so @@ -351,11 +368,12 @@ requires no explicit synchronization. ## Addendum: prebuilt CI tool installation (2026-09-03) -`cargo-llvm-cov` and `cargo-nextest` now follow separate installation -strategies. `cargo-llvm-cov` continues to install via a pinned `cargo-binstall`. -`cargo-nextest` instead downloads its pinned official release archive directly -from `nextest-rs/nextest` and never invokes Cargo, so a missing prebuilt binary -is a hard error rather than a source build. +`cargo-llvm-cov` and `cargo-nextest` follow separate installation strategies. +`cargo-llvm-cov` installed via a pinned `cargo-binstall` when this addendum was +written; since 2026-09-06 it installs from the tool manifest (see the design +decision of that date). `cargo-nextest` downloads its pinned official release +archive directly from `nextest-rs/nextest` and never invokes Cargo, so a missing +prebuilt binary is a hard error rather than a source build. ### Platform asset selection From f497a136ee3bfecfc5f991bfa90da7c10bc93c94 Mon Sep 17 00:00:00 2001 From: Payton McIntosh Date: Sun, 6 Sep 2026 03:34:33 +0100 Subject: [PATCH 07/11] Run the installer suite against both copies and document the change Review round four on #470. The test module loads the installer from generate-coverage and from ratchet-coverage in turn, so the ratchet copy is executed rather than assumed, and a test holds the two byte-identical. The developers' guide gains a usage example for probe_version and installed_at_pinned_version; the migration guide describes the move of cargo-llvm-cov to the tool manifest, its supported targets and the loss of cargo-binstall on PATH; ADR 0003 records the cache-path change in a dated addendum. --- .../tests/test_install_cargo_llvm_cov.py | 58 ++++++++++++++----- .../0003-sccache-owns-rust-compiler-output.md | 13 +++++ docs/developers-guide.md | 16 ++++- docs/migrating-to-verified-prebuilt-tools.md | 25 ++++++++ 4 files changed, 97 insertions(+), 15 deletions(-) diff --git a/.github/actions/generate-coverage/tests/test_install_cargo_llvm_cov.py b/.github/actions/generate-coverage/tests/test_install_cargo_llvm_cov.py index 930539160..c3a7dbfa7 100644 --- a/.github/actions/generate-coverage/tests/test_install_cargo_llvm_cov.py +++ b/.github/actions/generate-coverage/tests/test_install_cargo_llvm_cov.py @@ -45,14 +45,52 @@ _WRONG_VERSION_BINARY = b"#!/bin/sh\necho 'cargo-llvm-cov 0.6.24'\n" -@pytest.fixture -def install_llvm_cov_module(monkeypatch: pytest.MonkeyPatch) -> ModuleType: - """Return a freshly loaded installer with job-level side effects disabled.""" +ACTIONS_DIR = Path(__file__).resolve().parents[2] + +#: Both actions ship the installer; the suite runs against each copy so the +#: ratchet-coverage one is executed rather than assumed identical. +INSTALLER_COPIES = { + "generate-coverage": ACTIONS_DIR + / "generate-coverage" + / "scripts" + / "install_cargo_llvm_cov.py", + "ratchet-coverage": ACTIONS_DIR + / "ratchet-coverage" + / "scripts" + / "install_cargo_llvm_cov.py", +} + + +def _load_installer(script: Path, name: str) -> ModuleType: + """Load one installer copy from ``script`` under module name ``name``.""" + spec = importlib.util.spec_from_file_location(name, script) + assert spec is not None + assert spec.loader is not None + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +@pytest.fixture(params=list(INSTALLER_COPIES), ids=list(INSTALLER_COPIES)) +def install_llvm_cov_module( + request: pytest.FixtureRequest, monkeypatch: pytest.MonkeyPatch +) -> ModuleType: + """Return a freshly loaded installer copy with job-level side effects disabled.""" monkeypatch.delenv("GITHUB_STEP_SUMMARY", raising=False) monkeypatch.delenv("GITHUB_PATH", raising=False) monkeypatch.delenv("RUNNER_OS", raising=False) monkeypatch.delenv("RUNNER_ARCH", raising=False) - return _load_module(monkeypatch, "install_cargo_llvm_cov") + if request.param == "generate-coverage": + return _load_module(monkeypatch, "install_cargo_llvm_cov") + return _load_installer( + INSTALLER_COPIES[request.param], f"install_cargo_llvm_cov_{request.param}" + ) + + +def test_both_actions_ship_the_same_installer() -> None: + """The two copies are byte-identical, so a fix in one cannot miss the other.""" + contents = {name: path.read_bytes() for name, path in INSTALLER_COPIES.items()} + assert contents["generate-coverage"] == contents["ratchet-coverage"] @pytest.mark.parametrize("runner", list(RUNNERS.values()), ids=list(RUNNERS)) @@ -268,17 +306,9 @@ def _installer_module() -> ModuleType: fixtures would be shared across those examples, so the module is loaded directly here. """ - script = ( - Path(__file__).resolve().parents[1] / "scripts" / "install_cargo_llvm_cov.py" - ) - spec = importlib.util.spec_from_file_location( - "install_cargo_llvm_cov_property", script + return _load_installer( + INSTALLER_COPIES["generate-coverage"], "install_cargo_llvm_cov_property" ) - assert spec is not None - assert spec.loader is not None - module = importlib.util.module_from_spec(spec) - spec.loader.exec_module(module) - return module _PROBE_STATES = st.sampled_from(["absent", "unrunnable", "reported"]) diff --git a/docs/adr/0003-sccache-owns-rust-compiler-output.md b/docs/adr/0003-sccache-owns-rust-compiler-output.md index 8e306f7ae..4c4e94741 100644 --- a/docs/adr/0003-sccache-owns-rust-compiler-output.md +++ b/docs/adr/0003-sccache-owns-rust-compiler-output.md @@ -124,9 +124,22 @@ The decision itself stands: sccache owns compiler output, and no `target` archive returns. This records that owning it requires the two exports, which the original decision took for granted. +## Addendum, 2026-09-06: `cargo-binstall` leaves the coverage cache + +`generate-coverage` no longer provisions `cargo-binstall`: `cargo-llvm-cov` is +installed from the tool manifest at 0.9.0 by `install_cargo_llvm_cov.py`, and +`cargo-nextest` already came from its own pinned release, so nothing in the +action shells out to `cargo binstall`. The "Ensure cargo-binstall" step is +removed and `~/.cargo/bin/cargo-binstall` is dropped from the Cargo cache +paths, which now cover the `cargo-llvm-cov` and `cargo-nextest` binaries, the +registry and the Git index. The cache key and the `cache-provider` boundary are +unchanged, and the decision stands: sccache owns compiler output and no +`target` archive returns (`#470`). + ## References - Issue `#424`, PR `#425` - Issues `#437`, `#439` and `#441`, PRs `#438` and `#440` +- PR `#470` - `docs/developers-guide.md`, "Rust action cache ownership" - `docs/users-guide.md`, "Rust cache ownership" diff --git a/docs/developers-guide.md b/docs/developers-guide.md index 6c644704d..bb5685ffe 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -938,12 +938,26 @@ The script has one pure step and one effectful one: verifies its SHA-256 against the manifest, extracts exactly the manifest's `member`, stages it in a temporary directory beside the destination and publishes it with a rename, so a concurrent reader never sees a partially - written executable. Before publishing it probes the staged binary + written executable. Before publishing, it probes the staged binary (`probe_version`, the one place a process is spawned to read a version, returning a `VersionProbe` value over `absent`, `unrunnable` and `reported`) and the pure `installed_at_pinned_version` accepts only a reported line equal to `expected_version`. The same probe on the destination decides reuse. +The probe and the decision compose like this: + +```python +tool = resolve_tool() # ResolvedTool(expected_version="cargo-llvm-cov 0.9.0", ...) +probe = probe_version(cargo_bin() / tool.binary, tool.version_args) +# VersionProbe(state="reported", version="cargo-llvm-cov 0.9.0") -> reuse +# VersionProbe(state="reported", version="cargo-llvm-cov 0.6.24") -> install +# VersionProbe(state="absent", version=None) -> install +# VersionProbe(state="unrunnable", version=None) -> install +if installed_at_pinned_version(probe, tool.expected_version): + ... # reuse +probe.metric_state(tool.expected_version) # "pinned", "other-version", ... +``` + `main` is the command boundary: it turns a `ToolResolutionError` into an exit status and publishes the `cargo-llvm-cov.resolve`, `.download`, `.archive-digest` and `.install` metrics, each over a closed set of values, to diff --git a/docs/migrating-to-verified-prebuilt-tools.md b/docs/migrating-to-verified-prebuilt-tools.md index 8ee0838d4..ba417e0a6 100644 --- a/docs/migrating-to-verified-prebuilt-tools.md +++ b/docs/migrating-to-verified-prebuilt-tools.md @@ -165,6 +165,29 @@ instead of the previous cargo-binstall path. A caller that sets `use-cargo-nextest: "false"` is unaffected, since the action never installs `cargo-nextest` in that mode. +### `cargo-llvm-cov` from the tool manifest + +`generate-coverage` and `ratchet-coverage` install `cargo-llvm-cov` 0.9.0 from +the repository tool manifest (`.github/tool-manifest.toml`) through +`scripts/install_cargo_llvm_cov.py`, replacing the `cargo binstall` of 0.6.24. +The installer selects the archive for the runner's operating system and +architecture, verifies its SHA-256 against the manifest, extracts only the +named executable, and publishes it only when it reports exactly the pinned +version. The supported targets are the manifest entry's: x86_64 and aarch64 +Linux (glibc), x86_64 and aarch64 macOS, and x86_64 Windows. Any other runner +fails resolution with a bounded `cargo-llvm-cov.resolve=unsupported-runner` +metric rather than building from source. + +The version moved because cargo 1.100 nightlies enable Cargo's new build-dir +layout, which 0.6.24 cannot read: on such a toolchain coverage failed with +`failed to collect object files` after every test had passed. A caller on a +toolchain from 2026-08-22 or later needs this release of the action. + +`generate-coverage` no longer provisions `cargo-binstall` at all, and +`~/.cargo/bin/cargo-binstall` is no longer part of its Cargo cache. A caller +that relied on the action leaving a `cargo-binstall` on `PATH` for its own +later steps must install one itself. + ## Checklist - [ ] Confirm which `install-whitaker` and `generate-coverage` major tags you @@ -173,6 +196,8 @@ instead of the previous cargo-binstall path. A caller that sets - [ ] If you pin `installer-version` explicitly, confirm it is one of the versions listed in `installer-digests.sha256`, or supply a verified `installer-sha256`. +- [ ] If a later step of yours used the `cargo-binstall` that `generate-coverage` + used to leave on `PATH`, install one yourself; the action no longer does. - [ ] If you relied on a cargo-binstall QuickInstall substitute or a source build for `cargo-nextest`, replace that reliance with a version this repository pins, or preinstall a verified binary on `PATH` before From e1974e5be3eba4a9ca7f963e7e3084fd41ca79b2 Mon Sep 17 00:00:00 2001 From: Payton McIntosh Date: Sun, 6 Sep 2026 12:21:19 +0100 Subject: [PATCH 08/11] Hold the install steps by contract and align the ADR inventory Review round five on #470. Action-level tests parse both coverage action manifests: exactly one Install cargo-llvm-cov step each, running the manifest-driven script; the generate-coverage one gated on Rust or mixed projects and not on nextest; no step provisioning or invoking cargo-binstall; the Cargo cache without its binary. Mutation-tested by re-adding an Ensure cargo-binstall step and by dropping the mixed condition, both caught. The ADR cache inventory now matches its addendum and the migration checklist loses its second-person wording. --- .../tests/test_install_llvm_cov_steps.py | 79 +++++++++++++++++++ .../0003-sccache-owns-rust-compiler-output.md | 5 +- docs/migrating-to-verified-prebuilt-tools.md | 11 +-- 3 files changed, 88 insertions(+), 7 deletions(-) create mode 100644 .github/actions/generate-coverage/tests/test_install_llvm_cov_steps.py diff --git a/.github/actions/generate-coverage/tests/test_install_llvm_cov_steps.py b/.github/actions/generate-coverage/tests/test_install_llvm_cov_steps.py new file mode 100644 index 000000000..3dc8a83dd --- /dev/null +++ b/.github/actions/generate-coverage/tests/test_install_llvm_cov_steps.py @@ -0,0 +1,79 @@ +"""Contracts for the cargo-llvm-cov install steps in both coverage actions. + +The installer script is exercised by ``test_install_cargo_llvm_cov.py``; these +tests hold the action manifests to the step shape that invokes it: exactly one +``Install cargo-llvm-cov`` step per action, the generate-coverage one gated on +a Rust or mixed project, both running the manifest-driven script, no step +anywhere provisioning ``cargo-binstall``, and no cache path for it. +""" + +from __future__ import annotations + +import pathlib +import typing as typ + +import pytest +import yaml + +ACTIONS_DIR = pathlib.Path(__file__).resolve().parents[2] +ACTIONS = { + "generate-coverage": ACTIONS_DIR / "generate-coverage" / "action.yml", + "ratchet-coverage": ACTIONS_DIR / "ratchet-coverage" / "action.yml", +} +INSTALL_STEP = "Install cargo-llvm-cov" +INSTALLER = "install_cargo_llvm_cov.py" + + +def _steps(action: str) -> list[dict[str, typ.Any]]: + """Return the composite action's steps in order.""" + document = yaml.safe_load(ACTIONS[action].read_text(encoding="utf-8")) + return list(document["runs"]["steps"]) + + +def _install_steps(action: str) -> list[dict[str, typ.Any]]: + """Return the steps named after the installer.""" + return [step for step in _steps(action) if step.get("name") == INSTALL_STEP] + + +@pytest.mark.parametrize("action", list(ACTIONS), ids=list(ACTIONS)) +def test_exactly_one_install_step_runs_the_manifest_installer(action: str) -> None: + """One step installs cargo-llvm-cov, and it runs the manifest-driven script.""" + steps = _install_steps(action) + assert len(steps) == 1, f"{action} must have exactly one {INSTALL_STEP!r} step" + run = str(steps[0].get("run", "")) + assert "uv run --script" in run + assert run.rstrip().endswith(f'/scripts/{INSTALLER}"'), run + assert steps[0].get("shell") == "bash" + + +def test_generate_coverage_install_step_covers_rust_and_mixed_projects() -> None: + """The install step runs for Rust and mixed projects, and only those.""" + condition = str(_install_steps("generate-coverage")[0].get("if", "")) + assert "steps.detect.outputs.lang == 'rust'" in condition + assert "steps.detect.outputs.lang == 'mixed'" in condition + assert "use-cargo-nextest" not in condition, ( + "cargo-llvm-cov is needed whether or not nextest drives the tests" + ) + + +@pytest.mark.parametrize("action", list(ACTIONS), ids=list(ACTIONS)) +def test_nothing_provisions_or_invokes_cargo_binstall(action: str) -> None: + """No step is named for cargo-binstall and no run block calls it.""" + for step in _steps(action): + assert "binstall" not in str(step.get("name", "")).lower(), step.get("name") + assert "cargo binstall" not in str(step.get("run", "")), step.get("name") + assert "cargo-binstall" not in str(step.get("run", "")), step.get("name") + + +def test_generate_coverage_cargo_cache_omits_cargo_binstall() -> None: + """The Cargo cache no longer archives a binary nothing installs.""" + cache = next( + step + for step in _steps("generate-coverage") + if step.get("name") == "Cache cargo artefacts" + ) + paths = [ + line.strip() for line in str(cache["with"]["path"]).splitlines() if line.strip() + ] + assert "~/.cargo/bin/cargo-binstall" not in paths + assert "~/.cargo/bin/cargo-llvm-cov" in paths diff --git a/docs/adr/0003-sccache-owns-rust-compiler-output.md b/docs/adr/0003-sccache-owns-rust-compiler-output.md index 4c4e94741..8c47c577c 100644 --- a/docs/adr/0003-sccache-owns-rust-compiler-output.md +++ b/docs/adr/0003-sccache-owns-rust-compiler-output.md @@ -39,8 +39,9 @@ compiler output. profile-agnostic key `${{ runner.os }}-cargo-${{ hashFiles('rust-toolchain.toml', '**/Cargo.lock') }}`, with a matching `${{ runner.os }}-cargo-` restore prefix. `generate-coverage` -caches the `cargo-binstall`, `cargo-llvm-cov`, and `cargo-nextest` binaries -alongside the registry and Git index, under its existing key. +caches the `cargo-llvm-cov` and `cargo-nextest` binaries alongside the +registry and Git index, under its existing key (the `cargo-binstall` binary +left that list on 2026-09-06; see the addendum below). The `cache-provider` boundary is unchanged. `external` still disables the actions' own archive caches so a caller such as an Ubicloud or Namespace cache diff --git a/docs/migrating-to-verified-prebuilt-tools.md b/docs/migrating-to-verified-prebuilt-tools.md index ba417e0a6..adccf862c 100644 --- a/docs/migrating-to-verified-prebuilt-tools.md +++ b/docs/migrating-to-verified-prebuilt-tools.md @@ -184,9 +184,9 @@ layout, which 0.6.24 cannot read: on such a toolchain coverage failed with toolchain from 2026-08-22 or later needs this release of the action. `generate-coverage` no longer provisions `cargo-binstall` at all, and -`~/.cargo/bin/cargo-binstall` is no longer part of its Cargo cache. A caller -that relied on the action leaving a `cargo-binstall` on `PATH` for its own -later steps must install one itself. +`~/.cargo/bin/cargo-binstall` is no longer part of its Cargo cache. A workflow +that relied on the action leaving a `cargo-binstall` on `PATH` for later steps +must install one in those steps. ## Checklist @@ -196,8 +196,9 @@ later steps must install one itself. - [ ] If you pin `installer-version` explicitly, confirm it is one of the versions listed in `installer-digests.sha256`, or supply a verified `installer-sha256`. -- [ ] If a later step of yours used the `cargo-binstall` that `generate-coverage` - used to leave on `PATH`, install one yourself; the action no longer does. +- [ ] If a later workflow step used the `cargo-binstall` that `generate-coverage` + used to leave on `PATH`, install one in that workflow; the action no + longer does. - [ ] If you relied on a cargo-binstall QuickInstall substitute or a source build for `cargo-nextest`, replace that reliance with a version this repository pins, or preinstall a verified binary on `PATH` before From 9cce8790ae48514d37611c47bd8684d9aee8c8d2 Mon Sep 17 00:00:00 2001 From: Payton McIntosh Date: Mon, 7 Sep 2026 05:47:14 +0100 Subject: [PATCH 09/11] Make resolver loading injectable and typed Review round on #470. resolve_tool was documented as a side-effect-free query but loaded the install-tool resolver itself, executing another module's body through spec.loader.exec_module. A missing or broken resolver therefore escaped as OSError, ImportError or whatever that body raised, past main's only handler for ToolResolutionError, and one branch wrote to stderr and exited the process from inside the query. Loading now lives in load_resolver, which reads RESOLVER_PATH at call time, converts every failure to ToolResolutionError under one bounded kind, resolver-unavailable, and writes nothing. resolve_tool takes the resolver as an optional argument beside the manifest, so a caller or a test supplies it without touching the filesystem. main is unchanged and already turns a ToolResolutionError into the resolve metric and exit 1. Five tests cover it: an absent resolver and one that raises from its module body are both the bounded kind with no output; resolve_tool reports a failed load by kind; an injected resolver is used with RESOLVER_PATH pointing at nothing, so the call would fail were the dependency still read from disk; and main publishes cargo-llvm-cov.resolve=resolver-unavailable and exits 1. Also from the same round: the migration guide now states what changes for ratchet-coverage, which never provisioned or cached cargo-binstall but did require one on PATH and no longer does; and the ADR 0003 Decision section is restored to its historical text, leaving the dated addendum to record that cargo-binstall left the coverage cache. --- .../scripts/install_cargo_llvm_cov.py | 51 +++++++--- .../tests/test_install_cargo_llvm_cov.py | 93 +++++++++++++++++++ .../scripts/install_cargo_llvm_cov.py | 51 +++++++--- .../0003-sccache-owns-rust-compiler-output.md | 5 +- docs/developers-guide.md | 25 +++-- docs/migrating-to-verified-prebuilt-tools.md | 15 +++ 6 files changed, 199 insertions(+), 41 deletions(-) diff --git a/.github/actions/generate-coverage/scripts/install_cargo_llvm_cov.py b/.github/actions/generate-coverage/scripts/install_cargo_llvm_cov.py index a5a2d80e9..24571ccea 100755 --- a/.github/actions/generate-coverage/scripts/install_cargo_llvm_cov.py +++ b/.github/actions/generate-coverage/scripts/install_cargo_llvm_cov.py @@ -50,8 +50,9 @@ TOOL_NAME = "cargo-llvm-cov" -#: The resolver reports every other failure kind; this one is ours. +#: The resolver reports every other failure kind; these two are ours. MANIFEST_UNREADABLE = "manifest-unreadable" +RESOLVER_UNAVAILABLE = "resolver-unavailable" #: Where the manifest and the shared resolver live relative to this script: #: ``/.github/actions//scripts/``. @@ -97,17 +98,6 @@ def emit_metric(line: str) -> None: handle.write(f"metric {line}\n") -def _load_resolver() -> ModuleType: - """Import the install-tool resolver from its own script directory.""" - spec = importlib.util.spec_from_file_location("resolve_tool", RESOLVER_PATH) - if spec is None or spec.loader is None: - typer.echo(f"cannot load the tool resolver at {RESOLVER_PATH}", err=True) - raise typer.Exit(1) - module = importlib.util.module_from_spec(spec) - spec.loader.exec_module(module) - return module - - def runner_description() -> tuple[str, str]: """Return the runner OS and architecture as GitHub Actions names them. @@ -143,18 +133,49 @@ def load_manifest(manifest_path: Path = MANIFEST_PATH) -> dict[str, object]: raise ToolResolutionError(MANIFEST_UNREADABLE, message) from exc +def load_resolver(resolver_path: Path | None = None) -> ModuleType: + """Import the install-tool resolver, or raise ``ToolResolutionError``. + + Executing another script's module body can raise anything its top level + raises, so every failure is reported under one bounded kind rather than + escaping as ``OSError``, ``ImportError`` or a loader exception. The + command boundary publishes that kind as a metric; this query writes + nothing and never exits the process. ``resolver_path`` defaults to + ``RESOLVER_PATH`` read at call time, so a caller can redirect it. + """ + resolver_path = RESOLVER_PATH if resolver_path is None else resolver_path + spec = importlib.util.spec_from_file_location("resolve_tool", resolver_path) + if spec is None or spec.loader is None: + message = f"cannot load the tool resolver at {resolver_path}" + raise ToolResolutionError(RESOLVER_UNAVAILABLE, message) + module = importlib.util.module_from_spec(spec) + try: + spec.loader.exec_module(module) + # A module body raises whatever its top level raises, so the catch is as + # wide as the failure it must convert into a bounded kind. + except Exception as exc: + message = f"the tool resolver at {resolver_path} failed to load: {exc}" + raise ToolResolutionError(RESOLVER_UNAVAILABLE, message) from exc + return module + + def resolve_tool( version: str = CARGO_LLVM_COV_VERSION, *, manifest: dict[str, object] | None = None, runner: tuple[str, str] | None = None, + resolver: ModuleType | None = None, ) -> ResolvedTool: """Return the manifest entry for ``version`` on ``runner``. - A query with no side effects: it reads the manifest (or the one passed - in) and either returns the entry or raises ``ToolResolutionError``. + A query with no side effects: it reads the manifest and the resolver (or + the ones passed in) and either returns the entry or raises + ``ToolResolutionError``. Passing ``resolver`` makes the dependency + explicit, so a caller or a test can supply one without touching the + filesystem. """ - resolver = _load_resolver() + if resolver is None: + resolver = load_resolver() if manifest is None: manifest = load_manifest() schema = manifest.get("schema") diff --git a/.github/actions/generate-coverage/tests/test_install_cargo_llvm_cov.py b/.github/actions/generate-coverage/tests/test_install_cargo_llvm_cov.py index c3a7dbfa7..b533e075e 100644 --- a/.github/actions/generate-coverage/tests/test_install_cargo_llvm_cov.py +++ b/.github/actions/generate-coverage/tests/test_install_cargo_llvm_cov.py @@ -557,3 +557,96 @@ def test_entry_point_reports_a_resolution_failure_by_kind( assert "metric cargo-llvm-cov.resolve=unsupported-runner" in summary.read_text( encoding="utf-8" ) + + +def test_a_missing_resolver_is_a_typed_error_without_output( + install_llvm_cov_module: ModuleType, tmp_path: Path, capsys: pytest.CaptureFixture +) -> None: + """An absent resolver raises the bounded kind and the query stays silent. + + Loading the resolver used to exit the process from inside the query, so + a caller could not convert the failure into a metric. + """ + with pytest.raises(install_llvm_cov_module.ToolResolutionError) as excinfo: + install_llvm_cov_module.load_resolver(tmp_path / "absent.py") + + assert excinfo.value.kind == install_llvm_cov_module.RESOLVER_UNAVAILABLE + captured = capsys.readouterr() + assert captured.out == "" + assert captured.err == "" + + +def test_a_resolver_raising_on_import_is_a_typed_error( + install_llvm_cov_module: ModuleType, tmp_path: Path, capsys: pytest.CaptureFixture +) -> None: + """A resolver whose module body raises is reported by kind, not as a traceback.""" + resolver = tmp_path / "resolve_tool.py" + resolver.write_text('raise RuntimeError("resolver is broken")\n', encoding="utf-8") + + with pytest.raises(install_llvm_cov_module.ToolResolutionError) as excinfo: + install_llvm_cov_module.load_resolver(resolver) + + assert excinfo.value.kind == install_llvm_cov_module.RESOLVER_UNAVAILABLE + captured = capsys.readouterr() + assert captured.out == "" + assert captured.err == "" + + +def test_resolve_tool_reports_a_failing_resolver_load_by_kind( + install_llvm_cov_module: ModuleType, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """``resolve_tool`` surfaces a resolver-load failure as its own typed error.""" + monkeypatch.setattr( + install_llvm_cov_module, "RESOLVER_PATH", tmp_path / "absent.py" + ) + + with pytest.raises(install_llvm_cov_module.ToolResolutionError) as excinfo: + install_llvm_cov_module.resolve_tool(runner=RUNNERS["linux-x64"]) + + assert excinfo.value.kind == install_llvm_cov_module.RESOLVER_UNAVAILABLE + + +def test_resolve_tool_uses_an_injected_resolver( + install_llvm_cov_module: ModuleType, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """An injected resolver is used as given, so the query loads no file. + + ``RESOLVER_PATH`` points at nothing for the duration, which would fail the + call were the dependency still resolved from disk. + """ + monkeypatch.setattr( + install_llvm_cov_module, "RESOLVER_PATH", tmp_path / "absent.py" + ) + resolver = install_llvm_cov_module.load_resolver( + Path(install_llvm_cov_module.__file__).resolve().parents[3] + / "actions" + / "install-tool" + / "scripts" + / "resolve_tool.py" + ) + + tool = install_llvm_cov_module.resolve_tool( + runner=RUNNERS["linux-x64"], resolver=resolver + ) + + assert tool.expected_version == ( + f"cargo-llvm-cov {install_llvm_cov_module.CARGO_LLVM_COV_VERSION}" + ) + + +def test_main_reports_a_failing_resolver_load_as_a_metric( + install_llvm_cov_module: ModuleType, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """``main`` converts a resolver-load failure into the bounded metric and exit 1.""" + _binary, _github_path, summary = _job_environment(monkeypatch, tmp_path) + monkeypatch.setattr( + install_llvm_cov_module, "RESOLVER_PATH", tmp_path / "absent.py" + ) + + with pytest.raises(typer.Exit) as excinfo: + install_llvm_cov_module.main() + + assert _exit_code(excinfo.value) == 1 + assert "metric cargo-llvm-cov.resolve=resolver-unavailable" in summary.read_text( + encoding="utf-8" + ) diff --git a/.github/actions/ratchet-coverage/scripts/install_cargo_llvm_cov.py b/.github/actions/ratchet-coverage/scripts/install_cargo_llvm_cov.py index a5a2d80e9..24571ccea 100755 --- a/.github/actions/ratchet-coverage/scripts/install_cargo_llvm_cov.py +++ b/.github/actions/ratchet-coverage/scripts/install_cargo_llvm_cov.py @@ -50,8 +50,9 @@ TOOL_NAME = "cargo-llvm-cov" -#: The resolver reports every other failure kind; this one is ours. +#: The resolver reports every other failure kind; these two are ours. MANIFEST_UNREADABLE = "manifest-unreadable" +RESOLVER_UNAVAILABLE = "resolver-unavailable" #: Where the manifest and the shared resolver live relative to this script: #: ``/.github/actions//scripts/``. @@ -97,17 +98,6 @@ def emit_metric(line: str) -> None: handle.write(f"metric {line}\n") -def _load_resolver() -> ModuleType: - """Import the install-tool resolver from its own script directory.""" - spec = importlib.util.spec_from_file_location("resolve_tool", RESOLVER_PATH) - if spec is None or spec.loader is None: - typer.echo(f"cannot load the tool resolver at {RESOLVER_PATH}", err=True) - raise typer.Exit(1) - module = importlib.util.module_from_spec(spec) - spec.loader.exec_module(module) - return module - - def runner_description() -> tuple[str, str]: """Return the runner OS and architecture as GitHub Actions names them. @@ -143,18 +133,49 @@ def load_manifest(manifest_path: Path = MANIFEST_PATH) -> dict[str, object]: raise ToolResolutionError(MANIFEST_UNREADABLE, message) from exc +def load_resolver(resolver_path: Path | None = None) -> ModuleType: + """Import the install-tool resolver, or raise ``ToolResolutionError``. + + Executing another script's module body can raise anything its top level + raises, so every failure is reported under one bounded kind rather than + escaping as ``OSError``, ``ImportError`` or a loader exception. The + command boundary publishes that kind as a metric; this query writes + nothing and never exits the process. ``resolver_path`` defaults to + ``RESOLVER_PATH`` read at call time, so a caller can redirect it. + """ + resolver_path = RESOLVER_PATH if resolver_path is None else resolver_path + spec = importlib.util.spec_from_file_location("resolve_tool", resolver_path) + if spec is None or spec.loader is None: + message = f"cannot load the tool resolver at {resolver_path}" + raise ToolResolutionError(RESOLVER_UNAVAILABLE, message) + module = importlib.util.module_from_spec(spec) + try: + spec.loader.exec_module(module) + # A module body raises whatever its top level raises, so the catch is as + # wide as the failure it must convert into a bounded kind. + except Exception as exc: + message = f"the tool resolver at {resolver_path} failed to load: {exc}" + raise ToolResolutionError(RESOLVER_UNAVAILABLE, message) from exc + return module + + def resolve_tool( version: str = CARGO_LLVM_COV_VERSION, *, manifest: dict[str, object] | None = None, runner: tuple[str, str] | None = None, + resolver: ModuleType | None = None, ) -> ResolvedTool: """Return the manifest entry for ``version`` on ``runner``. - A query with no side effects: it reads the manifest (or the one passed - in) and either returns the entry or raises ``ToolResolutionError``. + A query with no side effects: it reads the manifest and the resolver (or + the ones passed in) and either returns the entry or raises + ``ToolResolutionError``. Passing ``resolver`` makes the dependency + explicit, so a caller or a test can supply one without touching the + filesystem. """ - resolver = _load_resolver() + if resolver is None: + resolver = load_resolver() if manifest is None: manifest = load_manifest() schema = manifest.get("schema") diff --git a/docs/adr/0003-sccache-owns-rust-compiler-output.md b/docs/adr/0003-sccache-owns-rust-compiler-output.md index 8c47c577c..4c4e94741 100644 --- a/docs/adr/0003-sccache-owns-rust-compiler-output.md +++ b/docs/adr/0003-sccache-owns-rust-compiler-output.md @@ -39,9 +39,8 @@ compiler output. profile-agnostic key `${{ runner.os }}-cargo-${{ hashFiles('rust-toolchain.toml', '**/Cargo.lock') }}`, with a matching `${{ runner.os }}-cargo-` restore prefix. `generate-coverage` -caches the `cargo-llvm-cov` and `cargo-nextest` binaries alongside the -registry and Git index, under its existing key (the `cargo-binstall` binary -left that list on 2026-09-06; see the addendum below). +caches the `cargo-binstall`, `cargo-llvm-cov`, and `cargo-nextest` binaries +alongside the registry and Git index, under its existing key. The `cache-provider` boundary is unchanged. `external` still disables the actions' own archive caches so a caller such as an Ubicloud or Namespace cache diff --git a/docs/developers-guide.md b/docs/developers-guide.md index bb5685ffe..602ba85c1 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -928,12 +928,18 @@ constant second. The script has one pure step and one effectful one: -- **Resolution** (`load_manifest`, `resolve_tool`) reads the manifest and - selects the archive for the runner with the `install-tool` resolver - (`.github/actions/install-tool/scripts/resolve_tool.py`), using `RUNNER_OS` - and `RUNNER_ARCH` inside a job and `platform` outside one. It returns a - `ResolvedTool` or raises `ToolResolutionError` carrying the resolver's - bounded failure kind; it publishes nothing. +- **Resolution** (`load_manifest`, `load_resolver`, `resolve_tool`) reads the + manifest and selects the archive for the runner with the `install-tool` + resolver (`.github/actions/install-tool/scripts/resolve_tool.py`), using + `RUNNER_OS` and `RUNNER_ARCH` inside a job and `platform` outside one. It + returns a `ResolvedTool` or raises `ToolResolutionError` carrying the + resolver's bounded failure kind; it publishes nothing and never exits the + process. `resolve_tool` takes both the manifest and the resolver as optional + arguments, so a caller or a test supplies either without touching the + filesystem. When neither is given it loads them, and a resolver that is + missing or raises from its own module body becomes + `ToolResolutionError(kind="resolver-unavailable")` rather than an `OSError`, + an `ImportError` or whatever that module body raised. - **Installation** (`install`) downloads the archive with a 200 MB cap, verifies its SHA-256 against the manifest, extracts exactly the manifest's `member`, stages it in a temporary directory beside the destination and @@ -969,8 +975,11 @@ The behavioural tests live in hold the pinned version to the manifest for every runner the resolver knows, drive `main` end to end against a temporary manifest whose entry points at a local HTTP server, and cover digest mismatch, a missing member, an oversized -download, a binary reporting another version, and the reuse path. A Hypothesis -property holds that only the exact expected version line counts as installed. +download, a binary reporting another version, and the reuse path. They also +inject a failing resolver, both absent and raising on import, and assert that +resolution reports `resolver-unavailable` while writing nothing, and that +`main` turns it into that metric and exit status. A Hypothesis property holds +that only the exact expected version line counts as installed. `generate-coverage` no longer provisions `cargo-binstall`: nothing in either action invokes `cargo binstall`. diff --git a/docs/migrating-to-verified-prebuilt-tools.md b/docs/migrating-to-verified-prebuilt-tools.md index adccf862c..eefaddd54 100644 --- a/docs/migrating-to-verified-prebuilt-tools.md +++ b/docs/migrating-to-verified-prebuilt-tools.md @@ -188,6 +188,15 @@ toolchain from 2026-08-22 or later needs this release of the action. that relied on the action leaving a `cargo-binstall` on `PATH` for later steps must install one in those steps. +The two actions differ here, so check which one you consume. +`ratchet-coverage` never provisioned or cached `cargo-binstall`: it invoked +whichever one the job already had on `PATH`, and it left nothing behind. Its +change is the other direction. It no longer needs a `cargo-binstall` on `PATH` +at all, so a job that installed one solely to satisfy `ratchet-coverage`, or +that ran `generate-coverage` first to obtain one, can drop that step. A later +step in such a job that used that `cargo-binstall` for its own purposes still +needs one installed explicitly. + ## Checklist - [ ] Confirm which `install-whitaker` and `generate-coverage` major tags you @@ -199,6 +208,12 @@ must install one in those steps. - [ ] If a later workflow step used the `cargo-binstall` that `generate-coverage` used to leave on `PATH`, install one in that workflow; the action no longer does. +- [ ] If you run `ratchet-coverage`, drop any step that installed + `cargo-binstall` only to satisfy it, and any ordering that ran + `generate-coverage` first for the same reason. `ratchet-coverage` now + installs `cargo-llvm-cov` from the manifest and calls no `cargo binstall` + of its own. Keep such a step only where a later step uses that + `cargo-binstall` itself. - [ ] If you relied on a cargo-binstall QuickInstall substitute or a source build for `cargo-nextest`, replace that reliance with a version this repository pins, or preinstall a verified binary on `PATH` before From 70c5bb653fe167d617a57cceb6b2a870dfbe98a2 Mon Sep 17 00:00:00 2001 From: Payton McIntosh Date: Mon, 7 Sep 2026 22:43:38 +0100 Subject: [PATCH 10/11] Prove the extraction takes only the named member Review round on #470. Every archive the installer suite built held one member, so a version of extract_member that unpacked the whole archive would have passed the lot: the wanted file lands either way and nothing looked for the rest. The missing- member case was tar-only, and an extractall implementation would have failed it late, when the staged binary turned out to be absent, rather than at the extraction it was meant to guard. Archives now carry decoys beside the wanted member. A parametrised test over tar.gz and zip asserts the destination holds the wanted payload and that its directory holds nothing else, so unpacking everything is caught at the boundary. The missing-member case runs for both formats, since the zip and tar paths raise from separate code. A tar entry with the member's name but a directory type covers the extractfile branch that returns None instead of raising. Mutation-tested: replacing the zip branch with extractall plus a copy fails test_extraction_takes_only_the_named_member for that copy alone. Also from the same round: the two sentences this pull request added to the migration guide are reworded without second-person pronouns, per the path instructions for Markdown outside README.md. --- .../tests/test_install_cargo_llvm_cov.py | 141 ++++++++++++++++++ docs/migrating-to-verified-prebuilt-tools.md | 20 +-- 2 files changed, 151 insertions(+), 10 deletions(-) diff --git a/.github/actions/generate-coverage/tests/test_install_cargo_llvm_cov.py b/.github/actions/generate-coverage/tests/test_install_cargo_llvm_cov.py index b533e075e..d32607ae7 100644 --- a/.github/actions/generate-coverage/tests/test_install_cargo_llvm_cov.py +++ b/.github/actions/generate-coverage/tests/test_install_cargo_llvm_cov.py @@ -176,6 +176,54 @@ def _zip_with(member: str, payload: bytes) -> bytes: return buffer.getvalue() +#: A second payload, so a test can tell the requested member from a decoy by +#: content rather than only by name. +_DECOY_PAYLOAD = b"#!/bin/sh\necho 'decoy'\n" + +#: Members an archive carries beside the one the manifest names. A correct +#: extraction writes none of them; ``extractall`` would write all of them. +_DECOY_MEMBERS = ("README.md", "completions/cargo-llvm-cov.bash") + + +def _tarball_with_members(members: dict[str, bytes]) -> bytes: + """Return a gzip tarball holding each ``member`` payload, mode 0o755.""" + buffer = io.BytesIO() + with tarfile.open(fileobj=buffer, mode="w:gz") as package: + for name, payload in members.items(): + info = tarfile.TarInfo(name) + info.size = len(payload) + info.mode = 0o755 + package.addfile(info, io.BytesIO(payload)) + return buffer.getvalue() + + +def _zip_with_members(members: dict[str, bytes]) -> bytes: + """Return a zip archive holding each ``member`` payload.""" + buffer = io.BytesIO() + with zipfile.ZipFile(buffer, mode="w") as package: + for name, payload in members.items(): + package.writestr(name, payload) + return buffer.getvalue() + + +def _tarball_with_a_directory_member(member: str) -> bytes: + """Return a gzip tarball whose ``member`` is a directory, not a file.""" + buffer = io.BytesIO() + with tarfile.open(fileobj=buffer, mode="w:gz") as package: + info = tarfile.TarInfo(member) + info.type = tarfile.DIRTYPE + info.mode = 0o755 + package.addfile(info) + return buffer.getvalue() + + +#: The two archive formats the manifest can name, with the builder for each. +_ARCHIVE_FORMATS = { + "tar.gz": _tarball_with_members, + "zip": _zip_with_members, +} + + def _fake_tool( module: ModuleType, archive: bytes, *, extension: str, member: str ) -> ResolvedTool: @@ -650,3 +698,96 @@ def test_main_reports_a_failing_resolver_load_as_a_metric( assert "metric cargo-llvm-cov.resolve=resolver-unavailable" in summary.read_text( encoding="utf-8" ) + + +@pytest.mark.parametrize( + "extension", list(_ARCHIVE_FORMATS), ids=list(_ARCHIVE_FORMATS) +) +def test_extraction_takes_only_the_named_member( + install_llvm_cov_module: ModuleType, tmp_path: Path, extension: str +) -> None: + """Only the manifest's member leaves the archive, whatever else it holds. + + The archive carries decoys beside the wanted member, so an implementation + that unpacked everything would be caught here. Without them an + ``extractall`` would satisfy every other test in this module, because a + single-member archive makes the two strategies indistinguishable. + """ + members = {"cargo-llvm-cov": _FAKE_BINARY} | dict.fromkeys( + _DECOY_MEMBERS, _DECOY_PAYLOAD + ) + archive_bytes = _ARCHIVE_FORMATS[extension](members) + archive = tmp_path / f"cargo-llvm-cov.{extension}" + archive.write_bytes(archive_bytes) + tool = _fake_tool( + install_llvm_cov_module, + archive_bytes, + extension=extension, + member="cargo-llvm-cov", + ) + destination = tmp_path / "bin" / "cargo-llvm-cov" + destination.parent.mkdir() + + install_llvm_cov_module.extract_member(archive, tool, destination) + + assert destination.read_bytes() == _FAKE_BINARY + assert [path.name for path in destination.parent.iterdir()] == ["cargo-llvm-cov"] + assert not (tmp_path / "README.md").exists() + assert not (tmp_path / "completions").exists() + + +@pytest.mark.parametrize( + "extension", list(_ARCHIVE_FORMATS), ids=list(_ARCHIVE_FORMATS) +) +def test_extraction_refuses_an_archive_without_the_named_member( + install_llvm_cov_module: ModuleType, tmp_path: Path, extension: str +) -> None: + """A member the manifest names but the archive lacks is rejected outright. + + Both formats are covered: the missing-member path is written separately + for zip and tar, so testing one leaves the other unguarded. + """ + members = dict.fromkeys(_DECOY_MEMBERS, _DECOY_PAYLOAD) + archive_bytes = _ARCHIVE_FORMATS[extension](members) + archive = tmp_path / f"cargo-llvm-cov.{extension}" + archive.write_bytes(archive_bytes) + tool = _fake_tool( + install_llvm_cov_module, + archive_bytes, + extension=extension, + member="cargo-llvm-cov", + ) + destination = tmp_path / "bin" / "cargo-llvm-cov" + destination.parent.mkdir() + + with pytest.raises(ValueError, match="missing from"): + install_llvm_cov_module.extract_member(archive, tool, destination) + + assert not destination.exists() + + +def test_extraction_refuses_a_tar_member_that_is_not_a_file( + install_llvm_cov_module: ModuleType, tmp_path: Path +) -> None: + """A tar entry with the member's name but a directory type is rejected. + + ``TarFile.extractfile`` returns ``None`` rather than raising for a + non-regular entry, so an unchecked implementation would carry that + ``None`` into the copy instead of failing here. + """ + archive_bytes = _tarball_with_a_directory_member("cargo-llvm-cov") + archive = tmp_path / "cargo-llvm-cov.tar.gz" + archive.write_bytes(archive_bytes) + tool = _fake_tool( + install_llvm_cov_module, + archive_bytes, + extension="tar.gz", + member="cargo-llvm-cov", + ) + destination = tmp_path / "bin" / "cargo-llvm-cov" + destination.parent.mkdir() + + with pytest.raises(ValueError, match="not a file"): + install_llvm_cov_module.extract_member(archive, tool, destination) + + assert not destination.exists() diff --git a/docs/migrating-to-verified-prebuilt-tools.md b/docs/migrating-to-verified-prebuilt-tools.md index eefaddd54..ff2dc4852 100644 --- a/docs/migrating-to-verified-prebuilt-tools.md +++ b/docs/migrating-to-verified-prebuilt-tools.md @@ -188,14 +188,14 @@ toolchain from 2026-08-22 or later needs this release of the action. that relied on the action leaving a `cargo-binstall` on `PATH` for later steps must install one in those steps. -The two actions differ here, so check which one you consume. -`ratchet-coverage` never provisioned or cached `cargo-binstall`: it invoked -whichever one the job already had on `PATH`, and it left nothing behind. Its -change is the other direction. It no longer needs a `cargo-binstall` on `PATH` -at all, so a job that installed one solely to satisfy `ratchet-coverage`, or -that ran `generate-coverage` first to obtain one, can drop that step. A later -step in such a job that used that `cargo-binstall` for its own purposes still -needs one installed explicitly. +The two actions differ here, so the consumed action determines the required +change. `ratchet-coverage` never provisioned or cached `cargo-binstall`: it +invoked whichever one the job already had on `PATH`, and it left nothing +behind. Its change is the other direction. It no longer needs a +`cargo-binstall` on `PATH` at all, so a job that installed one solely to +satisfy `ratchet-coverage`, or that ran `generate-coverage` first to obtain +one, can drop that step. A later step in such a job that used that +`cargo-binstall` for its own purposes still needs one installed explicitly. ## Checklist @@ -208,8 +208,8 @@ needs one installed explicitly. - [ ] If a later workflow step used the `cargo-binstall` that `generate-coverage` used to leave on `PATH`, install one in that workflow; the action no longer does. -- [ ] If you run `ratchet-coverage`, drop any step that installed - `cargo-binstall` only to satisfy it, and any ordering that ran +- [ ] For a workflow that runs `ratchet-coverage`, drop any step that + installed `cargo-binstall` only to satisfy it, and any ordering that ran `generate-coverage` first for the same reason. `ratchet-coverage` now installs `cargo-llvm-cov` from the manifest and calls no `cargo binstall` of its own. Keep such a step only where a later step uses that From b5a371e8ddf5fa509c5f6c58ff749955563fab83 Mon Sep 17 00:00:00 2001 From: Payton McIntosh Date: Mon, 7 Sep 2026 23:04:13 +0100 Subject: [PATCH 11/11] Split the installer suite by responsibility The extraction tests took the module from 38 functions to 44 and CodeScene reported Low Cohesion on it, five responsibilities against a threshold of four, dropping the file from 10.00 to 8.81 and failing the code health review on 70c5bb65. The suite now follows the split already used for cargo-nextest in this directory. test_install_cargo_llvm_cov.py keeps manifest resolution and the resolver dependency; test_install_cargo_llvm_cov_archives.py holds download bounds, digest verification, extraction, publication, probing and reuse; test_install_cargo_llvm_cov_entrypoint.py drives main over a real HTTP boundary. Shared constants, archive builders and the job-environment helper move to _llvm_cov_test_support.py, and the install_llvm_cov_module fixture moves to conftest.py, where install_nextest_module already lives and for the same reason. Every test and identifier is unchanged by the move; all 430 tests in the directory still pass. Each of the three modules and the conftest now score 10.00, and the support module does too after folding the single-member archive builders into the multi-member ones that subsumed them, which CodeScene flagged as duplication. The loader in the support module raises RuntimeError where it asserted: a helper that asserts reports a missing installer copy as a failing test in whichever module requested the fixture. --- .../tests/_llvm_cov_test_support.py | 162 +++++ .../generate-coverage/tests/conftest.py | 25 + .../tests/test_install_cargo_llvm_cov.py | 630 +----------------- .../test_install_cargo_llvm_cov_archives.py | 318 +++++++++ .../test_install_cargo_llvm_cov_entrypoint.py | 178 +++++ 5 files changed, 697 insertions(+), 616 deletions(-) create mode 100644 .github/actions/generate-coverage/tests/_llvm_cov_test_support.py create mode 100644 .github/actions/generate-coverage/tests/test_install_cargo_llvm_cov_archives.py create mode 100644 .github/actions/generate-coverage/tests/test_install_cargo_llvm_cov_entrypoint.py diff --git a/.github/actions/generate-coverage/tests/_llvm_cov_test_support.py b/.github/actions/generate-coverage/tests/_llvm_cov_test_support.py new file mode 100644 index 000000000..44052654e --- /dev/null +++ b/.github/actions/generate-coverage/tests/_llvm_cov_test_support.py @@ -0,0 +1,162 @@ +"""Helpers shared by the cargo-llvm-cov installer test modules. + +The installer's tests are split across three modules by responsibility, so the +constants, archive builders and job-environment helper they share live here +rather than being duplicated. The ``install_llvm_cov_module`` fixture lives in +``conftest.py`` for the same reason ``install_nextest_module`` does: a fixture +declared there is visible to every module in this directory without an import +that reads as unused. +""" + +from __future__ import annotations + +import hashlib +import importlib.util +import io +import tarfile +import typing as typ +import zipfile +from pathlib import Path + +if typ.TYPE_CHECKING: + from types import ModuleType + + import pytest + from install_cargo_llvm_cov import ResolvedTool + +RUNNERS = { + "linux-x64": ("Linux", "X64"), + "linux-arm64": ("Linux", "ARM64"), + "macos-x64": ("macOS", "X64"), + "macos-arm64": ("macOS", "ARM64"), + "windows-x64": ("Windows", "X64"), +} + +_FAKE_BINARY = b"#!/bin/sh\necho 'cargo-llvm-cov 0.9.0'\n" +_WRONG_VERSION_BINARY = b"#!/bin/sh\necho 'cargo-llvm-cov 0.6.24'\n" + + +ACTIONS_DIR = Path(__file__).resolve().parents[2] + +#: Both actions ship the installer; the suite runs against each copy so the +#: ratchet-coverage one is executed rather than assumed identical. +INSTALLER_COPIES = { + "generate-coverage": ACTIONS_DIR + / "generate-coverage" + / "scripts" + / "install_cargo_llvm_cov.py", + "ratchet-coverage": ACTIONS_DIR + / "ratchet-coverage" + / "scripts" + / "install_cargo_llvm_cov.py", +} + + +def _load_installer(script: Path, name: str) -> ModuleType: + """Load one installer copy from ``script`` under module name ``name``.""" + spec = importlib.util.spec_from_file_location(name, script) + if spec is None or spec.loader is None: + # A helper raises rather than asserts: an assertion here would report + # as a failing test in whichever module happened to request the + # fixture, rather than as the missing installer copy it is. + message = f"could not load {name} from {script}" + raise RuntimeError(message) + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +#: A second payload, so a test can tell the requested member from a decoy by +#: content rather than only by name. +_DECOY_PAYLOAD = b"#!/bin/sh\necho 'decoy'\n" + +#: Members an archive carries beside the one the manifest names. A correct +#: extraction writes none of them; ``extractall`` would write all of them. +_DECOY_MEMBERS = ("README.md", "completions/cargo-llvm-cov.bash") + + +def _tarball_with(members: dict[str, bytes]) -> bytes: + """Return a gzip tarball holding each ``member`` payload, mode 0o755.""" + buffer = io.BytesIO() + with tarfile.open(fileobj=buffer, mode="w:gz") as package: + for name, payload in members.items(): + info = tarfile.TarInfo(name) + info.size = len(payload) + info.mode = 0o755 + package.addfile(info, io.BytesIO(payload)) + return buffer.getvalue() + + +def _zip_with(members: dict[str, bytes]) -> bytes: + """Return a zip archive holding each ``member`` payload.""" + buffer = io.BytesIO() + with zipfile.ZipFile(buffer, mode="w") as package: + for name, payload in members.items(): + package.writestr(name, payload) + return buffer.getvalue() + + +def _tarball_with_a_directory_member(member: str) -> bytes: + """Return a gzip tarball whose ``member`` is a directory, not a file.""" + buffer = io.BytesIO() + with tarfile.open(fileobj=buffer, mode="w:gz") as package: + info = tarfile.TarInfo(member) + info.type = tarfile.DIRTYPE + info.mode = 0o755 + package.addfile(info) + return buffer.getvalue() + + +#: The two archive formats the manifest can name, with the builder for each. +_ARCHIVE_FORMATS = { + "tar.gz": _tarball_with, + "zip": _zip_with, +} + + +def _fake_tool( + module: ModuleType, archive: bytes, *, extension: str, member: str +) -> ResolvedTool: + """Return a ``ResolvedTool`` whose digest matches ``archive``.""" + url = ( + "https://github.com/taiki-e/cargo-llvm-cov/releases/download/v0.9.0/" + f"cargo-llvm-cov-x86_64-unknown-linux-gnu.{extension}" + ) + return typ.cast( + "ResolvedTool", + module.ResolvedTool( + triple="x86_64-unknown-linux-gnu", + url=url, + sha256=hashlib.sha256(archive).hexdigest(), + member=member, + extension=extension, + binary="cargo-llvm-cov", + version_args=("llvm-cov", "--version"), + expected_version="cargo-llvm-cov 0.9.0", + ), + ) + + +def _write_fetch(archive: bytes) -> typ.Callable[[object, Path], None]: + """Return a ``fetch`` stand-in that writes ``archive`` instead of downloading.""" + + def fetch(_tool: object, destination: Path) -> None: + """Write the canned archive to ``destination``.""" + destination.write_bytes(archive) + + return fetch + + +def _job_environment( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> tuple[Path, Path, Path]: + """Point CARGO_HOME, GITHUB_PATH and GITHUB_STEP_SUMMARY into ``tmp_path``.""" + cargo_home = tmp_path / "cargo" + github_path = tmp_path / "github_path" + summary = tmp_path / "summary.md" + monkeypatch.setenv("CARGO_HOME", str(cargo_home)) + monkeypatch.setenv("GITHUB_PATH", str(github_path)) + monkeypatch.setenv("GITHUB_STEP_SUMMARY", str(summary)) + monkeypatch.setenv("RUNNER_OS", "Linux") + monkeypatch.setenv("RUNNER_ARCH", "X64") + return cargo_home / "bin" / "cargo-llvm-cov", github_path, summary diff --git a/.github/actions/generate-coverage/tests/conftest.py b/.github/actions/generate-coverage/tests/conftest.py index 731ba397b..4654564b2 100644 --- a/.github/actions/generate-coverage/tests/conftest.py +++ b/.github/actions/generate-coverage/tests/conftest.py @@ -14,6 +14,7 @@ pytest.skip("cmd-mox IPC is unavailable on Windows", allow_module_level=True) from _coverage_test_support import _load_module +from _llvm_cov_test_support import INSTALLER_COPIES, _load_installer from test_support.cmd_mox_stub_adapter import StubManager @@ -56,3 +57,27 @@ def install_nextest_module(monkeypatch: pytest.MonkeyPatch) -> ModuleType: monkeypatch.delenv("GITHUB_STEP_SUMMARY", raising=False) monkeypatch.delenv("GITHUB_PATH", raising=False) return _load_module(monkeypatch, "install_cargo_nextest") + + +@pytest.fixture(params=list(INSTALLER_COPIES), ids=list(INSTALLER_COPIES)) +def install_llvm_cov_module( + request: pytest.FixtureRequest, monkeypatch: pytest.MonkeyPatch +) -> ModuleType: + """Return a freshly loaded installer copy with job-level side effects disabled. + + Declared here rather than in one test module because the installer's tests + are split across three modules by responsibility and each needs it; a + conftest fixture is visible to all of them without an import that reads as + unused. Both actions ship the installer, so the fixture is parametrised + over the two copies and the ratchet-coverage one is executed rather than + assumed identical. + """ + monkeypatch.delenv("GITHUB_STEP_SUMMARY", raising=False) + monkeypatch.delenv("GITHUB_PATH", raising=False) + monkeypatch.delenv("RUNNER_OS", raising=False) + monkeypatch.delenv("RUNNER_ARCH", raising=False) + if request.param == "generate-coverage": + return _load_module(monkeypatch, "install_cargo_llvm_cov") + return _load_installer( + INSTALLER_COPIES[request.param], f"install_cargo_llvm_cov_{request.param}" + ) diff --git a/.github/actions/generate-coverage/tests/test_install_cargo_llvm_cov.py b/.github/actions/generate-coverage/tests/test_install_cargo_llvm_cov.py index d32607ae7..b945f2700 100644 --- a/.github/actions/generate-coverage/tests/test_install_cargo_llvm_cov.py +++ b/.github/actions/generate-coverage/tests/test_install_cargo_llvm_cov.py @@ -1,91 +1,32 @@ -"""Verify the manifest-driven cargo-llvm-cov installer. +"""Verify how the cargo-llvm-cov installer resolves its manifest entry. The installer resolves its entry from ``.github/tool-manifest.toml`` with the -``install-tool`` resolver, so these tests hold the pinned version to the -manifest for every runner the resolver knows, exercise download, digest -verification, extraction and installation against local archives, drive the -whole entry point across a real HTTP boundary, and check that only a binary -reporting exactly the pinned version is reused. +``install-tool`` resolver, so this module holds the pinned version to the +manifest for every runner the resolver knows and covers the typed failures +that resolution reports: an unknown version, an unreadable manifest, a schema +this installer does not read, and a resolver that cannot be loaded. + +Archive handling lives in ``test_install_cargo_llvm_cov_archives.py`` and the +entry point in ``test_install_cargo_llvm_cov_entrypoint.py``. """ from __future__ import annotations -import dataclasses -import functools -import hashlib -import http.server -import importlib.util -import io -import tarfile -import threading import typing as typ -import zipfile from pathlib import Path import pytest import typer -from _coverage_test_support import _exit_code, _load_module -from hypothesis import given -from hypothesis import strategies as st +from _coverage_test_support import _exit_code +from _llvm_cov_test_support import ( + INSTALLER_COPIES, + RUNNERS, + _job_environment, +) if typ.TYPE_CHECKING: from types import ModuleType - from install_cargo_llvm_cov import ResolvedTool - -RUNNERS = { - "linux-x64": ("Linux", "X64"), - "linux-arm64": ("Linux", "ARM64"), - "macos-x64": ("macOS", "X64"), - "macos-arm64": ("macOS", "ARM64"), - "windows-x64": ("Windows", "X64"), -} - -_FAKE_BINARY = b"#!/bin/sh\necho 'cargo-llvm-cov 0.9.0'\n" -_WRONG_VERSION_BINARY = b"#!/bin/sh\necho 'cargo-llvm-cov 0.6.24'\n" - - -ACTIONS_DIR = Path(__file__).resolve().parents[2] - -#: Both actions ship the installer; the suite runs against each copy so the -#: ratchet-coverage one is executed rather than assumed identical. -INSTALLER_COPIES = { - "generate-coverage": ACTIONS_DIR - / "generate-coverage" - / "scripts" - / "install_cargo_llvm_cov.py", - "ratchet-coverage": ACTIONS_DIR - / "ratchet-coverage" - / "scripts" - / "install_cargo_llvm_cov.py", -} - - -def _load_installer(script: Path, name: str) -> ModuleType: - """Load one installer copy from ``script`` under module name ``name``.""" - spec = importlib.util.spec_from_file_location(name, script) - assert spec is not None - assert spec.loader is not None - module = importlib.util.module_from_spec(spec) - spec.loader.exec_module(module) - return module - - -@pytest.fixture(params=list(INSTALLER_COPIES), ids=list(INSTALLER_COPIES)) -def install_llvm_cov_module( - request: pytest.FixtureRequest, monkeypatch: pytest.MonkeyPatch -) -> ModuleType: - """Return a freshly loaded installer copy with job-level side effects disabled.""" - monkeypatch.delenv("GITHUB_STEP_SUMMARY", raising=False) - monkeypatch.delenv("GITHUB_PATH", raising=False) - monkeypatch.delenv("RUNNER_OS", raising=False) - monkeypatch.delenv("RUNNER_ARCH", raising=False) - if request.param == "generate-coverage": - return _load_module(monkeypatch, "install_cargo_llvm_cov") - return _load_installer( - INSTALLER_COPIES[request.param], f"install_cargo_llvm_cov_{request.param}" - ) - def test_both_actions_ship_the_same_installer() -> None: """The two copies are byte-identical, so a fix in one cannot miss the other.""" @@ -157,456 +98,6 @@ def test_unreadable_manifest_is_a_typed_error( assert excinfo.value.kind == install_llvm_cov_module.MANIFEST_UNREADABLE -def _tarball_with(member: str, payload: bytes) -> bytes: - """Return a gzip tarball holding ``payload`` at ``member``, mode 0o755.""" - buffer = io.BytesIO() - with tarfile.open(fileobj=buffer, mode="w:gz") as package: - info = tarfile.TarInfo(member) - info.size = len(payload) - info.mode = 0o755 - package.addfile(info, io.BytesIO(payload)) - return buffer.getvalue() - - -def _zip_with(member: str, payload: bytes) -> bytes: - """Return a zip archive holding ``payload`` at ``member``.""" - buffer = io.BytesIO() - with zipfile.ZipFile(buffer, mode="w") as package: - package.writestr(member, payload) - return buffer.getvalue() - - -#: A second payload, so a test can tell the requested member from a decoy by -#: content rather than only by name. -_DECOY_PAYLOAD = b"#!/bin/sh\necho 'decoy'\n" - -#: Members an archive carries beside the one the manifest names. A correct -#: extraction writes none of them; ``extractall`` would write all of them. -_DECOY_MEMBERS = ("README.md", "completions/cargo-llvm-cov.bash") - - -def _tarball_with_members(members: dict[str, bytes]) -> bytes: - """Return a gzip tarball holding each ``member`` payload, mode 0o755.""" - buffer = io.BytesIO() - with tarfile.open(fileobj=buffer, mode="w:gz") as package: - for name, payload in members.items(): - info = tarfile.TarInfo(name) - info.size = len(payload) - info.mode = 0o755 - package.addfile(info, io.BytesIO(payload)) - return buffer.getvalue() - - -def _zip_with_members(members: dict[str, bytes]) -> bytes: - """Return a zip archive holding each ``member`` payload.""" - buffer = io.BytesIO() - with zipfile.ZipFile(buffer, mode="w") as package: - for name, payload in members.items(): - package.writestr(name, payload) - return buffer.getvalue() - - -def _tarball_with_a_directory_member(member: str) -> bytes: - """Return a gzip tarball whose ``member`` is a directory, not a file.""" - buffer = io.BytesIO() - with tarfile.open(fileobj=buffer, mode="w:gz") as package: - info = tarfile.TarInfo(member) - info.type = tarfile.DIRTYPE - info.mode = 0o755 - package.addfile(info) - return buffer.getvalue() - - -#: The two archive formats the manifest can name, with the builder for each. -_ARCHIVE_FORMATS = { - "tar.gz": _tarball_with_members, - "zip": _zip_with_members, -} - - -def _fake_tool( - module: ModuleType, archive: bytes, *, extension: str, member: str -) -> ResolvedTool: - """Return a ``ResolvedTool`` whose digest matches ``archive``.""" - url = ( - "https://github.com/taiki-e/cargo-llvm-cov/releases/download/v0.9.0/" - f"cargo-llvm-cov-x86_64-unknown-linux-gnu.{extension}" - ) - return typ.cast( - "ResolvedTool", - module.ResolvedTool( - triple="x86_64-unknown-linux-gnu", - url=url, - sha256=hashlib.sha256(archive).hexdigest(), - member=member, - extension=extension, - binary="cargo-llvm-cov", - version_args=("llvm-cov", "--version"), - expected_version="cargo-llvm-cov 0.9.0", - ), - ) - - -def _write_fetch(archive: bytes) -> typ.Callable[[object, Path], None]: - """Return a ``fetch`` stand-in that writes ``archive`` instead of downloading.""" - - def fetch(_tool: object, destination: Path) -> None: - """Write the canned archive to ``destination``.""" - destination.write_bytes(archive) - - return fetch - - -@pytest.mark.parametrize("extension", ["tar.gz", "zip"], ids=["tarball", "zip"]) -def test_install_extracts_the_manifest_member_and_verifies_it( - install_llvm_cov_module: ModuleType, tmp_path: Path, extension: str -) -> None: - """A digest-verified archive installs exactly its member and reports the version.""" - archive = ( - _tarball_with("cargo-llvm-cov", _FAKE_BINARY) - if extension == "tar.gz" - else _zip_with("cargo-llvm-cov", _FAKE_BINARY) - ) - tool = _fake_tool( - install_llvm_cov_module, archive, extension=extension, member="cargo-llvm-cov" - ) - destination = tmp_path / "bin" / "cargo-llvm-cov" - - install_llvm_cov_module.install(tool, destination, fetch=_write_fetch(archive)) - - assert destination.read_bytes() == _FAKE_BINARY - assert destination.stat().st_mode & 0o111 - probe = install_llvm_cov_module.probe_version(destination, tool.version_args) - assert install_llvm_cov_module.installed_at_pinned_version( - probe, tool.expected_version - ) - assert [p.name for p in destination.parent.iterdir()] == ["cargo-llvm-cov"], ( - "the staging directory must not outlive the install" - ) - - -@dataclasses.dataclass(frozen=True) -class _RejectedArchive: - """One way a downloaded archive can be unusable.""" - - member: str - tamper_digest: bool - - -@pytest.mark.parametrize( - "case", - [ - pytest.param( - _RejectedArchive(member="cargo-llvm-cov", tamper_digest=True), - id="digest-mismatch", - ), - pytest.param( - _RejectedArchive(member="some-other-binary", tamper_digest=False), - id="missing-member", - ), - ], -) -def test_rejected_archive_fails_and_preserves_the_existing_binary( - install_llvm_cov_module: ModuleType, tmp_path: Path, case: _RejectedArchive -) -> None: - """A tampered or malformed archive is an error that leaves the binary alone.""" - archive = _tarball_with(case.member, _FAKE_BINARY) - tool = _fake_tool( - install_llvm_cov_module, archive, extension="tar.gz", member="cargo-llvm-cov" - ) - if case.tamper_digest: - tool = tool._replace(sha256="0" * 64) - destination = tmp_path / "bin" / "cargo-llvm-cov" - destination.parent.mkdir() - destination.write_bytes(b"previous") - - with pytest.raises(typer.Exit) as excinfo: - install_llvm_cov_module.install(tool, destination, fetch=_write_fetch(archive)) - - assert _exit_code(excinfo.value) == 1 - assert destination.read_bytes() == b"previous" - - -def test_installed_binary_reporting_another_version_fails_the_install( - install_llvm_cov_module: ModuleType, tmp_path: Path -) -> None: - """A verified archive whose binary reports the wrong version is not published.""" - archive = _tarball_with("cargo-llvm-cov", _WRONG_VERSION_BINARY) - tool = _fake_tool( - install_llvm_cov_module, archive, extension="tar.gz", member="cargo-llvm-cov" - ) - destination = tmp_path / "bin" / "cargo-llvm-cov" - destination.parent.mkdir() - destination.write_bytes(b"previous") - - with pytest.raises(typer.Exit) as excinfo: - install_llvm_cov_module.install(tool, destination, fetch=_write_fetch(archive)) - - assert _exit_code(excinfo.value) == 1 - assert destination.read_bytes() == b"previous" - assert [p.name for p in destination.parent.iterdir()] == ["cargo-llvm-cov"] - - -def _installer_module() -> ModuleType: - """Load the installer without pytest fixtures, for the property test. - - Hypothesis runs the test body many times per call and function-scoped - fixtures would be shared across those examples, so the module is loaded - directly here. - """ - return _load_installer( - INSTALLER_COPIES["generate-coverage"], "install_cargo_llvm_cov_property" - ) - - -_PROBE_STATES = st.sampled_from(["absent", "unrunnable", "reported"]) - - -@given(state=_PROBE_STATES, version=st.one_of(st.none(), st.text(max_size=40))) -def test_only_a_reported_exact_version_counts_as_installed( - state: str, version: str | None -) -> None: - """``installed_at_pinned_version`` accepts one probe outcome and nothing else. - - A prefix match would accept ``cargo-llvm-cov 0.9.0-rc1`` or - ``cargo-llvm-cov 0.9.01``; a substring match would accept a longer line - that merely mentions the version; and an absent or unrunnable binary must - never count, whatever ``version`` says. - """ - module = _installer_module() - probe = module.VersionProbe(state, version) - - outcome = module.installed_at_pinned_version(probe, "cargo-llvm-cov 0.9.0") - - assert outcome == (state == "reported" and version == "cargo-llvm-cov 0.9.0") - assert probe.metric_state("cargo-llvm-cov 0.9.0") == ( - state if state != "reported" else ("pinned" if outcome else "other-version") - ) - - -@pytest.mark.parametrize( - ("binary", "expected"), - [ - pytest.param(None, ("absent", None), id="absent"), - pytest.param(b"not executable", ("unrunnable", None), id="unrunnable"), - pytest.param(_FAKE_BINARY, ("reported", "cargo-llvm-cov 0.9.0"), id="reported"), - ], -) -def test_probe_version_reports_each_outcome_as_a_value( - install_llvm_cov_module: ModuleType, - tmp_path: Path, - binary: bytes | None, - expected: tuple[str, str | None], -) -> None: - """A missing, unrunnable and reporting binary are three distinct probe states.""" - path = tmp_path / "cargo-llvm-cov" - if binary is not None: - path.write_bytes(binary) - if binary == _FAKE_BINARY: - path.chmod(0o755) - - probe = install_llvm_cov_module.probe_version(path, ("llvm-cov", "--version")) - - assert tuple(probe) == expected - - -def test_oversized_download_is_discarded( - install_llvm_cov_module: ModuleType, tmp_path: Path, monkeypatch: pytest.MonkeyPatch -) -> None: - """A response beyond the byte cap is cut off and removed.""" - monkeypatch.setattr(install_llvm_cov_module, "_MAX_ARCHIVE_BYTES", 16) - - class _Response(io.BytesIO): - """A urlopen response that streams more bytes than the cap allows.""" - - def __enter__(self) -> _Response: - """Enter the response context.""" - return self - - def __exit__(self, *_args: object) -> None: - """Close the response.""" - self.close() - - monkeypatch.setattr( - install_llvm_cov_module.urllib.request, - "urlopen", - lambda *_a, **_k: _Response(b"x" * 64), - ) - tool = _fake_tool( - install_llvm_cov_module, b"", extension="tar.gz", member="cargo-llvm-cov" - ) - destination = tmp_path / tool.filename - - with pytest.raises(typer.Exit) as excinfo: - install_llvm_cov_module.download_archive(tool, destination) - - assert _exit_code(excinfo.value) == 1 - assert not destination.exists() - - -def _job_environment( - monkeypatch: pytest.MonkeyPatch, tmp_path: Path -) -> tuple[Path, Path, Path]: - """Point CARGO_HOME, GITHUB_PATH and GITHUB_STEP_SUMMARY into ``tmp_path``.""" - cargo_home = tmp_path / "cargo" - github_path = tmp_path / "github_path" - summary = tmp_path / "summary.md" - monkeypatch.setenv("CARGO_HOME", str(cargo_home)) - monkeypatch.setenv("GITHUB_PATH", str(github_path)) - monkeypatch.setenv("GITHUB_STEP_SUMMARY", str(summary)) - monkeypatch.setenv("RUNNER_OS", "Linux") - monkeypatch.setenv("RUNNER_ARCH", "X64") - return cargo_home / "bin" / "cargo-llvm-cov", github_path, summary - - -def test_main_reuses_an_installed_binary_at_the_pinned_version( - install_llvm_cov_module: ModuleType, tmp_path: Path, monkeypatch: pytest.MonkeyPatch -) -> None: - """A binary already reporting the pinned version is kept and exported to PATH.""" - binary, github_path, summary = _job_environment(monkeypatch, tmp_path) - binary.parent.mkdir(parents=True) - binary.write_bytes(_FAKE_BINARY) - binary.chmod(0o755) - - def fail_install(*_args: object, **_kwargs: object) -> None: - """Fail the test if the installer tries to install.""" - message = "install must not run for a reused binary" - raise AssertionError(message) - - monkeypatch.setattr(install_llvm_cov_module, "install", fail_install) - - install_llvm_cov_module.main() - - assert github_path.read_text(encoding="utf-8").strip() == str(binary.parent) - assert "metric cargo-llvm-cov.install=reused" in summary.read_text(encoding="utf-8") - - -def test_main_replaces_an_installed_binary_at_another_version( - install_llvm_cov_module: ModuleType, tmp_path: Path, monkeypatch: pytest.MonkeyPatch -) -> None: - """A binary reporting a different version triggers a fresh install.""" - binary, _github_path, _summary = _job_environment(monkeypatch, tmp_path) - binary.parent.mkdir(parents=True) - binary.write_bytes(_WRONG_VERSION_BINARY) - binary.chmod(0o755) - calls: list[Path] = [] - - monkeypatch.setattr( - install_llvm_cov_module, - "install", - lambda _tool, destination, **_kwargs: calls.append(destination), - ) - - install_llvm_cov_module.main() - - assert calls == [binary] - - -class _ArchiveHandler(http.server.BaseHTTPRequestHandler): - """Serve one archive at one path; anything else is a 404.""" - - archive: typ.ClassVar[bytes] = b"" - path_served: typ.ClassVar[str] = "" - - def do_GET(self) -> None: - if self.path != self.path_served: - self.send_error(404) - return - self.send_response(200) - self.send_header("Content-Length", str(len(self.archive))) - self.end_headers() - self.wfile.write(self.archive) - - def log_message(self, *_args: object) -> None: - """Keep the server quiet during the test.""" - return - - -@pytest.fixture -def archive_server() -> typ.Iterator[typ.Callable[[bytes, str], str]]: - """Start a local HTTP server and return ``serve(archive, path) -> url``.""" - server = http.server.ThreadingHTTPServer(("127.0.0.1", 0), _ArchiveHandler) - thread = threading.Thread(target=server.serve_forever, daemon=True) - thread.start() - - def serve(archive: bytes, path: str) -> str: - """Publish ``archive`` at ``path`` and return its URL.""" - _ArchiveHandler.archive = archive - _ArchiveHandler.path_served = path - return f"http://127.0.0.1:{server.server_port}{path}" - - yield serve - server.shutdown() - server.server_close() - - -def test_entry_point_installs_from_a_manifest_over_http( - install_llvm_cov_module: ModuleType, - tmp_path: Path, - monkeypatch: pytest.MonkeyPatch, - archive_server: typ.Callable[[bytes, str], str], -) -> None: - """``main`` resolves, downloads, verifies, installs and exports, end to end. - - The manifest is a temporary one whose entry points at a local HTTP - server, so the real download path runs without leaving the machine. - """ - binary, github_path, summary = _job_environment(monkeypatch, tmp_path) - archive = _tarball_with("cargo-llvm-cov", _FAKE_BINARY) - url = archive_server( - archive, "/v0.9.0/cargo-llvm-cov-x86_64-unknown-linux-gnu.tar.gz" - ) - manifest = tmp_path / "tool-manifest.toml" - manifest.write_text( - "schema = 1\n\n" - "[[tool]]\n" - 'name = "cargo-llvm-cov"\n' - f'version = "{install_llvm_cov_module.CARGO_LLVM_COV_VERSION}"\n' - 'binary = "cargo-llvm-cov"\n' - 'version-args = ["llvm-cov", "--version"]\n\n' - " [[tool.target]]\n" - ' triple = "x86_64-unknown-linux-gnu"\n' - f' url = "{url}"\n' - f' sha256 = "{hashlib.sha256(archive).hexdigest()}"\n' - ' member = "cargo-llvm-cov"\n' - ' sidecar-verified = "absent"\n', - encoding="utf-8", - ) - monkeypatch.setattr(install_llvm_cov_module, "MANIFEST_PATH", manifest) - monkeypatch.setattr( - install_llvm_cov_module, - "load_manifest", - functools.partial(install_llvm_cov_module.load_manifest, manifest), - ) - - install_llvm_cov_module.main() - - assert binary.read_bytes() == _FAKE_BINARY - assert binary.stat().st_mode & 0o111 - assert github_path.read_text(encoding="utf-8").strip() == str(binary.parent) - metrics = summary.read_text(encoding="utf-8") - assert "metric cargo-llvm-cov.resolve=ok" in metrics - assert "metric cargo-llvm-cov.download=ok" in metrics - assert "metric cargo-llvm-cov.archive-digest=ok" in metrics - assert "metric cargo-llvm-cov.install=ok" in metrics - - -def test_entry_point_reports_a_resolution_failure_by_kind( - install_llvm_cov_module: ModuleType, tmp_path: Path, monkeypatch: pytest.MonkeyPatch -) -> None: - """An unsupported runner exits 1 with a bounded resolve metric.""" - _binary, _github_path, summary = _job_environment(monkeypatch, tmp_path) - monkeypatch.setenv("RUNNER_OS", "Plan9") - - with pytest.raises(typer.Exit) as excinfo: - install_llvm_cov_module.main() - - assert _exit_code(excinfo.value) == 1 - assert "metric cargo-llvm-cov.resolve=unsupported-runner" in summary.read_text( - encoding="utf-8" - ) - - def test_a_missing_resolver_is_a_typed_error_without_output( install_llvm_cov_module: ModuleType, tmp_path: Path, capsys: pytest.CaptureFixture ) -> None: @@ -698,96 +189,3 @@ def test_main_reports_a_failing_resolver_load_as_a_metric( assert "metric cargo-llvm-cov.resolve=resolver-unavailable" in summary.read_text( encoding="utf-8" ) - - -@pytest.mark.parametrize( - "extension", list(_ARCHIVE_FORMATS), ids=list(_ARCHIVE_FORMATS) -) -def test_extraction_takes_only_the_named_member( - install_llvm_cov_module: ModuleType, tmp_path: Path, extension: str -) -> None: - """Only the manifest's member leaves the archive, whatever else it holds. - - The archive carries decoys beside the wanted member, so an implementation - that unpacked everything would be caught here. Without them an - ``extractall`` would satisfy every other test in this module, because a - single-member archive makes the two strategies indistinguishable. - """ - members = {"cargo-llvm-cov": _FAKE_BINARY} | dict.fromkeys( - _DECOY_MEMBERS, _DECOY_PAYLOAD - ) - archive_bytes = _ARCHIVE_FORMATS[extension](members) - archive = tmp_path / f"cargo-llvm-cov.{extension}" - archive.write_bytes(archive_bytes) - tool = _fake_tool( - install_llvm_cov_module, - archive_bytes, - extension=extension, - member="cargo-llvm-cov", - ) - destination = tmp_path / "bin" / "cargo-llvm-cov" - destination.parent.mkdir() - - install_llvm_cov_module.extract_member(archive, tool, destination) - - assert destination.read_bytes() == _FAKE_BINARY - assert [path.name for path in destination.parent.iterdir()] == ["cargo-llvm-cov"] - assert not (tmp_path / "README.md").exists() - assert not (tmp_path / "completions").exists() - - -@pytest.mark.parametrize( - "extension", list(_ARCHIVE_FORMATS), ids=list(_ARCHIVE_FORMATS) -) -def test_extraction_refuses_an_archive_without_the_named_member( - install_llvm_cov_module: ModuleType, tmp_path: Path, extension: str -) -> None: - """A member the manifest names but the archive lacks is rejected outright. - - Both formats are covered: the missing-member path is written separately - for zip and tar, so testing one leaves the other unguarded. - """ - members = dict.fromkeys(_DECOY_MEMBERS, _DECOY_PAYLOAD) - archive_bytes = _ARCHIVE_FORMATS[extension](members) - archive = tmp_path / f"cargo-llvm-cov.{extension}" - archive.write_bytes(archive_bytes) - tool = _fake_tool( - install_llvm_cov_module, - archive_bytes, - extension=extension, - member="cargo-llvm-cov", - ) - destination = tmp_path / "bin" / "cargo-llvm-cov" - destination.parent.mkdir() - - with pytest.raises(ValueError, match="missing from"): - install_llvm_cov_module.extract_member(archive, tool, destination) - - assert not destination.exists() - - -def test_extraction_refuses_a_tar_member_that_is_not_a_file( - install_llvm_cov_module: ModuleType, tmp_path: Path -) -> None: - """A tar entry with the member's name but a directory type is rejected. - - ``TarFile.extractfile`` returns ``None`` rather than raising for a - non-regular entry, so an unchecked implementation would carry that - ``None`` into the copy instead of failing here. - """ - archive_bytes = _tarball_with_a_directory_member("cargo-llvm-cov") - archive = tmp_path / "cargo-llvm-cov.tar.gz" - archive.write_bytes(archive_bytes) - tool = _fake_tool( - install_llvm_cov_module, - archive_bytes, - extension="tar.gz", - member="cargo-llvm-cov", - ) - destination = tmp_path / "bin" / "cargo-llvm-cov" - destination.parent.mkdir() - - with pytest.raises(ValueError, match="not a file"): - install_llvm_cov_module.extract_member(archive, tool, destination) - - assert not destination.exists() diff --git a/.github/actions/generate-coverage/tests/test_install_cargo_llvm_cov_archives.py b/.github/actions/generate-coverage/tests/test_install_cargo_llvm_cov_archives.py new file mode 100644 index 000000000..ed90a0309 --- /dev/null +++ b/.github/actions/generate-coverage/tests/test_install_cargo_llvm_cov_archives.py @@ -0,0 +1,318 @@ +"""Verify the cargo-llvm-cov installer's archive and installation handling. + +Split from ``test_install_cargo_llvm_cov.py``, which keeps manifest +resolution, so neither module carries more responsibilities than the code +health rules allow. This module covers download bounds, digest verification, +selective extraction, atomic publication, version probing and the reuse +decision. The tests and their identifiers are unchanged by the move. +""" + +from __future__ import annotations + +import dataclasses +import io +import typing as typ + +import pytest +import typer +from _coverage_test_support import _exit_code +from _llvm_cov_test_support import ( + _ARCHIVE_FORMATS, + _DECOY_MEMBERS, + _DECOY_PAYLOAD, + _FAKE_BINARY, + _WRONG_VERSION_BINARY, + INSTALLER_COPIES, + _fake_tool, + _load_installer, + _tarball_with, + _tarball_with_a_directory_member, + _write_fetch, + _zip_with, +) +from hypothesis import given +from hypothesis import strategies as st + +if typ.TYPE_CHECKING: + from pathlib import Path + from types import ModuleType + + +@pytest.mark.parametrize("extension", ["tar.gz", "zip"], ids=["tarball", "zip"]) +def test_install_extracts_the_manifest_member_and_verifies_it( + install_llvm_cov_module: ModuleType, tmp_path: Path, extension: str +) -> None: + """A digest-verified archive installs exactly its member and reports the version.""" + archive = ( + _tarball_with({"cargo-llvm-cov": _FAKE_BINARY}) + if extension == "tar.gz" + else _zip_with({"cargo-llvm-cov": _FAKE_BINARY}) + ) + tool = _fake_tool( + install_llvm_cov_module, archive, extension=extension, member="cargo-llvm-cov" + ) + destination = tmp_path / "bin" / "cargo-llvm-cov" + + install_llvm_cov_module.install(tool, destination, fetch=_write_fetch(archive)) + + assert destination.read_bytes() == _FAKE_BINARY + assert destination.stat().st_mode & 0o111 + probe = install_llvm_cov_module.probe_version(destination, tool.version_args) + assert install_llvm_cov_module.installed_at_pinned_version( + probe, tool.expected_version + ) + assert [p.name for p in destination.parent.iterdir()] == ["cargo-llvm-cov"], ( + "the staging directory must not outlive the install" + ) + + +@dataclasses.dataclass(frozen=True) +class _RejectedArchive: + """One way a downloaded archive can be unusable.""" + + member: str + tamper_digest: bool + + +@pytest.mark.parametrize( + "case", + [ + pytest.param( + _RejectedArchive(member="cargo-llvm-cov", tamper_digest=True), + id="digest-mismatch", + ), + pytest.param( + _RejectedArchive(member="some-other-binary", tamper_digest=False), + id="missing-member", + ), + ], +) +def test_rejected_archive_fails_and_preserves_the_existing_binary( + install_llvm_cov_module: ModuleType, tmp_path: Path, case: _RejectedArchive +) -> None: + """A tampered or malformed archive is an error that leaves the binary alone.""" + archive = _tarball_with({case.member: _FAKE_BINARY}) + tool = _fake_tool( + install_llvm_cov_module, archive, extension="tar.gz", member="cargo-llvm-cov" + ) + if case.tamper_digest: + tool = tool._replace(sha256="0" * 64) + destination = tmp_path / "bin" / "cargo-llvm-cov" + destination.parent.mkdir() + destination.write_bytes(b"previous") + + with pytest.raises(typer.Exit) as excinfo: + install_llvm_cov_module.install(tool, destination, fetch=_write_fetch(archive)) + + assert _exit_code(excinfo.value) == 1 + assert destination.read_bytes() == b"previous" + + +def test_installed_binary_reporting_another_version_fails_the_install( + install_llvm_cov_module: ModuleType, tmp_path: Path +) -> None: + """A verified archive whose binary reports the wrong version is not published.""" + archive = _tarball_with({"cargo-llvm-cov": _WRONG_VERSION_BINARY}) + tool = _fake_tool( + install_llvm_cov_module, archive, extension="tar.gz", member="cargo-llvm-cov" + ) + destination = tmp_path / "bin" / "cargo-llvm-cov" + destination.parent.mkdir() + destination.write_bytes(b"previous") + + with pytest.raises(typer.Exit) as excinfo: + install_llvm_cov_module.install(tool, destination, fetch=_write_fetch(archive)) + + assert _exit_code(excinfo.value) == 1 + assert destination.read_bytes() == b"previous" + assert [p.name for p in destination.parent.iterdir()] == ["cargo-llvm-cov"] + + +def _installer_module() -> ModuleType: + """Load the installer without pytest fixtures, for the property test. + + Hypothesis runs the test body many times per call and function-scoped + fixtures would be shared across those examples, so the module is loaded + directly here. + """ + return _load_installer( + INSTALLER_COPIES["generate-coverage"], "install_cargo_llvm_cov_property" + ) + + +_PROBE_STATES = st.sampled_from(["absent", "unrunnable", "reported"]) + + +@given(state=_PROBE_STATES, version=st.one_of(st.none(), st.text(max_size=40))) +def test_only_a_reported_exact_version_counts_as_installed( + state: str, version: str | None +) -> None: + """``installed_at_pinned_version`` accepts one probe outcome and nothing else. + + A prefix match would accept ``cargo-llvm-cov 0.9.0-rc1`` or + ``cargo-llvm-cov 0.9.01``; a substring match would accept a longer line + that merely mentions the version; and an absent or unrunnable binary must + never count, whatever ``version`` says. + """ + module = _installer_module() + probe = module.VersionProbe(state, version) + + outcome = module.installed_at_pinned_version(probe, "cargo-llvm-cov 0.9.0") + + assert outcome == (state == "reported" and version == "cargo-llvm-cov 0.9.0") + assert probe.metric_state("cargo-llvm-cov 0.9.0") == ( + state if state != "reported" else ("pinned" if outcome else "other-version") + ) + + +@pytest.mark.parametrize( + ("binary", "expected"), + [ + pytest.param(None, ("absent", None), id="absent"), + pytest.param(b"not executable", ("unrunnable", None), id="unrunnable"), + pytest.param(_FAKE_BINARY, ("reported", "cargo-llvm-cov 0.9.0"), id="reported"), + ], +) +def test_probe_version_reports_each_outcome_as_a_value( + install_llvm_cov_module: ModuleType, + tmp_path: Path, + binary: bytes | None, + expected: tuple[str, str | None], +) -> None: + """A missing, unrunnable and reporting binary are three distinct probe states.""" + path = tmp_path / "cargo-llvm-cov" + if binary is not None: + path.write_bytes(binary) + if binary == _FAKE_BINARY: + path.chmod(0o755) + + probe = install_llvm_cov_module.probe_version(path, ("llvm-cov", "--version")) + + assert tuple(probe) == expected + + +def test_oversized_download_is_discarded( + install_llvm_cov_module: ModuleType, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """A response beyond the byte cap is cut off and removed.""" + monkeypatch.setattr(install_llvm_cov_module, "_MAX_ARCHIVE_BYTES", 16) + + class _Response(io.BytesIO): + """A urlopen response that streams more bytes than the cap allows.""" + + def __enter__(self) -> _Response: + """Enter the response context.""" + return self + + def __exit__(self, *_args: object) -> None: + """Close the response.""" + self.close() + + monkeypatch.setattr( + install_llvm_cov_module.urllib.request, + "urlopen", + lambda *_a, **_k: _Response(b"x" * 64), + ) + tool = _fake_tool( + install_llvm_cov_module, b"", extension="tar.gz", member="cargo-llvm-cov" + ) + destination = tmp_path / tool.filename + + with pytest.raises(typer.Exit) as excinfo: + install_llvm_cov_module.download_archive(tool, destination) + + assert _exit_code(excinfo.value) == 1 + assert not destination.exists() + + +@pytest.mark.parametrize( + "extension", list(_ARCHIVE_FORMATS), ids=list(_ARCHIVE_FORMATS) +) +def test_extraction_takes_only_the_named_member( + install_llvm_cov_module: ModuleType, tmp_path: Path, extension: str +) -> None: + """Only the manifest's member leaves the archive, whatever else it holds. + + The archive carries decoys beside the wanted member, so an implementation + that unpacked everything would be caught here. Without them an + ``extractall`` would satisfy every other test in this module, because a + single-member archive makes the two strategies indistinguishable. + """ + members = {"cargo-llvm-cov": _FAKE_BINARY} | dict.fromkeys( + _DECOY_MEMBERS, _DECOY_PAYLOAD + ) + archive_bytes = _ARCHIVE_FORMATS[extension](members) + archive = tmp_path / f"cargo-llvm-cov.{extension}" + archive.write_bytes(archive_bytes) + tool = _fake_tool( + install_llvm_cov_module, + archive_bytes, + extension=extension, + member="cargo-llvm-cov", + ) + destination = tmp_path / "bin" / "cargo-llvm-cov" + destination.parent.mkdir() + + install_llvm_cov_module.extract_member(archive, tool, destination) + + assert destination.read_bytes() == _FAKE_BINARY + assert [path.name for path in destination.parent.iterdir()] == ["cargo-llvm-cov"] + assert not (tmp_path / "README.md").exists() + assert not (tmp_path / "completions").exists() + + +@pytest.mark.parametrize( + "extension", list(_ARCHIVE_FORMATS), ids=list(_ARCHIVE_FORMATS) +) +def test_extraction_refuses_an_archive_without_the_named_member( + install_llvm_cov_module: ModuleType, tmp_path: Path, extension: str +) -> None: + """A member the manifest names but the archive lacks is rejected outright. + + Both formats are covered: the missing-member path is written separately + for zip and tar, so testing one leaves the other unguarded. + """ + members = dict.fromkeys(_DECOY_MEMBERS, _DECOY_PAYLOAD) + archive_bytes = _ARCHIVE_FORMATS[extension](members) + archive = tmp_path / f"cargo-llvm-cov.{extension}" + archive.write_bytes(archive_bytes) + tool = _fake_tool( + install_llvm_cov_module, + archive_bytes, + extension=extension, + member="cargo-llvm-cov", + ) + destination = tmp_path / "bin" / "cargo-llvm-cov" + destination.parent.mkdir() + + with pytest.raises(ValueError, match="missing from"): + install_llvm_cov_module.extract_member(archive, tool, destination) + + assert not destination.exists() + + +def test_extraction_refuses_a_tar_member_that_is_not_a_file( + install_llvm_cov_module: ModuleType, tmp_path: Path +) -> None: + """A tar entry with the member's name but a directory type is rejected. + + ``TarFile.extractfile`` returns ``None`` rather than raising for a + non-regular entry, so an unchecked implementation would carry that + ``None`` into the copy instead of failing here. + """ + archive_bytes = _tarball_with_a_directory_member("cargo-llvm-cov") + archive = tmp_path / "cargo-llvm-cov.tar.gz" + archive.write_bytes(archive_bytes) + tool = _fake_tool( + install_llvm_cov_module, + archive_bytes, + extension="tar.gz", + member="cargo-llvm-cov", + ) + destination = tmp_path / "bin" / "cargo-llvm-cov" + destination.parent.mkdir() + + with pytest.raises(ValueError, match="not a file"): + install_llvm_cov_module.extract_member(archive, tool, destination) + + assert not destination.exists() diff --git a/.github/actions/generate-coverage/tests/test_install_cargo_llvm_cov_entrypoint.py b/.github/actions/generate-coverage/tests/test_install_cargo_llvm_cov_entrypoint.py new file mode 100644 index 000000000..73db847f5 --- /dev/null +++ b/.github/actions/generate-coverage/tests/test_install_cargo_llvm_cov_entrypoint.py @@ -0,0 +1,178 @@ +"""Verify the cargo-llvm-cov installer's entry point end to end. + +Split from ``test_install_cargo_llvm_cov.py`` so that module keeps manifest +resolution alone. This module drives ``main`` across a real HTTP boundary +against a temporary manifest, and covers the reuse and replace decisions and +the bounded metrics the command boundary publishes. The tests and their +identifiers are unchanged by the move. +""" + +from __future__ import annotations + +import functools +import hashlib +import http.server +import threading +import typing as typ + +import pytest +import typer +from _coverage_test_support import _exit_code +from _llvm_cov_test_support import ( + _FAKE_BINARY, + _WRONG_VERSION_BINARY, + _job_environment, + _tarball_with, +) + +if typ.TYPE_CHECKING: + from pathlib import Path + from types import ModuleType + + +def test_main_reuses_an_installed_binary_at_the_pinned_version( + install_llvm_cov_module: ModuleType, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """A binary already reporting the pinned version is kept and exported to PATH.""" + binary, github_path, summary = _job_environment(monkeypatch, tmp_path) + binary.parent.mkdir(parents=True) + binary.write_bytes(_FAKE_BINARY) + binary.chmod(0o755) + + def fail_install(*_args: object, **_kwargs: object) -> None: + """Fail the test if the installer tries to install.""" + message = "install must not run for a reused binary" + raise AssertionError(message) + + monkeypatch.setattr(install_llvm_cov_module, "install", fail_install) + + install_llvm_cov_module.main() + + assert github_path.read_text(encoding="utf-8").strip() == str(binary.parent) + assert "metric cargo-llvm-cov.install=reused" in summary.read_text(encoding="utf-8") + + +def test_main_replaces_an_installed_binary_at_another_version( + install_llvm_cov_module: ModuleType, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """A binary reporting a different version triggers a fresh install.""" + binary, _github_path, _summary = _job_environment(monkeypatch, tmp_path) + binary.parent.mkdir(parents=True) + binary.write_bytes(_WRONG_VERSION_BINARY) + binary.chmod(0o755) + calls: list[Path] = [] + + monkeypatch.setattr( + install_llvm_cov_module, + "install", + lambda _tool, destination, **_kwargs: calls.append(destination), + ) + + install_llvm_cov_module.main() + + assert calls == [binary] + + +class _ArchiveHandler(http.server.BaseHTTPRequestHandler): + """Serve one archive at one path; anything else is a 404.""" + + archive: typ.ClassVar[bytes] = b"" + path_served: typ.ClassVar[str] = "" + + def do_GET(self) -> None: + if self.path != self.path_served: + self.send_error(404) + return + self.send_response(200) + self.send_header("Content-Length", str(len(self.archive))) + self.end_headers() + self.wfile.write(self.archive) + + def log_message(self, *_args: object) -> None: + """Keep the server quiet during the test.""" + return + + +@pytest.fixture +def archive_server() -> typ.Iterator[typ.Callable[[bytes, str], str]]: + """Start a local HTTP server and return ``serve(archive, path) -> url``.""" + server = http.server.ThreadingHTTPServer(("127.0.0.1", 0), _ArchiveHandler) + thread = threading.Thread(target=server.serve_forever, daemon=True) + thread.start() + + def serve(archive: bytes, path: str) -> str: + """Publish ``archive`` at ``path`` and return its URL.""" + _ArchiveHandler.archive = archive + _ArchiveHandler.path_served = path + return f"http://127.0.0.1:{server.server_port}{path}" + + yield serve + server.shutdown() + server.server_close() + + +def test_entry_point_installs_from_a_manifest_over_http( + install_llvm_cov_module: ModuleType, + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + archive_server: typ.Callable[[bytes, str], str], +) -> None: + """``main`` resolves, downloads, verifies, installs and exports, end to end. + + The manifest is a temporary one whose entry points at a local HTTP + server, so the real download path runs without leaving the machine. + """ + binary, github_path, summary = _job_environment(monkeypatch, tmp_path) + archive = _tarball_with({"cargo-llvm-cov": _FAKE_BINARY}) + url = archive_server( + archive, "/v0.9.0/cargo-llvm-cov-x86_64-unknown-linux-gnu.tar.gz" + ) + manifest = tmp_path / "tool-manifest.toml" + manifest.write_text( + "schema = 1\n\n" + "[[tool]]\n" + 'name = "cargo-llvm-cov"\n' + f'version = "{install_llvm_cov_module.CARGO_LLVM_COV_VERSION}"\n' + 'binary = "cargo-llvm-cov"\n' + 'version-args = ["llvm-cov", "--version"]\n\n' + " [[tool.target]]\n" + ' triple = "x86_64-unknown-linux-gnu"\n' + f' url = "{url}"\n' + f' sha256 = "{hashlib.sha256(archive).hexdigest()}"\n' + ' member = "cargo-llvm-cov"\n' + ' sidecar-verified = "absent"\n', + encoding="utf-8", + ) + monkeypatch.setattr(install_llvm_cov_module, "MANIFEST_PATH", manifest) + monkeypatch.setattr( + install_llvm_cov_module, + "load_manifest", + functools.partial(install_llvm_cov_module.load_manifest, manifest), + ) + + install_llvm_cov_module.main() + + assert binary.read_bytes() == _FAKE_BINARY + assert binary.stat().st_mode & 0o111 + assert github_path.read_text(encoding="utf-8").strip() == str(binary.parent) + metrics = summary.read_text(encoding="utf-8") + assert "metric cargo-llvm-cov.resolve=ok" in metrics + assert "metric cargo-llvm-cov.download=ok" in metrics + assert "metric cargo-llvm-cov.archive-digest=ok" in metrics + assert "metric cargo-llvm-cov.install=ok" in metrics + + +def test_entry_point_reports_a_resolution_failure_by_kind( + install_llvm_cov_module: ModuleType, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """An unsupported runner exits 1 with a bounded resolve metric.""" + _binary, _github_path, summary = _job_environment(monkeypatch, tmp_path) + monkeypatch.setenv("RUNNER_OS", "Plan9") + + with pytest.raises(typer.Exit) as excinfo: + install_llvm_cov_module.main() + + assert _exit_code(excinfo.value) == 1 + assert "metric cargo-llvm-cov.resolve=unsupported-runner" in summary.read_text( + encoding="utf-8" + )