STF-1287: Build and publish PIE precompiled binaries on tag

[claude]

Requested by Greg Oschwald · Slack thread

Since this was first opened. maxmind/MaxMind-DB-Reader-php#265 merged, so
the submodule now carries ext/libmaxminddb and the bundled build mode. Three
things follow, and they are why the branch was rebuilt into four commits:

• The Linux and macOS lanes build for the first time. Until the bump they
  skipped on every run, because setup found no ext/libmaxminddb in the
  submodule tree. With them live, build-ext.sh past its guard,
  gate-extension.sh, stage-asset.sh, the clean-container load and
  collect-assets.sh's BINARIES=true arithmetic have all now executed. The
  measured glibc floor is 2.33 against the documented 2.36 ceiling.
• The gate and the load check come from the submodule. This repository had
  its own copies; they had drifted to 95 lines against 234 and still carried
  two fail-open bugs the consolidation fixed. setup publishes the
  libmaxminddb version so every load check asserts MMDB_LIB_VERSION, Windows
  included. gate-extension.ps1 stays here — it has no upstream counterpart.
• workflow_dispatch now checks out the tag it is given. No checkout
  passed a ref:, so a re-cut built the dispatching branch on the Linux, macOS
  and tarball lanes while Windows correctly built the tag, and the mixture was
  published under the tag's name.

Today pie install maxmind-db/reader-ext always compiles from source, so a user
needs a toolchain and libmaxminddb-dev headers. After this, PIE downloads a
prebuilt maxminddb.so on Linux (x86_64/arm64) and macOS (arm64) for PHP
8.2–8.5, NTS and ZTS, plus Windows — and still falls back to building from
source everywhere else, unchanged.

The one line that enables it is "download-url-method": ["pre-packaged-binary", "pre-packaged-source"]. PIE tries the methods in order, so an uncovered
platform misses the binary and falls through to the tarball.

.github/workflows/release.yml runs on v* tags, on pull requests, and on
workflow_dispatch (with a tag input, to re-cut assets without re-tagging, or
without one for a dry run). A tag stages everything onto a
draft release — assets cannot be added to a published, immutable release — and
only un-drafts at the end. Each Linux/macOS build is gated before it is staged by
dev-bin/gate-extension.sh from the submodule, so the objects published
here and the ones maxmind/MaxMind-DB-Reader-php tests are held to one
implementation of the same bar: nothing but the C runtime in NEEDED, no
RUNPATH/RPATH, no undefined or exported MMDB_ symbols, get_module still
exported, no GLIBC_PRIVATE, a measured glibc floor within the documented
maximum, and a load-and-query of a real database in a clean container. Windows is gated too: the
php_maxminddb.dll it built must export get_module, and must load under php -n and answer a real lookup.

The source tarball PIE falls back to is built from an explicit member list: the
extension's own sources and LICENSE, plus only the libmaxminddb sources
config.m4 compiles, the headers it puts on the
include path, and that library's LICENSE and NOTICE, which Apache-2.0 §4(d)
requires a redistribution to carry. A bare glob would publish the vendored
checkout's gitfile, CI configuration, test suite and nested test-data
submodules — 101 entries where 16 are needed — and the verification step now
asserts that. The layout stays flat, with config.m4 at the archive root,
because that is where PIE looks; the extension's LICENSE lives a level above
ext/, so tar appends it with -C .. and it lands flat there too.

Every binary lane builds from the submodule tree, so a source-build job
builds the staged tarball itself: extracted into an empty directory, in a
container with only that directory mounted and no system libmaxminddb to stand
in for a source the tarball failed to ship, then loaded for a real lookup.
Without it, a libmaxminddb reshuffle would break source-installing users while
every other gate stayed green. It runs alongside the binary lanes, and
publish waits for it.

publish asserts the exact expected asset count and refuses to publish if it
does not match. That assertion is deliberate: the Python repo once shipped a
release with an sdist and zero wheels because an artifact glob silently matched
nothing.

Everything except the release itself also runs on pull requests, so a change
to this workflow is tested by the pull request that makes it rather than by the
next tag. A pull-request run builds and gates every lane, builds the tarball and
builds from the tarball, and asserts the asset count in a verify-assets job
— running the same dev-bin/collect-assets.sh that publish runs immediately
before uploading, so the assertion cannot rot into release-only code. What it
skips is everything that writes: draft-release, the upload/attest/un-draft
steps, and smoke, which installs from Packagist and so would have nothing to
install. contents: write and id-token: write belong to publish and
draft-release alone and neither is reachable without a tag, which also means a
fork's pull request never needs a permission it cannot be given.

There is no tag on a pull request, so setup stands in
v0.0.0-pr<number>-<short sha>. That goes through the same sanitisation regex a
real tag does rather than around it, and it keeps the leading v, because PIE
matches assets on Composer's v-prefixed pretty version and the naming code must
have one rule and not two. The Windows lane clones MaxMind-DB-Reader-php by
ref and no tag of that name exists there, so on a tagless run it builds the
commit the submodule points at — which is what every other lane builds, and is
also what decides whether it gets the bundled flag.

Pull requests build a reduced matrix: one PHP version instead of four, so 11
assets instead of 41. Both TS modes, both Linux architectures, macOS and Windows
all still run, because those are the legs whose steps actually differ; only PHP
versions are dropped, which costs no coverage of the workflow itself. The knob is
a single assignment in setup, immediately below the full lists it narrows —
widen it by adding versions there. A tag always builds the full matrix, since
publish refuses to ship a partial release, and workflow_dispatch takes a
full-matrix input to ask for the whole thing on a branch. setup is still the
only place both the matrices and the count assertion read from, so a reduced run
asserts a reduced count. Superseded pull-request runs are cancelled; a release in
flight is not.

The build, gate, load-and-query and staging shell lives in dev-bin/ rather
than duplicated per-OS in the workflow, so there is one copy of each and it can
be run outside Actions. Windows shares neither the tools nor the shell with the
Unix lanes, so its gate is a sibling script there, gate-extension.ps1, rather
than a branch inside the bash one.

Linux binaries are built in digest-pinned php:{version}-{cli,zts}-bookworm
containers rather than on the runner, so the glibc floor (2.36) is a reviewable
choice that cannot drift with runner images. The 2.38 floor reported for the
bundled sources is what building outside those containers costs: glibc 2.38
headers redirect the strtol libmaxminddb calls to
__isoc23_strtol@GLIBC_2.38 and PHP compiles with -D_GNU_SOURCE, which
enables the redirect. Inside bookworm that symbol does not exist, and the gate
fails closed if a build ever escapes. arm64 comes from a native
ubuntu-24.04-arm runner, not qemu. macOS has no container equivalent, so the
floor there is a pinned MACOSX_DEPLOYMENT_TARGET, asserted against
LC_BUILD_VERSION.

Dependencies, both in maxmind/MaxMind-DB-Reader-php:

• Every binary lane needs the --with-maxminddb-bundled configure flag (PR in
  flight, branch stf-1287-…), which compiles libmaxminddb's vendored sources
  into the extension so the .so needs only libc. Until the submodule points at
  a tag that has it, a tag run fails fast with an explicit message and a
  pull-request run skips the Linux and macOS lanes with a warning — setup
  looks for ext/libmaxminddb in the submodule's tree, and consults that only
  when there is no tag. A pull request should not be red for a dependency it has
  nothing to do with; a release whose binaries are silently missing is worse than
  no release. The expected asset count follows the same flag, so a run with the
  lanes skipped still asserts the assets it did expect.
• The Windows lane reads the same condition but cannot skip on it, because it is
  the only source of the windows-binary assets. With the vendored sources present
  it passes --with-maxminddb-bundled and asks for no libraries at all; without
  them it falls back to fetching the libmaxminddb PHP publishes, which is 1.5.0
  and needs #263 so that config.w32 accepts the lib-prefixed name that
  libmaxminddb-1.5.0-vs16-*.zip actually ships. Either way it produces the same
  assets, so the count does not move.

dev-bin/release.sh there must also stop building the tarball and creating the
release itself and just push the tag, or the two will collide. That is a
follow-up PR in the other repo, not this one.

This workflow is now exercised by its own pull request, so the run on this
branch is a dry run of everything but the release. Publishing itself still first
happens for real on a tag; a workflow_dispatch against an existing tag remains
the cheapest way to smoke that part. What was verified statically: actionlint,
zizmor (auditor persona), shellcheck over dev-bin/, php -l, and bash -n on every run: block extracted with a YAML parser are all clean; every job
reachable on a pull request was confirmed to hold no write permission and no
reachable gh release command or attestation; every pinned action SHA was
checked against
the tag named in its comment; composer.json is valid JSON and the php-ext
block matches PIE's documented schema; all eight pinned image digests were
confirmed to exist as multi-arch indexes with amd64 and arm64; the tarball was
reproduced in both submodule states, before and after the bundled-sources PR,
with LICENSE asserted at the archive root in each, and the source-build job
was run against both — building and querying in the post-PR state, skipping
with a warning in today's, and failing as intended when a header is withheld
from the tarball; the glibc claim was measured rather than assumed, compiling
libmaxminddb with and without -D_GNU_SOURCE (strtol@GLIBC_2.2.5 versus
__isoc23_strtol@GLIBC_2.38, the only symbol in the built object above 2.36)
and against a bookworm glibc 2.36 sysroot, where the floor drops to 2.33; and
the asset-count assertion was simulated for both the full and the reduced matrix
over a realistically shaped dist/ tree, with negative cases for each
platform's assets missing, a mismatched tarball name and two artifacts sharing a
filename, and the pull-request shape in which the binary lanes are skipped; the
version resolution and the bundled-sources probe were run through every event
shape -- tag push, dispatch with and without a tag, pull request -- including
two tags the sanitisation must reject; and the staging cross-checks and the
lookup assertion were each exercised locally, negative cases included.

The bundled lanes were exercised for real before this merges. They gate on
setup's binaries output, which probes the submodule for ext/libmaxminddb,
and the submodule points at v1.13.1, which has neither those sources nor the
bundled mode in config.m4/config.w32. So on this branch as it stands Linux
and macOS skip and Windows takes the fetched-library fallback — which meant the
bundled path's first real execution would have been a live release, since
dev-bin/release.sh moves that pointer to the new tag as part of cutting one.

So the pointer was temporarily moved to the --with-maxminddb-bundled branch of
maxmind/MaxMind-DB-Reader-php (#265), the full set of lanes was run, and the
pointer was reverted. Those two commits are deliberately not part of this branch
— its tree is byte-identical to before they were made — so the evidence is here
instead.

The first attempt found a real defect, which is the entire reason for doing this
early: all four Windows legs, x64 and x86, nts and ts, built and linked
successfully and then produced a DLL PHP refused to load —

Warning: PHP Startup: Invalid library (maybe not a PHP library)
'...\x64\Release\php_maxminddb.dll' in Unknown on line 0

which is what PHP says when get_module is not exported. ext/maxminddb.c and
ext/libmaxminddb/src/maxminddb.c share a basename, and while ADD_SOURCES
normally derives each object's directory from its first argument — how php-src's
ext/gd builds libgd/gd.c beside gd.c — under phpize confutils.js takes a
branch that ignores it and uses only the directory part of each file_list
entry. Ours are bare filenames, so both resolved to $(BUILD_DIR)\maxminddb.obj,
the makefile got two rules for one target, and the extension's own object lost.
Fixed in #265 by passing the object directory explicitly; it needs no change
here.

With that fix, run
30840832024
was green across all fourteen build and verification jobs, the three release-only
jobs skipping as designed:

• Windows x64 and x86, nts and ts: options: --with-maxminddb --with-maxminddb-bundled with php_libraries: and extension_libraries:
  both empty, so nothing was fetched and the link line carries no libmaxminddb
  import library — the vendored 1.13.3 sources really are compiled in, where the
  fallback would have given Windows the 1.5.0 PHP publishes. All three objects
  now compile to distinct paths, and the extension's own test suite goes from
  2 skipped plus 1 failed to Tests passed: 3 (100.0%).
• Linux x86_64 and arm64, nts and zts: NEEDED: libc.so.6 alone, no
  RUNPATH, no undefined MMDB_, and Measured glibc floor: 2.33 (documented maximum 2.36) — the number this description predicts for a bookworm build,
  measured rather than assumed. The ceiling did not fire and MAX_GLIBC was not
  touched.
• macOS arm64, nts and zts: otool -L lists only
  /usr/lib/libSystem.B.dylib, no LC_RPATH, no undefined _MMDB_, and
  Measured minimum macOS: 11.0 (documented maximum 11.0).
• Every Linux, macOS and source-build object loaded and queried a real database
  in a clean container under php -n: ok maxminddb 1.13.1 on PHP 8.4.24 (nodeCount=1453).
• source-build: the tarball carried libmaxminddb this time, so the job
  stopped skipping and built from the tarball alone, in a container asserted to
  have no system libmaxminddb.
• verify-assets: Expecting 11 assets, found 11 — 1 tarball, 4 Linux, 2
  macOS, 4 Windows, exactly the reduced pull-request matrix.

The run after the revert skips the binary lanes again, as before.

Nothing in the Windows lane asserted that failure directly. 001-load.phpt and
002-final.phpt both --SKIPIF-- on !extension_loaded('maxminddb'), so they
skipped rather than failed; only 003, which has no --SKIPIF--, failed. A build
whose release asset PHP could not load reported 2 skipped, 1 failed, which is far
too quiet for something a release depends on. 003's failure does reach the
step's exit code, though, which turns out to matter for what this gate can be
shown to do -- see the last two paragraphs of this section.
The lane now gates the DLL it built. dumpbin /exports must show
get_module, and dev-bin/verify-extension.php must load that DLL under php -n and complete a real lookup with it. Both are fatal, which is the point: PHP
only warns about a library it could not use and leaves the exit code at 0.
dumpbin ships with MSVC but is not on PATH outside a developer prompt, so the
gate asks vswhere where it went. There is deliberately no Windows analogue of
the ldd check — the libmaxminddb PHP publishes for Windows is a static .lib
with no DLL beside it, so the extension imports no libmaxminddb whichever way it
was built, and an assertion that holds in both modes would say nothing about
which one ran. Dropping those two --SKIPIF-- guards is the general form of the
fix and belongs in maxmind/MaxMind-DB-Reader-php, so it is a follow-up there.

The gate was exercised in both Windows build modes, since a pull request only
ever reaches one of them.

Run
30842452545
is this branch as it stands: the pointer at v1.13.1, so binaries=false and
Windows takes the fetched-library fallback. All four legs green, and the gate
ran on each rather than skipping:

Gating D:\a\...\ext\386c2e09\x64\Release\php_maxminddb.dll with D:\a\...\ext\386c2e09\php-bin\php.exe
Reading exports with C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.29.30133\bin\HostX64\x64\dumpbin.exe
get_module export: 1    0 00001D50 get_module = get_module
Loading the extension and querying a database
ok maxminddb 1.13.1 on PHP 8.4.24 (nodeCount=1453)

Run
30842932447
moved the pointer to 1643998 again so that binaries=true and Windows took the
bundled build — the mode a release will use. Green across all fourteen build
and verification jobs, Expecting 11 assets, found 11, and the gate on all four
legs there too:

Gating D:\a\...\ext\d0f50265\x64\Release\php_maxminddb.dll with D:\a\...\ext\d0f50265\php-bin\php.exe
Reading exports with C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.29.30133\bin\HostX64\x64\dumpbin.exe
get_module export: 1    0 00001D50 get_module = get_module
Loading the extension and querying a database
ok maxminddb 1.13.1 on PHP 8.4.24 (nodeCount=1453)

x86 reports the same through its decorated name, get_module = _get_module, and
ts through Release_TS\php_maxminddb.dll. vswhere located dumpbin on all
eight legs; nothing degraded to a warning. Nothing in the gate is conditioned on
the build mode, and there is nothing in it that could only hold in one. The
pointer was reverted straight afterwards, so the tree is once again identical to
what it was before those two commits and git ls-tree HEAD MaxMind-DB-Reader-php
is back at 2194f58.

Its rejection path, though, is still unobserved. Run
30844570591
moved the pointer to b969d83 -- 1643998 with that fourth ADD_SOURCES argument
taken back off -- to watch the gate reject a DLL it ought to reject. The defect
reproduced exactly, and the makefile said so out loud:

NMAKE : warning U4004: too many rules for target '...\x64\Release\maxminddb.obj'
...\maxminddb.c                        -> ...\x64\Release\maxminddb.obj
...\libmaxminddb\src\maxminddb.c      -> ...\x64\Release\maxminddb.obj
...\x64\Release\maxminddb.obj : warning LNK4042: object specified more than once; extras ignored
EXT maxminddb build complete
√ maxminddb Extension maxminddb built successfully

Both translation units claimed one object, libmaxminddb's compiled second and
won, and the link then succeeded anyway with only an LNK4042. So the build
proper did succeed, exactly as the original report describes. What follows is
still inside the same Build the extension step, because the action runs the
phpt suite there too:

Warning: PHP Startup: Invalid library (maybe not a PHP library) '...\x64\Release\php_maxminddb.dll'
FAIL openbase_dir is followed [...\tests\003-open-basedir.phpt]
Tests skipped   :     2 ( 66.7%)
Tests failed    :     1 ( 33.3%)
✗ maxminddb Failed to run tests successfully
##[error]Process completed with exit code 1.

So the gate never ran. Invoke-Tests.ps1 throws, the step goes red on all four
legs, its own Upload the build artifact is skipped, and Gate the built
DLL -- which carries no if:, so it defaults to success() -- is skipped too.
The distinction that matters: the build succeeded and the step still failed, so
the gate was never reached.

So the defect is caught today, but by the action's own test run rather than by
this gate. Run
30840224616,
the same defect before the gate existed, failed the same step on the same four
legs, and neither run uploaded a Windows artifact -- 7 apiece, none of them a
DLL, against 11 on the green bundled run above. The gate is still worth keeping:
it asserts under php -n with no php.ini to help, and it does not depend on the
action continuing to run tests for us. But it has not been shown to stop
anything, and the honest summary is that it is unproven rather than proven.
Giving the step if: ${{ !cancelled() }}, so it also runs when the build step
fails, is what would both put it in front of this defect and make its rejection
path observable; that is a change to the gate's wiring and belongs in its own
commit, so it is a follow-up.

---

Generated by Claude Code

[oschwald]

Claude here, on behalf of Greg. Pushed three fixup commits from a review pass.

fixup! Ship only the sources a build needs in the release tarball — the member-list step had no pipefail. An inline run: gets GitHub's default bash -e {0}, and the list is generated on the left of a pipe, so a find that fails partway through kills the group, the remaining members are never emitted, sort still exits 0 and the step produces a silently incomplete tarball. Reproduced locally: with ext/libmaxminddb/ present but empty, the pipeline exits 0 and the member list loses every libmaxminddb entry including LICENSE and NOTICE. Both downstream guards then degrade to warnings on exactly that input, so the run stays green. Also switches [ -d libmaxminddb ] to [ -f libmaxminddb/src/maxminddb.c ], matching the other three places that ask the same question — the -d form is what lets a half-initialised submodule through.

fixup! Build from the release tarball on every run — source-build decided whether to test itself by asking the tarball, so the one condition under which it would fail was the condition that made it skip, and that skip was not scoped to non-release runs. It now uses needs.setup.outputs.binaries as the oracle: a tarball missing sources the submodule does carry is an error rather than a warning.

fixup! Build and publish PIE precompiled binaries on tag — gh release view succeeds for published releases too, so draft-release would print "reusing it" for one and publish would then --clobber into a live release. This asserts isDraft, and distinguishes a 404 from auth/rate-limit/5xx failures rather than treating every error as "does not exist".

Two other gate fixes from the same pass went to maxmind/MaxMind-DB-Reader-php#265 instead, which now carries a shared dev-bin/gate-extension.sh that both repos can run. Follow-up once that merges and the submodule bumps: delete this repo's dev-bin/gate-extension.sh and call the submodule's. The two copies had already drifted — this one wrapped the glibc measurement in || true and skipped the assertion on an empty result, so a missing objdump certified the binary; #265's copy always treated that as fatal.

Not addressed here: workflow_dispatch with a tag builds github.ref rather than the tag, because no checkout passes ref:. Dispatching tag: v1.13.1 from main would build main's HEAD on Linux/macOS/source while Windows correctly builds the tag, then publish it all as v1.13.1. It needs ref: on all five checkouts, which felt too invasive to land unannounced while this branch is moving.

[horgh]

I wonder if 8.5 would make sense. Maybe that other spot would need to be bumped? Just thinking maybe using the latest in such cases would be desirable.

[oschwald]

Done — the reduced matrix is now ["8.2","8.5"] rather than ["8.4"], and
source-build's pinned image moves to the 8.5 digest so it stays a version the
matrix builds ("that other spot" was exactly right).

Went with both ends rather than just the latest because of a gap this exposed:
the submodule has no macOS job at all, and upstream's Windows job is
x64/nts/8.4 only, so whatever the reduced list omits is first compiled for those
platforms on a tag — where publish refuses a partial release, so the tag then
blocks on the least-tested leg. 8.2 and 8.5 are where an ABI break shows up.

Six of the eight pinned image digests were also never pulled on a pull request,
so a GC'd digest would have passed CI and failed the release. Two of those six
are now exercised.

Dry-run asset count goes 11 → 21.

🤖 Claude, replying on behalf of Greg.

[horgh]

I wonder if dependabot will be able to bump this (or if it should).

[oschwald]

Partly. There was no .github/dependabot.yml in this repository at all, so
nothing was being bumped — that's now added, covering github-actions and
gitsubmodule on the same schedule the php repo uses.

It will not bump this line. Dependabot's docker ecosystem reads Dockerfiles
and compose files, not image references sitting in a run: block or, at line 538,
inside a shell case. Same for the ubuntu:24.04 digest in the smoke job and the
pie.phar URL.

Rather than leave that to be rediscovered, the new dependabot.yml ends with a
comment listing the three things it deliberately does not cover and why. Worth
noting that editing those digests is how the glibc floor is raised, so their
being manual is the intended design — but nothing prompts it, which is the real
gap.

🤖 Claude, replying on behalf of Greg.

[horgh]

Some of these comments (above too) are phrased expecting the other PR to not be merged yet. Would updating them make sense?

[oschwald]

Fixed, though not the way this suggests — the branch those comments describe is
gone.

The whole binaries=false fallback turned out to be unreachable: setup's
"Check out the bundled libmaxminddb" step runs git submodule update --init ext/libmaxminddb with no guard, so a submodule tree lacking that gitlink exits 1
there, two steps before binaries=false is computed. The warning, the lane
skips, source-build's bundled=false branch, the Windows system-library
fallback and binary_count=0 were all dead, along with about forty lines of
comments across two files.

So it is deleted rather than reworded, and replaced by an explicit precondition
with a legible message. One of those dead branches was also a route by which a
release could ship the tarball and no binaries at all.

🤖 Claude, replying on behalf of Greg.

[horgh]

Similar comment to the above regarding dependabot for these here.

[oschwald]

See my reply on line 371 — same answer, and this case is the specific place Dependabot cannot reach.

🤖 Claude, replying on behalf of Greg.

[horgh]

I wonder if this could be in mise or something.

I guess the apt commands above may need bumping sometimes too.

Somewhat related to other comments, I see we're using 8.3 for those apt packages.

[oschwald]

Three separate things here, and the honest answer differs for each.

pie.phar — still fetched by release tag, not digest. mise would not help:
this runs inside the smoke container, which is deliberately minimal (no
compiler, no phpize) precisely so a missing prebuilt asset cannot be papered over
by a source build. Installing mise there would work against that. What I did fix
is the comment: "Non-Windows prebuilt binaries need PIE >= 1.4.0" sat above pie --version, which asserts nothing — the constraint is actually satisfied by the
hardcoded 1.4.9 in the URL, which carried no comment at all. The note now sits
on the pin.

The apt packages — php8.3-cli is what ubuntu:24.04 ships. Moving to 8.4 or
8.5 means adding the ondrej PPA, which is a third-party repository in the one
container whose whole purpose is to be a clean, minimal target. The PHP version
here is also not really under test: what is under test is whether PIE can resolve
and install a prebuilt asset without a compiler present.

Bumping generally — the smoke image digest and this apt line are both in the
"Dependabot cannot see it" list I added to dependabot.yml.

🤖 Claude, replying on behalf of Greg.

[oschwald]

You're right and my answer was wrong — done in 1fea279.

My objection assumed mise would have to run inside the smoke container. It doesn't. mise runs on the runner; only the phar crosses over, mounted into the directory the job already mounts. The container is unchanged — barer, actually, since the curl binary was only ever installed to fetch pie and is now gone.

[tools."github:php/pie"]
version = "latest"
asset_pattern = "pie.phar"

mise.lock now carries the sha256, and mise verified GitHub's artifact attestations on download — same as the php repo's composer.phar entry, which is where I should have looked first.

asset_pattern matters more than it looks: that release also ships pie-Linux-X64 and friends, which platform autodetection prefers, and those are static binaries bundling their own PHP. pie install has to target the PHP a user would install into, so the phar run by the container's PHP is the only asset that tests the right thing.

Two things fell out of it:

• pie --version is now an assertion. PIE ≥ 1.4.0 is a real constraint and that line checked nothing. Verified it accepts 1.4.0/1.4.9/1.10.0/2.0.0 and rejects 1.3.7/1.3.13.
• Caching is off on mise-action here. It's on by default and zizmor flags it as cache poisoning in a workflow that publishes release artifacts. links.yml keeps its cache; it publishes nothing.

On automation: Dependabot has no mise ecosystem, so nothing bumps pie until the Renovate migration — Renovate's mise manager does understand the github: backend. Worth noting Renovate could also take the container image digests through a regex manager, which is the thing Dependabot cannot be configured into doing at all. I've recorded both in dependabot.yml.

On the PHP version: I checked, and I don't think mise can help here yet. Both backends compile from source — asdf:mise-plugins/asdf-php, and vfox:mise-plugins/vfox-php, whose README says "Compiles from source: Uses official PHP source releases" and lists autoconf/bison/re2c as prerequisites. A compiler is exactly what this container must not contain. I also searched mise's release notes from v2026.7.14 to v2026.8.2 and found no precompiled-PHP change; if you've seen something more recent, point me at it and I'll revisit.

So php8.3-cli stays, and I've documented why: it's ubuntu:24.04's own, anything newer means a third-party apt repository in the one image whose value is being minimal, and the PHP version is incidental to what the job proves.

Caveat worth stating plainly: smoke only runs on a tag, so none of this is exercised by CI going green. That was already true, and this change doesn't alter it.

🤖 Claude, replying on behalf of Greg.

[horgh]

Similar to above comments re: image tag / dependabot.

[oschwald]

Covered in my reply on line 908 — the image digest is in the manual list in the new dependabot.yml, and the pie.phar comment has moved onto the pin that actually enforces the constraint.

🤖 Claude, replying on behalf of Greg.

[horgh]

A review from Claude.

[horgh]

Critical: this reuse path is unreachable — REST cannot find draft releases by tag.

GET /repos/{owner}/{repo}/releases/tags/{tag} is documented as "Get a published release with the specified tag". Drafts have no git tag until publication. gh's own pkg/cmd/release/shared/fetch.go says it outright: "First use GraphQL to find a draft release by pending tag name, since REST doesn't have this ability."

So jq -r .draft can never return true and the "Draft release $TAG already exists; reusing it." branch below is dead code.

The published-release guard does work, but for a different reason than the comment gives: if this endpoint returns anything at all it is published, so the != true branch correctly refuses.

What breaks is the re-cut workflow — the stated purpose of workflow_dispatch with a tag. An existing draft 404s, so this falls through to gh release create "$TAG" --draft. GitHub does not require draft tag names to be unique, so that likely creates a second draft rather than 422ing. publish then resolves the draft via GraphQL (gh release upload/edit do see drafts) and picks one — so assets can land on one draft while the other gets un-drafted.

Suggest resolving the draft the way gh does (gh release view "$TAG" --json isDraft, or list-and-filter) while keeping the 404-vs-other-failure discrimination, which is the genuinely good part of this block.

I did not verify this end to end, because doing so would mean creating a real draft release on the repo.

🤖 Comment by Claude (Claude Code) on behalf of Will.

[oschwald]

Confirmed and fixed. GET /releases/tags/{tag} returns published releases only,
so jq -r .draft could never be true and the reuse branch was dead.

Both questions are now asked with the mechanism that can answer them:

• Published still uses that endpoint, which is exact and needs no pagination
  — but on the correct grounds, as you say: if it returns anything at all the
  release is published, so there is nothing to inspect.
• Drafts are found by listing, which is what gh falls back to GraphQL for.

The 404-vs-other-failure discrimination is kept on both.

The re-cut hazard is handled explicitly rather than incidentally: more than one
draft on a tag is now its own error naming the situation, instead of something
publish resolves arbitrarily.

Also fixed here, which your comment on php#266 line 249 found: this was the only
job in the workflow with no checkout, and --notes-from-tag reads the annotation
from a local clone — so it always failed and the notes were always GitHub's
generated commit list. It now checks the tag out.

🤖 Claude, replying on behalf of Greg.

[horgh]

Critical: on a release, Windows builds a different commit than every other lane.

ext_ref is passed to php/php-windows-builder, which clones maxmind/MaxMind-DB-Reader-php — a different repository from this shim. On a release that is upstream's tag $TAG, while Linux, macOS and the source tarball all build this repo's submodule pointer. Nothing asserts they are the same commit.

Verified: the submodule currently points at 0b33422, which git describe reports as v1.13.1-102-g0b33422 — an untagged commit, 102 commits past v1.13.1. For the two to agree, someone must tag upstream at exactly the submodule commit.

The MMDB_VERSION check cannot catch this: it compares libmaxminddb's PACKAGE_VERSION, not the extension's own C sources.

The else branch two lines down already does the right thing. Suggest using the submodule SHA on releases too, or asserting git rev-parse HEAD:MaxMind-DB-Reader-php equals upstream's $TAG in setup. As written this is the same "mixture published under the tag's name" class of bug that lines 152-158 claim to have fixed.

🤖 Comment by Claude (Claude Code) on behalf of Will.

[oschwald]

Fixed by removing the divergence rather than asserting it away — as you point
out, the else branch was already doing the right thing.

ext_ref is now the submodule pointer on every run, including releases. That is
this repository's own statement of what the release contains, and the dry-run
path has been handing php/php-windows-builder a bare commit on every run, so a
SHA is already proven to work as extension-ref.

Your note that MMDB_VERSION cannot catch this is what decided it: an assertion
comparing the two would have to be right about which one is authoritative, and
the pointer plainly is.

🤖 Claude, replying on behalf of Greg.

[horgh]

Critical: the "refuse a partial release" assertion can be satisfied by a tarball-only release.

expected is derived from the same needs.setup.outputs.* values that gate whether the binary lanes run at all (release.yml:487, 598). When a lane does not run, the expectation shrinks by exactly the assets it would have produced — the two faults always agree.

Verified by running this script against a dist/ containing only the source tarball:

BINARIES	result
''	Expecting 1 assets, found 1 → exit 0
false	Expecting 1 assets, found 1 → exit 0
true	Expecting 7 assets, found 1 → exit 1

Today the release path is protected solely by the if at release.yml:127 forcing binaries=true. One output rename, one added if: on setup, or a value spelled True turns a skipped lane into a green, published, tarball-only release — verify-assets tolerates skipped needs by design (release.yml:795), and this script lowers its own bar to match.

Suggest asserting the expected asset names rather than a scalar count. Every input needed to generate all 32 Linux/macOS filenames is already in scope here.

🤖 Comment by Claude (Claude Code) on behalf of Will.

[oschwald]

The mechanism you identified is gone, though the name-based assertion is not
done.

BINARIES no longer exists. The whole fallback was unreachable — see my reply on
line 65 — so binary_count is now computed unconditionally and the expectation
can no longer shrink to match a lane that did not run. That closes the specific
"two faults always agree" coupling.

Asserting the expected filenames rather than a count is the stronger fix and it
is not done; deferred deliberately with Greg. Recording it here as still
open, along with the WINDOWS_COUNT sibling below, which is fixed.

🤖 Claude, replying on behalf of Greg.

[horgh]

The invariant this comment asserts is never actually checked.

"setup only allows this on a non-release run" is true of release.yml:127 today, but this script is not told whether it is on a release run — RELEASE is not in its environment in either caller (release.yml:812-820, 849-857). So the comment documents an assumption rather than an enforced property, and the branch below is the mechanism by which a release can ship with zero binaries.

Suggest passing RELEASE and making it explicit:

if [ "$RELEASE" = true ] && [ "$BINARIES" != true ]; then
    fail "Release run with BINARIES=$BINARIES; the binary lanes did not run."
fi

🤖 Comment by Claude (Claude Code) on behalf of Will.

[oschwald]

Fixed by deletion — RELEASE never needed passing because the branch it guarded is gone. See line 65.

🤖 Claude, replying on behalf of Greg.

[horgh]

Critical: an empty WINDOWS_COUNT silently becomes 0 here, even under set -euo pipefail.

Verified:

$ ( set -euo pipefail; WINDOWS_COUNT=""; binary_count=0; expected=$((1 + binary_count + WINDOWS_COUNT)); echo "expected=$expected" )
expected=1
exit=0

set -u does not fire because the variable is set-but-empty, and arithmetic coerces it to 0. So any path that delivers an empty WINDOWS_COUNT to publish expects zero Windows assets — and if none arrived, the count matches and the release ships without them.

This is a second, independent route to the same outcome as the BINARIES finding above. Suggest requiring a positive integer:

case "$WINDOWS_COUNT" in ''|*[!0-9]*) fail "WINDOWS_COUNT is '$WINDOWS_COUNT', not a count." ;; esac

🤖 Comment by Claude (Claude Code) on behalf of Will.

[oschwald]

Confirmed and fixed. set -u does not fire on set-but-empty and arithmetic
coerces it to 0, exactly as you show.

collect-assets.sh now requires a count before using one:

case "${WINDOWS_COUNT:-}" in
'' | *[!0-9]*) fail "WINDOWS_COUNT is '${WINDOWS_COUNT:-}', which is not a count." ;;
esac

Verified against '', null, 1 6, -1 and 16. The upstream guard in
windows-matrix is fixed too — see your comment on line 698, which is the same
bug one step earlier.

🤖 Claude, replying on behalf of Greg.

[horgh]

This include: only annotates rows because linux-arches is always both values.

A matrix include: entry whose values do not match an existing combination adds a leg rather than annotating one. Harmless as written, but collect-assets.sh:37 computes arch_count from linux-arches, which implies that list is meant to be variable. If it were ever narrowed to one arch, the non-matching include entry would create a phantom row with no php-version or ts — and the product arithmetic in collect-assets.sh does not model added rows, so the count would silently disagree.

That slightly weakens the "single source of truth" claim at 135-136.

🤖 Comment by Claude (Claude Code) on behalf of Will.

[oschwald]

Fixed as a comment. The include: entries now say they annotate rather than add
only because linux-arches always holds both values, that an entry matching no
combination adds a leg, and that collect-assets.sh models a product rather than
added rows — so narrowing the list means fixing the arithmetic in the same
change.

Left as documentation rather than code because the coupling is real and the
comment is where someone narrowing that list will be looking.

🤖 Claude, replying on behalf of Greg.

[horgh]

os and libc are the only name components not cross-checked against the machine.

abi, ts_suffix and arch are all verified against what the build and the runner report — which is the good instinct that makes the Windows gap visible. But os and libc come from the call site (linux glibc / darwin bsdlibc) and are never checked against uname -s, so a copy-paste error in the workflow ships macOS binaries named -linux-glibc.

Partly caught today by the duplicate-filename guard in collect-assets.sh:32 — but only because the arch strings happen to collide. A future linux musl lane would sail through. uname -s is one more case.

🤖 Comment by Claude (Claude Code) on behalf of Will.

[oschwald]

Correct, and not fixed. Recording it as open.

The reasoning is worth keeping: abi, ts_suffix and arch are all cross-checked
against what the build and the runner report, which is precisely what makes the
os/libc gap visible — and the duplicate-filename guard only masks it today
because the arch strings happen to collide, which a linux musl lane would end.
uname -s is one more case.

🤖 Claude, replying on behalf of Greg.

[oschwald]

Now fixed in cbb7c23 — I'd marked this open, but it was small enough that leaving it wasn't worth it.

os comes from uname -s. libc can't, so the pairing is asserted instead, since the set of libcs per OS is small and fixed:

case "$expected_os-$libc" in
linux-glibc | linux-musl | darwin-bsdlibc) ;;
*) fail "'$libc' is not a libc that occurs on $expected_os." ;;
esac

Nothing distinguishes glibc from musl on Linux — there's no musl lane, and gate-extension.sh's glibc floor measurement only succeeds against glibc, which corroborates the one combination we actually build. linux-musl is allowed so that adding the lane you mention doesn't require touching this.

Verified all four cases: a darwin name on a Linux runner rejected, linux gnu rejected (a plausible mistake, since gnu is what several toolchains call it), linux glibc and linux musl accepted.

Your framing is what made it worth doing rather than noting — that the duplicate-filename guard catches this today only because the two lanes' arch strings happen to collide is exactly the kind of accidental coverage that disappears without anyone noticing.

🤖 Claude, replying on behalf of Greg.

[horgh]

Minor asymmetry: this regex accepts a trailing dash, and mmdb_version gets no regex at all.

^[A-Za-z0-9][A-Za-z0-9._+-]*$ permits a trailing -, so a truncated GITHUB_SHA yields v0.0.0- and passes. Harmless for filenames, noted only because this guard is otherwise careful.

The more substantive half: mmdb_version (lines 168-173) gets only an emptiness check despite being interpolated unquoted into a pwsh command line at 781. Given the care taken over TAG here, closing that gap seems worth it — see my comment on 781.

🤖 Comment by Claude (Claude Code) on behalf of Will.

[oschwald]

Both fixed. The regex is now
^[A-Za-z0-9]([A-Za-z0-9._+-]*[A-Za-z0-9])?$ — verified it still accepts
v1.14.0, v1.14.0-beta1, 1.2.3+build and the v0.0.0-pr2-abc1234 stand-in,
and now rejects v0.0.0-.

mmdb_version gets the same regex and moves into env: — see your comment on
line 781.

🤖 Claude, replying on behalf of Greg.

[horgh]

pie.phar is fetched by version tag while everything else is digest-pinned.

Every action here is SHA-pinned and every build image is digest-pinned; this downloads a .phar from a mutable release URL. Blast radius is confined to the smoke test, so this is a consistency nit rather than a risk.

Related: the "Non-Windows prebuilt binaries need PIE >= 1.4.0" comment two lines down sits above pie --version, which prints and asserts nothing. The constraint is actually satisfied by the hardcoded 1.4.9 in this URL, which carries no comment. Suggest moving the note here, or making it a real assertion.

🤖 Comment by Claude (Claude Code) on behalf of Will.

[oschwald]

Half fixed. The comment has moved onto the pin that actually enforces the
constraint, since pie --version asserts nothing — that was the more misleading
half.

The .phar is still fetched by release tag. Pinning it by digest is reasonable
and not done; noting your own assessment that the blast radius is confined to the
smoke test, which is why it did not make this pass.

🤖 Claude, replying on behalf of Greg.

[horgh]

This loop cannot distinguish a broken asset from Packagist lag.

A genuinely malformed asset burns five minutes and then reports "pie install never succeeded for $version" — the same message Packagist lag produces. Since this job is post-publication and non-blocking anyway, it is diagnosis quality rather than gating, but "PIE found no matching asset" and "Packagist does not know this version yet" are very different emergencies, and only one of them is an emergency at all.

🤖 Comment by Claude (Claude Code) on behalf of Will.

[oschwald]

Fair, and not fixed. Recording it as open.

Agreed on the framing that this is diagnosis quality rather than gating — but
"PIE found no matching asset" and "Packagist does not know this version yet" being
indistinguishable is worth fixing precisely because only one of them is an
emergency, and this job reports after the release is already public.

🤖 Claude, replying on behalf of Greg.

[oschwald]

Thanks for the review — I've replied to each comment individually. Overview of what changed, and what deliberately did not.

15 new commits, one per concern. No fixups, since the branch has now been reviewed.

The findings that could ship something broken

Draft releases were unfindable	/releases/tags/{tag} returns published releases only, so the reuse branch was dead code and a workflow_dispatch re-cut would create a second draft. Drafts are now found by listing; more than one on a tag is its own error.
--notes-from-tag never worked	draft-release was the only job with no checkout, and the flag reads the annotation from a local clone. Every release would have got generated notes behind a warning.
The upload step had no pipefail	A partial find uploaded a subset, xargs exited 0, attestation then covered the complete local set while an incomplete release was published.
An empty WINDOWS_COUNT became 0	So a release missing every Windows asset matched its own expectation. Both this and the windows-matrix guard now validate shape before magnitude.
Windows built a different commit	On a release it built upstream's $TAG while every other lane built the submodule pointer, with nothing asserting they matched. Now always the pointer.
The macOS floor checked itself	MACOSX_DEPLOYMENT_TARGET was both the compiler input and the gate's ceiling. Now declared separately on each step so the two can disagree.
publish could overwrite a published release	Re-running a single job reuses draft-release's prior result. The draft assertion is now repeated where contents: write lives.

The dead fallback

setup's submodule step exits 1 before binaries=false is ever computed, so the entire documented degradation path was unreachable — lane skips, source-build's bundled=false, the Windows system-library fallback, binary_count=0, and ~40 lines of comments. Deleted rather than repaired, with Greg's agreement, and replaced by an explicit precondition. binary_count=0 was also one of two routes to a binary-less release.

Coverage

• dev-bin/test-gate-extension-windows.sh — 15 cases for the Windows gate, run in CI, with verify-assets depending on it. The accept path is now testable off Windows via a dumpbin shim, which is the case that matters most: without it, a gate broken into refusing everything passes every rejection test. Confirmed load-bearing by breaking the gate two ways and watching it go red.
• source-build's object is now gated — it was the only object in the pipeline held to no self-containment bar.
• The reduced matrix builds 8.2 and 8.5 — the submodule has no macOS job at all, so six of eight macOS legs previously first ran on a tag.

Also

dependabot.yml (there was none), dev-bin/lib.sh for the thrice-copied fail(), pkg-config presence probe, with-maxminddb-bundled declared for PIE source installs, the PIE ≥ 1.4.0 minimum in the README, and a comment-accuracy pass — including one claim I introduced in the previous commit that was already false at this submodule pointer.

Not done, on purpose

Deferred with Greg as a follow-up, all real:

• Asset names are still not asserted — collect-assets.sh counts. The BINARIES coupling that made a tarball-only release pass is gone, but names are the stronger check.
• Nothing reconciles the draft's actual contents before un-drafting, and --clobber never deletes.
• No Windows zip is ever opened. The Unix lanes re-open theirs and assert exact contents.
• No Windows arch/ABI cross-check — a leg building x86 while labelled x64 gets a unique, wrong filename that no guard catches.
• smoke still runs after publish, covering 1 of ~41 assets.
• os/libc are not cross-checked against uname -s in stage-asset.sh.
• The tarball member check is still a denylist with no entry-count bound.

🤖 Claude, on behalf of Greg.

Edited: the script was named dev-bin/test-gate-extension.sh when this was written. Renamed to dev-bin/test-gate-extension-windows.sh, because this directory holds no gate-extension.sh and the submodule has a same-named test for the bash gate.

[oschwald]

Filed ENG-5218 for the container image digests, blocked on ENG-5215 (Renovate for mise dependencies).

That covers the three dependabot questions on this PR — the source-build image, the digest case statement, and the smoke job's image — so they don't depend on this PR being remembered.

The short version of what's in it: Dependabot cannot do this, and no configuration fixes it. Its docker ecosystem scans Dockerfiles, compose files and Kubernetes manifests, never workflow YAML; its github-actions ecosystem scans workflows but only uses: references. All ten image references here sit in run: blocks or env:, so neither sees any of them. The decoy-Dockerfile trick is possible and I've recommended against it — nothing would build it, so it drifts from real usage.

Renovate can, via a custom regex manager: one entry, no annotation comments, no change to release.yml. One trap worth naming, and it's in the issue: left alone Renovate treats 8.2-cli-bookworm as a version and would "upgrade" the 8.2 lane to 8.5, silently changing what that leg builds while the matrix still calls it 8.2. It needs a packageRule restricting it to digest updates.

The issue also notes an existing wart to clear up whenever this is touched: source-build's IMAGE duplicates the Linux lane's 8.5-nts digest as a separate literal.

In this PR, .github/dependabot.yml records the three as manually bumped.

🤖 Claude, on behalf of Greg.