Conversation
There was a problem hiding this comment.
Pull request overview
Updates the PHP container build pipeline to include PHP 8.5 images and corresponding extension assets, while also bumping several pinned component versions used during image builds.
Changes:
- Add PHP 8.5 to CI build matrices and Makefile extension build targets.
- Bump grpc extension build/download versioning to
1.78.0and adjust asset naming to include the grpc version. - Update base image inputs (serversideup image version, supercronic) and add
opentelemetryextension; change defaultPHP_MAX_EXECUTION_TIME.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
php/common/usr/local/bin/docker-php-serversideup-setup |
Pins grpc version and updates the S3 download filename to a versioned .so. |
php/base.Dockerfile |
Bumps upstream image/supercronic versions, installs opentelemetry, and changes default PHP execution time. |
extensions-builder.Dockerfile |
Bumps grpc build version and outputs versioned grpc artifact filenames. |
Makefile |
Adds 8.5 extension build targets and comments out alpine+ZTS extension builds across versions. |
.github/workflows/build.yml |
Extends the build matrices to include PHP 8.5. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ENV PHP_OPCACHE_VALIDATE_TIMESTAMPS=0 | ||
| ENV PHP_OPCACHE_INTERNED_STRINGS_BUFFER=16 | ||
| ENV PHP_MAX_EXECUTION_TIME=900 | ||
| ENV PHP_MAX_EXECUTION_TIME=55 |
There was a problem hiding this comment.
PHP_MAX_EXECUTION_TIME is reduced from 900 to 55 seconds. This is a behavior change that can break long-running requests/CLI tasks (e.g., artisan commands, migrations, queue workers) for existing users of this image. If this is intentional, consider documenting the rationale and/or making it configurable per image/tag instead of changing the default so drastically.
| iconv \ | ||
| imagick \ | ||
| intl \ | ||
| opentelemetry \ | ||
| pdo_sqlite \ |
There was a problem hiding this comment.
This PR is titled "Php 8.5", but it also adds the opentelemetry extension (and changes other runtime defaults). Consider updating the PR title/description or splitting these unrelated changes so it's clear what consumers should expect when upgrading.
| docker buildx build --pull --platform=linux/amd64 --build-arg PHP_VERSION=8.3 -f extensions-builder.Dockerfile -o=./assets/extensions/ . | ||
| docker buildx build --pull --platform=linux/amd64 --build-arg PHP_VERSION=8.3-zts -f extensions-builder.Dockerfile -o=./assets/extensions/ . | ||
| docker buildx build --pull --platform=linux/amd64 --build-arg PHP_VERSION=8.3 --build-arg OS_VARIANT=alpine -f extensions-builder.Dockerfile -o=./assets/extensions/ . | ||
| docker buildx build --pull --platform=linux/amd64 --build-arg PHP_VERSION=8.3-zts --build-arg OS_VARIANT=alpine -f extensions-builder.Dockerfile -o=./assets/extensions/ . | ||
| # docker buildx build --pull --platform=linux/amd64 --build-arg PHP_VERSION=8.3-zts --build-arg OS_VARIANT=alpine -f extensions-builder.Dockerfile -o=./assets/extensions/ . |
There was a problem hiding this comment.
The alpine ZTS extension build command is now commented out for 8.3. If ZTS+alpine images (or extension assets) are still supported, this will stop producing the required artifacts and the runtime grpc download may fail. If ZTS+alpine is intentionally unsupported, consider removing the dead targets (or adding an explicit comment explaining why they are disabled) to avoid confusion.
| docker buildx build --pull --platform=linux/amd64 --build-arg PHP_VERSION=8.4 -f extensions-builder.Dockerfile -o=./assets/extensions/ . | ||
| docker buildx build --pull --platform=linux/amd64 --build-arg PHP_VERSION=8.4-zts -f extensions-builder.Dockerfile -o=./assets/extensions/ . | ||
| docker buildx build --pull --platform=linux/amd64 --build-arg PHP_VERSION=8.4 --build-arg OS_VARIANT=alpine -f extensions-builder.Dockerfile -o=./assets/extensions/ . | ||
| docker buildx build --pull --platform=linux/amd64 --build-arg PHP_VERSION=8.4-zts --build-arg OS_VARIANT=alpine -f extensions-builder.Dockerfile -o=./assets/extensions/ . | ||
| # docker buildx build --pull --platform=linux/amd64 --build-arg PHP_VERSION=8.4-zts --build-arg OS_VARIANT=alpine -f extensions-builder.Dockerfile -o=./assets/extensions/ . |
There was a problem hiding this comment.
The alpine ZTS extension build command is now commented out for 8.4. If ZTS+alpine artifacts are still expected, this will leave missing grpc assets for that matrix. If it's intentionally disabled, please add a brief rationale or remove the commented command to keep the Makefile unambiguous.
| docker buildx build --pull --platform=linux/amd64 --build-arg PHP_VERSION=8.5 -f extensions-builder.Dockerfile -o=./assets/extensions/ . | ||
| docker buildx build --pull --platform=linux/amd64 --build-arg PHP_VERSION=8.5-zts -f extensions-builder.Dockerfile -o=./assets/extensions/ . | ||
| docker buildx build --pull --platform=linux/amd64 --build-arg PHP_VERSION=8.5 --build-arg OS_VARIANT=alpine -f extensions-builder.Dockerfile -o=./assets/extensions/ . | ||
| # docker buildx build --pull --platform=linux/amd64 --build-arg PHP_VERSION=8.5-zts --build-arg OS_VARIANT=alpine -f extensions-builder.Dockerfile -o=./assets/extensions/ . |
There was a problem hiding this comment.
The alpine ZTS extension build command is commented out for the new 8.5 targets. Either restore the build if ZTS+alpine is supported, or explicitly document/remove the ZTS+alpine targets so users don't assume those assets are being produced.
No description provided.