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

Add support for PHP 8.3 #92

Closed
wants to merge 3 commits into from
Closed

Add support for PHP 8.3 #92

wants to merge 3 commits into from

Conversation

jessedobbelaere
Copy link
Contributor

@jessedobbelaere jessedobbelaere commented Apr 9, 2024

Description

PHP 8.2 will stop to be actively supported in just 7 months. Let's get ready for that by supporting PHP 8.3 which was released in November 2023?

Screenshot 2024-04-09 at 23 51 30@2x

Important changes:

  • Started from a copy from the 8.2 folder
  • Only issue was with Imagick that throws an error during build. There are issues/PR's open in the imagick repo but no clear timeline about a release that contains the fix. It seems to be fixed in the develop/master branch though but not released on PECL. The project seems to be a bit abandoned as well. I followed the fix that Yii framework had to do here. See line 96 of the Dockerfile
  • Bumped mozjpeg version from 3.3.1 to 4.1.1. They switched to use cmake to build, so it required a change in Dockerfile.

Related issues

#90

@jessedobbelaere
Copy link
Contributor Author

@timkelty can you take a look when you find some time 🤞

Also, not sure how to trigger the GitHub build action – but it builds locally with make local.

@timkelty
Copy link
Collaborator

@jessedobbelaere yep, sorry for the delay.

FWIW - we're working on migrating away from alpine, which is why we've been dragging on these images for a while.

@jasonmccallister
Copy link
Member

@jessedobbelaere the new images that we are actively maintaining are here https://github.com/craftcms/image

Would love some feedback, when I was working on these images initially with Alpine, we made them very opinionated and semi-designed for Nitro.

The new images only act as a base and come with what you need the php-fpm. We have been using these on other projects with a lot of success. Also, PHP 8.3 is the default branch now that has regular builds/updates.

@jessedobbelaere
Copy link
Contributor Author

Great! I'll try to integrate it in my production docker build and post findings if any 👍

@jessedobbelaere
Copy link
Contributor Author

jessedobbelaere commented Apr 18, 2024

@jasonmccallister One thing i'm missing in craftcms/image is the imagick extension and the optimizers were nice too (jpegoptim, optipng, ...). The craft docs say that imagick is recommended. Should it be installed by default in craftcms/image or do you suggest to leave it to be added by the end-user?

@jasonmccallister
Copy link
Member

jasonmccallister commented Apr 18, 2024

Should it be installed by default in craftcms/image or do you suggest to leave it to be added by the end-user?

We've gone back and forth on this a few times, for our enterprise hosting clients we end up using a third party service and with Cloud we don't include those because they run on Lambda and the CDN handles transforms at the edge...

Considering these are not for Cloud, we should probably consider adding them or at least documenting how to add those.

@timkelty I know we talked about removing that requirement from core, but I don't remember if we actually did that or not?

@timkelty
Copy link
Collaborator

@timkelty I know we talked about removing that requirement from core, but I don't remember if we actually did that or not?

Craft 5 still must have at least GD, but that is largely for backward compatibility. I would guess Craft 6 wont have an explicit requirement and instead will be tied to which image transformer you are using.

jpegoptim, optipng, et al. were never Craft requirements, so I think the most we should do there is show examples of how to include those. ImageMagick is technically suggested, so we should probably have some guidance there. That said, I think including it by default is probably a bad idea – there has already been talk of supporting libvips as an alternative, so the landscape is looking more like BYO-image-tools.

@jessedobbelaere
Copy link
Contributor Author

Thanks, makes sense 👍

I'm using Imager-X (without third-party service for images) for generating avif/webp/jpg/png with optimizers. So I'll add the necessary dependencies back in Docker. Some examples would be quite helpful for the community because mozjpeg and others can be a pain to add (needs to be built from scratch).

@jessedobbelaere
Copy link
Contributor Author

I added cavif (rust-based), cwebp, jpegoptim, optipng and imagemagick.
I skipped mozjpeg because of the painful install (no apt package, needs to be built from scratch in a separate stage) and JPG's are becoming less important with Avif and Webp anyways 💯

ARG CAVIF_VERSION="1.5.5"
RUN curl -L -o cavif.zip https://github.com/kornelski/cavif-rs/releases/download/v${CAVIF_VERSION}/cavif-${CAVIF_VERSION}.zip && \
    unzip cavif.zip -d cavif/ && \
    mv cavif/linux-generic/cavif /usr/local/bin/cavif && \
    rm -rf cavif.zip cavif/

RUN apt-get update -y && \
    apt-get install --no-install-recommends -y imagemagick php8.3-imagick jpegoptim optipng webp && \
    apt-get clean -y && \
    rm -rf /var/lib/apt/lists/*

Will try a deployment on a staging environment 👍 But overall looks pretty good!

I'll close this PHP 8.3 PR meanwhile

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