Reconcile final-stage apt state before perl-dependent postinsts run - #2
Conversation
The final stages of the php images never upgrade the base packages, so when the builder stage rebuilds fresh (new PHP source invalidates its cache) while curl:latest's apt layers are cache hits, the wholesale COPY --from=builder /usr/bin/ overwrites /usr/bin/perl with a newer interpreter than the base image's XS modules. Debian sid's perl 5.40->5.42 transition turned that skew into dpkg postinst failures (Perl API version v5.40.0 of Cwd does not match v5.42.0) in the build php-zts-base-master step. Add apt-get dist-upgrade -y to the five final stages that lack an upgrade (zts-base, base, testing, zts-testing, franken-testing); fpm, zts, and franken already have one. dist-upgrade rather than upgrade because a mid-transition perl-base upgrade can require new packages, which plain upgrade silently holds back. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015tYeFcz7Z3Faz4hVMvvayk
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PHP Dockerfiles now use ChangesPHP runtime package upgrades
Estimated code review effort: 1 (Trivial) | ~5 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Add --no-install-recommends to the dist-upgrade lines so replacement packages pulled mid-transition do not drag in Recommends, and a comment explaining why dist-upgrade is used instead of upgrade, so a future harmonizing pass does not revert it. Convert the fpm, zts, and franken final stages from upgrade to the same dist-upgrade line: they share the wholesale /usr/bin copy pattern, and plain upgrade silently holds back perl-base when the transition needs package adds or removals. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015tYeFcz7Z3Faz4hVMvvayk
|
Verification update: the no-cache re-run of master (31271329533) failed with the same Perl mismatch, which upgrades this PR from nice-to-have to required. The log shows why: even on a fully fresh build, Now verifying with a chain dispatched on this branch: 31272663046. |
|
Branch verification run 31272663046: the Perl fix is confirmed — The run then failed at Separate observation from the same log: xcaddy downloads fpm-side verification (exercises |
|
Verification complete ✅ — fpm chain 31276236043 succeeded end-to-end: The log shows the fix working exactly as designed in No Remaining known issue (independent of this PR): |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@php/base/Dockerfile`:
- Around line 203-204: Add and vary a cache-busting build argument before the
package-refresh RUN instruction in php/base/Dockerfile lines 203-204 and
php/zts-base/Dockerfile lines 191-192, ensuring both base stages invalidate
their APT upgrade layers. Also verify CI varies the existing CACHE_BUSTER
argument for derived images.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 96920c9c-3a66-4b95-a84a-cf2f5a357385
📒 Files selected for processing (8)
php/base/Dockerfilephp/fpm/Dockerfilephp/franken-testing/Dockerfilephp/franken/Dockerfilephp/testing/Dockerfilephp/zts-base/Dockerfilephp/zts-testing/Dockerfilephp/zts/Dockerfile
📜 Review details
🧰 Additional context used
🔍 Remote MCP Context7, DeepWiki
Additional review context
-
Debian APT documents that
apt-get upgradeholds back packages when dependency changes require installing or removing packages. -
apt-get dist-upgraderesolves changing dependencies and may install or remove packages. This supports the PR’s rationale for Perl transitions, but reviewers should verify the resulting package plan does not remove required runtime packages. -
APT recommends running
apt-get updatebefore either upgrade command, so the command ordering is correct.
Repository-specific DeepWiki context was unavailable because the public endpoint required authentication.
🔇 Additional comments (1)
php/base/Dockerfile (1)
203-204: 🩺 Stability & AvailabilityVerify the
dist-upgraderemoval plan across all final images.
apt-get dist-upgrademay install or remove packages during dependency resolution. (manpages.debian.org) Each stage upgrades the image after its runtime state is assembled and then runsapt-get autoremove. Capture the simulated or real APT plan for every stage. Fail or alert when a runtime-critical package is removed.
php/base/Dockerfile#L203-L204: verify the copied PHP runtime and explicit library set.php/fpm/Dockerfile#L150-L151: verify the PHP-FPM runtime and explicit library set.php/franken-testing/Dockerfile#L126-L127: verify FrankenPHP and its watcher library.php/franken/Dockerfile#L126-L127: verify FrankenPHP and its watcher library.php/testing/Dockerfile#L115-L116: verify inherited PHP-FPM dependencies.php/zts-base/Dockerfile#L191-L192: verify the copied PHP ZTS runtime and explicit library set.php/zts-testing/Dockerfile#L113-L114: verify inherited PHP ZTS dependencies.php/zts/Dockerfile#L148-L149: verify the PHP ZTS runtime and explicit library set.Source: MCP tools
| # dist-upgrade, not upgrade: sid transitions can need package adds/removals to move perl-base; plain upgrade holds it back and perl postinsts break on the skew | ||
| && apt-get dist-upgrade -y --no-install-recommends \ |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔵 Trivial
Add a recurring cache trigger to both base stages.
If the base and builder layers remain cache hits, Docker can reuse the package-upgrade RUN layer without checking current APT state. (docs.docker.com) Add and vary a cache argument for both base images, or schedule no-cache rebuilds. Also verify that CI varies the existing CACHE_BUSTER argument in the derived images.
php/base/Dockerfile#L203-L204: add the cache trigger for the PHP base refresh.php/zts-base/Dockerfile#L191-L192: add the cache trigger for the PHP ZTS base refresh.
📍 Affects 2 files
php/base/Dockerfile#L203-L204(this comment)php/zts-base/Dockerfile#L191-L192
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@php/base/Dockerfile` around lines 203 - 204, Add and vary a cache-busting
build argument before the package-refresh RUN instruction in php/base/Dockerfile
lines 203-204 and php/zts-base/Dockerfile lines 191-192, ensuring both base
stages invalidate their APT upgrade layers. Also verify CI varies the existing
CACHE_BUSTER argument for derived images.
Why
Run 31266620318 failed at
build php-zts-base-masterwith:Debian sid transitioned perl 5.40 → 5.42. The zts-base builder stage rebuilt fresh (new
php/source/content invalidated its cache) and pulled perl 5.42, whilecurl:latest's apt layers were cache hits (no working cache-buster exists —ARG CACHE_BUSTERis never referenced in anyRUNnor passed by the build actions), leaving its perl-base modules at 5.40.COPY --from=builder /usr/bin/ /usr/bin/then overwrote/usr/bin/perlwith the 5.42 binary, and the final-stageapt-get install's perl-based maintainer scripts (deb-systemd-helper,update-rc.d) failed on the module mismatch → dpkg exit 100.What
Add
apt-get dist-upgrade -yafterapt-get updatein the five final stages that lack an upgrade, so the base converges to current sid before perl-dependent postinsts run:php/zts-base/Dockerfilephp/base/Dockerfilephp/testing/Dockerfile/php/zts-testing/Dockerfile(beforeautoremove— no install line there)php/franken-testing/Dockerfile(parity withphp/franken, which already upgrades)php/fpmandphp/ztsalready runupgrade -yand are untouched.dist-upgraderather thanupgradebecause a mid-transitionperl-baseupgrade can require new packages, which plainupgradesilently holds back (exit 0).How to verify
curl → php-franken-testing-masterchain re-dispatched withno_cache=true(immediate unblock, independent of this PR).php-fpm-base-master → php-fpm-socket-masterchain exercises thephp/basechange — the zts chain never builds it.dist-upgradeupgrading perl-base and noPerl API version … does not matchlines.What to watch for
$CACHE_BUSTERin the aptRUNs and passinggithub.run_idfrom the build actions) or a scheduledno_cache=truerebuild.COPY --from=builder /usr/bin/ /usr/bin/ships the build toolchain (gcc, git, gdb, valgrind…) into runtime images and is the generic trigger for this skew class.🤖 Generated with Claude Code
https://claude.ai/code/session_015tYeFcz7Z3Faz4hVMvvayk