Imagick
PHP 8.3 compatibility (and 8.x prototypes fixes)
Work in progress, before:
========DIFF========
001- success
001+ Fatal error: Imagick::getResourceLimit(): Return value must be of type int, float returned in Unknown on line 0
========DONE========
FAIL Imagick::setResourceLimit test [tests/014-setresourcelimit.phpt]
001- Ok
001+ Fatal error: Arginfo / zpp mismatch during call of ImagickPixel::setColorValueQuantum() in /work/GIT/pecl-and-ext/imagick/tests/172_ImagickPixel_setColorValueQuantum_basic.php on line 11
========DONE========
FAIL Test ImagickPixel, setColorValueQuantum [tests/172_ImagickPixel_setColorValueQuantum_basic.phpt]
========DIFF========
001- %s: ImagickPixelIterator::newPixelIterator is deprecated. ImagickPixelIterator::getPixelIterator should be used instead in %s on line %d
002- done
001+ php: /work/build/phpmaster/Zend/zend_execute_API.c:975: zend_call_function: Assertion `!(call->func->common.fn_flags & (1 << 13)) || zend_verify_internal_return_type(call->func, fci->retval)' failed.
002+
003+ Termsig=6
========DONE========
FAIL Pixel Iterator tests [tests/020-pixeliterator.phpt]
========DIFF========
001- Complete
001+ Fatal error: Arginfo / zpp mismatch during call of ImagickKernel::fromMatrix() in /work/GIT/pecl-and-ext/imagick/tests/145_imagickkernel_coverage.php on line 10
========DONE========
FAIL ImagickKernel::fromMatrix test [tests/145_imagickkernel_coverage.phpt]
========DIFF========
001- Temporary-path was empty at start.
002- Temporary path was set correctly.
003- Temporary path was listed correctly.
004- This is fine.
001+ Fatal error: Imagick::getRegistry(): Return value must be of type string, false returned in Unknown on line 0
========DONE========
FAIL Test Imagick, setRegistry and getRegistry [tests/150_Imagick_setregistry.phpt]
========DIFF========
001- Ok
001+ Fatal error: Arginfo / zpp mismatch during call of ImagickKernel::fromMatrix() in /work/GIT/pecl-and-ext/imagick/tests/047_Imagick_convolveImage_7.php on line 13
========DONE========
FAIL Test Imagick, convolveImage [tests/047_Imagick_convolveImage_7.phpt]
001- height : 25
002- width : 25
003- x : 50
004- y : 50
005- Ok
001+ Fatal error: Arginfo / zpp mismatch during call of Imagick::subimageMatch() in /work/GIT/pecl-and-ext/imagick/tests/151_Imagick_subImageMatch_basic.php on line 18
========DONE========
FAIL Test Imagick, subImageMatch [tests/151_Imagick_subImageMatch_basic.phpt]
========DIFF========
format: png (portable network graphics)
units: undefined
type: palette
004- Image geometry 640x480
004+ Image geometry 640x480[Wed May 31 08:15:00 2023] Script: '/work/GIT/pecl-and-ext/imagick/tests/236_Imagick_identify_basic.php'
005+ /opt/php83/include/php/Zend/zend_string.h(174) : Freeing 0x00007fb9bfc04070 (72 bytes), script=/work/GIT/pecl-and-ext/imagick/tests/236_Imagick_identify_basic.php
006+ Last leak repeated 598 times
007+ [Wed May 31 08:15:00 2023] Script: '/work/GIT/pecl-and-ext/imagick/tests/236_Imagick_identify_basic.php'
008+ /work/build/phpmaster/Zend/zend_hash.c(291) : Freeing 0x00007fb9bfcaa480 (56 bytes), script=/work/GIT/pecl-and-ext/imagick/tests/236_Imagick_identify_basic.php
009+ === Total 600 memory leaks detected ===
========DONE========
FAIL Test Imagick, identifyImage [tests/236_Imagick_identify_basic.phpt]
001- Ok
001+ php: /work/build/phpmaster/Zend/zend_execute_API.c:975: zend_call_function: Assertion `!(call->func->common.fn_flags & (1 << 13)) || zend_verify_internal_return_type(call->func, fci->retval)' failed.
002+
003+ Termsig=6
========DONE========
FAIL Test Tutorial, fxAnalyzeImage [tests/229_Tutorial_fxAnalyzeImage_case1.phpt]
========DIFF========
--
Frame: 3
Frame: 4
Frame: 5
007- Ok
007+
008+ Fatal error: Imagick::optimizeImageLayers(): Return value must be of type bool, Imagick returned in Unknown on line 0
========DONE========
FAIL Test Imagick::optimizeimagelayers and Imagick::optimizeimagetransparency [tests/278_Imagick_optimaze_gif.phpt]
========DIFF========
001- Ok
001+ php: /work/build/phpmaster/Zend/zend_execute_API.c:975: zend_call_function: Assertion `!(call->func->common.fn_flags & (1 << 13)) || zend_verify_internal_return_type(call->func, fci->retval)' failed.
002+
003+ Termsig=6
========DONE========
FAIL Test Imagick, getPixelIterator [tests/083_Imagick_getPixelIterator_basic.phpt]
========DIFF========
001- Ok
001+ php: /work/build/phpmaster/Zend/zend_execute_API.c:975: zend_call_function: Assertion `!(call->func->common.fn_flags & (1 << 13)) || zend_verify_internal_return_type(call->func, fci->retval)' failed.
002+
003+ Termsig=6
========DONE========
FAIL Test Imagick, getPixelRegionIterator [tests/084_Imagick_getPixelRegionIterator_basic.phpt]
========DIFF========
Depth is 16
002- Ok
002+
003+ Fatal error: Imagick::getImageBlob(): Return value must be of type string, null returned in Unknown on line 0
========DONE========
FAIL Test Imagick, setDepth [tests/325_Imagick_setDepth.phpt]
========DIFF========
001- Ok
001+ php: /work/build/phpmaster/Zend/zend_vm_execute.h:1877: ZEND_DO_FCALL_SPEC_RETVAL_UNUSED_HANDLER: Assertion `!(call->func->common.fn_flags & (1 << 13)) || zend_verify_internal_return_type(call->func, ret)' failed.
002+
003+ Termsig=6
========DONE========
FAIL Test Imagick, setImageMask basic [tests/286_Imagick_setMask_basic.phpt]
========DIFF========
001- Ok
001+ php: /work/build/phpmaster/Zend/zend_vm_execute.h:1877: ZEND_DO_FCALL_SPEC_RETVAL_UNUSED_HANDLER: Assertion `!(call->func->common.fn_flags & (1 << 13)) || zend_verify_internal_return_type(call->func, ret)' failed.
002+
003+ Termsig=6
========DONE========
FAIL Test Imagick, medianFilterImage [tests/289_Imagick_setImageMask_basic.phpt]
001- 0
002- 0
003- 1
004- 1
005- 2
006- 2
007- 0
008- 1
009- 2
010- 0
011- 1
012- 2
013- still 2 as hasn't changed
014- Exception: Unable to set image index
015- Ok
001+ php: /work/build/phpmaster/Zend/zend_execute_API.c:975: zend_call_function: Assertion `!(call->func->common.fn_flags & (1 << 13)) || zend_verify_internal_return_type(call->func, fci->retval)' failed.
002+
003+ Termsig=6
========DONE========
FAIL Test iterating over images works [tests/292_index_iterator.phpt]
========DIFF========
true
002- false
003- true
002+ php: /work/build/phpmaster/Zend/zend_vm_execute.h:1877: ZEND_DO_FCALL_SPEC_RETVAL_UNUSED_HANDLER: Assertion `!(call->func->common.fn_flags & (1 << 13)) || zend_verify_internal_return_type(call->func, ret)' failed.
003+
004+ Termsig=6
========DONE========
FAIL Test pseudo formats [tests/246_antialias_image.phpt]
========DIFF========
001- Ok
001+ php: /work/build/phpmaster/Zend/zend_execute_API.c:975: zend_call_function: Assertion `!(call->func->common.fn_flags & (1 << 13)) || zend_verify_internal_return_type(call->func, fci->retval)' failed.
002+
003+ Termsig=6
========DONE========
FAIL Test ImagickPixelIterator, construct [tests/247_ImagickPixelIterator_construct_basic.phpt]
========DIFF========
001- Ok
001+ php: /work/build/phpmaster/Zend/zend_execute_API.c:975: zend_call_function: Assertion `!(call->func->common.fn_flags & (1 << 13)) || zend_verify_internal_return_type(call->func, fci->retval)' failed.
002+
003+ Termsig=6
========DONE========
FAIL Test ImagickPixelIterator, clear [tests/248_ImagickPixelIterator_clear_basic.phpt]
001- Ok
001+ Fatal error: ImagickPixelIterator::getNextIteratorRow(): Return value must be of type array, null returned in Unknown on line 0
========DONE========
FAIL Test ImagickPixelIterator, getNextIteratorRow [tests/249_ImagickPixelIterator_getNextIteratorRow_basic.phpt]
001- Ok
001+ php: /work/build/phpmaster/Zend/zend_execute_API.c:975: zend_call_function: Assertion `!(call->func->common.fn_flags & (1 << 13)) || zend_verify_internal_return_type(call->func, fci->retval)' failed.
002+
003+ Termsig=6
========DONE========
FAIL Test ImagickPixelIterator, resetIterator [tests/250_ImagickPixelIterator_resetIterator_basic.phpt]
========DIFF========
001- Ok
001+ php: /work/build/phpmaster/Zend/zend_execute_API.c:975: zend_call_function: Assertion `!(call->func->common.fn_flags & (1 << 13)) || zend_verify_internal_return_type(call->func, fci->retval)' failed.
002+
003+ Termsig=6
========DONE========
FAIL Test ImagickPixelIterator, construct [tests/252_ImagickPixelIterator_construct_basic.phpt]
========DIFF========
001- Ok
001+ Fatal error: Imagick::evaluateImages(): Return value must be of type bool, Imagick returned in Unknown on line 0
========DONE========
FAIL Test Imagick, Imagick::evaluateImages [tests/258_Imagick_evaluateImages_basic.phpt]
========DIFF========
001- Ok
001+ php: /work/build/phpmaster/Zend/zend_vm_execute.h:1877: ZEND_DO_FCALL_SPEC_RETVAL_UNUSED_HANDLER: Assertion `!(call->func->common.fn_flags & (1 << 13)) || zend_verify_internal_return_type(call->func, ret)' failed.
002+
003+ Termsig=6
========DONE========
FAIL Test localContrastImage [tests/260_localContrastImage.phpt]
========DIFF========
001- Ok
001+ php: /work/build/phpmaster/Zend/zend_vm_execute.h:1877: ZEND_DO_FCALL_SPEC_RETVAL_UNUSED_HANDLER: Assertion `!(call->func->common.fn_flags & (1 << 13)) || zend_verify_internal_return_type(call->func, ret)' failed.
002+
003+ Termsig=6
========DONE========
========DIFF========
001- Ok
001+ php: /work/build/phpmaster/Zend/zend_vm_execute.h:1877: ZEND_DO_FCALL_SPEC_RETVAL_UNUSED_HANDLER: Assertion `!(call->func->common.fn_flags & (1 << 13)) || zend_verify_internal_return_type(call->func, ret)' failed.
002+
003+ Termsig=6
========DONE========
FAIL Test autoGammaImage [tests/263_autoGammaImage.phpt]
========DIFF========
001- Ok
001+ php: /work/build/phpmaster/Zend/zend_vm_execute.h:1877: ZEND_DO_FCALL_SPEC_RETVAL_UNUSED_HANDLER: Assertion `!(call->func->common.fn_flags & (1 << 13)) || zend_verify_internal_return_type(call->func, ret)' failed.
002+
003+ Termsig=6
========DONE========
FAIL Test Imagick, mergeImageLayers [tests/092_Imagick_mergeImageLayers_basic.phpt]
Current state, single failure, probably not related to 8.3
========DIFF========
format: png (portable network graphics)
units: undefined
type: palette
004- Image geometry 640x480
004+ Image geometry 640x480[Wed May 31 12:18:34 2023] Script: '/work/GIT/pecl-and-ext/imagick/tests/236_Imagick_identify_basic.php'
005+ /opt/php83/include/php/Zend/zend_string.h(174) : Freeing 0x00007fcf55004070 (72 bytes), script=/work/GIT/pecl-and-ext/imagick/tests/236_Imagick_identify_basic.php
006+ Last leak repeated 598 times
007+ [Wed May 31 12:18:34 2023] Script: '/work/GIT/pecl-and-ext/imagick/tests/236_Imagick_identify_basic.php'
008+ /work/build/phpmaster/Zend/zend_hash.c(291) : Freeing 0x00007fcf550aa480 (56 bytes), script=/work/GIT/pecl-and-ext/imagick/tests/236_Imagick_identify_basic.php
009+ === Total 600 memory leaks detected ===
========DONE========
FAIL Test Imagick, identifyImage [tests/236_Imagick_identify_basic.phpt]
AND
=====================================================================
TEST RESULT SUMMARY
---------------------------------------------------------------------
Exts skipped : 0
Exts tested : 35
---------------------------------------------------------------------
Number of tests : 333 322
Tests skipped : 11 ( 3.3%) --------
Tests warned : 3 ( 0.9%) ( 0.9%)
Tests failed : 1 ( 0.3%) ( 0.3%)
Expected fail : 1 ( 0.3%) ( 0.3%)
Tests passed : 317 ( 95.2%) ( 98.4%)
---------------------------------------------------------------------
Time taken : 13 seconds
=====================================================================
=====================================================================
EXPECTED FAILED TEST SUMMARY
---------------------------------------------------------------------
Test Imagick, affineTransformImage [tests/031_Imagick_affineTransformImage_basic.phpt] XFAIL REASON: I don't understand what values are returned in which elements of getImageChannelStatistics
=====================================================================
=====================================================================
FAILED TEST SUMMARY
---------------------------------------------------------------------
Test Imagick, identifyImage [tests/236_Imagick_identify_basic.phpt]
=====================================================================
=====================================================================
WARNED TEST SUMMARY
---------------------------------------------------------------------
ImagickPixel iterator [tests/bug_73840.phpt] (warn: XFAIL section but test passes)
Test ImagickDraw, getDensity [tests/268_ImagickDraw_getDensity_basic.phpt] (warn: XFAIL section but test passes)
Test Imagick, GetImageChannelRange basic [tests/287_Imagick_GetImageChannelRange_basic.phpt] (warn: XFAIL section but test passes)
=====================================================================
@Danack Ready for review
Notice:
There is lot of place with
if (php_imagick_ensure_not_empty (intern->magick_wand) == 0)
return;
So this implies allowing null for the return type (ex: public function autoOrient(): ?bool {})
An alternative way will be to raise an exception in such cases.
@Danack Ready for review
Cool, thanks.
An alternative way will be to raise an exception in such cases.
I think it does actually raise an exception, but it would probably be good to change the name to make that more obvious.
but it would probably be good to change the name to make that more obvious.
Or perhaps use RETURN_THROW() everywhere after exception ?
@Danack last commit use RETURN_THROWS everywhere in ImagickDraw (which allows me to find some more minor issues), If you are OK, I will add same for other classes.
Despite initial work was for 8.3, the same errors appear on 8.2 (using 8.2.8-dev debug build)
So only the review object_handlers commit is really specific for 8.3
The RETURN_THROWS for the parameter passing looks good.
I'm thinking that maybe the code that is currently:
intern = Z_IMAGICK_P(getThis());
if (php_imagick_ensure_not_empty (intern->magick_wand) == 0)
return;
Could be replaced by a macro, as there is an awful lot of repetition of that in the code. What do you think?
Could be replaced by a macro, as there is an awful lot of repetition of that in the code. What do you think?
Indeed... 334 usages See last commit.
CI is happy again, so ready for review.
I have tried to remove all unneeded "return;"
All remaining have to be checked later, as this means the method may return null (unset return_value), so have to be in its proto, but this PR already has too much work... and the review will be painful, so don't want to add more (other PR may come later)
@remicollet Thank you, I appreciate the time you invested in preparing this pull request and for all of the work you've contributed to the PHP community.
You too @Danack, Thank you for the time and energy you've spent maintaining this project.
I appreciate you both!
Have a great weekend! ✌🏾
Just wanted to confirm this fixes issue with installing Imagick on PHP8.3 during Docker build (php:8.3.0RC5-fpm-bullseye). Thank you very much for this @remicollet and fingers crossed it will get merged and released soon 🙂.
28f27044e435a2b203e32675e942eb8de620ee58 compiled well on php:8.3.0RC5-fpm-alpine. See https://github.com/waja/docker-php83-fpm/blob/development/Dockerfile
FYI: we were able to build it on PHP 8.3 for amd64, but not arm64
#52 2615.4 In /tmp/pear/temp/imagick/Imagick.stub.php:
#52 2615.4 Unterminated preprocessor conditions
#52 2615.4 make: *** [Makefile:196: /tmp/pear/temp/imagick/Imagick_arginfo.h] Error 1
#52 2615.4 ERROR: `make -j4 INSTALL_ROOT="/tmp/pear/temp/pear-build-defaultuseryTW5hI/install-imagick-3.7.0" install' failed
https://github.com/yiisoft/yii2-docker/actions/runs/6979511769/job/18992949588#step:13:5408
FYI: we were able to build it on PHP 8.3 for
amd64, but notarm64https://github.com/yiisoft/yii2-docker/actions/runs/6979511769/job/18992949588#step:13:5408
IN log:
#52 2612.3 running: make -j4 INSTALL_ROOT="/tmp/pear/temp/pear-build-defaultuseryTW5hI/install-imagick-3.7.0" install
#52 2612.5 Parse /tmp/pear/temp/imagick/ImagickDraw.stub.php to generate /tmp/pear/temp/imagick/ImagickDraw_arginfo.h
#52 2612.5 Parse /tmp/pear/temp/imagick/ImagickPixel.stub.php to generate /tmp/pear/temp/imagick/ImagickPixel_arginfo.h
#52 2612.5 Parse /tmp/pear/temp/imagick/Imagick.stub.php to generate /tmp/pear/temp/imagick/Imagick_arginfo.h
This should not happen, such generated files are not usable (in this ext) A touch on the headers may be a workaround
Just tested on aarch64, with PHP 8.3.0
[remi@builder2 imagick (issue-php83)]$ make test TESTS='-j64 --show-diff'
=====================================================================
PHP : /usr/bin/php
PHP_SAPI : cli
PHP_VERSION : 8.3.0
ZEND_VERSION: 4.3.0
PHP_OS : Linux - Linux builder2.remirepo.net 5.14.0-362.8.1.el9_3.aarch64 #1 SMP PREEMPT_DYNAMIC Tue Oct 3 11:57:53 EDT 2023 aarch64
INI actual : /home/remi/work/imagick/tmp-php.ini
More .INIs :
---------------------------------------------------------------------
...
=====================================================================
TEST RESULT SUMMARY
---------------------------------------------------------------------
Exts skipped : 0
Exts tested : 17
---------------------------------------------------------------------
Number of tests : 333 322
Tests skipped : 11 ( 3.3%) --------
Tests warned : 3 ( 0.9%) ( 0.9%)
Tests failed : 0 ( 0.0%) ( 0.0%)
Expected fail : 1 ( 0.3%) ( 0.3%)
Tests passed : 318 ( 95.5%) ( 98.8%)
---------------------------------------------------------------------
Time taken : 11 seconds
=====================================================================
@Danack sorry for pinging, but is there any particular reason this was not reviewed and merged? We would like to bump our Docker runtime to PHP 8.3 but don't want to rely on fork. I have this in my PoC Dockerfile:
ARG IMAGICK_PHP83_FIX_COMMIT=9df92616f577e38625b96b7b903582a46c064739
...
&& curl -L https://github.com/remicollet/imagick/archive/${IMAGICK_PHP83_FIX_COMMIT}.zip -o /tmp/imagick-issue-php83.zip \
&& unzip /tmp/imagick-issue-php83.zip -d /tmp \
&& pecl install /tmp/imagick-${IMAGICK_PHP83_FIX_COMMIT}/package.xml \
but as you may know it's not a solution you would like to rely in such a big, client-facing application like GetResponse 😉.
FYI: here's a gist with Docker build output that shows installation failure for Imagick 3.7.0 on PHP 8.3.2 (same was on 8.3.0RC5). Using this PR's code for installation (at least commit 9df92616) fixed it for us.
Hi @schmunk42,
FYI: we were able to build it on PHP 8.3 for
amd64, but notarm64
If you still have issues, please open a separate issue.
Hi @schmunk42,
FYI: we were able to build it on PHP 8.3 for
amd64, but notarm64If you still have issues, please open a separate issue.
Just for the record ... 8.3 builds with default versions: https://github.com/yiisoft/yii2-docker/actions/runs/9908435638 :+1: