Skip to content
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

fix(docker): replace php:8.1-fpm-alpine image by php:8.1-fpm-bookworm: alpine doesn't support GLOB_BRACE #1360

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

IrAlfred
Copy link
Member

@IrAlfred IrAlfred commented Nov 17, 2024

Replace php:8.1-fpm-alpine docker base image by php:8.1-fpm-bookworm

This merge request replaces the php:8.1-fpm-alpine image with php:8.1-fpm-bookworm in our Docker setup. The change addresses the challenges caused by the minimalistic nature of the Alpine-based image, which often lacks key features and extensions required for smooth development and runtime environments.

@mercihabam
Copy link
Member

@IrAlfred, why shouldn't we consider changing the PHP image we're using? The issue arises due to the minimalistic nature of the Alpine image, which omits certain features. While it does reduce the Docker image size, is it worth the development effort required to work around the missing extensions?

@IrAlfred
Copy link
Member Author

@IrAlfred, why shouldn't we consider changing the PHP image we're using? The issue arises due to the minimalistic nature of the Alpine image, which omits certain features. While it does reduce the Docker image size, is it worth the development effort required to work around the missing extensions?

I've thought about it for a while... I agree with switching the base image. Which image would you recommend we use ?

@indridieinarsson
Copy link
Contributor

@IrAlfred, why shouldn't we consider changing the PHP image we're using? The issue arises due to the minimalistic nature of the Alpine image, which omits certain features. While it does reduce the Docker image size, is it worth the development effort required to work around the missing extensions?

I've thought about it for a while... I agree with switching the base image. Which image would you recommend we use ?

wouldn't php:8.1-fpm-bookworm be the one to use? Same php version, but running on the most up-to-date debian. Same publisher as the alpine-based image in use now.

@mercihabam
Copy link
Member

Thank you @indridieinarsson for the suggestion! @IrAlfred please try this one and keep us informed of the outcome.

@IrAlfred
Copy link
Member Author

Thank you @jacob-js , I work on it

@IrAlfred IrAlfred force-pushed the fix-docker-image-broken-glob-brace-undefined branch from 1ac9f16 to 395ddee Compare November 24, 2024 09:06
@IrAlfred IrAlfred changed the title [FIX] Docker image broken - GLOB_BRACE undefined [FIX] Replace php:8.1-fpm-alpine image by php:8.1-fpm-bookworm: alpine doesn't support GLOB_BRACE Nov 24, 2024
@IrAlfred IrAlfred force-pushed the fix-docker-image-broken-glob-brace-undefined branch 2 times, most recently from dd5b969 to fa71c5d Compare November 24, 2024 10:13
@IrAlfred IrAlfred changed the title [FIX] Replace php:8.1-fpm-alpine image by php:8.1-fpm-bookworm: alpine doesn't support GLOB_BRACE fix: Replace php:8.1-fpm-alpine image by php:8.1-fpm-bookworm: alpine doesn't support GLOB_BRACE Nov 24, 2024
@IrAlfred IrAlfred changed the title fix: Replace php:8.1-fpm-alpine image by php:8.1-fpm-bookworm: alpine doesn't support GLOB_BRACE fix(docker): Replace php:8.1-fpm-alpine image by php:8.1-fpm-bookworm: alpine doesn't support GLOB_BRACE Nov 24, 2024
@IrAlfred IrAlfred changed the title fix(docker): Replace php:8.1-fpm-alpine image by php:8.1-fpm-bookworm: alpine doesn't support GLOB_BRACE fix(docker): replace php:8.1-fpm-alpine image by php:8.1-fpm-bookworm: alpine doesn't support GLOB_BRACE Nov 24, 2024
@IrAlfred
Copy link
Member Author

Please review @jacob-js and @indridieinarsson

@mercihabam
Copy link
Member

@indridieinarsson, please share some feedback as soon as you can!

@indridieinarsson
Copy link
Contributor

@mercihabam @IrAlfred. Not really an insightful comment, but I did test the PR, and it works. Great job.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants