search

yiisoft / yii2-docker

Official Docker images suitable for Yii 2.0
https://www.yiiframework.com/
BSD 3-Clause "New" or "Revised" License
381 stars 202 forks source link

Recent PHP 8 changes have broken imagick install in PHP 7.4 image #131

Closed tfrancisci closed 3 years ago

tfrancisci commented 3 years ago

Hey all.

Today we had some failing pipelines that were using the yii2-php:7.4-apache image. Composer would fail due to an incompatible version of imagick. It was reporting that the installed version was @PACKAGE_VERSION@

I won't give you my build logs as there is an easy way to reproduce the problem without them. I noticed that the problem was introduced on the new version of the image that was pushed 3 days ago (April 30th 2021) with digest d0b43c241834. I went back to a previous version of 7.4 and found 7.4-apache-21.3.1 from a month ago which does not display the problem.

Old version:

docker run --rm -ti yiisoftware/yii2-php:7.4-apache-21.3.1 php -i | grep “imagick module version”
imagick module version => 3.4.4

New version:

docker run --rm -ti yiisoftware/yii2-php:7.4-apache php -i | grep “imagick module version” 
imagick module version => @PACKAGE_VERSION@

I think I tracked it down to the following changes: Issue: https://github.com/yiisoft/yii2-docker/issues/110 PR: https://github.com/yiisoft/yii2-docker/pull/123 Commit: https://github.com/yiisoft/yii2-docker/commit/c7c94e40b50aae8ad2287cf384a59cedf9b24292

Prior to the above change, imagick was installed with pecl. After the change it is installed by downloading a package from github.

When I follow the exact same install steps from a vanilla PHP 7.4 image, I get the same problem where php -i returns @PACKAGE_VERSION@ instead of the version number. Interestingly the same occurs even when running it against a PHP 8 image. I guess that install method is broken/incomplete?

Also, is there a suitable immutable image tag that we should be using for this image?

Thanks,

Tom...

schmunk42 commented 3 years ago

Could you test and check these images https://github.com/yiisoft/yii-docker ?

tfrancisci commented 3 years ago

That looks good.

docker run --rm -ti yiisoftware/yii-php:7.4-apache php -i | grep "imagick module version"
imagick module version => 3.4.4

Looks like it uses a different mechanism to install though. It uses install-php-extensions instead of pecl or the manual process.

schmunk42 commented 3 years ago

And this?

Status: Downloaded newer image for yiisoftware/yii-php:8.0-apache
imagick module version => git--master-132a11fd26675db9eb9f0e9a3e2887c161875206

Yes, we're using https://github.com/mlocati/docker-php-extension-installer in yii-docker, since it was more reliable.

tfrancisci commented 3 years ago

Yes I get that too:

docker run --rm -ti yiisoftware/yii-php:8.0-apache php -i | grep "imagick module version"
imagick module version => git--master-132a11fd26675db9eb9f0e9a3e2887c161875206

That does not look good either.

schmunk42 commented 3 years ago

I remember reading that imagemagick has issues on PHP 8, so that might be the reason why they install from master.

https://github.com/Imagick/imagick/issues/358

tfrancisci commented 3 years ago

I get that, and that will somehow need to be resolved for PHP 8.

But in the meantime, the existing image for PHP 7.4 has been broken. It used to be installed just fine in the PHP 7.4 version of the image and now it's broken. Can this be reverted, at least for the PHP 7.4 image?

schmunk42 commented 3 years ago

So, I'd rather go with the scripts from https://github.com/mlocati/docker-php-extension-installer if they are working fine, WDYT?

tfrancisci commented 3 years ago

Now I see why you were asking about that yii-php image :)

Well, if you go that route, it will (should?) work in 7.4, but will remain broken in PHP 8. I guess that's a net win, but work would still need to be done for PHP 8 down the line.

As I understand, due to using Docker build arguments, it's non-trivial to have a separate install process for 7.4? I guess you would need a separate Dockerfile (or an include) to do that which would mean too much refactoring to fix this by reverting to pecl just for 7.4. Is that accurate?

Given that, then I say yeah, go for the docker-php-extension-installer.

Question: In the yii-php repo you COPY from mlocati/php-extension-installer and not mlocati/docker-php-extension-installer. The former repo does not exist. Am I missing something there?

schmunk42 commented 3 years ago

Question: In the yii-php repo you COPY from mlocati/php-extension-installer and not mlocati/docker-php-extension-installer. The former repo does not exist. Am I missing something there?

I guess that's just the image vs. repo name.

schmunk42 commented 3 years ago

As I understand, due to using Docker build arguments, it's non-trivial to have a separate install process for 7.4?

That's what the script collection from mlocati manages.