libvips / php-vips

php binding for libvips
MIT License
618 stars 25 forks source link

PHP 8 Support #118

Closed jesseforrest closed 2 years ago

jesseforrest commented 3 years ago

Hi, I was wondering if there are installation instructions available for PHP 8. We use php-vips heavily while on 7.1, but are trying to upgrade to PHP 8. I'm struggling to find available documentation for this.

jcupitt commented 3 years ago

Hi @jesseforrest, sorry, no idea, I've never tried php8.

I had a quick look at some guides and I think it'll probably just work, but someone would need to do some testing.

jcupitt commented 3 years ago

It seems quite a few tests fail. Looks like there will need to be a bit of minor dev work.

john@banana ~/GIT/php-vips-ext (master) $ make test

Build complete.
Don't forget to run 'make test'.

=====================================================================
PHP         : /home/john/vips/bin/php 
PHP_SAPI    : cli
PHP_VERSION : 8.0.3
ZEND_VERSION: 4.0.3
PHP_OS      : Linux - Linux banana 5.8.0-49-generic #55-Ubuntu SMP Wed Mar 24 14:45:45 UTC 2021 x86_64
INI actual  : /home/john/GIT/php-vips-ext/tmp-php.ini
More .INIs  :   
---------------------------------------------------------------------
PHP         : /home/john/vips/bin/phpdbg 
PHP_SAPI    : phpdbg
PHP_VERSION : 8.0.3
ZEND_VERSION: 4.0.3
PHP_OS      : Linux - Linux banana 5.8.0-49-generic #55-Ubuntu SMP Wed Mar 24 14:45:45 UTC 2021 x86_64
INI actual  : /home/john/GIT/php-vips-ext/tmp-php.ini
More .INIs  : 
---------------------------------------------------------------------
CWD         : /home/john/GIT/php-vips-ext
Extra dirs  : 
VALGRIND    : Not used
=====================================================================
TIME START 2021-04-17 10:38:15
=====================================================================
PASS check for vips presence [tests/001.phpt] 
FAIL vips can load a file [tests/002.phpt] 
FAIL vips can save a file [tests/003.phpt] 
FAIL vips can get image header fields [tests/004.phpt] 
FAIL we can call any vips operation [tests/005.phpt] 
FAIL vips_call supports optional input and output args [tests/006.phpt] 
PASS new_from_file supports optional args [tests/007.phpt] 
FAIL input array double args work [tests/008.phpt] 
FAIL input array image args work [tests/009.phpt] 
FAIL output int arrays work [tests/010.phpt] 
FAIL typeof works [tests/011.phpt] 
FAIL new_from_buffer works [tests/012.phpt] 
FAIL write_to_buffer works [tests/013.phpt] 
FAIL new_from_array works [tests/014.phpt] 
FAIL new_from_array sets values correctly [tests/015.phpt] 
FAIL new_from_array has optional scale and offset [tests/016.phpt] 
FAIL can set enum from int [tests/017.phpt] 
FAIL can use 2D array as image [tests/026.phpt] 
FAIL can use 1D array as constant image [tests/027.phpt] 
FAIL can call draw operations [tests/028.phpt] 
FAIL can get error messages [tests/029.phpt] 
FAIL enum fields are returned as strings [tests/030.phpt] 
FAIL write_to_file can set options [tests/031.phpt] 
PASS foreign_find_load_buffer works [tests/032.phpt] 
PASS foreign_find_load works [tests/033.phpt] 
FAIL can copy to memory [tests/034.phpt] 
PASS can make an interpolator [tests/035.phpt] 
FAIL can enlarge with a bicubic interpolator [tests/036.phpt] 
PASS can make an image from memory [tests/037.phpt] 
PASS can write to memory [tests/038.phpt] 
PASS can get version [tests/039.phpt] 
PASS can get info about cache and concurrency [tests/040.phpt] 
FAIL write_to_array works [tests/041.phpt] 
FAIL can set metadata [tests/042.phpt] 
jcupitt commented 3 years ago

For reference, I built php8 with:

./configure --prefix=/home/john/vips --enable-debug --enable-cgi --enable-cli \
    --with-readline --with-openssl --with-zlib --enable-mbstring --with-pear
kleisauke commented 3 years ago

According to https://blog.remirepo.net/pages/PECL-extensions-RPM-status, the PECL extension should be able to run with PHP 8 without any problems.

fwiw, I didn't observe any test failures after running the test suite on my Fedora PC.

Details ``` $ make test Build complete. Don't forget to run 'make test'. ===================================================================== PHP : /usr/bin/php PHP_SAPI : cli PHP_VERSION : 8.0.3 ZEND_VERSION: 4.0.3 PHP_OS : Linux - Linux pc-kaw 5.11.12-200.fc33.x86_64 #1 SMP Thu Apr 8 02:34:17 UTC 2021 x86_64 INI actual : /home/kleisauke/php-vips-ext/tmp-php.ini More .INIs : CWD : /home/kleisauke/php-vips-ext Extra dirs : VALGRIND : Not used ===================================================================== TIME START 2021-04-17 11:01:12 ===================================================================== PASS check for vips presence [tests/001.phpt] PASS vips can load a file [tests/002.phpt] PASS vips can save a file [tests/003.phpt] PASS vips can get image header fields [tests/004.phpt] PASS we can call any vips operation [tests/005.phpt] PASS vips_call supports optional input and output args [tests/006.phpt] PASS new_from_file supports optional args [tests/007.phpt] PASS input array double args work [tests/008.phpt] PASS input array image args work [tests/009.phpt] PASS output int arrays work [tests/010.phpt] PASS typeof works [tests/011.phpt] PASS new_from_buffer works [tests/012.phpt] PASS write_to_buffer works [tests/013.phpt] PASS new_from_array works [tests/014.phpt] PASS new_from_array sets values correctly [tests/015.phpt] PASS new_from_array has optional scale and offset [tests/016.phpt] PASS can set enum from int [tests/017.phpt] PASS can use 2D array as image [tests/026.phpt] PASS can use 1D array as constant image [tests/027.phpt] PASS can call draw operations [tests/028.phpt] PASS can get error messages [tests/029.phpt] PASS enum fields are returned as strings [tests/030.phpt] PASS write_to_file can set options [tests/031.phpt] PASS foreign_find_load_buffer works [tests/032.phpt] PASS foreign_find_load works [tests/033.phpt] PASS can copy to memory [tests/034.phpt] PASS can make an interpolator [tests/035.phpt] PASS can enlarge with a bicubic interpolator [tests/036.phpt] PASS can make an image from memory [tests/037.phpt] PASS can write to memory [tests/038.phpt] PASS can get version [tests/039.phpt] PASS can get info about cache and concurrency [tests/040.phpt] PASS write_to_array works [tests/041.phpt] PASS can set metadata [tests/042.phpt] ===================================================================== TIME END 2021-04-17 11:01:14 ===================================================================== TEST RESULT SUMMARY --------------------------------------------------------------------- Exts skipped : 0 Exts tested : 16 --------------------------------------------------------------------- Number of tests : 34 34 Tests skipped : 0 ( 0.0%) -------- Tests warned : 0 ( 0.0%) ( 0.0%) Tests failed : 0 ( 0.0%) ( 0.0%) Tests passed : 34 (100.0%) (100.0%) --------------------------------------------------------------------- Time taken : 2 seconds ===================================================================== ```
jcupitt commented 3 years ago

Oh, interesting, I see eg.:

$ more tests/002.out 
Fatal error: Arginfo / zpp mismatch during call of vips_image_new_from_file() in
 /home/john/GIT/php-vips-ext/tests/002.php on line 3

I suppose I've accidentally built my php with strict arg checking enabled.

It wouldn't hurt to add support for argument type hints. Let's leave this open as an enhancement.

jcupitt commented 3 years ago

Here's a branch with type hints added, it fixes most of the tests.

https://github.com/libvips/php-vips-ext/tree/add-type-hints

Though vips_call is still broken, I must have got the varadic hint wrong.

kleisauke commented 3 years ago

Here's a patch that seems to resolve the test failures on that branch with PHP 8 in debug mode:

diff --git a/vips.c b/vips.c
index 1111111..2222222 100644
--- a/vips.c
+++ b/vips.c
@@ -2233,7 +2233,7 @@ ZEND_BEGIN_ARG_INFO_EX(arginfo_vips_image_new_from_memory, 0, 0, 5)
    ZEND_ARG_TYPE_INFO(0, width, IS_LONG, 0)
    ZEND_ARG_TYPE_INFO(0, height, IS_LONG, 0)
    ZEND_ARG_TYPE_INFO(0, bands, IS_LONG, 0)
-   ZEND_ARG_TYPE_INFO(0, format, IS_LONG, 0)
+   ZEND_ARG_TYPE_INFO(0, format, IS_STRING, 0)
 ZEND_END_ARG_INFO()

 ZEND_BEGIN_ARG_INFO_EX(arginfo_vips_image_write_to_memory, 0, 0, 1)
@@ -2258,7 +2258,7 @@ ZEND_END_ARG_INFO()

 ZEND_BEGIN_ARG_INFO_EX(arginfo_vips_call, 0, 0, 2)
    ZEND_ARG_TYPE_INFO(0, name, IS_STRING, 0)
-   ZEND_ARG_TYPE_INFO(0, resource, IS_RESOURCE, 0)
+   ZEND_ARG_TYPE_INFO(0, resource, IS_RESOURCE, 1)
    ZEND_ARG_VARIADIC_INFO(0, vars)
 ZEND_END_ARG_INFO()

@@ -2284,7 +2284,7 @@ ZEND_END_ARG_INFO()

 ZEND_BEGIN_ARG_INFO_EX(arginfo_vips_image_set_type, 0, 0, 4)
    ZEND_ARG_TYPE_INFO(0, image, IS_RESOURCE, 0)
-   ZEND_ARG_TYPE_INFO(0, type, IS_LONG, 0)
+   ZEND_ARG_TYPE_MASK(0, type, MAY_BE_LONG|MAY_BE_STRING, NULL)
    ZEND_ARG_TYPE_INFO(0, field, IS_STRING, 0)
    ZEND_ARG_INFO(0, value)
 ZEND_END_ARG_INFO()

Do you think we should also need to hint the return type with ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX / ZEND_BEGIN_ARG_WITH_RETURN_TYPE_MASK_EX?

jcupitt commented 3 years ago

Brilliant! Yes, all tests pass now.

I agree, return type hints would be a good idea too.

jesseforrest commented 3 years ago

Really appreciate the fast response and turn-around fellas! I'll be digging into this and I'll keep you posted if I run into any issues on my end.

jcupitt commented 3 years ago

@jesseforrest after Kleis's work, it looks like git master php-vips-ext should now be OK with php8. Would you be able to test before we push out a new version?

sadektouati commented 3 years ago

@jesseforrest after Kleis's work, it looks like git master php-vips-ext should now be OK with php8. Would you be able to test before we push out a new version?

Hey I'm on Centos8 and php8 (from remi)

***** pecl install vips downloading vips-1.0.12.tgz ... Starting to download vips-1.0.12.tgz (561,450 bytes) .................................................................................................................done: 561,450 bytes 4 source files, building running: phpize

It just stops at running: phpize Any clues?

kleisauke commented 3 years ago

@sadektouati I think you can just do dnf install php-pecl-vips if you're using Remi's RPM repository.

sadektouati commented 3 years ago

@kleisauke Thank you very much, it worked. Do you know why it doesn't with pecl?

kleisauke commented 3 years ago

You'll need additional devel packages to build the extension from source, such as vips-devel php-devel php-pear gcc. However, I can recommend just installing the extension through the package manager with the command mentioned above.

sadektouati commented 3 years ago

You'll need additional devel packages to build the extension from source, such as vips-devel php-devel php-pear gcc. However, I can recommend just installing the extension through the package manager with the command mentioned above.

Enlightening bro. You saved me hours, thanks

sadektouati commented 3 years ago

@kleisauke I want to convert to AVIF format, it gives me this error

Uncaught Jcupitt\Vips\Exception: VipsForeignSave: "1636717374734212.avif" is not a known file format

it works for jpeg and webp though

I think I need to install one of these, not sure which one(s) vips-devel.x86_64 8.11.4-1.el8.remi remi-safe vips-full.x86_64 8.10.6-2.el8.remi remi-safe vips-full-devel.x86_64 8.10.6-2.el8.remi remi-safe vips-full-tools.x86_64 8.10.6-2.el8.remi remi-safe vips-heif.x86_64 8.11.4-1.el8.remi remi-safe vips-magick-gm.x86_64 8.11.4-1.el8.remi remi-safe vips-magick-im7.x86_64 8.11.4-1.el8.remi remi-safe vips-tools.x86_64 and these give some dependency errors like cannot install the best candidate for the job

Any ideas? please

kleisauke commented 3 years ago

You need to enable the EPEL, RPM Fusion and PowerTools repository for AVIF support. After that, you can install the vips-heif module from Remi's RPM repository.

dnf install epel-release
dnf install --nogpgcheck https://mirrors.rpmfusion.org/free/el/rpmfusion-free-release-8.noarch.rpm
dnf config-manager --set-enabled powertools
dnf install vips-heif

See also: https://github.com/remicollet/remirepo/issues/171. Please open a new issue if something is not clear.

sadektouati commented 3 years ago

@kleisauke Thanks for your help

I know you told me to open an issue with the remi repos. I have an issue but not sure where to go for solution With your help, now it's working. Just not giving a proper result.

`require_once DIR . '/vendor/autoload.php';

use Jcupitt\Vips;

$image = Vips\Image::newFromFile('x1636717374734212.jpeg'); $image->writeToFile('x1636ddddddddddddddddddd717374734212.avif');`

Here's the result Sans-titre-1

Any help?

xvilo commented 2 years ago

@sadektouati Maybe a bit late to the party but:

sadektouati commented 2 years ago

@sadektouati Maybe a bit late to the party but:

  • Did you happen to echo the file without setting the right content-type header?
  • As far as I know, chrome is not yet able to display avif images directly this way, maybe that could be the cause.
  • Have you tried downloading the avif file, and then try to open it in some other program?

Indeed, it was the Content-Type header issue, but not in Chrome, Apache doesn't set the correct header for avif images automatically, instead one should set a directive for that.

Now I have another problem: the colors are not right.

xvilo commented 2 years ago

@jesseforrest and/or @jcupitt what exactly is the current state of PHP 8+ support? Should it be there already? Or isn't it finished yet? If it's done, maybe close this one. If new bugs arise, new issues can be created

jcupitt commented 2 years ago

php8 was working with git master php-vips-ext, but we hadn't pushed out a pecl package. I've done that now, so 1.0.13 should work:

https://pecl.php.net/package/vips/1.0.13

All tests pass for me with php 8.0.8.

Looking further ahead, I've started rewriting php-vips to use the new php FFI system, so we can get rid of the annoying binary extension. It should make it much simpler to maintain and distribute.

The branch is here:

https://github.com/libvips/php-vips/tree/switch-to-php-ffi

It mostly works, with (I think) 6 tests currently still failing. A few more days, hopefully.

jcupitt commented 2 years ago

php8 support should be released and working. Please open a new issue if anyone finds a problem.