Port php c extension to php8 #7793
Conversation
TeBoring
commented
Aug 11, 2020
TeBoring
commented
- Only ported c extension to php8.
- Didn't fixed the issue of throwing warnings for missing arginfo in bundled files.
- Tests not fixed, because syntax of phpunit (<7 vs >9.3) are not compatible.
- In next release, needs to drop php5 and php7.0 support (in order to use phpunit > 7)
tests.sh
Outdated
| popd | ||
| } | ||
|
|
||
| build_php8.0_zts_c() { |
There was a problem hiding this comment.
Do we need to test the ZTS build separately? I thought that the API changes for ZTS had disappeared as of PHP 7 (the TSRM stuff) and a ZTS build won't actually exercise any parallelism I don't think.
php/ext/google/protobuf/message.c
Outdated
| } | ||
|
|
||
| upb_msg_set(intern->msg, f, msgval, arena); | ||
| #if PHP_VERSION_ID < 80000 |
There was a problem hiding this comment.
I think this might be a PHP 7.4 change, not 8.0:
https://github.com/php/php-src/blob/PHP-7.4.0/UPGRADING.INTERNALS#L171-L173
Do we have any tests for 7.4?
There was a problem hiding this comment.
We have tests https://github.com/protocolbuffers/protobuf/blob/master/tests.sh#L665
This change just causes warning so far
php/ext/google/protobuf/protobuf.h
Outdated
| #define PROTO_MSG_P(obj) (Message*)Z_OBJ_P(obj) | ||
| #define PROTO_STRVAL_P(obj) Z_STRVAL_P(obj) | ||
| #define PROTO_STRLEN_P(obj) Z_STRLEN_P(obj) | ||
| #define PROTO_RETURN_VAL void |
There was a problem hiding this comment.
I think this is for 7.4, not 8.0:
https://github.com/php/php-src/blob/PHP-7.4.0/UPGRADING.INTERNALS#L171-L173
| @@ -43,6 +43,24 @@ const zval *get_generated_pool(); | |||
| #define GC_DELREF(h) --GC_REFCOUNT(h) | |||
| #endif | |||
|
|
|||
| #if PHP_VERSION_ID < 80000 | |||
There was a problem hiding this comment.
Please add a link to this, to help explain these changes:
https://github.com/php/php-src/blob/898bb97706326311a367aaef35b3f95510100f3c/UPGRADING.INTERNALS#L37-L39
* Only ported c extension to php8. * Didn't fixed the issue of throwing warnings for missing arginfo in bundled files. * Tests not fixed, because syntax of phpunit (<7 vs >9.3) are not compatible. * In next release, needs to drop php5 and php7.0 support (in order to use phpunit > 7)
