Generate the CI matrices from one versions file - #13
Merged
Conversation
Bumping ESP-IDF meant editing four matrix blocks plus two Dockerfile ARG defaults, and getting it half right did not fail: old tags live on in GHCR, so a manifest job with a stale block republishes the previous version's images under the new latest. The repository was already carrying one instance of that drift - CI passed PIO_VERSION=v6.1.18 while the Dockerfile defaulted to 6.1.18, so every CI build installed platformio==v6.1.18 and only PEP 440 normalisation made it work. images/versions.json now holds what gets built: platforms mapped to runner pools, the variants published per image, their build args, which one is primary, and for a derived image the base tag each variant is built on. scripts/versions-matrix.sh turns that into the build and manifest matrices; a prepare job emits them and every other job reads fromJSON. No matrix is written in a workflow any more. scripts/check-versions.sh enforces what the data cannot say about itself, each check standing for a failure that used to be silent: every image has a Dockerfile and every Dockerfile an image; exactly one variant is primary, so latest is decided in the data rather than by whichever manifest job finished last; tags are unique within an image; every platform has a runner pool, since one without leaves runs-on empty and that leg never starts; the primary variant's args match the Dockerfile ARG defaults, which is what caught PIO_VERSION; every base_tag exists among the base image's tags; and the base image's own ARG default names the primary variant's base tag, so a local build stands on the same ESP-IDF as CI. Verified by breaking it four ways - the PIO_VERSION mismatch, a base tag the base image does not publish, two primaries, and an ARG BASE_IMAGE default pointing at the wrong version - and confirming each is reported. Then built platformio with the generated build args: PlatformIO Core, version 6.1.18. images/versions.json and both scripts are in every paths: filter, or the one commit that only bumps a version would trigger nothing at all.
Five defects, all real, and the two P1s both bite exactly when a second variant is added - which is what PR-5 does. Build-arg names are now checked for every variant, not just the primary one. Docker silently ignores an unconsumed --build-arg, so `IDF_BASE_TGA` in a non-primary variant would have built the Dockerfile's default version and published it under the other version's tag, with nothing failing. Values are still compared only for the primary variant, since that is the one a local build reproduces. base_arg was declared in versions.json and ignored by the workflow, which hardcoded BASE_IMAGE. Renaming the argument would have passed the checker while the build silently fell back to the Dockerfile's mutable default - defeating the digest pin this series just introduced. The matrix carries base_arg now and the build step uses it. Runner labels are validated against .github/actionlint.yaml. Moving runs-on to a runtime expression took it out of actionlint's reach, so a typo would no longer be a lint error - it would be a leg queued for 24 hours with no diagnostic. The checker compares each pool against that file, plus a list of GitHub-hosted labels, which are deliberately absent from it. timeout_minutes was centralized in versions.json and then ignored: the jobs still hardcoded their own. Changing it would have had no effect. Now taken from the matrix. platform_tag used sub, replacing one slash; linux/arm/v7 would have produced `linux-arm/v7`, which upload-artifact rejects as a name. gsub. Verified by breaking each new check: a mistyped ARG name in a non-primary variant and a mistyped runner pool are both reported.
Moving the versions into a data file removed guarantees that the workflow's
structure had been providing for free. Ten defects, all confirmed; the mechanism
ones are fixed here.
`tag` used to be assembled from the same fields that fed build-args, so it could
not disagree with them. As free text it can: bump `args` and the Dockerfile but
forget `tag`, and CI republishes idf-v5.4.1-matter-v1.4.2 - plus latest and
sha-<commit> - containing Matter v1.5.0. The checker now requires every version a
variant is built with, and its base_tag, to appear in its tag.
A variant with `"args": {}` passed every check and built the Dockerfile's default
version under the other version's tag. The arg-name check could not see it: it
iterates over the keys that are present. Every variant must now pass the same set
of arg names as the primary one.
Digests are downloaded with `digest-<image>-<tag>-*`, so one tag being a prefix of
another - idf-v5.4.1 and idf-v5.4.10 - would pull in the other variant's platforms
and publish a mixed manifest. Uniqueness does not rule that out; the checker
rejects prefixes now.
`EXPECTED_PLATFORMS: 2` stayed hardcoded in three manifest jobs while the platform
list moved to a file where it is a map of arbitrary length. Adding a third platform
would have burned three full builds and then failed the manifest. It comes from the
matrix now.
`echo "key=$(generator)"` takes echo's exit status, so a failing generator wrote an
empty matrix and left the step green - surfacing later as an opaque fromJSON error
in another job. Assigned first, echoed second.
`$b.args | to_entries` crashed on a variant with no args at all, which is a
legitimate shape for a derived image that only changes its base. `(.args // {})`.
Documentation: the new check-versions block had been inserted between
./scripts/lint.sh and the paragraph describing it, so that paragraph read as
documentation for the wrong script. CLAUDE.md lists what the checker actually
enforces, and states the cost of one shared versions file: it is in every paths
filter, so a bump to one image rebuilds the others.
There was a problem hiding this comment.
Pull request overview
Centralizes Docker image build/manifest configuration into a single images/versions.json file, generates GitHub Actions matrices from it, and adds a consistency checker to prevent CI/build-arg drift across images and workflows.
Changes:
- Introduces
images/versions.jsonas the single source of truth for image variants, platforms→runner pools, build args, and primary variant selection. - Adds
scripts/versions-matrix.sh(matrix generator) andscripts/check-versions.sh(invariant enforcement against Dockerfile ARG defaults and other constraints). - Refactors
esp-idf.ymlandplatformio.ymlworkflows to use apreparejob andfromJSON(needs.prepare.outputs.*)matrices; updates docs inREADME.md/CLAUDE.md.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| scripts/versions-matrix.sh | Generates build/manifest matrices from images/versions.json for workflow fromJSON consumption. |
| scripts/check-versions.sh | Validates images/versions.json invariants (Dockerfile presence, primary uniqueness, tag rules, runner pools, ARG consistency, base-tag correctness). |
| README.md | Documents the new “check versions before a bump” workflow and updates repo tree listing. |
| images/versions.json | Adds the single source-of-truth data for CI builds (platforms, variants, args, primaries, base relationships). |
| CLAUDE.md | Updates repo conventions to describe the new versions/matrix/checker mechanism and invariants. |
| .github/workflows/platformio.yml | Switches workflow matrices to generated outputs; adds prepare job and consistency check. |
| .github/workflows/esp-idf.yml | Switches esp-idf/esp-matter matrices to generated outputs; adds prepare job and consistency check. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Copilot caught that the pool check embedded the label in an ERE unescaped. Every label here contains dots, so ubuntu-24.04-arm as a pattern would also match ubuntu-24X04-arm - accepting exactly the typo the check exists to catch. The pools are now read from .github/actionlint.yaml into an array and compared as strings, with an empty list treated as a hard failure rather than a permissive one.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.
Suppressed comments (1)
scripts/versions-matrix.sh:11
- The header comment says the generator fails when a platform has no runner pool, but
versions-matrix.shdoes not validate runner values (it will happily emitrunner: ""). It’sscripts/check-versions.shthat enforces non-empty runners, so the comment is currently misleading.
# Every entry carries `runner`, so a platform without a pool is impossible to
# express here - the generator fails instead of emitting a leg whose `runs-on`
# resolves to the empty string and never starts.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bumping ESP-IDF meant editing four matrix blocks plus two Dockerfile
ARGdefaults — and getting it half right did not fail. Old tags live on in GHCR, so a
manifest job with a stale block quietly republishes the previous version's images
under the new
latest.The repository was already carrying one instance of that drift: CI passed
PIO_VERSION=v6.1.18while the Dockerfile defaulted to6.1.18, so every CI buildinstalled
platformio==v6.1.18and only PEP 440 normalisation made it work.One file
images/versions.jsonholds what gets built — platforms mapped to runner pools,the variants published per image, their build args, which one is
primary, and fora derived image the base tag each variant stands on.
scripts/versions-matrix.shturns that into the build and manifest matrices, apreparejob emits them, and every other job readsfromJSON. No matrix iswritten in a workflow any more.
A bump is now one edit here plus the matching
ARGdefault — and the checker failsthe build if you do only one.
What the checker enforces
Each check stands for a failure that used to be silent:
primaryvariantlatestdecided by whichever manifest job finished lastdigest-<image>-<tag>-*glob pulling in another variant's platformsargswith a forgottentagpublishing a name that contradicts its contentsARGdefaults.github/actionlint.yamlruns-onis now runtime, so actionlint cannot see itbase_tagexists among the base image's tagsidf-v<old>published onv<new>ARGdefault names primary's base tag./scripts/build.shstanding on a different ESP-IDF than CIVerification
Every check was verified by breaking it and confirming the report:
PIO_VERSIONmismatch — the first run of the checker caught itARG BASE_IMAGEdefault pointing at the wrong version"args": {}idf-v5.4.1as a prefix ofidf-v5.4.10Then built platformio with the generated build args:
PlatformIO Core, version 6.1.18.Reviewed by
codex-review(xhigh) — five defects, two of them P1, both biting exactly when asecond variant is added, which is what PR-5 does: arg names were only checked on
the primary variant, and
base_argwas declared in the data while the workflowhardcoded
BASE_IMAGE. Also:timeout_minuteswas centralised and then ignored,runner labels lost actionlint's coverage when
runs-onbecame a runtimeexpression, and
platform_tagusedsub, leaving a slash inlinux/arm/v7.The
code-reviewworkflow (high) — ten defects, all confirmed. The mechanism onesare the table above plus:
EXPECTED_PLATFORMS: 2hardcoded in three jobs against aplatform map of arbitrary length,
echo "key=$(generator)"swallowing thegenerator's exit status, and
$b.args | to_entriescrashing on a variant with noargs — a legitimate shape for a derived image that only changes its base.
Left for later, deliberately: the manifest tag-assembly block is now duplicated
three times across two workflows. That is what reusable workflows are for, and it
is a mechanical change once this lands.