Drop FROM php:8-alpine in favor of FROM alpine:3.23 (~55% smaller image)#800
Open
akirayamamoto wants to merge 2 commits intolibrespeed:masterfrom
Open
Drop FROM php:8-alpine in favor of FROM alpine:3.23 (~55% smaller image)#800akirayamamoto wants to merge 2 commits intolibrespeed:masterfrom
akirayamamoto wants to merge 2 commits intolibrespeed:masterfrom
Conversation
The current Dockerfile.alpine pulls FROM php:8-alpine and then `apk add php-apache2`. The result is two PHP installs side by side: the docker-library PHP at /usr/local/bin/php (~30 MB, never used by Apache) and the apk-installed PHP at /usr/bin/php (which mod_php actually loads). Pinning a fresh Alpine release and installing the apk packages directly drops the dead /usr/local/bin/php install entirely. Expected wins: - Smaller image (~30 MB less; 2024 baseline was ~120 MB) - One PHP binary, no $PATH ambiguity - Cleaner story for any follow-up that touches PHP config
Contributor
Review Summary by QodoReplace php:8-alpine with alpine:3.23 for smaller image
WalkthroughsDescription• Replace FROM php:8-alpine with FROM alpine:3.23 base image • Install PHP via apk packages instead of inheriting unused binary • Remove dead docker-php-extension-installer comment block • Reduce image size by ~55% (227 MB to 102 MB) Diagramflowchart LR
A["FROM php:8-alpine<br/>227 MB"] -->|Remove unused binary<br/>and build artifacts| B["FROM alpine:3.23<br/>102 MB"]
B -->|Add php package| C["Single PHP install<br/>via apk"]
C -->|mod_php loads from| D["php-apache2 package<br/>php84"]
File Changes1. Dockerfile.alpine
|
Contributor
Code Review by Qodo
1.
|
ca-certificates-bundle (which provides /etc/ssl/certs/ca-certificates.crt) is already pulled in transitively by apache2 / php-apache2 on alpine:3.23, so live HTTPS calls from PHP work today (verified with file_get_contents against ipinfo.io). But the master image (FROM php:8-alpine) installs the full ca-certificates package explicitly, and operators with IPINFO_APIKEY configured rely on outbound HTTPS. Make the dependency explicit to: * Match master's package set rather than relying on a transitive pull that some future apk dep change could drop. * Document the runtime TLS requirement at the Dockerfile level instead of leaving it implicit. ~50 KB image-size cost; the umbrella ca-certificates package adds the update-ca-certificates CLI on top of the bundle. Per Qodo's review on PR librespeed#800.
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.
Fixes #799.
Replace
FROM php:8-alpinewithFROM alpine:3.23inDockerfile.alpineand install PHP entirely via apk. See #799 for the full context. Short version: the FROM image's/usr/local/bin/phpis never used by Apache (mod_php loads from the apk packages instead), and the FROM image also carries PHP source/build artifacts the runtime doesn't need.Change
The dead
docker-php-extension-installercomment block also goes; it never applied to the alpine variant.Image size
The savings come from dropping the unused
/usr/local/bin/phpbinary plus the/usr/src/phpheaders and build artifacts that the docker-libraryphp:8-alpineimage carries. ~55% smaller.Verification
Inside the new image:
One PHP install, no FROM-image leftovers, and mod_php is still served by the same
php-apache2package — which on alpine:3.23 resolves to php84, same major as master'sFROM php:8-alpine.Full Playwright e2e suite passes against this image: 12/12, 22.4s. Specs: classic-standalone-regression, design-switch, modes, title-special-chars across standalone/backend/frontend/dual configs.
Note on PHP version
alpine:3.23'sphp-apache2meta resolves tophp84-apache2, matching the PHP major that master'sFROM php:8-alpinelands on today. No version regression and no need to pin a versioned package. Earlier alpine releases (3.21, 3.22) defaulted to php83; if a future Alpine bump shifts the default again, pinningphp84-*explicitly is the escape hatch.Dependabot
.github/dependabot.yml's docker ecosystem at/auto-discovers FROM lines. After this PR, Dependabot dropsphp:8-alpineand picks upalpine:3.23. No config change needed. One subtle shift: Alpine bumps now carry the implicit risk of a PHP major bump when the apk meta shifts — see the PHP-version note above for the lock-down option.