diff --git a/.github/workflows/brain-ci.yml b/.github/workflows/brain-ci.yml index f0aeca87e..b7c36d3a8 100644 --- a/.github/workflows/brain-ci.yml +++ b/.github/workflows/brain-ci.yml @@ -63,16 +63,20 @@ jobs: if: needs.detect-build.outputs.present == 'true' timeout-minutes: 5 steps: - - name: Setup Zig - uses: mlugg/setup-zig@v2 - with: - version: 0.16.0 - + # Checkout before setup-zig, in every job here. setup-zig restores its + # cache into $GITHUB_WORKSPACE/.zig-cache, and a later actions/checkout + # empties the workspace ("Deleting the contents of ..."), so with the + # old order every hit was deleted before a build used it (#844). - name: Checkout Trinity uses: actions/checkout@v4 with: submodules: recursive + - name: Setup Zig + uses: mlugg/setup-zig@v2 + with: + version: 0.16.0 + - name: Build TRI Binary # `tri-compile`, not `tri`: the `tri` step RUNS the CLI and installs # nothing, so ./zig-out/bin/tri never existed for the steps below. @@ -200,16 +204,16 @@ jobs: if: needs.detect-build.outputs.present == 'true' timeout-minutes: 5 steps: - - name: Setup Zig - uses: mlugg/setup-zig@v2 - with: - version: 0.16.0 - - name: Checkout Trinity uses: actions/checkout@v4 with: submodules: recursive + - name: Setup Zig + uses: mlugg/setup-zig@v2 + with: + version: 0.16.0 + - name: Build TRI Binary # `tri-compile`, not `tri`: the `tri` step RUNS the CLI and installs # nothing, so ./zig-out/bin/tri never existed for the steps below. @@ -241,16 +245,20 @@ jobs: - intraparietal - hslm steps: - - name: Setup Zig - uses: mlugg/setup-zig@v2 - with: - version: 0.16.0 - - name: Checkout Trinity uses: actions/checkout@v4 with: submodules: recursive + - name: Setup Zig + uses: mlugg/setup-zig@v2 + with: + version: 0.16.0 + # One entry per region. Without it the five legs share one key: the + # first to finish saves, the other four are refused ("Unable to + # reserve cache"), and every leg restores that one region's build. + cache-key: ${{ matrix.region }} + - name: Test ${{ matrix.region }} run: | zig build test-${{ matrix.region }} -Dci=true @@ -265,16 +273,16 @@ jobs: if: needs.detect-build.outputs.present == 'true' timeout-minutes: 10 steps: - - name: Setup Zig - uses: mlugg/setup-zig@v2 - with: - version: 0.16.0 - - name: Checkout Trinity uses: actions/checkout@v4 with: submodules: recursive + - name: Setup Zig + uses: mlugg/setup-zig@v2 + with: + version: 0.16.0 + - name: Test Brain Integration run: | zig build test-brain -Dci=true @@ -289,16 +297,16 @@ jobs: if: needs.detect-build.outputs.present == 'true' timeout-minutes: 15 steps: - - name: Setup Zig - uses: mlugg/setup-zig@v2 - with: - version: 0.16.0 - - name: Checkout Trinity uses: actions/checkout@v4 with: submodules: recursive + - name: Setup Zig + uses: mlugg/setup-zig@v2 + with: + version: 0.16.0 + - name: Run Stress Test id: stress run: | @@ -418,16 +426,16 @@ jobs: if: needs.detect-build.outputs.present == 'true' timeout-minutes: 5 steps: - - name: Setup Zig - uses: mlugg/setup-zig@v2 - with: - version: 0.16.0 - - name: Checkout Trinity uses: actions/checkout@v4 with: submodules: recursive + - name: Setup Zig + uses: mlugg/setup-zig@v2 + with: + version: 0.16.0 + - name: Build TRI Binary # `tri-compile`, not `tri`: the `tri` step RUNS the CLI and installs # nothing, so ./zig-out/bin/tri never existed for the steps below. @@ -459,16 +467,16 @@ jobs: if: needs.detect-build.outputs.present == 'true' timeout-minutes: 5 steps: - - name: Setup Zig - uses: mlugg/setup-zig@v2 - with: - version: 0.16.0 - - name: Checkout Trinity uses: actions/checkout@v4 with: submodules: recursive + - name: Setup Zig + uses: mlugg/setup-zig@v2 + with: + version: 0.16.0 + - name: Build TRI Binary # `tri-compile`, not `tri`: the `tri` step RUNS the CLI and installs # nothing, so ./zig-out/bin/tri never existed for the steps below. @@ -566,16 +574,16 @@ jobs: if: github.event_name == 'schedule' timeout-minutes: 10 steps: - - name: Setup Zig - uses: mlugg/setup-zig@v2 - with: - version: 0.16.0 - - name: Checkout Trinity uses: actions/checkout@v4 with: submodules: recursive + - name: Setup Zig + uses: mlugg/setup-zig@v2 + with: + version: 0.16.0 + - name: Build TRI Binary # `tri-compile`, not `tri`: the `tri` step RUNS the CLI and installs # nothing, so ./zig-out/bin/tri never existed for the steps below. diff --git a/.github/workflows/zig-cache-order.yml b/.github/workflows/zig-cache-order.yml new file mode 100644 index 000000000..2c8ba2f53 --- /dev/null +++ b/.github/workflows/zig-cache-order.yml @@ -0,0 +1,44 @@ +name: Zig cache order gate + +# mlugg/setup-zig restores the Zig cache into $GITHUB_WORKSPACE/.zig-cache, and +# an actions/checkout of the workspace that runs after it deletes the lot. +# brain-ci did this in all 8 of its Zig jobs: 238 restores in 25 runs, none +# used, while its entries held most of the repository's Actions cache (#844, +# #890). See tools/check_setup_zig_order.py; tools/test_check_setup_zig_order.py +# plants every case it claims to decide, and runs first. + +on: + push: + branches: [main] + paths: + - '.github/workflows/**' + - 'tools/check_setup_zig_order.py' + - 'tools/test_check_setup_zig_order.py' + pull_request: + paths: + - '.github/workflows/**' + - 'tools/check_setup_zig_order.py' + - 'tools/test_check_setup_zig_order.py' + workflow_dispatch: + +concurrency: + group: ${{ github.workflow_ref }}-${{ github.event_name == 'workflow_dispatch' && github.run_id || github.event_name }} + cancel-in-progress: ${{ github.event_name == 'pull_request' }} + +jobs: + zig-cache-order: + runs-on: ubuntu-latest + timeout-minutes: 5 + steps: + - uses: actions/checkout@v4 + # The checker reads workflows with PyYAML, as the runner reads them. + # Without PyYAML it answers "cannot tell" (rc 2), never clean. + - name: PyYAML for the checker + run: | + python3 -c 'import yaml' 2>/dev/null \ + || python3 -m pip install --quiet --user --break-system-packages pyyaml + python3 -c 'import yaml; print("PyYAML", yaml.__version__)' + - name: The checker decides every planted case + run: python3 tools/test_check_setup_zig_order.py + - name: No job checks out over its restored Zig cache + run: python3 tools/check_setup_zig_order.py diff --git a/tools/check_setup_zig_order.py b/tools/check_setup_zig_order.py new file mode 100644 index 000000000..103e7196f --- /dev/null +++ b/tools/check_setup_zig_order.py @@ -0,0 +1,128 @@ +#!/usr/bin/env python3 +"""Does any job restore its Zig cache and then check out over it? + +mlugg/setup-zig restores the job's cache into `$GITHUB_WORKSPACE/.zig-cache` +and points both ZIG_GLOBAL_CACHE_DIR and ZIG_LOCAL_CACHE_DIR at it +(`getZigCachePath` in its common.js). If an actions/checkout of the workspace +runs after it, checkout logs "Deleting the contents of ''" and the +restored cache goes with the rest. The job then builds from nothing and saves a +new entry anyway. brain-ci did this in all 8 of its Zig jobs. In 25 runs it +made 238 restores and used none of them, and its entries took 8.43 of the +repository's 10.83 GB of Actions cache (#844, #890). + +A job fails this gate when a caching setup-zig step comes before a checkout +of the workspace itself. Caching means `use-cache` is absent or anything but +a literal `false`. The workspace itself means `path` is absent, `.` or +`${{ github.workspace }}`. A checkout into a subdirectory leaves `.zig-cache` +alone and passes, as does a job that never checks out. + +Exit 0 clean, 1 a job checks out over a restored cache, 2 cannot tell (no +PyYAML, no workflows, an unreadable workflow, or no job uses setup-zig, so +there is nothing to guard and a pass would mean nothing). + +Known limits: a workspace wiped by a script (`git clean -xdf`, `rm -rf`) or +by another action is not read, and neither is a setup-zig vendored under a +different name. Each such case would need its own rule. +""" +from __future__ import annotations + +import argparse +import pathlib +import sys + +ROOT = pathlib.Path(__file__).resolve().parents[1] +WORKSPACE_PATHS = {"", ".", "./", "${{ github.workspace }}"} + + +def action_name(step: dict) -> str: + return str(step.get("uses") or "").split("@")[0].strip().lower() + + +def restores_cache(step: dict) -> bool: + use_cache = str((step.get("with") or {}).get("use-cache", "true")).strip().lower() + return use_cache != "false" + + +def checks_out_workspace(step: dict) -> bool: + path = str((step.get("with") or {}).get("path") or "").strip() + return path in WORKSPACE_PATHS + + +def job_findings(workflow: str, job_id: str, job: dict) -> tuple[bool, list[str]]: + """(uses setup-zig at all, one line per checkout that wipes a restored cache).""" + steps = job.get("steps") or [] + restored_at = None + uses_zig = False + out = [] + for i, step in enumerate(steps, 1): + not_a_step = not isinstance(step, dict) + if not_a_step: + continue + name = action_name(step) + is_zig = name == "mlugg/setup-zig" + uses_zig = uses_zig or is_zig + restores = is_zig and restores_cache(step) + if restores: + restored_at = i + wipes = restored_at is not None and name == "actions/checkout" and checks_out_workspace(step) + if wipes: + out.append(f"{workflow} :: {job_id} :: setup-zig (step {restored_at}) restores .zig-cache, " + f"then checkout (step {i}) deletes it") + return uses_zig, out + + +def main(argv: list[str] | None = None) -> int: + ap = argparse.ArgumentParser(description=__doc__.splitlines()[0]) + ap.add_argument("--workflows", default=str(ROOT / ".github" / "workflows"), + help="directory of workflow files (default: this tree's)") + args = ap.parse_args(argv) + try: + import yaml + except ImportError: + print("cannot tell: PyYAML is not installed") + return 2 + wf_dir = pathlib.Path(args.workflows) + files = sorted(list(wf_dir.glob("*.yml")) + list(wf_dir.glob("*.yaml"))) + no_files = not files + if no_files: + print(f"cannot tell: no workflow file under {wf_dir}") + return 2 + findings: list[str] = [] + unreadable: list[str] = [] + zig_jobs = 0 + for wf in files: + try: + doc = yaml.safe_load(wf.read_text(encoding="utf-8")) + except yaml.YAMLError as e: + unreadable.append(f"{wf.name}: {str(e).splitlines()[0]}") + continue + jobs = doc.get("jobs") if isinstance(doc, dict) else None + jobs_ok = isinstance(jobs, dict) + if not jobs_ok: + continue + for job_id, job in jobs.items(): + job_ok = isinstance(job, dict) + if not job_ok: + continue + uses_zig, found = job_findings(wf.name, str(job_id), job) + zig_jobs += uses_zig + findings.extend(found) + print(f"{len(files)} workflows, {zig_jobs} jobs use mlugg/setup-zig, " + f"{len(findings)} check out over its restored cache") + for line in findings: + print(f" {line}") + for line in unreadable: + print(f" cannot read {line}") + wiped_any = bool(findings) + if wiped_any: + print("Move actions/checkout above setup-zig in each job listed (see #890).") + return 1 + blind = bool(unreadable) or zig_jobs == 0 + if blind: + print("cannot tell: " + ("a workflow did not parse" if unreadable else "no job uses mlugg/setup-zig")) + return 2 + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/tools/test_check_setup_zig_order.py b/tools/test_check_setup_zig_order.py new file mode 100644 index 000000000..80d084429 --- /dev/null +++ b/tools/test_check_setup_zig_order.py @@ -0,0 +1,88 @@ +#!/usr/bin/env python3 +"""Plants every case tools/check_setup_zig_order.py claims to decide, and +checks its exit code: 0 clean, 1 a checkout over a restored cache, 2 cannot +tell. + +Each case writes one workflow into an empty directory and runs the checker's +main() on it. Case 1 is brain-ci's layout before #890. + +Exit 0 when every case gives its expected code, 1 otherwise. +""" +from __future__ import annotations + +import contextlib +import importlib.util +import io +import pathlib +import sys +import tempfile + +HERE = pathlib.Path(__file__).resolve().parent +spec = importlib.util.spec_from_file_location("csz", HERE / "check_setup_zig_order.py") +csz = importlib.util.module_from_spec(spec) +spec.loader.exec_module(csz) + +H = "on: push\njobs:\n j:\n runs-on: ubuntu-latest\n steps:\n" +ZIG = " - uses: mlugg/setup-zig@v2\n with:\n version: 0.16.0\n" +CO = " - uses: actions/checkout@v4\n" +BUILD = " - run: zig build\n" + +# (name, expected rc, workflow text; None writes no workflow at all) +CASES: list[tuple[str, int, str | None]] = [ + ("setup-zig, then checkout (brain-ci before #890)", 1, H + ZIG + CO + BUILD), + ("checkout, then setup-zig (#890)", 0, H + CO + ZIG + BUILD), + ("setup-zig and no checkout", 0, H + ZIG + BUILD), + ("use-cache: false, then checkout", 0, + H + " - uses: mlugg/setup-zig@v2\n with:\n use-cache: false\n" + CO), + ("use-cache: 'false' as a string", 0, + H + " - uses: mlugg/setup-zig@v2\n with:\n use-cache: 'false'\n" + CO), + ("use-cache from an expression counts as caching", 1, + H + " - uses: mlugg/setup-zig@v2\n with:\n use-cache: ${{ matrix.c }}\n" + CO), + ("checkout into a subdirectory", 0, + H + ZIG + " - uses: actions/checkout@v4\n with:\n path: sub\n"), + ("checkout with path: .", 1, + H + ZIG + " - uses: actions/checkout@v4\n with:\n path: .\n"), + ("checkout with path: ${{ github.workspace }}", 1, + H + ZIG + " - uses: actions/checkout@v4\n with:\n path: ${{ github.workspace }}\n"), + ("a pinned SHA is still setup-zig", 1, + H + " - uses: mlugg/setup-zig@8d6198c65fb0feaa111df26e6b467fea8345e46f\n" + CO), + ("a second checkout after setup-zig", 1, H + CO + ZIG + CO), + ("upper case action name", 1, H + " - uses: MLugg/Setup-Zig@v2\n" + CO), + ("the second of two jobs", 1, + H + CO + ZIG + " k:\n runs-on: ubuntu-latest\n steps:\n" + ZIG + CO), + ("a reusable-workflow job has no steps", 0, + H + CO + ZIG + " k:\n uses: ./.github/workflows/other.yml\n"), + ("no job uses setup-zig: nothing to guard", 2, H + CO + BUILD), + ("a workflow that does not parse", 2, "on: push\njobs: [\n"), + ("no workflow at all", 2, None), +] + + +def run_case(text: str | None) -> tuple[int, str]: + with tempfile.TemporaryDirectory() as d: + wf_dir = pathlib.Path(d) + has_text = text is not None + if has_text: + (wf_dir / "t.yml").write_text(text) + out = io.StringIO() + with contextlib.redirect_stdout(out): + rc = csz.main(["--workflows", str(wf_dir)]) + return rc, out.getvalue() + + +def main() -> int: + bad = 0 + for name, want, text in CASES: + rc, out = run_case(text) + ok = rc == want + bad += not ok + print(f"{'ok ' if ok else 'FAIL'} rc {rc} (want {want}) {name}") + show = not ok + if show: + print(" " + out.replace("\n", "\n ").rstrip()) + print(f"{len(CASES) - bad}/{len(CASES)} planted cases decided as claimed") + return 1 if bad else 0 + + +if __name__ == "__main__": + sys.exit(main())