diff --git a/.github/workflows/acceptance-205.yml b/.github/workflows/acceptance-205.yml index 07a78431..c8e4875f 100644 --- a/.github/workflows/acceptance-205.yml +++ b/.github/workflows/acceptance-205.yml @@ -94,6 +94,7 @@ jobs: - name: Setup soldr uses: zackees/setup-soldr@v0 with: + cache-key-suffix: acceptance-205-${{ matrix.test_bin }} cache: true build-cache: true target-cache: true diff --git a/.github/workflows/bench-205.yml b/.github/workflows/bench-205.yml index 7d2ec223..cca5b104 100644 --- a/.github/workflows/bench-205.yml +++ b/.github/workflows/bench-205.yml @@ -47,6 +47,7 @@ jobs: - name: Setup soldr uses: zackees/setup-soldr@v0 with: + cache-key-suffix: bench-205-${{ matrix.bench }} cache: true build-cache: true target-cache: true @@ -89,6 +90,7 @@ jobs: - name: Setup soldr uses: zackees/setup-soldr@v0 with: + cache-key-suffix: bench-205-fastled-examples cache: true build-cache: true target-cache: true diff --git a/.github/workflows/benchmark-build-comparison.yml b/.github/workflows/benchmark-build-comparison.yml index b95d934b..de6da152 100644 --- a/.github/workflows/benchmark-build-comparison.yml +++ b/.github/workflows/benchmark-build-comparison.yml @@ -43,6 +43,7 @@ jobs: - name: Setup soldr uses: zackees/setup-soldr@v0 with: + cache-key-suffix: benchmark-build-comparison-benchmark cache: true build-cache: true target-cache: true diff --git a/.github/workflows/check-macos.yml b/.github/workflows/check-macos.yml index 9d0a3414..b075466d 100644 --- a/.github/workflows/check-macos.yml +++ b/.github/workflows/check-macos.yml @@ -38,6 +38,7 @@ jobs: python-version: "3.13" - uses: zackees/setup-soldr@v0 with: + cache-key-suffix: check-macos-test cache: true build-cache: true target-cache: true diff --git a/.github/workflows/check-ubuntu.yml b/.github/workflows/check-ubuntu.yml index c1ba342e..2f167f4f 100644 --- a/.github/workflows/check-ubuntu.yml +++ b/.github/workflows/check-ubuntu.yml @@ -63,6 +63,11 @@ jobs: # the steady-state size so the action only flags genuinely # runaway caches. Hard cap stays at the action default (6 GiB). cache-payload-warn-bytes: 2GiB + # Distinct from python-facade-tests below. With one shared key the + # job that finishes first owns the immutable entry: the facade job's + # 166MB fbuild-python-only cache won over this job's 479MB save, and + # PRs restored it at a 28-32% hit rate. + cache-key-suffix: check-ubuntu # No separate `cargo check`: clippy type-checks the same # `--workspace --all-targets` set, so a prior check only re-did that work. @@ -114,6 +119,7 @@ jobs: target-cache: true prebuild-deps: none linker: platform-default + cache-key-suffix: python-facades - name: Run embedded-CPython facade tests run: | uv python install 3.10 diff --git a/.github/workflows/check-windows.yml b/.github/workflows/check-windows.yml index baa9a9e1..ae3c53e7 100644 --- a/.github/workflows/check-windows.yml +++ b/.github/workflows/check-windows.yml @@ -37,6 +37,7 @@ jobs: id: setup-soldr uses: zackees/setup-soldr@67ed4018aca013f8388050ac9bc264244f9b742c with: + cache-key-suffix: check-windows-check cache: true build-cache: true target-cache: true diff --git a/.github/workflows/esp32s3-size-parity.yml b/.github/workflows/esp32s3-size-parity.yml index 876f22fe..5729df40 100644 --- a/.github/workflows/esp32s3-size-parity.yml +++ b/.github/workflows/esp32s3-size-parity.yml @@ -37,6 +37,7 @@ jobs: - name: Setup soldr uses: zackees/setup-soldr@v0 with: + cache-key-suffix: esp32s3-size-parity-size-parity cache: true build-cache: true target-cache: true diff --git a/.github/workflows/fmt.yml b/.github/workflows/fmt.yml index 4fa02f77..bb1fc5c1 100644 --- a/.github/workflows/fmt.yml +++ b/.github/workflows/fmt.yml @@ -37,6 +37,7 @@ jobs: id: setup-soldr uses: zackees/setup-soldr@v0 with: + cache-key-suffix: fmt-fmt cache: true build-cache: true target-cache: false diff --git a/.github/workflows/platform-boundary-research.yml b/.github/workflows/platform-boundary-research.yml index 000a93d6..7fc89a3f 100644 --- a/.github/workflows/platform-boundary-research.yml +++ b/.github/workflows/platform-boundary-research.yml @@ -35,6 +35,7 @@ jobs: - uses: astral-sh/setup-uv@v3 - uses: zackees/setup-soldr@v0 with: + cache-key-suffix: platform-boundary-research-inventory cache: true toolchain: 1.95.0 prebuild-deps: none diff --git a/.github/workflows/qemu-linux-runtime.yml b/.github/workflows/qemu-linux-runtime.yml index f45d9a4e..566b1e13 100644 --- a/.github/workflows/qemu-linux-runtime.yml +++ b/.github/workflows/qemu-linux-runtime.yml @@ -64,6 +64,7 @@ jobs: - name: Setup soldr uses: zackees/setup-soldr@v0 with: + cache-key-suffix: qemu-linux-runtime-qemu_runtime cache: true build-cache: true target-cache: true diff --git a/ci/README.md b/ci/README.md index 411b9b7b..3249f269 100644 --- a/ci/README.md +++ b/ci/README.md @@ -16,6 +16,7 @@ Python scripts for CI, packaging, and development tooling. All invoked via `uv r - **`platform_boundary_research.py`** -- Host-independent phase-1 inventory and cross-host drift check for FastLED/fbuild#1307 - **`render_workflows.py`** -- Re-renders the `on:` and `concurrency:` blocks of `.github/workflows/build-*.yml` and the full `nightly-platforms.yml` from `board_families.json` + `ci_common_paths.txt`. CI invokes `--check` to enforce no drift. See [docs/DEVELOPMENT.md](../docs/DEVELOPMENT.md#ci-per-board-build-triggers) and FastLED/fbuild#835. - **`select_boards.py`** -- Picks which `build-.yml` workflows `nightly-platforms.yml` dispatches: path-selected from a push diff, or `--all` for the nightly badge refresh. Tested by `test_select_boards.py`. +- **`test_setup_soldr_cache_keys.py`** -- Every saving `zackees/setup-soldr` step must set a `cache-key-suffix` unique per runner; jobs sharing a key race to own one immutable cache entry. - **`board_families.json`** -- SOT: per-board metadata (workflow / test_dir / env_name / family) plus the family → crate-path mapping consumed by `render_workflows.py`. - **`ci_common_paths.txt`** -- SOT: paths whose changes force-run *every* per-board build workflow. - **`test.py`** -- Workspace test runner with `--full` (stress + integration) and per-crate filtering diff --git a/ci/test_setup_soldr_cache_keys.py b/ci/test_setup_soldr_cache_keys.py new file mode 100644 index 00000000..c23a697a --- /dev/null +++ b/ci/test_setup_soldr_cache_keys.py @@ -0,0 +1,108 @@ +"""Every setup-soldr call that can save a cache must own its cache key. + +setup-soldr derives its build/target cache keys from toolchain + lockfile + +`cache-key-suffix`. Two jobs on the same runner OS with the same (or no) +suffix therefore share one immutable GitHub cache entry, and whichever job +finishes first on main owns it. On fbuild the embedded-CPython facade job +(166MB, fbuild-python only) beat the workspace Check job's 479MB save, so +every PR restored the wrong cache at a 28-32% hit rate. +""" +from __future__ import annotations + +import unittest +from collections import defaultdict +from pathlib import Path + +import yaml + +WORKFLOWS = Path(__file__).resolve().parent.parent / ".github" / "workflows" + +# Suffixes deliberately shared because every owner compiles the identical +# thing, so whichever saves first saves the same content. +SHARED_BY_DESIGN = { + # `soldr cargo build -p fbuild-cli -p fbuild-daemon` (debug): the shared + # fbuild_bin job and template_build.yml's standalone fallback. + "fbuild-rust-debug", +} + + +def matrix_cells(job): + """Every concrete matrix combination of `job` (one empty cell if none).""" + matrix = (job.get("strategy") or {}).get("matrix") + if not isinstance(matrix, dict): + return [{}] + axes = {k: v for k, v in matrix.items() if k not in ("include", "exclude") and isinstance(v, list)} + cells = [{}] + for key, values in axes.items(): + cells = [{**cell, key: value} for cell in cells for value in values] + includes = matrix.get("include") or [] + if includes and not axes: + cells = [dict(entry) for entry in includes] + elif includes: + cells += [dict(entry) for entry in includes] + return cells + + +def substitute(value, cell): + text = str(value) + for key, cell_value in cell.items(): + for spelling in (f"${{{{ matrix.{key} }}}}", f"${{{{matrix.{key}}}}}"): + text = text.replace(spelling, str(cell_value)) + return text + + +def saves(with_): + # yaml.safe_load turns `save-cache: false` into the boolean False. + value = with_.get("save-cache", "auto") + return value is not False and str(value).strip().lower() != "false" + + +def setup_soldr_calls(): + """(workflow, job, runner, suffix) per saving step and matrix cell.""" + for path in sorted(WORKFLOWS.glob("*.yml")): + doc = yaml.safe_load(path.read_text(encoding="utf-8")) or {} + for job_id, job in (doc.get("jobs") or {}).items(): + for step in job.get("steps") or []: + if "zackees/setup-soldr@" not in str(step.get("uses", "")): + continue + with_ = step.get("with") or {} + if not saves(with_): + continue + suffix = with_.get("cache-key-suffix") + for cell in matrix_cells(job): + yield ( + path.name, + job_id, + substitute(job.get("runs-on"), cell), + substitute(suffix, cell) if suffix else None, + ) + + +class SetupSoldrCacheKeyTests(unittest.TestCase): + def test_saving_calls_have_a_suffix(self): + missing = [f"{wf}:{job}" for wf, job, _, suffix in setup_soldr_calls() if not suffix] + self.assertEqual([], missing, "setup-soldr steps that save caches need a cache-key-suffix") + + def test_suffixes_are_unique_per_runner(self): + owners = defaultdict(list) + for wf, job, runner, suffix in setup_soldr_calls(): + if suffix and suffix not in SHARED_BY_DESIGN: + owners[(runner, suffix)].append(f"{wf}:{job}") + shared = {key: jobs for key, jobs in owners.items() if len(jobs) > 1} + self.assertEqual({}, shared, "jobs sharing a setup-soldr cache key race to own it") + + def test_matrix_cells_expand_include_entries(self): + job = {"strategy": {"matrix": {"include": [{"bench": "a"}, {"bench": "b"}]}}} + self.assertEqual( + ["x-a", "x-b"], + [substitute("x-${{ matrix.bench }}", cell) for cell in matrix_cells(job)], + ) + + def test_boolean_false_save_cache_is_restore_only(self): + self.assertFalse(saves({"save-cache": False})) + self.assertFalse(saves({"save-cache": "false"})) + self.assertTrue(saves({})) + + +if __name__ == "__main__": + unittest.main()