-
-
Notifications
You must be signed in to change notification settings - Fork 0
feat(apache): Update to PHP 8.5.
#32
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughUpdated PHP version references from 8.4 to 8.5 across environment files, CI workflows, docs, and examples. The Apache Dockerfile also adds conditional build/install/cleanup steps to install Microsoft SQL Server drivers (sqlsrv/pdo_sqlsrv), using a source-compile path when PHP_VERSION_ID >= 80500 and a pecl fallback otherwise. Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @.github/workflows/build.yml:
- Around line 27-28: The workflow sets PHP_VERSION to "8.5" which will cause
image-pull failures because the official PHP Docker images don't publish 8.5
tags yet; update the env variable PHP_VERSION in the workflow to a supported
official tag (e.g., "8.4" or "8.3") or change the image reference to a
maintained third-party image that provides 8.5; specifically edit the env
PHP_VERSION value in .github/workflows/build.yml (symbol: PHP_VERSION) to a
published official version or adjust the job's container/image references to an
alternative registry if you intentionally need 8.5.
In `@src/flavor/apache/Dockerfile`:
- Around line 2-7: The Dockerfile uses PHP_VERSION and references
php:${PHP_VERSION}-fpm-trixie which does not exist for 8.5; update the
Dockerfile to use an available base image by changing the PHP_VERSION default to
a released tag (e.g., set ARG PHP_VERSION=8.4) or switch the base image
reference to an existing tag (e.g., replace php:${PHP_VERSION}-fpm-trixie with a
released variant like php:8.4-fpm-trixie or php:8.5-fpm-bullseye if intended),
ensuring the ARG PHP_VERSION value and the FROM reference remain consistent so
builds succeed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/flavor/apache/Dockerfile (1)
2-12: The base image tagphp:8.5.2-fpm-trixiedoes not exist in the official Docker PHP repository.The official PHP images do not publish a Debian trixie FPM variant for version 8.5. Available PHP 8.5 FPM tags are limited to Alpine variants (e.g.,
php:8.5.2-fpm-alpine3.23). While PHP 8.4 had thefpm-trixievariant, this is not available for PHP 8.5.Update the base image to an available tag such as
php:8.5.2-fpm-alpine3.23or check the official Docker Hub PHP image tags for the latest supported Debian variant.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/flavor/apache/Dockerfile`:
- Around line 188-209: The Dockerfile currently forces building/patching
pdo_sqlsrv-5.12.0 for PHP >= 8.5 (the php -r PHP_VERSION_ID check) which is
unsupported; change the logic around the pecl/install and manual build
(references: pecl install sqlsrv-5.12.0, pdo_sqlsrv-5.12.0 download/tar/extract,
the sed patches to pdo_dbh.cpp and php_pdo_sqlsrv_int.h, and
docker-php-ext-enable sqlsrv pdo_sqlsrv) to only install/build when
PHP_VERSION_ID corresponds to officially supported releases (PHP 8.1–8.3), and
for other PHP versions either skip installation with a clear error/notice or
gate this whole block behind an explicit EXPERIMENTAL_SQLSRV build-time flag and
emit a documented warning; ensure any automated patching/build steps remain
inside that guarded branch and do not run for PHP 8.5 unless EXPERIMENTAL_SQLSRV
is set.
♻️ Duplicate comments (1)
src/flavor/apache/Dockerfile (1)
2-12: Base imagephp:8.5-fpm-trixieavailability concern already noted.This issue was flagged in a previous review. The
php:8.5-fpm-trixieimage is not yet available on Docker Hub as PHP 8.5 is still in development. The build will fail until the official image is released.
🧹 Nitpick comments (1)
src/flavor/apache/Dockerfile (1)
196-197: Fragile source patching viasedmay silently fail.If the upstream
pdo_sqlsrvsource changes (different whitespace, renamed variables, etc.), thesesedreplacements will silently produce no changes, potentially causing compilation failures or runtime issues. Consider adding verification that the patterns actually matched:💡 Suggested verification approach
&& sed -i 's/= dbh->error_mode/= (enum pdo_error_mode) dbh->error_mode/' pdo_dbh.cpp \ + && grep -q '(enum pdo_error_mode) dbh->error_mode' pdo_dbh.cpp \ && sed -i 's/zval_ptr_dtor( &dbh->query_stmt_zval );/OBJ_RELEASE(dbh->query_stmt_obj);dbh->query_stmt_obj=NULL;/' php_pdo_sqlsrv_int.h \ + && grep -q 'OBJ_RELEASE(dbh->query_stmt_obj)' php_pdo_sqlsrv_int.h \
…SQL Server repository.
…tallation in Dockerfile.
Pull Request
Summary by CodeRabbit
Chores
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.