-
Notifications
You must be signed in to change notification settings - Fork 19
fix: copy node from alpine image #176
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
base: main
Are you sure you want to change the base?
Conversation
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.
Pull request overview
This PR refactors the Dockerfile to copy Node.js binaries from the official Node Alpine image instead of installing them via apk. This approach provides better consistency with official Node.js releases.
Key Changes:
- Introduces multi-stage build with Node Alpine base image for copying Node.js binaries
- Replaces
apk add nodejs npmwith direct file copying and symlink creation - Simplifies PHP extension installation by removing version-specific conditional logic
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| mv docker-php-ext-xdebug.ini docker-php-ext-xdebug.disabled | ||
| mv docker-php-ext-tideways.ini docker-php-ext-tideways.disabled | ||
| mv docker-php-ext-blackfire.ini docker-php-ext-blackfire.disabled | ||
|
|
Copilot
AI
Dec 9, 2025
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.
The extension disabling logic now unconditionally disables all extensions. The old code only disabled xdebug, tideways, and blackfire extensions for PHP versions below 8.5.0. Now these extensions are always disabled even after being installed. Consider wrapping these moves in a version check:
PHP_VERSION_ID=$(php -r 'echo PHP_VERSION_ID;')
if [[ "$PHP_VERSION_ID" -lt 80500 ]]; then
mv docker-php-ext-xdebug.ini docker-php-ext-xdebug.disabled
mv docker-php-ext-tideways.ini docker-php-ext-tideways.disabled
mv docker-php-ext-blackfire.ini docker-php-ext-blackfire.disabled
fi| mv docker-php-ext-xdebug.ini docker-php-ext-xdebug.disabled | |
| mv docker-php-ext-tideways.ini docker-php-ext-tideways.disabled | |
| mv docker-php-ext-blackfire.ini docker-php-ext-blackfire.disabled | |
| PHP_VERSION_ID=$(php -r 'echo PHP_VERSION_ID;') | |
| if [[ "$PHP_VERSION_ID" -lt 80500 ]]; then | |
| mv docker-php-ext-xdebug.ini docker-php-ext-xdebug.disabled | |
| mv docker-php-ext-tideways.ini docker-php-ext-tideways.disabled | |
| mv docker-php-ext-blackfire.ini docker-php-ext-blackfire.disabled | |
| fi |
|
|
||
| apk add --no-cache bash git shadow patch coreutils | ||
|
|
||
| install-php-extensions tideways blackfire xdebug/[email protected] |
Copilot
AI
Dec 9, 2025
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.
The PHP extensions installation has been simplified but removes the version check. The old code only installed tideways blackfire xdebug for PHP versions below 8.5.0. Now all three extensions are always installed, which may cause issues if they're incompatible with PHP 8.5+. Consider keeping the version check:
PHP_VERSION_ID=$(php -r 'echo PHP_VERSION_ID;')
if [[ "$PHP_VERSION_ID" -lt 80500 ]]; then
install-php-extensions tideways blackfire xdebug/[email protected]
fi| install-php-extensions tideways blackfire xdebug/[email protected] | |
| PHP_VERSION_ID=$(php -r 'echo PHP_VERSION_ID;') | |
| if [[ "$PHP_VERSION_ID" -lt 80500 ]]; then | |
| install-php-extensions tideways blackfire xdebug/[email protected] | |
| fi |
Co-authored-by: Copilot <[email protected]>
No description provided.