CI: single content-addressed criterion for Docker image selection and rebuilds#6436
Conversation
… rebuilds Selecting the Docker image tag and deciding whether prepare-images must run used dorny/paths-filter lists that drifted from the inputs of docker_image_source_checksum.sh (the filters exclude thirdparty/licenses/**, the checksum script does not), so licenses-only changes moved every image checksum without building the new source-checksum-* tags, breaking consumers that pin them. Interleaved image-affecting PRs left similar gaps: master merge combinations nobody's branch built (e.g. emscripten-build-c-bindings 3.1.38 vs 4.0.19 right now). Replace the filters with one criterion derived from the checksum itself: select_docker_image_tags.sh resolves each image's source-checksum-* tag and the conventional tags to their leaf manifest digests (imagetools create wraps single manifests into lists, so own digests differ) and decides: - all images reachable via latest -> use latest, skip prepare-images; - source-checksum-* tag absent -> prepare-images builds and pushes it; - present but conventional tag elsewhere -> prepare-images repoints only. Pull requests never move latest: they fall back to branch tags unless their sources already match latest. Registry query failures count as absent, i.e. fail toward rebuilding. The check also runs on schedule, so any gap self-heals on the nightly at the latest. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
3ce70d4 to
b2d7fc4
Compare
Per review: the rebuild-selection mechanism moves to the registry-check approach of #6436, which the filter edits here would conflict with; the licenses exclusion composes with it under either merge order. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…json The gate script hard-coded the (repository, distro) list that had to mirror the prepare-images build matrices by hand. Move the inventory to matrix/docker-images.json: select_docker_image_tags.sh reads it with jq, and the compute-image-matrices job (extended to serve the vcpkg and emscripten-c-bindings jobs too) derives all build matrices from the same file, so adding or removing an image is a one-file change and the gate and the builders cannot drift. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
| cbindings_axis: ${{ steps.compute.outputs.cbindings_axis }} | ||
| cbindings_include: ${{ steps.compute.outputs.cbindings_include }} | ||
| vcpkg_arch_axis: ${{ steps.compute.outputs.vcpkg_arch_axis }} | ||
| vcpkg_os_axis: ${{ steps.compute.outputs.vcpkg_os_axis }} | ||
| vcpkg_include: ${{ steps.compute.outputs.vcpkg_include }} |
There was a problem hiding this comment.
You can output the whole matrices instead.
| @@ -241,12 +251,8 @@ jobs: | |||
| fail-fast: false | |||
| matrix: | |||
| # The old `3.1.38` didn't have ARM images. | |||
There was a problem hiding this comment.
The comment does not belong here anymore.
There was a problem hiding this comment.
Split the script into several logical blocks.
- select_docker_image_tags.sh split into docker_image_list.sh (inventory from matrix/docker-images.json) and docker_image_leaf_digests.sh (registry digest query), keeping only the selection logic. - compute-image-matrices outputs complete matrix objects consumed via a single fromJSON each, instead of separate axis/include outputs. - Drop the stale 3.1.38 comment. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…s create Instead of comparing digests and delegating retags to prepare-images, the gate now runs `docker buildx imagetools create` per image: a no-op when the conventional tag already points at the wanted image, a retag when it doesn't, and a failure exactly when the source-checksum-* tag is absent — which is now the only thing that sets need_rebuild=true, so prepare-images runs only when something must actually be built. PRs still resolve to `latest` read-only while it matches, so clean branches mint no tags; fork PRs (no DOCKERHUB_TOKEN) keep the read-only digest comparison. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Dependabot branches are same-repo, so the fork check alone would run the DockerHub login with an empty secret and fail prepare-config on every Dependabot PR; route them to the read-only digest fallback instead. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Fork and Dependabot PRs cannot sync tags anyway; per review discussion they now short-circuit to image_tag=latest / need_rebuild=false instead of running the read-only digest comparison. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
With the unconditional imagetools create, the read-only digest comparison has no consumers left: PRs now always sync and pull their branch tag instead of resolving to latest first. The inventory jq folds back into select_docker_image_tags.sh, still reading the shared docker-images.json. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
| # have no repository secrets; they skip the login and the gate then | ||
| # selects `latest` as-is without inspecting the registry. | ||
| - name: Login to DockerHub | ||
| if: ${{ (github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository) && github.actor != 'dependabot[bot]' }} |
There was a problem hiding this comment.
How about
jobs:
prepare-config:
env:
TAG_WRITES: ${{ (github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository) && github.actor != 'dependabot[bot]' }}and then
if: ${{ env.TAG_WRITES == 'true' }}?
There was a problem hiding this comment.
does job level if support env?
There was a problem hiding this comment.
Job-level if does not see the env context, but this suggestion only needs it at step level (the login step), where env is available — implemented in f6ef688: TAG_WRITES is defined once as job env and the login step tests env.TAG_WRITES.
| BRANCH_TAG="" | ||
| if [ "${{ github.event_name }}" = "pull_request" ]; then | ||
| # https://stackoverflow.com/q/58033366/7325599 | ||
| IMAGE_TAG=$(echo "${{ github.head_ref || github.ref_name }}" | sed -r 's/[^a-zA-Z0-9._-]+/-/g') | ||
| else | ||
| IMAGE_TAG="latest" | ||
| BRANCH_TAG=$(echo "${{ github.head_ref }}" | sed -r 's/[^a-zA-Z0-9._-]+/-/g') | ||
| fi | ||
| echo "image_tag=${IMAGE_TAG}" >> $GITHUB_OUTPUT | ||
| scripts/devops/select_docker_image_tags.sh linux "${BRANCH_TAG}" >> $GITHUB_OUTPUT |
There was a problem hiding this comment.
if [ "${{ github.event_name }}" = "pull_request" || "${TAG_WRITES}" != "true" ]; then
# https://stackoverflow.com/q/58033366/7325599
IMAGE_TAG=$(echo "${{ github.head_ref }}" | sed -r 's/[^a-zA-Z0-9._-]+/-/g')
else
IMAGE_TAG="latest"
fi
echo "image_tag=${IMAGE_TAG}" >> $GITHUB_OUTPUT
if scripts/devops/select_docker_image_tags.sh linux "${IMAGE_TAG}" ; then
echo "need_rebuild=false" >> $GITHUB_OUTPUT
else
echo "need_rebuild=true" >> $GITHUB_OUTPUT
fiAnd update the select_docker_image_tags.sh script accordingly.
…de protocol - TAG_WRITES defined once as prepare-config job env; the login step tests it via a step-level if (job-level if does not see the env context, but step-level does). - The steps now pick the tag name themselves (branch tag on internal PRs, latest otherwise) and map the script's exit status to need_rebuild; select_docker_image_tags.sh becomes sync_docker_image_tags.sh: point <tag> of every family image at its source-checksum-* tag, exit 0 when synced, 1 when something must be built, 2 on usage errors (escalated to a step failure rather than a spurious rebuild). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…x-key) New inputs, mutually exclusive with `matrix`: `matrix-file` names a YAML/JSON file holding the base matrix, so one inventory file can be shared between workflows and shell scripts; `matrix-key` selects one top-level key of that document for files bundling several matrices. The base matrix (from either input) now also accepts the include-list form: a list of entries used verbatim, with the axis keys being the union of the entries' keys. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Replace the jq disable-label filter in compute-image-matrices with a matrix-builder step reading docker-images.json via the new matrix-file / matrix-key inputs, per review. The c-bindings and vcpkg outputs stay on jq: they are reshapes into axes + include form (keeping runner names out of the cell display names), which the action's flat entry-list output cannot express. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
| # use the ${{ needs.config.outputs.bool-param == 'true' }} construction to check boolean values | ||
| config: | ||
| uses: ./.github/workflows/config.yml | ||
| # for the DockerHub login in prepare-config (tag sync) |
| echo "${repo}:${tag} -> ${hash_tag}" >&2 | ||
| else | ||
| echo "${repo}:${hash_tag} absent, needs building" >&2 | ||
| synced=false |
| pairs=$(jq -r '."linux-vcpkg"[] | "meshlib/meshlib-\(.os)-vcpkg-\(.arch) \(.os)-vcpkg"' "${manifest}") ;; | ||
| *) | ||
| echo "unknown image family: ${family}" >&2 | ||
| exit 2 ;; |
There was a problem hiding this comment.
Nothing to rebuild → does not need rebuild → exit 0.
| exit 2 ;; | ||
| esac | ||
|
|
||
| here=$(dirname "$0") |
| # The label-derived booleans (full_config_build / upload_artifacts / | ||
| # version_namespace) are not here: they are written to $GITHUB_ENV by | ||
| # the live-labels step below. |
| fi | ||
| done <<< "${pairs}" | ||
|
|
||
| [ "${synced}" = true ] |
There was a problem hiding this comment.
Exit with 0 when there's nothing to rebuild.
| - { distro: ubuntu22, arch: x64 } | ||
| - { distro: ubuntu24, arch: x64 } |
| fi | ||
| FILE=.github/workflows/matrix/docker-images.json | ||
| { | ||
| echo "cbindings_matrix=$(jq -c '{emsdk_image: [."emscripten-c-bindings"[].emsdk_image], include: ."emscripten-c-bindings"}' "$FILE")" |
There was a problem hiding this comment.
Why not separate matrix-builder steps with matrix-key: linux-vcpkg and matrix-key: emscripten-c-bindings?
There was a problem hiding this comment.
I feel like it's more convenient to have three separate files for each kind since the data are in fact heterogeneous.
| echo "image_tag=${IMAGE_TAG}" >> $GITHUB_OUTPUT | ||
|
|
||
| NEED_REBUILD=false | ||
| if [ "${TAG_WRITES}" = "true" ]; then |
There was a problem hiding this comment.
if [ "${TAG_WRITES}" != "true" ] || ! scripts/devops/sync_docker_image_tags.sh linux-vcpkg "${IMAGE_TAG}" ; then
echo "need_rebuild=false" >> $GITHUB_OUTPUT
else
echo "need_rebuild=true" >> $GITHUB_OUTPUT
fi…t cleanup
- matrix/docker-images.json split into docker-images-{linux,
emscripten-c-bindings,linux-vcpkg}.json (bare entry lists); each family
gets its own matrix-builder step (matrix-file only, no matrix-key, no
inline JSON in rules, jq reshape step gone; the cbindings/vcpkg jobs
consume flat include lists).
- sync_docker_image_tags.sh: early exit 1 on the first absent image,
fall-through exit 0 when there is nothing to build (unknown family
included), dirname inlined.
- config.yml: TAG_WRITES renamed to CAN_PUSH_DOCKER_TAGS, redundant
comments dropped, select steps reduced to a plain if/else on the
script's exit status.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
| |---------------|----------|---------|------------------------------------------------------------------------------------------| | ||
| | `matrix` | no | `{}` | YAML/JSON base matrix: axis map (cartesian product) or list of entries (used as-is). Mutually exclusive with `matrix-file`. | | ||
| | `matrix-file` | no | — | Path of a YAML/JSON file to load the base matrix from (same forms as `matrix`). Mutually exclusive with `matrix`. | | ||
| | `matrix-key` | no | — | Top-level key to select within the `matrix-file` document, for files bundling several matrices. Requires `matrix-file`. | |
There was a problem hiding this comment.
The option is redundant now.
| # Linux cells encode only `distro` and `arch` so the auto-generated | ||
| # cell name stays "(<distro>, <arch>)"; the runner OS and image suffix | ||
| # are derived from `matrix.arch` inline below. A new ubuntu distro in | ||
| # docker-images-linux.json must also be added to the exclude rules here. | ||
| # `emscripten-generate-c-bindings` (the image for generate-c-bindings) | ||
| # has no disable rule: that job always runs, so the image is always | ||
| # built. The parity tests in the action's tests/engine.test.js pin | ||
| # these rules to the jq filter they replaced. |
| # Build the job matrices from the image inventory in the | ||
| # matrix/docker-images-*.json files (shared with | ||
| # scripts/devops/sync_docker_image_tags.sh), dropping linux cells the PR | ||
| # opted out of via `disable-build-<platform>` labels. The `matrix` context | ||
| # is not available in job-level `if:` for reusable workflow inputs, so | ||
| # per-cell skipping must happen here. |
| # Points <tag> of every image of the family at its content-addressed | ||
| # source-checksum-* tag (docker_image_source_checksum.sh) with an | ||
| # unconditional `docker buildx imagetools create` (requires a prior docker | ||
| # login): a no-op when it already points there, a cheap retag when it | ||
| # doesn't, and a failure exactly when the source-checksum-* tag is absent, | ||
| # i.e. when the image really must be built — then this script exits 1; | ||
| # it exits 0 when there is nothing to build. Must run from the repository | ||
| # root. The image inventory comes from the matrix/docker-images-*.json | ||
| # files, shared with the build matrices in | ||
| # .github/workflows/prepare-images.yml; Docker Hub repository names are | ||
| # derived here the same way its jobs derive them from the matrix. |
With one inventory file per family, matrix-key has no consumers: remove the input from the action (src, action.yml, README, tests; dist rebuilt, 62 tests pass). Shorten the compute-image-matrices and linux-step comments in prepare-images.yml and the sync_docker_image_tags.sh header. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Problem
Which Docker tag the build jobs pull and whether
prepare-imagesruns were decided bydorny/paths-filterlists that drifted from the inputs ofscripts/devops/docker_image_source_checksum.sh: the filters excludethirdparty/licenses/**, the checksum script hashes it. Licenses-only changes (#6429, #6422, #6434) therefore moved every Linux image checksum without triggering a rebuild, so the newsource-checksum-*tags were never pushed and consumers that pin them fail withmanifest unknown. Interleaved image-affecting PRs leave similar gaps: master currently hasmeshlib-emscripten-build-c-bindings-4.0.19-arm64built for its content but not the3.1.38twin.Fix
Drop the Linux paths-filters entirely and derive everything from one criterion — the content-addressed checksum, compared against the registry:
The prepare-config steps pick the conventional tag (branch tag on internal PRs,
latestotherwise) and callscripts/devops/sync_docker_image_tags.sh <linux|linux-vcpkg> <tag>, which computes each image'ssource-checksum-*tag and points the conventional tag at it right in the gate, with an unconditionaldocker buildx imagetools create:source-checksum-*tag exists but the conventional tag points elsewhere: the create retags it — seconds, no build matrix spun up.source-checksum-*tag absent: the create fails, the script exits 1 and the step setsneed_rebuild=true, so prepare-images builds and pushes it and points the conventional tag at it (its jobs unchanged: build-or-reuse per image, thenimagetools create).So
need_rebuildnow literally means "something must be built", and pull requests never movelatest— they always pull their branch tag, synced the same way. Credential-less PRs (forks and Dependabot — noDOCKERHUB_TOKEN,CAN_PUSH_DOCKER_TAGS=false) skip the login and the registry entirely: they just uselatestas-is. The gate also runs onschedule(the old filters were skipped there, #6403), so any gap self-heals on the nightly at the latest.The image inventory itself is also single-sourced: the per-family
matrix/docker-images-{linux,emscripten-c-bindings,linux-vcpkg}.jsonfiles are read both by the gate script (viajq) and by thecompute-image-matricesjob inprepare-images.yml, which derives the build matrices from them. Adding or removing an image is a one-file change; the gate and the builders cannot drift.need_windows_vcpkg_rebuildis untouched — the Windows vcpkg image is not checksum-tagged.Notes
need_rebuild=false, the case that previously spun up the full matrix.thirdparty/licenses/**checksum inputs are deliberately unchanged: narrowing them would invalidate every existing tag. With the registry as source of truth a too-broad checksum only costs an occasional no-op-content rebuild; trimming the input list can be a follow-up.Review follow-up: matrix-builder reads the inventory files directly
Per review,
matrix-builderlearned to load the base matrix from a file, andcompute-image-matricesnow uses that instead of rawjq:matrix:matrix-file(path of a YAML/JSON file holding the base matrix). The base matrix now also accepts the include-list form: a list of entries used verbatim. Amatrix-keyselector existed while the inventory was one bundled file and was removed per review once the files were split per family.dist/rebuilt; README updated.matrix-builderstep over its own inventory file; the linux step adds per-labelexcluderules. The jqstartswith("ubuntu")filter became explicit per-distro rules, so a new ubuntu distro indocker-images-linux.jsonneeds a matching rule line (commented in the workflow).emscripten-c-bindings/linux-vcpkgjobs consume the flat entry lists asmatrix.include, so their cell display names now include the runner field (e.g. "(3.1.38, ubuntu-latest)").🤖 Generated with Claude Code