forked from RoushTech/docker
fix(ubuntu-php): get the Ubuntu PHP images working, and gate them #41
Loading…
Reference in a new issue
No description provided.
Delete branch "claude/nervous-sinoussi-ff5f90"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
The
ubuntu-phpassertions were commented out intest.yml, and theubuntu-phphealthchecks intest-healthchecks.yml, because the images could not pass validation. Reported symptom:That was accurate, but it was the first of five faults. The nginx version alone would not have let either gate be re-enabled.
The last two only became visible once the images were run rather than merely built — which is what the disabled healthchecks were hiding, and why 8.x had been shipping with no PHP extensions and a dead web server for as long as it had.
1. nginx version recorded as 1.28 on a base that ships 1.24
All five stages build
FROM ./Ubuntu.Dockerfile#ubuntu-24-base, and noble ships1.24.0-2ubuntu7.15. Theondrej/phpPPA, added before nginx installs, does not bump it.Set to
1.24rather than adding nginx.org.NGINX_VERSIONis descriptive, not prescriptive — it feeds theooo.grey.version.nginxlabel and the assertion, and selects nothing.PHP.Dockerfilealready varies it per base (1.28 / 1.24 / 1.20) to match what each Alpine ships, so Alpine running 1.28 is what its repo happens to provide rather than a target being pursued. Sourcing from nginx.org would mean a third-party repo and signing key, and would make Ubuntu the only image bypassing its distro.2. No PHP extensions loaded on any 8.x image
The shared
fs/php-nginxoverlay uses Alpine's flat/etc/php, which works there only becauseln -s /etc/php /etc/php${PHP_VER}covers the one path PHP reads. Ondrej compiles in/etc/php/<version>/{cli,fpm}— inside/etc/php, where that symlink cannot reach — soconf.dwas never scanned.FIX_PERMSreshaped the tree only underif [[ "$PHP_VER" -eq 7 ]], leaving every 8.x image on its built-ins alone: 18 modules while 51.sofiles sat unused.Now linked for all versions, and as symlinks rather than the copies the 7.4 branch used — those copies meant the runtime toggles in
services.d/php/run, which rewrite/etc/php/conf.d, were editing files PHP never read. That branch is now just thepm.max_spawn_ratefixup it was documented to be.ubuntu-php-84ubuntu-php-743. Startup warnings on stdout
Once
conf.dwas scanned, PHP warned about every extension ondrej builds in (openssl,pcntl,session,sodium) or omits (gmp,odbc,snmp,tidy, ...). Those ini files can never load, so they are pruned.The remaining warning — opcache disabling JIT because xdebug overrides
zend_execute_ex()— is legitimate. It broke validation only becausephp.shreadphp --version | head -n 1and PHP prints startup warnings to stdout, so the parse landed on a warning. It now selects the version line.That warning also went to stdout on every CLI call, which would corrupt any script being piped. Of the settings that suppress it, only
opcache.enable_cliis CLI-scoped:opcache.jitandjit_buffer_sizelive in the sharedconf.dand would cost FPM its JIT, anddisplay_errorsthere would silently override thePHP_DISPLAY_ERRORStunable, sinceconf.dis scanned afterphp.ini. JIT is inert beside xdebug anyway and a CLI process is too short-lived for the opcode cache, so it is off for the CLI only.4. The CLI never loaded
php.iniOndrej looks for it at
/etc/php/<version>/cli/, and only FPM was covered, by the explicitphp-fpm -c /etc/php/in the service script. So composer and everydocker run <img> php ...quietly used upstream defaults instead of the tuned config. Now linked for both SAPIs:memory_limitreads128Mfromphp.ini,expose_phpisOff.5. nginx had never started
The overlay's
nginx.confincludes/etc/nginx/mime.types, which only the distro package provides, butINSTALL_PHPremoved/etc/nginxwholesale. Alpine removes only/etc/php*, which is why it never showed there. nginx crash-looped:Only Ubuntu's own vhost wiring is removed now. The 7.4 healthcheck block also carried a mis-indented
configs:that would not have parsed.Interaction with
mainMerged
origin/main, which lands pcov (#37) on these images. The two changes reinforce each other rather than collide:pcov.sois on disk and the prune correctly keeps51_pcov.ini. pcov loads withpcov.enabled='0', andphp.shreports "pcov present and correctly disabled by default: OK".$(php -r 'echo ini_get("pcov.enabled") ? "on" : "";')and fails if non-empty. On Ubuntu 8.x that would have captured the JIT warning and falsely reported "pcov is enabled by default" — fix 3 is what makes it pass.51_pcov.inidocuments that correct coverage requiresopcache.enable_cli=0, calling it a correctness requirement that fails silently. Fix 3 makes that the Ubuntu default.Verification
All five assertions and all five healthchecks pass, and are wired into the aggregator. The healthchecks were checked beyond the health flag: HTTP 200, 98KB phpinfo, PHP 8.4.24, zero nginx errors.
fs/php-nginx/etc/validate.d/php.shis shared, so the other families were re-verified:php-85,php-83,php-74,magento-php-85,php-85-node,frankenphp-85all pass.frankenphpcarries its own copy and is untouched.Still out of scope
fs/php-frankenphp/etc/validate.d/php.shkeeps the same fragilehead -n 1. It passes today, so the duplication is left rather than changing a file with no failing case.teacannot operate on this repo at all — its git library rejects it withcore.repositoryformatversion does not support extension: worktreeconfig, from both a worktree and the main checkout. This PR and its comments were made through the Forgejo API. CLAUDE.md directs PR work throughtea, so that instruction will fail for anyone with the same config.🤖 Generated with Claude Code
The ubuntu-php assertions have been commented out in test.yml because the images could not pass their build-time validate. Three separate faults were in the way. NGINX_VERSION was declared 1.28 on every Ubuntu stage, but all of them build FROM ubuntu-24-base and noble ships nginx 1.24 (the ondrej PPA does not carry nginx, so nothing bumps it). The ARG is descriptive, not prescriptive -- it feeds the ooo.grey.version.nginx label and the validate assertion and selects nothing -- and PHP.Dockerfile already varies it per base, 1.28/1.24/1.20, to match what each Alpine actually ships. Alpine running 1.28 is what its repo happens to ship rather than a target we pursue, so record 1.24 here instead of adding nginx.org's repo and making Ubuntu the only image bypassing its distro. Extensions never loaded on 8.x. The shared fs/php-nginx overlay uses Alpine's flat /etc/php, which works there because `ln -s /etc/php /etc/php${PHP_VER}` covers the only path PHP reads. Ondrej compiles in /etc/php/<version>/{cli,fpm} instead, which sits inside /etc/php where that symlink cannot reach, so conf.d was never scanned. FIX_PERMS reshaped the tree into the versioned layout only under `if [[ "$PHP_VER" -eq 7 ]]`, leaving every 8.x image with just its built-ins -- php -m listed 18 modules while 51 .so files sat unused. Link the versioned paths back at the flat ones for all versions, as symlinks rather than the copies the 7.4 branch used, so the runtime toggles in services.d/php/run still reach the files PHP reads. That branch is now only the pm.max_spawn_rate fixup it was documented to be. ubuntu-php-84 goes 18 -> 65 modules. Once conf.d was scanned, PHP warned about every extension in it that ondrej builds in or omits, on every invocation, so prune the ini files that can never load. The last of those warnings, opcache disabling JIT because xdebug overrides zend_execute_ex, is legitimate and stays; it broke validate only because php.sh read `php --version | head -n 1` and PHP prints startup warnings to stdout, so select the version line explicitly. All five ubuntu-php assertions now pass and are wired into the test aggregator. Verified no regression in the php, magento, php-node and frankenphp images, which share validate.d/php.sh (frankenphp carries its own copy, untouched). Co-Authored-By: Claude <noreply@anthropic.com>CI: run 294 failed, but not on this change
Run 294 is red. The build and validation phases were entirely clean — 41 ×
All validations passed, 0 ×Validation failedacross the whole matrix, with 8 nginx checks landing on1.24. All fivedocker-ubuntu-php-*-assertions-1containers reachedCreated.It broke ~19 minutes in, during
compose upcontainer recreation:A container-removal race on the shared Swarm daemon, on a Java Tomcat service that this diff does not touch. There were ~5 runs executing concurrently at the time — the contention mode CLAUDE.md warns about, where the failure says nothing about the change under test.
Re-pushed as
ab148fbwith two runs in flight instead of five; run 303 is monitoring.One real thing the log did catch
The 294 log showed this in every 8.x build:
Self-inflicted by this PR. The new extension prune already drops
00_json.ini— ondrej builds json into the binary and leaves nojson.so— so the later json-specific removal always missed.|| trueswallowed the exit code but not the message. Nowrm -f, which swallows both. Fixed inab148fb.CI green
Run 303 succeeded — both
BuildandRun container tests.Validation failed: 01.24: 5 (one per Ubuntu stage)docker-ubuntu-php-*-assertions-1containersCreated→Started→ completed successfully, so the newly uncommented gate intest.ymlis genuinely being enforced rather than skippedrm: cannot remove '*_json.ini'noise is goneThis run was scoped rather than full-matrix:
ab148fbonly touchesPHP.Ubuntu-fragment.Dockerfile, sochanged-targetscorrectly narrowed it to theubuntu-phpimages. The full-matrix coverage came from run 294, whose build and validation phases were clean end to end and which failed only on the shared-daemon container-removal race described above.fix(ubuntu-php): make the Ubuntu PHP images pass validate, and gate themto fix(ubuntu-php): get the Ubuntu PHP images working, and gate themRebased onto current
main, and the pcov work lines upmainhad moved on a fair way, including two changes that touch this PR directly. Merged it in (0fc7af3) — no conflicts, and both sides survived:NGINX_VERSION=1.24alongside the newphp<ver>-pcovpackages, and the robust version parse alongside the new pcov assertion block.The pcov work (#37) and this PR reinforce each other:
pcov.solands on disk and the extension prune here correctly keeps51_pcov.ini. Verified on a fresh 8.5 build: pcov loaded,pcov.enabled='0', andphp.shreporting "pcov present and correctly disabled by default: OK".$(php -r 'echo ini_get("pcov.enabled") ? "on" : "";')and fails when that is non-empty. On Ubuntu 8.x it would have captured theJIT disabledwarning on stdout and falsely failed with "pcov is enabled by default". Theopcache.enable_clifix in this PR is what keeps that probe clean.51_pcov.inicallsopcache.enable_cli=0a correctness requirement for coverage that otherwise fails silently. This PR makes it the Ubuntu default.One thing worth knowing for anyone verifying locally: #43 changed the compose project name, so built images are now
docker-<runid>-*rather than<dir>-*. I inspected the old names at first and got a stale answer — pcov looked absent when it was in fact installed.All ten Ubuntu services (5 assertions + 5 healthchecks) pass against merged
main.Run 308 shows cancelled, not failed: pushing the merge to the same branch superseded it. Run 312 covers the merge and is being monitored.
68171a931af07086cd48View command line instructions
Checkout
From your project repository, check out a new branch and test the changes.Merge
Merge the changes and update on Forgejo.Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.