build: enforce keep-sorted in pre-commit and CI (#4453) Several files use `# keep-sorted` directives, but nothing verified or sorted them automatically, allowing stale markers and unsorted blocks to slip in. Add `google/keep-sorted` to `.pre-commit-config.yaml` and a cached `keep-sorted` lint job in GitHub Actions CI, backed by a shared `.github/actions/download-tool` action and verified download module reused by `setup-bazel`. Also fix existing `keep-sorted` directives so all tracked files pass.
diff --git a/.agents/rules/code-style.md b/.agents/rules/code-style.md index e2c0aa8..eb4ebf4 100644 --- a/.agents/rules/code-style.md +++ b/.agents/rules/code-style.md
@@ -59,8 +59,8 @@ error messages for CLI flags that bypass safety checks. ## Sorting -* Add `keep-sorted start` and `keep-sorted end` markers on list and dict config - or registry tables, even when they have a single entry. +* Add `keep-sorted` `start` and `end` markers on list and dict config or + registry tables, even when they have a single entry. ## Bulk Logging * In bulk operations, log a single summary line on success; on failure, list
diff --git a/.github/actions/download-tool/BUILD.bazel b/.github/actions/download-tool/BUILD.bazel new file mode 100644 index 0000000..383bac9 --- /dev/null +++ b/.github/actions/download-tool/BUILD.bazel
@@ -0,0 +1,7 @@ +package(default_visibility = ["//tests:__subpackages__"]) + +filegroup( + name = "all_files", + srcs = ["action.yaml"], + visibility = ["//tests:__subpackages__"], +)
diff --git a/.github/actions/download-tool/action.yaml b/.github/actions/download-tool/action.yaml new file mode 100644 index 0000000..165b7f5 --- /dev/null +++ b/.github/actions/download-tool/action.yaml
@@ -0,0 +1,57 @@ +name: "Download Tool" +description: "Download, SHA-256-verify, and cache a tool binary" + +inputs: + name: + description: "Tool binary name (used in the cache key and cached filename)" + required: true + urls: + description: "Comma- or newline-separated list of URLs to download the tool binary from" + required: true + sha256: + description: "Expected hex SHA-256 digest of the tool binary" + required: true + +outputs: + path: + description: "Path to the verified executable binary" + value: ${{ steps.tool_path.outputs.path }} + +runs: + using: "composite" + steps: + - name: Resolve tool path + id: tool_path + shell: bash + env: + TOOL_NAME: ${{ inputs.name }} + TOOL_SHA256: ${{ inputs.sha256 }} + run: >- + [[ "${HOME}" != *[$'\r\n']* + && "${TOOL_NAME}" =~ ^[a-zA-Z0-9._-]+$ + && "${TOOL_SHA256}" =~ ^[0-9a-fA-F]{64}$ ]] + || { printf '::error::Invalid tool name or sha256 digest\n'; exit 1; }; + printf 'path=%s\n' + "${HOME}/.cache/download-tool/${TOOL_SHA256}/${TOOL_NAME}" + >> "${GITHUB_OUTPUT}" + + - name: Cache tool + id: cache + uses: actions/cache@v6 + with: + path: ${{ steps.tool_path.outputs.path }} + key: ${{ inputs.name }}-${{ inputs.sha256 }} + + - name: Download and verify tool + if: steps.cache.outputs.cache-hit != 'true' + shell: bash + env: + TOOL_PATH: ${{ steps.tool_path.outputs.path }} + TOOL_SHA256: ${{ inputs.sha256 }} + TOOL_URLS: ${{ inputs.urls }} + run: >- + "${GITHUB_ACTION_PATH}/../../../dev/verified_download.py" + --urls "${TOOL_URLS}" + --sha256 "${TOOL_SHA256}" + --executable + --output "${TOOL_PATH}"
diff --git a/.github/dependabot.yml b/.github/dependabot.yml index 8ee450b..1ac1f1e 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml
@@ -28,8 +28,10 @@ - package-ecosystem: "github-actions" multi-ecosystem-group: "dev-only" directories: + # keep-sorted start - "/" - - "/.github/actions/setup-bazel" + - "/.github/actions/*" + # keep-sorted end patterns: - "*"
diff --git a/.github/workflows/check_keep_sorted.py b/.github/workflows/check_keep_sorted.py new file mode 100755 index 0000000..be7ba4b --- /dev/null +++ b/.github/workflows/check_keep_sorted.py
@@ -0,0 +1,214 @@ +#!/usr/bin/env python3 +"""Runs google/keep-sorted in lint mode across tracked files. + +Usage: + check_keep_sorted.py /path/to/keep-sorted +""" + +from __future__ import annotations + +import argparse +import asyncio +import json +import os +import subprocess +import sys +from collections.abc import Sequence +from pathlib import Path + +# Cap file arguments per subprocess invocation well below Linux ARG_MAX (2 MiB). +FILES_PER_BATCH = 200 +_MAX_CONCURRENCY = os.cpu_count() or 4 + + +def _parse_args(argv: list[str]) -> Path: + """Parses CLI arguments and returns the `keep-sorted` executable path.""" + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument( + "binary", + type=Path, + help="Path to the keep-sorted executable.", + ) + args = parser.parse_args(argv) + return args.binary + + +def _format_subprocess_error( + prefix: str, proc: subprocess.CompletedProcess[str] +) -> str: + """Formats a failed subprocess result with demarcated stdout/stderr.""" + return ( + f"{prefix} (exit {proc.returncode}):\n" + f"==================== STDOUT BEGIN ====================\n" + f"{proc.stdout}" + f"==================== STDOUT END ====================\n" + f"==================== STDERR BEGIN ====================\n" + f"{proc.stderr}" + f"==================== STDERR END ====================" + ) + + +async def _run_process( + cmd: Sequence[str], *, cwd: Path | None = None +) -> subprocess.CompletedProcess[str]: + """Runs `cmd` and captures UTF-8 stdout and stderr.""" + proc = await asyncio.create_subprocess_exec( + *cmd, + cwd=cwd, + stdout=asyncio.subprocess.PIPE, + stderr=asyncio.subprocess.PIPE, + ) + stdout_bytes, stderr_bytes = await proc.communicate() + assert proc.returncode is not None # type assert + return subprocess.CompletedProcess( + args=list(cmd), + returncode=proc.returncode, + stdout=stdout_bytes.decode("utf-8"), + stderr=stderr_bytes.decode("utf-8"), + ) + + +def _filter_regular_files(repo_root: Path, raw_entries: Sequence[str]) -> list[str]: + """Filters NUL-split git paths down to existing non-symlink regular files.""" + tracked: list[str] = [] + for rel_path in raw_entries: + if not rel_path: + continue + full_path = repo_root / rel_path + if full_path.is_file() and not full_path.is_symlink(): + tracked.append(rel_path) + return tracked + + +async def list_tracked_files(repo_root: Path) -> list[str]: + """Lists existing regular files tracked by git under `repo_root`. + + Args: + repo_root: Repository root directory. + + Returns: + Paths relative to `repo_root`. + """ + # -z (no long form) NUL-delimits paths without quoting special characters. + proc = await _run_process(["git", "ls-files", "-z"], cwd=repo_root) + if proc.returncode != 0: + raise RuntimeError(_format_subprocess_error("git ls-files failed", proc)) + raw_entries = proc.stdout.split("\0") + return await asyncio.to_thread(_filter_regular_files, repo_root, raw_entries) + + +def _format_finding(finding: dict[str, object]) -> str: + """Formats a single `keep-sorted --mode=lint` JSON finding line. + + Args: + finding: Parsed JSON finding object with `path`, `lines`, and `message`. + + Returns: + Formatted `<path>:<lines>: <message>` string with `path` relative to the + repository root. + """ + path = str(finding["path"]) + lines_obj = finding["lines"] + if not isinstance(lines_obj, dict): + raise ValueError(f"Unexpected 'lines' payload in finding: {finding!r}") + start = int(lines_obj["start"]) + end = int(lines_obj["end"]) + message = str(finding["message"]) + if start == end: + location = f"{path}:{start}" + else: + location = f"{path}:{start}-{end}" + return f"{location}: {message}" + + +async def _lint_batch( + binary: Path, + repo_root: Path, + batch: Sequence[str], + semaphore: asyncio.Semaphore, +) -> list[str]: + """Runs `keep-sorted --mode=lint` on a single batch of files.""" + async with semaphore: + # --mode=lint checks without modifying files and emits JSON findings on stdout. + proc = await _run_process( + [str(binary), "--mode=lint", "--", *batch], + cwd=repo_root, + ) + if proc.returncode == 0: + return [] + stdout = proc.stdout.strip() + if not stdout: + raise RuntimeError(_format_subprocess_error("keep-sorted failed", proc)) + try: + findings = json.loads(stdout) + except json.JSONDecodeError as err: + raise RuntimeError( + _format_subprocess_error("Failed to parse keep-sorted JSON output", proc) + ) from err + if not isinstance(findings, list) or not findings: + raise RuntimeError( + _format_subprocess_error("Unexpected keep-sorted output", proc) + ) + batch_failures: list[str] = [] + for item in findings: + if not isinstance(item, dict): + raise RuntimeError(f"Unexpected finding entry: {item!r}") + batch_failures.append(_format_finding(item)) + return batch_failures + + +async def lint_files(binary: Path, repo_root: Path, files: Sequence[str]) -> list[str]: + """Runs `keep-sorted --mode=lint` concurrently across bounded batches. + + Args: + binary: Path to the `keep-sorted` executable. + repo_root: Repository root directory from which relative `files` resolve. + files: File paths relative to `repo_root`. + + Returns: + Unsorted list of formatted failure strings (empty when all files pass). + """ + semaphore = asyncio.Semaphore(_MAX_CONCURRENCY) + batches = [ + files[offset : offset + FILES_PER_BATCH] + for offset in range(0, len(files), FILES_PER_BATCH) + ] + results = await asyncio.gather( + *(_lint_batch(binary, repo_root, batch, semaphore) for batch in batches) + ) + failures: list[str] = [] + for batch_result in results: + failures.extend(batch_result) + return failures + + +async def _repo_root() -> Path: + """Returns the git repository root directory.""" + proc = await _run_process(["git", "rev-parse", "--show-toplevel"]) + if proc.returncode != 0: + raise RuntimeError(_format_subprocess_error("git rev-parse failed", proc)) + return Path(proc.stdout.strip()) + + +async def async_main(argv: list[str]) -> int: + """Async CLI entry point.""" + binary = _parse_args(argv).resolve() + repo_root = await _repo_root() + files = await list_tracked_files(repo_root) + failures = await lint_files(binary, repo_root, files) + if not failures: + print(f"Checked {len(files)} tracked files: all keep-sorted blocks sorted.") + return 0 + + for line in sorted(failures): + print(line, file=sys.stderr) + return 1 + + +def main(argv: list[str]) -> int: + """Synchronous CLI entry point.""" + return asyncio.run(async_main(argv)) + + +if __name__ == "__main__": + sys.exit(main(sys.argv[1:]))
diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 48bb5a1..b5a547a 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml
@@ -31,6 +31,25 @@ version: 0.15.14 args: format --check --exclude testdata + keep_sorted: + name: keep-sorted + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v7 + - name: Download keep-sorted + id: keep-sorted + uses: ./.github/actions/download-tool + with: + name: keep-sorted + # Keep in sync with .pre-commit-config.yaml + urls: | + https://github.com/google/keep-sorted/releases/download/v0.10.0/keep-sorted_linux_amd64 + sha256: 079710c01619cc3dc11c9ddce18ac8db8d86701e7d12f9c5b62b8c4c4a664cca + - name: Check keep-sorted + env: + KEEP_SORTED_BIN: ${{ steps.keep-sorted.outputs.path }} + run: ./.github/workflows/check_keep_sorted.py -- "$KEEP_SORTED_BIN" + build_release_artifacts: name: Build release artifacts uses: ./.github/workflows/release_build_artifacts.yaml
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index a2ff683..cd11aa5 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml
@@ -20,6 +20,11 @@ rev: v5.0.0 # Use the ref you want to point at hooks: - id: check-merge-conflict + - repo: https://github.com/google/keep-sorted + # Keep in sync with .github/workflows/ci.yaml + rev: v0.10.0 + hooks: + - id: keep-sorted - repo: https://github.com/keith/pre-commit-buildifier rev: 8.2.1 hooks:
diff --git a/BUILD.bazel b/BUILD.bazel index 9f57081..1af3bc6 100644 --- a/BUILD.bazel +++ b/BUILD.bazel
@@ -63,6 +63,7 @@ [ ".bazelrc", ".bazelversion", + ".pre-commit-config.yaml", "MODULE.bazel", "buildbuddy.bazelrc", ],
diff --git a/MODULE.bazel b/MODULE.bazel index 30fbc28..aa1f4eb 100644 --- a/MODULE.bazel +++ b/MODULE.bazel
@@ -111,17 +111,16 @@ dev_dependency = TOOL_DEPS_DEV_ONLY, ) -# keep-sorted start block=yes by_regex=package\s*=\s*"([^"]+)" newline_separated=yes +# keep-sorted start block=yes by_regex=package\s*=\s*"([^"]+)" rust_crates.spec( package = "anyhow", version = "1.0.104", ) - -# Not used directly; it's a transitive dep of zip. zip's `deflate-flate2` -# feature enables flate2 without selecting a backend, so a top-level spec is -# needed to pick the pure-Rust `rust_backend` and avoid the C zlib backend, -# which can't cross-compile for dev/platforms. rust_crates.spec( + # Not used directly; it's a transitive dep of zip. zip's `deflate-flate2` + # feature enables flate2 without selecting a backend, so a top-level spec is + # needed to pick the pure-Rust `rust_backend` and avoid the C zlib backend, + # which can't cross-compile for dev/platforms. default_features = False, features = ["rust_backend"], package = "flate2", @@ -135,12 +134,11 @@ package = "sha2", version = "0.10.8", ) - -# NOTE: zip 9.x added mandatory target-conditional OS deps (getrandom, nix, -# windows-sys) that fail on powerpc64le-unknown-linux-musl until rules_rust -# defines a powerpc64le @rules_rust//rust/platform config_setting. Keep on the -# latest 8.x release until rules_rust supports powerpc64le platform selection. rust_crates.spec( + # NOTE: zip 9.x added mandatory target-conditional OS deps (getrandom, nix, + # windows-sys) that fail on powerpc64le-unknown-linux-musl until rules_rust + # defines a powerpc64le @rules_rust//rust/platform config_setting. Keep on the + # latest 8.x release until rules_rust supports powerpc64le platform selection. default_features = False, features = ["deflate-flate2"], package = "zip",
diff --git a/dev/BUILD.bazel b/dev/BUILD.bazel index 056dfa4..a5e3661 100644 --- a/dev/BUILD.bazel +++ b/dev/BUILD.bazel
@@ -4,7 +4,10 @@ licenses(["notice"]) exports_files( - ["download_bazel.py"], + [ + "download_bazel.py", + "verified_download.py", + ], visibility = ["//:__subpackages__"], )
diff --git a/dev/download_bazel.py b/dev/download_bazel.py index f64ce1d..856d263 100755 --- a/dev/download_bazel.py +++ b/dev/download_bazel.py
@@ -22,50 +22,50 @@ from __future__ import annotations import asyncio -import hashlib -import io import os import platform import shutil import stat import sys -import tarfile -import urllib.request from pathlib import Path -DOWNLOAD_TIMEOUT_SECONDS = 120 -MAX_ATTEMPTS = 5 -INITIAL_DELAY_SECONDS = 2.0 +import verified_download -# Maps "<os>-<arch>" to a list of (url, sha256) tuples tried in order. +DOWNLOAD_TIMEOUT_SECONDS = verified_download.DEFAULT_TIMEOUT_SECONDS +MAX_ATTEMPTS = verified_download.DEFAULT_MAX_ATTEMPTS +INITIAL_DELAY_SECONDS = verified_download.DEFAULT_INITIAL_DELAY_SECONDS + +# Maps "<os>-<arch>" to (url, sha256, archive_member) tuples tried in order. # Primary URLs use v1.30.0 (latest GitHub release); the npm mirror fallback uses # 1.28.1 (latest version published to the npm registry). # fmt: off -BAZELISK_URLS: dict[str, list[tuple[str, str]]] = { +BAZELISK_URLS: dict[str, list[tuple[str, str, str]]] = { + # keep-sorted start block=yes "darwin-amd64": [ - ("https://github.com/bazelbuild/bazelisk/releases/download/v1.30.0/bazelisk-darwin-amd64", "a0e6468b0c6b812f782d8cbfb05824121f6980c0bd70aaa1774fb8272bb7b2ad"), - ("https://registry.npmjs.org/@bazel/bazelisk/-/bazelisk-1.28.1.tgz", "023225736cea5dc88f2b0807d5b1af4eb0f69a4ed45e3994b2c18c263bc80e48"), + ("https://github.com/bazelbuild/bazelisk/releases/download/v1.30.0/bazelisk-darwin-amd64", "a0e6468b0c6b812f782d8cbfb05824121f6980c0bd70aaa1774fb8272bb7b2ad", ""), + ("https://registry.npmjs.org/@bazel/bazelisk/-/bazelisk-1.28.1.tgz", "023225736cea5dc88f2b0807d5b1af4eb0f69a4ed45e3994b2c18c263bc80e48", "package/bazelisk-darwin_amd64"), ], "darwin-arm64": [ - ("https://github.com/bazelbuild/bazelisk/releases/download/v1.30.0/bazelisk-darwin-arm64", "555d2ff912ccd336d0060bdb5251ef2f62629c6f8fbadb59005a45d5625793ec"), - ("https://registry.npmjs.org/@bazel/bazelisk/-/bazelisk-1.28.1.tgz", "dea3f3f5de2dbc5e269e0132cdd369d5efe738f7b973d5d4eb2b4f7055a97b39"), + ("https://github.com/bazelbuild/bazelisk/releases/download/v1.30.0/bazelisk-darwin-arm64", "555d2ff912ccd336d0060bdb5251ef2f62629c6f8fbadb59005a45d5625793ec", ""), + ("https://registry.npmjs.org/@bazel/bazelisk/-/bazelisk-1.28.1.tgz", "dea3f3f5de2dbc5e269e0132cdd369d5efe738f7b973d5d4eb2b4f7055a97b39", "package/bazelisk-darwin_arm64"), ], "linux-amd64": [ - ("https://github.com/bazelbuild/bazelisk/releases/download/v1.30.0/bazelisk-linux-amd64", "a248ae2b2b62b5baebb748f6fba7203bff1a27e5d01b55399a8406744cb539e8"), - ("https://registry.npmjs.org/@bazel/bazelisk/-/bazelisk-1.28.1.tgz", "22e7d3a188699982f661cf4687137ee52d1f24fec1ec893d91a6c4d791a75de8"), + ("https://github.com/bazelbuild/bazelisk/releases/download/v1.30.0/bazelisk-linux-amd64", "a248ae2b2b62b5baebb748f6fba7203bff1a27e5d01b55399a8406744cb539e8", ""), + ("https://registry.npmjs.org/@bazel/bazelisk/-/bazelisk-1.28.1.tgz", "22e7d3a188699982f661cf4687137ee52d1f24fec1ec893d91a6c4d791a75de8", "package/bazelisk-linux_amd64"), ], "linux-arm64": [ - ("https://github.com/bazelbuild/bazelisk/releases/download/v1.30.0/bazelisk-linux-arm64", "b2bcc040c213b44586b121bda6d983c35df121b917c3a7a66fb16f2fa0df1a4f"), - ("https://registry.npmjs.org/@bazel/bazelisk/-/bazelisk-1.28.1.tgz", "8ded44b58a0d9425a4178af26cf17693feac3b87bdcfef0a2a0898fcd1afc9f2"), + ("https://github.com/bazelbuild/bazelisk/releases/download/v1.30.0/bazelisk-linux-arm64", "b2bcc040c213b44586b121bda6d983c35df121b917c3a7a66fb16f2fa0df1a4f", ""), + ("https://registry.npmjs.org/@bazel/bazelisk/-/bazelisk-1.28.1.tgz", "8ded44b58a0d9425a4178af26cf17693feac3b87bdcfef0a2a0898fcd1afc9f2", "package/bazelisk-linux_arm64"), ], "windows-amd64": [ - ("https://github.com/bazelbuild/bazelisk/releases/download/v1.30.0/bazelisk-windows-amd64.exe", "5d14a31e9d8a363c48ba1900dbd56764d730835232eb0d113cce255856ffe3ef"), - ("https://registry.npmjs.org/@bazel/bazelisk/-/bazelisk-1.28.1.tgz", "b9d65a1f7c2d7af885a96a4fd5aa36b40fb41816d30944390569eef908bdc954"), + ("https://github.com/bazelbuild/bazelisk/releases/download/v1.30.0/bazelisk-windows-amd64.exe", "5d14a31e9d8a363c48ba1900dbd56764d730835232eb0d113cce255856ffe3ef", ""), + ("https://registry.npmjs.org/@bazel/bazelisk/-/bazelisk-1.28.1.tgz", "b9d65a1f7c2d7af885a96a4fd5aa36b40fb41816d30944390569eef908bdc954", "package/bazelisk-windows_amd64.exe"), ], "windows-arm64": [ - ("https://github.com/bazelbuild/bazelisk/releases/download/v1.30.0/bazelisk-windows-arm64.exe", "cb832b1d157e0dc5f89108da426f9c6fe087f5b15873e072480ec49cc5afe488"), - ("https://registry.npmjs.org/@bazel/bazelisk/-/bazelisk-1.28.1.tgz", "85ba3d92a8bdcbecc06657b8c0ae30f4307b552d601d9d6246f8a98aec36c346"), + ("https://github.com/bazelbuild/bazelisk/releases/download/v1.30.0/bazelisk-windows-arm64.exe", "cb832b1d157e0dc5f89108da426f9c6fe087f5b15873e072480ec49cc5afe488", ""), + ("https://registry.npmjs.org/@bazel/bazelisk/-/bazelisk-1.28.1.tgz", "85ba3d92a8bdcbecc06657b8c0ae30f4307b552d601d9d6246f8a98aec36c346", "package/bazelisk-windows_arm64.exe"), ], + # keep-sorted end } # fmt: on @@ -90,17 +90,6 @@ raise RuntimeError(f"Unsupported architecture: {raw}") -def _verify_sha256_bytes(data: bytes, expected: str) -> bool: - actual = hashlib.sha256(data).hexdigest().lower() - if actual != expected.lower(): - print( - f"::warning::SHA-256 mismatch: expected {expected.lower()}, got {actual}", - flush=True, - ) - return False - return True - - def _bazelisk_cache_dir(os_name: str) -> Path: bazelisk_home = os.environ.get("BAZELISK_HOME") if bazelisk_home: @@ -142,41 +131,6 @@ ) -def _read_url(url: str) -> bytes: - with urllib.request.urlopen(url, timeout=DOWNLOAD_TIMEOUT_SECONDS) as resp: - return resp.read() - - -async def _fetch_bazelisk( - url: str, os_name: str, arch_name: str, ext: str -) -> bytes | None: - try: - data = await asyncio.to_thread(_read_url, url) - if url.endswith(".tgz"): - member_name = f"package/bazelisk-{os_name}_{arch_name}{ext}" - with tarfile.open(fileobj=io.BytesIO(data), mode="r:gz") as tar: - member = tar.extractfile(member_name) - if member is None: - return None - return member.read() - return data - except Exception as exc: # noqa: BLE001 - print(f"::warning::Failed to fetch {url}: {exc}", flush=True) - return None - - -def _install_bazelisk_binary( - data: bytes, bin_dir: Path, bazel_bin: Path, bazelisk_bin: Path -) -> None: - bin_dir.mkdir(parents=True, exist_ok=True) - bazel_bin.write_bytes(data) - exec_bits = stat.S_IXUSR | stat.S_IXGRP | stat.S_IXOTH - bazel_bin.chmod(bazel_bin.stat().st_mode | exec_bits) - shutil.copy2(bazel_bin, bazelisk_bin) - bazelisk_bin.chmod(bazelisk_bin.stat().st_mode | exec_bits) - _add_to_path(bin_dir) - - async def _setup_bazelisk( max_attempts: int = MAX_ATTEMPTS, initial_delay: float = INITIAL_DELAY_SECONDS, @@ -193,47 +147,38 @@ bazel_bin = bin_dir / f"bazel{ext}" bazelisk_bin = bin_dir / f"bazelisk{ext}" - if bazel_bin.is_file(): - actual_sha = hashlib.sha256(bazel_bin.read_bytes()).hexdigest().lower() - if any(actual_sha == sha256.lower() for _, sha256 in sources): - exec_bits = stat.S_IXUSR | stat.S_IXGRP | stat.S_IXOTH - bazel_bin.chmod(bazel_bin.stat().st_mode | exec_bits) - if not bazelisk_bin.is_file(): - shutil.copy2(bazel_bin, bazelisk_bin) - bazelisk_bin.chmod(bazelisk_bin.stat().st_mode | exec_bits) - _add_to_path(bin_dir) - return + try: + await verified_download.download_verified_file( + bazel_bin, + sources, + executable=True, + timeout=DOWNLOAD_TIMEOUT_SECONDS, + max_attempts=max_attempts, + initial_delay=initial_delay, + ) + except verified_download.DownloadError: + print( + f"::error::Failed to download Bazelisk after {max_attempts} attempts", + flush=True, + ) + _clean_bazelisk_cache() + sys.exit(1) - delay = initial_delay - for attempt in range(1, max_attempts + 1): - for url, expected_sha in sources: - data = await _fetch_bazelisk(url, os_name, arch_name, ext) - if data is not None and _verify_sha256_bytes(data, expected_sha): - _install_bazelisk_binary(data, bin_dir, bazel_bin, bazelisk_bin) - return - - if attempt < max_attempts: - print( - f"::warning::Failed to download Bazelisk " - f"(attempt {attempt}/{max_attempts}); retrying in {delay:g}s...", - flush=True, - ) - await asyncio.sleep(delay) - delay *= 2 - - print( - f"::error::Failed to download Bazelisk after {max_attempts} attempts", - flush=True, - ) - _clean_bazelisk_cache() - sys.exit(1) + bazel_sha = verified_download.sha256_bytes(bazel_bin.read_bytes()) + if ( + not bazelisk_bin.is_file() + or verified_download.sha256_bytes(bazelisk_bin.read_bytes()) != bazel_sha + ): + shutil.copy2(bazel_bin, bazelisk_bin) + exec_bits = stat.S_IXUSR | stat.S_IXGRP | stat.S_IXOTH + bazelisk_bin.chmod(bazelisk_bin.stat().st_mode | exec_bits) + _add_to_path(bin_dir) async def _run_bazel_version() -> bool: bazel_exe = shutil.which("bazel", path=os.environ.get("PATH")) or "bazel" proc = await asyncio.create_subprocess_exec(bazel_exe, "--version") - returncode = await proc.wait() - return returncode == 0 + return (await proc.wait()) == 0 async def _download_bazel(
diff --git a/dev/verified_download.py b/dev/verified_download.py new file mode 100755 index 0000000..d7c25c5 --- /dev/null +++ b/dev/verified_download.py
@@ -0,0 +1,294 @@ +#!/usr/bin/env python3 +"""Downloads files with retries, mirrors, and SHA-256 checks. + +Usage: + verified_download.py \\ + --urls https://primary.example.com/tool,https://mirror.example.com/tool \\ + --sha256 <hex-digest> \\ + --executable \\ + --output /path/to/tool + +Input format for `--urls`: + Comma- or newline-delimited list of candidate URLs tried in order per attempt. +""" + +from __future__ import annotations + +import argparse +import asyncio +import hashlib +import io +import stat +import sys +import tarfile +import urllib.request +import zipfile +from collections.abc import Sequence +from pathlib import Path + +# Allow up to 120s per HTTP request before treating the connection as stalled. +DEFAULT_TIMEOUT_SECONDS = 120.0 +# Retry up to 5 times (2s, 4s, 8s, 16s) to ride out transient 5xx/reset flakes. +DEFAULT_MAX_ATTEMPTS = 5 +DEFAULT_INITIAL_DELAY_SECONDS = 2.0 + +_USER_AGENT = "rules_python-verified-download/1.0" +_EXEC_BITS = stat.S_IXUSR | stat.S_IXGRP | stat.S_IXOTH + + +def _parse_args(argv: list[str]) -> argparse.Namespace: + """Parses CLI flags for single-file verified downloads.""" + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument( + "--urls", + required=True, + help=( + "Comma- or newline-delimited list of candidate URLs to download, " + "tried in order." + ), + ) + parser.add_argument( + "--sha256", + required=True, + help="Expected lowercase or uppercase hex SHA-256 digest.", + ) + parser.add_argument( + "--output", + type=Path, + required=True, + help="Destination file path.", + ) + parser.add_argument( + "--executable", + action="store_true", + help="Set user/group/other executable permission bits (+x) on output.", + ) + return parser.parse_args(argv) + + +def parse_url_list(raw_urls: str) -> list[str]: + """Extracts ordered non-empty URLs from comma- or newline-delimited text.""" + urls: list[str] = [] + for line in raw_urls.splitlines(): + for part in line.split(","): + url = part.strip() + if url: + urls.append(url) + if not urls: + raise ValueError("At least one URL must be provided") + return urls + + +def sha256_bytes(data: bytes) -> str: + """Returns the lowercase hex SHA-256 digest of `data`.""" + return hashlib.sha256(data).hexdigest().lower() + + +def verify_sha256(data: bytes, expected_sha256: str) -> bool: + """Checks `data` against `expected_sha256`, logging a CI warning on mismatch. + + Args: + data: Raw payload bytes to hash. + expected_sha256: Expected hex SHA-256 digest. + + Returns: + True when the digest matches, False otherwise. + """ + actual = sha256_bytes(data) + expected = expected_sha256.lower() + if actual != expected: + print( + f"::warning::SHA-256 mismatch: expected {expected}, got {actual}", + flush=True, + ) + return False + return True + + +def _set_executable(path: Path) -> None: + """Sets `+x` permission bits (`0o111`) on `path` while keeping existing bits.""" + path.chmod(path.stat().st_mode | _EXEC_BITS) + + +def _extract_archive_member(data: bytes, url: str, archive_member: str) -> bytes | None: + """Extracts `archive_member` from a zip or tar archive payload.""" + if zipfile.is_zipfile(io.BytesIO(data)): + with zipfile.ZipFile(io.BytesIO(data)) as zf: + if archive_member not in zf.namelist(): + print( + f"::warning::Member {archive_member!r} not found in {url}", + flush=True, + ) + return None + return zf.read(archive_member) + + with tarfile.open(fileobj=io.BytesIO(data), mode="r:*") as tar: + member = tar.extractfile(archive_member) + if member is None: + print( + f"::warning::Member {archive_member!r} not found in {url}", + flush=True, + ) + return None + return member.read() + + +def _read_source_bytes(source: tuple[str, str, str], timeout: float) -> bytes | None: + """Blocking worker that fetches `url` and extracts `archive_member`.""" + url, _, archive_member = source + req = urllib.request.Request(url, headers={"User-Agent": _USER_AGENT}) + with urllib.request.urlopen(req, timeout=timeout) as response: + data = response.read() + if archive_member: + return _extract_archive_member(data, url, archive_member) + return data + + +async def _fetch_source(source: tuple[str, str, str], timeout: float) -> bytes | None: + """Fetches `source` and extracts its optional archive member. + + Args: + source: `(url, sha256, archive_member)` tuple. + timeout: Per-request deadline in seconds. + + Returns: + Payload bytes on success, or `None` if fetching or extraction fails. + """ + url = source[0] + try: + return await asyncio.wait_for( + asyncio.to_thread(_read_source_bytes, source, timeout), + timeout=timeout, + ) + except Exception as exc: # Retry transient network, HTTP, and archive errors. + print(f"::warning::Failed to fetch {url}: {exc}", flush=True) + return None + + +async def download_verified_bytes( + sources: Sequence[tuple[str, str, str]], + *, + timeout: float = DEFAULT_TIMEOUT_SECONDS, + max_attempts: int = DEFAULT_MAX_ATTEMPTS, + initial_delay: float = DEFAULT_INITIAL_DELAY_SECONDS, +) -> bytes: + """Downloads and SHA-256-verifies bytes with retries. + + Args: + sources: Ordered `(url, sha256, archive_member)` tuples tried on each + attempt until one succeeds and matches its SHA-256 digest. Pass `""` + for `archive_member` when `url` serves the raw file directly. + timeout: Per-request deadline in seconds. + max_attempts: Maximum outer retry attempts across `sources`. + initial_delay: Initial backoff delay in seconds (doubled each retry). + + Returns: + Verified payload bytes from the first matching source. + + Raises: + ValueError: If `sources` is empty. + DownloadError: If all retry attempts fail to produce verified bytes. + """ + if not sources: + raise ValueError("sources must not be empty") + + delay = initial_delay + for attempt in range(1, max_attempts + 1): + for source in sources: + _, expected_sha256, _ = source + data = await _fetch_source(source, timeout) + if data is not None and verify_sha256(data, expected_sha256): + return data + + if attempt < max_attempts: + print( + f"::warning::Download failed (attempt {attempt}/{max_attempts}); " + f"retrying in {delay:g}s...", + flush=True, + ) + await asyncio.sleep(delay) + delay *= 2 + + raise DownloadError(f"Failed to download after {max_attempts} attempts") + + +def _reuse_cached_file(dest: Path, expected_shas: set[str], executable: bool) -> bool: + """Returns True if `dest` exists and matches any digest in `expected_shas`.""" + if not dest.is_file(): + return False + if sha256_bytes(dest.read_bytes()) not in expected_shas: + return False + if executable: + _set_executable(dest) + return True + + +def _write_output_file(dest: Path, data: bytes, executable: bool) -> None: + """Writes verified `data` to `dest` and optionally marks it executable.""" + dest.parent.mkdir(parents=True, exist_ok=True) + dest.write_bytes(data) + if executable: + _set_executable(dest) + + +async def download_verified_file( + dest: Path, + sources: Sequence[tuple[str, str, str]], + *, + executable: bool = False, + timeout: float = DEFAULT_TIMEOUT_SECONDS, + max_attempts: int = DEFAULT_MAX_ATTEMPTS, + initial_delay: float = DEFAULT_INITIAL_DELAY_SECONDS, +) -> None: + """Ensures `dest` contains SHA-256-verified bytes. + + Reuses `dest` without network requests if `dest` already exists and its + SHA-256 matches any entry in `sources`. + + Args: + dest: Destination file path to verify or write. + sources: Ordered `(url, sha256, archive_member)` tuples. Pass `""` for + `archive_member` when `url` serves the raw file directly. + executable: Whether to mark `dest` executable (`+x`). + timeout: Per-request deadline in seconds. + max_attempts: Maximum outer retry attempts across `sources`. + initial_delay: Initial backoff delay in seconds (doubled each retry). + """ + expected_shas = {sha256.lower() for _, sha256, _ in sources} + if await asyncio.to_thread(_reuse_cached_file, dest, expected_shas, executable): + return + + data = await download_verified_bytes( + sources, + timeout=timeout, + max_attempts=max_attempts, + initial_delay=initial_delay, + ) + await asyncio.to_thread(_write_output_file, dest, data, executable) + + +class DownloadError(Exception): + """Raised when all candidate sources and retry attempts fail verification.""" + + +async def _async_main(argv: list[str]) -> int: + """Async CLI entry point.""" + args = _parse_args(argv) + urls = parse_url_list(args.urls) + sources = [(url, args.sha256, "") for url in urls] + await download_verified_file( + args.output, + sources, + executable=args.executable, + ) + print(f"Downloaded and verified {args.output}") + return 0 + + +def main(argv: list[str]) -> int: + """Synchronous CLI entry point.""" + return asyncio.run(_async_main(argv)) + + +if __name__ == "__main__": + sys.exit(main(sys.argv[1:]))
diff --git a/python/private/common.bzl b/python/private/common.bzl index cbea882..2d4c48d 100644 --- a/python/private/common.bzl +++ b/python/private/common.bzl
@@ -145,9 +145,10 @@ A "BinarySemantics" struct. """ return struct( - # keep-sorted + # keep-sorted start get_native_deps_dso_name = get_native_deps_dso_name, should_build_native_deps_dso = should_build_native_deps_dso, + # keep-sorted end ) def create_cc_details_struct(
diff --git a/tests/support/py_runtime_info_subject.bzl b/tests/support/py_runtime_info_subject.bzl index 0c6c672..06fd701 100644 --- a/tests/support/py_runtime_info_subject.bzl +++ b/tests/support/py_runtime_info_subject.bzl
@@ -30,7 +30,7 @@ # buildifier: disable=uninitialized public = struct( - # go/keep-sorted start + # go/keep-sorted start block=yes actual = info, bootstrap_template = lambda *a, **k: _py_runtime_info_subject_bootstrap_template(self, *a, **k), coverage_files = lambda *a, **k: _py_runtime_info_subject_coverage_files(self, *a, **k),
diff --git a/tests/workflows/BUILD.bazel b/tests/workflows/BUILD.bazel index 0b1a539..fdb6d0f 100644 --- a/tests/workflows/BUILD.bazel +++ b/tests/workflows/BUILD.bazel
@@ -3,6 +3,52 @@ load("//tests/support:support.bzl", "NOT_WINDOWS") py_library( + name = "verified_download", + testonly = True, + srcs = ["//dev:verified_download.py"], + imports = ["../../dev"], + target_compatible_with = NOT_WINDOWS, +) + +pytest_test( + name = "verified_download_test", + srcs = ["verified_download_test.py"], + data = [ + "//.github/actions/download-tool:all_files", + "//dev:verified_download.py", + ], + target_compatible_with = NOT_WINDOWS, + deps = [ + ":verified_download", + "//python/runfiles", + "@pypi//pyyaml", + ], +) + +py_library( + name = "check_keep_sorted", + testonly = True, + srcs = ["//.github/workflows:check_keep_sorted.py"], + imports = ["../../.github/workflows"], + target_compatible_with = NOT_WINDOWS, +) + +pytest_test( + name = "check_keep_sorted_test", + srcs = ["check_keep_sorted_test.py"], + data = [ + "//:.pre-commit-config.yaml", + "//.github/workflows:all_files", + ], + target_compatible_with = NOT_WINDOWS, + deps = [ + ":check_keep_sorted", + "//python/runfiles", + "@pypi//pyyaml", + ], +) + +py_library( name = "on_comment", testonly = True, srcs = ["//.github/workflows:on_comment.py"], @@ -86,6 +132,7 @@ srcs = ["//dev:download_bazel.py"], imports = ["../../dev"], target_compatible_with = NOT_WINDOWS, + deps = [":verified_download"], ) pytest_test( @@ -96,6 +143,7 @@ "//:buildbuddy.bazelrc", "//.github/actions/setup-bazel:all_files", "//dev:download_bazel.py", + "//dev:verified_download.py", ], target_compatible_with = NOT_WINDOWS, deps = [
diff --git a/tests/workflows/check_keep_sorted_test.py b/tests/workflows/check_keep_sorted_test.py new file mode 100644 index 0000000..a55da46 --- /dev/null +++ b/tests/workflows/check_keep_sorted_test.py
@@ -0,0 +1,179 @@ +"""Tests for .github/workflows/check_keep_sorted.py and CI configuration.""" + +from __future__ import annotations + +import asyncio +import contextlib +import json +import os +import subprocess +from collections.abc import Iterator +from pathlib import Path + +import check_keep_sorted +import pytest +import yaml + +from python.runfiles import runfiles + + +def _repo_file(rel_path: str) -> Path: + return runfiles.CreateOrRaise().root() / f"rules_python/{rel_path}" + + +@contextlib.contextmanager +def _chdir(target: Path) -> Iterator[None]: + prev = Path.cwd() + os.chdir(target) + try: + yield + finally: + os.chdir(prev) + + +def _write_fake_keep_sorted(path: Path, body: str) -> Path: + path.write_text(f"#!/usr/bin/env python3\n{body}\n", encoding="utf-8") + path.chmod(0o755) + return path + + +def test_ci_workflow_uses_download_tool_in_sync_with_pre_commit() -> None: + pre_commit = yaml.safe_load( + _repo_file(".pre-commit-config.yaml").read_text(encoding="utf-8") + ) + repos = pre_commit["repos"] + matching = [ + repo + for repo in repos + if repo.get("repo") == "https://github.com/google/keep-sorted" + ] + assert len(matching) == 1 + entry = matching[0] + rev = entry["rev"] + hook_ids = [hook["id"] for hook in entry["hooks"]] + assert hook_ids == ["keep-sorted"] + + ci = yaml.safe_load( + _repo_file(".github/workflows/ci.yaml").read_text(encoding="utf-8") + ) + job = ci["jobs"]["keep_sorted"] + assert job["runs-on"] == "ubuntu-latest" + steps = job["steps"] + + download_step = next(s for s in steps if s.get("id") == "keep-sorted") + assert download_step["uses"] == "./.github/actions/download-tool" + assert download_step["with"]["name"] == "keep-sorted" + urls = [ + line.strip() + for line in download_step["with"]["urls"].splitlines() + if line.strip() + ] + assert urls == [ + f"https://github.com/google/keep-sorted/releases/download/{rev}/keep-sorted_linux_amd64" + ] + assert len(download_step["with"]["sha256"]) == 64 + + check_step = next(s for s in steps if s.get("name") == "Check keep-sorted") + assert check_step["env"] == { + "KEEP_SORTED_BIN": "${{ steps.keep-sorted.outputs.path }}" + } + assert ( + check_step["run"] + == './.github/workflows/check_keep_sorted.py -- "$KEEP_SORTED_BIN"' + ) + + +def test_lint_files_formats_findings_across_concurrent_batches( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + monkeypatch.setattr(check_keep_sorted, "FILES_PER_BATCH", 2) + invocations_dir = tmp_path / "invocations" + invocations_dir.mkdir() + binary = _write_fake_keep_sorted( + tmp_path / "keep-sorted", + f""" +import json, os, pathlib, sys +args = sys.argv[1:] +assert args[:2] == ["--mode=lint", "--"], args +files = args[2:] +(pathlib.Path({str(invocations_dir)!r}) / str(os.getpid())).write_text("\\n".join(files)) +findings = [] +if "b.py" in files: + findings.append({{"path": "b.py", "lines": {{"start": 10, "end": 14}}, "message": "These lines are out of order."}}) +if "a.bzl" in files: + findings.append({{"path": "a.bzl", "lines": {{"start": 5, "end": 5}}, "message": "Unmatched directive."}}) +if findings: + print(json.dumps(findings)) + sys.exit(1) +""", + ) + + failures = asyncio.run( + check_keep_sorted.lint_files(binary, tmp_path, ["b.py", "c.py", "a.bzl"]) + ) + assert len(list(invocations_dir.iterdir())) == 2 + assert failures == [ + "b.py:10-14: These lines are out of order.", + "a.bzl:5: Unmatched directive.", + ] + + +def test_lint_files_fails_closed_on_non_json_error(tmp_path: Path) -> None: + binary = _write_fake_keep_sorted( + tmp_path / "keep-sorted", + """ +import sys +sys.stderr.write("fatal flag error\\n") +sys.exit(2) +""", + ) + + with pytest.raises(RuntimeError, match="fatal flag error"): + asyncio.run(check_keep_sorted.lint_files(binary, tmp_path, ["a.py"])) + + +def test_end_to_end_git_tracked_files_success_and_sorted_failures( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + repo = tmp_path / "repo" + repo.mkdir() + (repo / "z.py").write_text("z\n", encoding="utf-8") + (repo / "a.py").write_text("a\n", encoding="utf-8") + (repo / "link.py").symlink_to("a.py") + subprocess.run(["git", "init", "--quiet"], cwd=repo, check=True) + subprocess.run(["git", "add", "a.py", "z.py", "link.py"], cwd=repo, check=True) + + ok_bin = _write_fake_keep_sorted( + tmp_path / "keep-sorted-ok", + """ +import sys +# Symlinks must be filtered out before invoking keep-sorted. +assert "link.py" not in sys.argv +sys.exit(0) +""", + ) + fail_bin = _write_fake_keep_sorted( + tmp_path / "keep-sorted-fail", + f""" +import json, sys +print(json.dumps([ + {json.dumps({"path": "z.py", "lines": {"start": 2, "end": 4}, "message": "These lines are out of order."})}, + {json.dumps({"path": "a.py", "lines": {"start": 9, "end": 9}, "message": "Unmatched directive."})}, +])) +sys.exit(1) +""", + ) + + with _chdir(repo): + assert check_keep_sorted.main([str(ok_bin)]) == 0 + captured = capsys.readouterr() + assert "Checked 2 tracked files" in captured.out + assert captured.err == "" + + assert check_keep_sorted.main([str(fail_bin)]) == 1 + captured = capsys.readouterr() + assert captured.out == "" + assert captured.err.splitlines() == [ + "a.py:9: Unmatched directive.", + "z.py:2-4: These lines are out of order.", + ]
diff --git a/tests/workflows/setup_bazel_test.py b/tests/workflows/setup_bazel_test.py index 4b1ff6b..20e017c 100644 --- a/tests/workflows/setup_bazel_test.py +++ b/tests/workflows/setup_bazel_test.py
@@ -5,7 +5,10 @@ import asyncio import hashlib import os +import shutil import stat +import subprocess +import sys import tarfile from pathlib import Path @@ -34,8 +37,8 @@ return hashlib.sha256(path.read_bytes()).hexdigest() -def test_action_yaml_invokes_executable_download_script(): - """Verifies setup-bazel action.yaml runs download_bazel.py and omits bazelisk-version.""" +def test_action_yaml_invokes_executable_download_script(tmp_path: Path): + """Verifies setup-bazel action.yaml runs dev/download_bazel.py and omits bazelisk-version.""" rf = runfiles.CreateOrRaise() action_dir = _action_dir() script = rf.root() / "rules_python/dev/download_bazel.py" @@ -50,6 +53,24 @@ setup_step = next(s for s in steps if s.get("name") == "Setup Bazel") assert "bazelisk-version" not in setup_step["with"] + # Stage dev/ scripts outside Bazel runfiles and verify dev/download_bazel.py + # resolves sibling dev/verified_download.py with an empty environment (as in + # Read the Docs and GitHub Actions). + staged_dev = tmp_path / "dev" + staged_dev.mkdir(parents=True) + staged_script = staged_dev / "download_bazel.py" + shutil.copy2(script, staged_script) + shutil.copy2( + rf.root() / "rules_python/dev/verified_download.py", + staged_dev / "verified_download.py", + ) + subprocess.run( + [sys.executable, "-B", "-m", "doctest", "download_bazel.py"], + check=True, + cwd=staged_dev, + env={}, + ) + def test_action_yaml_enables_buildbuddy_on_all_os(): """Verifies setup-bazel action.yaml enables BuildBuddy across all OSes.""" @@ -174,7 +195,7 @@ monkeypatch.setitem( download_bazel.BAZELISK_URLS, "linux-amd64", - [(asset.as_uri(), _sha256_file(asset))], + [(asset.as_uri(), _sha256_file(asset), "")], ) asyncio.run(download_bazel.main()) @@ -218,7 +239,10 @@ monkeypatch.setitem( download_bazel.BAZELISK_URLS, "linux-amd64", - [(missing_url, sha), (tgz_path.as_uri(), sha)], + [ + (missing_url, sha, ""), + (tgz_path.as_uri(), sha, "package/bazelisk-linux_amd64"), + ], ) asyncio.run(download_bazel.main()) @@ -251,7 +275,7 @@ monkeypatch.setitem( download_bazel.BAZELISK_URLS, "linux-amd64", - [((tmp_path / "missing").as_uri(), _sha256_file(cached_bazel))], + [((tmp_path / "missing").as_uri(), _sha256_file(cached_bazel), "")], ) asyncio.run(download_bazel.main()) @@ -280,8 +304,12 @@ download_bazel.BAZELISK_URLS, "linux-amd64", [ - ((tmp_path / "missing").as_uri(), "0" * 64), - ((tmp_path / "missing.tgz").as_uri(), "0" * 64), + ((tmp_path / "missing").as_uri(), "0" * 64, ""), + ( + (tmp_path / "missing.tgz").as_uri(), + "0" * 64, + "package/bazelisk-linux_amd64", + ), ], )
diff --git a/tests/workflows/verified_download_test.py b/tests/workflows/verified_download_test.py new file mode 100644 index 0000000..5b279e2 --- /dev/null +++ b/tests/workflows/verified_download_test.py
@@ -0,0 +1,204 @@ +"""Tests for the .github/actions/download-tool composite action and module.""" + +from __future__ import annotations + +import asyncio +import hashlib +import stat +import tarfile +import zipfile +from pathlib import Path + +import pytest +import verified_download +import yaml +from verified_download import ( + DownloadError, + download_verified_file, + parse_url_list, +) + +from python.runfiles import runfiles + + +def _action_dir() -> Path: + return ( + runfiles.CreateOrRaise().root() / "rules_python/.github/actions/download-tool" + ) + + +def _sha256(data: bytes) -> str: + return hashlib.sha256(data).hexdigest() + + +def test_action_yaml_accepts_urls_and_caches_by_name_and_sha256() -> None: + rf = runfiles.CreateOrRaise() + action_dir = _action_dir() + script = rf.root() / "rules_python/dev/verified_download.py" + assert script.is_file() + assert bool(script.stat().st_mode & stat.S_IXUSR) + + action = yaml.safe_load((action_dir / "action.yaml").read_text(encoding="utf-8")) + assert set(action["inputs"].keys()) == {"name", "urls", "sha256"} + assert action["outputs"]["path"]["value"] == "${{ steps.tool_path.outputs.path }}" + + steps = action["runs"]["steps"] + for step in steps: + assert "${{" not in step.get("run", "") + + cache_step = next(s for s in steps if s.get("id") == "cache") + assert cache_step["uses"].startswith("actions/cache@") + assert cache_step["with"]["path"] == "${{ steps.tool_path.outputs.path }}" + assert cache_step["with"]["key"] == "${{ inputs.name }}-${{ inputs.sha256 }}" + + download_step = next( + s for s in steps if s.get("name") == "Download and verify tool" + ) + assert download_step["if"] == "steps.cache.outputs.cache-hit != 'true'" + assert download_step["env"]["TOOL_URLS"] == "${{ inputs.urls }}" + assert '--urls "${TOOL_URLS}"' in download_step["run"] + assert "dev/verified_download.py" in download_step["run"] + + +def test_parse_url_list_splits_commas_and_newlines() -> None: + urls = parse_url_list( + "https://primary.example.com/tool, https://mirror.example.com/tool,\n" + " https://backup.example.com/tool\n" + ) + assert urls == [ + "https://primary.example.com/tool", + "https://mirror.example.com/tool", + "https://backup.example.com/tool", + ] + with pytest.raises(ValueError, match="At least one URL"): + parse_url_list(" ,\n ") + + +def test_downloads_from_delimited_url_list_with_fallback( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + payload = b"#!/bin/sh\nexit 0\n" + src_file = tmp_path / "src-bin" + src_file.write_bytes(payload) + missing_a = (tmp_path / "missing-a").as_uri() + missing_b = (tmp_path / "missing-b").as_uri() + dest = tmp_path / "out" / "tool" + + rc = verified_download.main( + [ + "--urls", + f"{missing_a},{missing_b}\n{src_file.as_uri()}\n", + "--sha256", + _sha256(payload), + "--executable", + "--output", + str(dest), + ] + ) + assert rc == 0 + assert dest.read_bytes() == payload + assert bool(dest.stat().st_mode & stat.S_IXUSR) + out = capsys.readouterr().out + assert f"::warning::Failed to fetch {missing_a}" in out + assert f"::warning::Failed to fetch {missing_b}" in out + + +def test_falls_back_to_tgz_and_zip_archive_tuples_and_reuses_cached_file( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + payload = b"extracted-binary-data" + member_file = tmp_path / "member" + member_file.write_bytes(payload) + tgz_path = tmp_path / "archive.tgz" + with tarfile.open(tgz_path, "w:gz") as tar: + tar.add(member_file, arcname="pkg/tool") + + missing_url = (tmp_path / "missing").as_uri() + digest = _sha256(payload) + dest = tmp_path / "cached" / "tool" + sources = [ + (missing_url, digest, ""), + (tgz_path.as_uri(), digest, "pkg/tool"), + ] + + asyncio.run(download_verified_file(dest, sources, executable=True, initial_delay=0)) + assert dest.read_bytes() == payload + assert f"::warning::Failed to fetch {missing_url}" in capsys.readouterr().out + + # Second call with only a broken URL succeeds via SHA-256 cache match. + asyncio.run( + download_verified_file( + dest, + [(missing_url, digest, "")], + executable=True, + initial_delay=0, + ) + ) + assert capsys.readouterr().out == "" + + # Verify zip archive_member tuple extraction as well. + zip_path = tmp_path / "archive.zip" + with zipfile.ZipFile(zip_path, "w") as zf: + zf.writestr("bin/tool.exe", payload) + zip_dest = tmp_path / "zip-out" / "tool.exe" + asyncio.run( + download_verified_file( + zip_dest, + [(zip_path.as_uri(), digest, "bin/tool.exe")], + executable=True, + initial_delay=0, + ) + ) + assert zip_dest.read_bytes() == payload + + +def test_concurrent_downloads(tmp_path: Path) -> None: + payload_a = b"tool-alpha" + payload_b = b"tool-beta" + src_a = tmp_path / "src_a" + src_b = tmp_path / "src_b" + src_a.write_bytes(payload_a) + src_b.write_bytes(payload_b) + dest_a = tmp_path / "out" / "a" + dest_b = tmp_path / "out" / "b" + + async def _run_both() -> None: + await asyncio.gather( + download_verified_file( + dest_a, + [(src_a.as_uri(), _sha256(payload_a), "")], + executable=True, + ), + download_verified_file( + dest_b, + [(src_b.as_uri(), _sha256(payload_b), "")], + executable=True, + ), + ) + + asyncio.run(_run_both()) + assert dest_a.read_bytes() == payload_a + assert dest_b.read_bytes() == payload_b + + +def test_rejects_bad_sha256_and_exhausts_retries( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + src_file = tmp_path / "corrupted" + src_file.write_bytes(b"corrupted") + dest = tmp_path / "out" + + with pytest.raises(DownloadError, match="Failed to download after 2 attempts"): + asyncio.run( + download_verified_file( + dest, + [(src_file.as_uri(), "0" * 64, "")], + max_attempts=2, + initial_delay=0, + ) + ) + + out = capsys.readouterr().out + assert "::warning::SHA-256 mismatch:" in out + assert "::warning::Download failed (attempt 1/2)" in out + assert not dest.exists()