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

[PHP] Upgrade to 8.0 #3655

Merged
merged 35 commits into from
Sep 10, 2021
Merged

[PHP] Upgrade to 8.0 #3655

merged 35 commits into from
Sep 10, 2021

Conversation

waghanza
Copy link
Collaborator

@waghanza waghanza commented Dec 26, 2020

Hi,

This PR replaces #3606, it upgrade PHP to 8.0

Regards,

@waghanza waghanza changed the title Php/upgrade/v8 [PHP] Upgrade to 8.0 Dec 26, 2020
@waghanza
Copy link
Collaborator Author

waghanza commented Dec 27, 2020

@dominikzogg
Copy link
Member

@waghanza as i can see, you already saw the open PR: dflydev/dflydev-fig-cookies#39

@waghanza waghanza force-pushed the php/upgrade/v8 branch 4 times, most recently from 7ed3c74 to 53ce736 Compare January 11, 2021 20:36
@waghanza waghanza marked this pull request as draft January 13, 2021 11:38
@dominikzogg
Copy link
Member

@waghanza the dependency that was not ready for php 8 (dflydev/dflydev-fig-cookies) is ready now, means the chubbyphp-swoole and slim-swoole should run on php8

@waghanza
Copy link
Collaborator Author

let see what's CI say

@dominikzogg
Copy link
Member

@waghanza it seems that the newly added libevent does not work (all workerman stuff fails)

@waghanza
Copy link
Collaborator Author

You mean event php module configured in

I though this module was required on production

@dominikzogg
Copy link
Member

@waghanza it's much better with the extension (from what I read) but it seems to fail compiling

@waghanza
Copy link
Collaborator Author

waghanza commented Jan 23, 2021

@waghanza it's much better with the extension (from what I read) but it seems to fail compiling

And idea @walkor ?

As I can see event has php8 support https://pecl.php.net/package-changelog.php?package=event&release=3.0.2

@dominikzogg see that an outdate version of event is used https://github.com/the-benchmarker/web-frameworks/runs/1753805064?check_suite_focus=true#step:6:975, I'll open an issue

@laylatichy
Copy link
Contributor

laylatichy commented Jan 23, 2021

@waghanza it's much better with the extension (from what I read) but it seems to fail compiling

And idea @walkor ?

As I can see event has php8 support https://pecl.php.net/package-changelog.php?package=event&release=3.0.2

@dominikzogg see that an outdate version of event is used https://github.com/the-benchmarker/web-frameworks/runs/1753805064?check_suite_focus=true#step:6:975, I'll open an issue

@waghanza pecl and pear are not available in php8 by default - see https://externals.io/message/103977

instead of pecl install xxx you have to run

mkdir -p /usr/src/php/ext/xxx && curl -fsSL https://pecl.php.net/get/xxx | tar xvz -C "/usr/src/php/ext/xxx" --strip 1 && docker-php-ext-install xxx \

@waghanza
Copy link
Collaborator Author

pecl and pear are not available in php8

However I have access to this command on php8 containers

@dominikzogg Having an non-compatible extension is 🆗 since PHP 8 event is still in beta. We have to wait

@laylatichy
Copy link
Contributor

laylatichy commented Jan 23, 2021

pecl and pear are not available in php8

However I have access to this command on php8 containers

@dominikzogg Having an non-compatible extension is 🆗 since PHP 8 event is still in beta. We have to wait

yes, but as you can see - /tmp/pear/temp/event/php7 - php7 pear, not 8, so that's why it's grabbing event for a php7 not v3

php8-pear is in community repo on alpine https://pkgs.alpinelinux.org/packages?name=php8-pear&branch=edge

btw workerman will work without event, I disabled it in my nano due to php8 pecl shenanigan's ;p will add it later on as well

@walkor
Copy link
Contributor

walkor commented Jan 23, 2021

@waghanza try command pecl install event-3.0.2 instead of pecl install event.

@waghanza
Copy link
Collaborator Author

@waghanza try command pecl install event-3.0.2 instead of pecl install event.

It's working, but I prefer avoid this. I think the only solution is to wait

@waghanza
Copy link
Collaborator Author

waghanza commented Jan 23, 2021

Thanks @laylatichy. I've made a little fix to containers definitions, so as pecl for php8 is used.

The changes is in php core, and seems not to be temp : php/php-src#3781

However, I'm a little bit confused about this : a C based extension should not be installed with https://github.com/FriendsOfPHP/pickle in php8 ?

@laylatichy
Copy link
Contributor

Thanks @laylatichy. I've made a little fix to containers definitions, so as pecl for php8 is used.

The changes is in php core, and seems not to be temp : php/php-src#3781

However, I'm a little bit confused about this : a C based extension should not be installed with https://github.com/FriendsOfPHP/pickle in php8 ?

pickle last time I checked was still in a bit of beta state, we still have to wait for some frameworks here that use laminas deps for example so maybe it'll be ready by then

@waghanza
Copy link
Collaborator Author

seems that pickle is ready to be used @laylatichy

@waghanza
Copy link
Collaborator Author

We should have been noticed by renovatebot. Could you make a PR @limingxinleo ?

@limingxinleo
Copy link
Member

@waghanza #4477

@waghanza waghanza marked this pull request as ready for review July 18, 2021 06:09
@waghanza
Copy link
Collaborator Author

@leocavalcante @sy-records I have finished this feature (I have to include hyperf beta and phalcon alpha to unblock this PR). May you quick review this ?

leocavalcante
leocavalcante previously approved these changes Jul 18, 2021
sy-records
sy-records previously approved these changes Jul 18, 2021
@waghanza
Copy link
Collaborator Author

@waghanza waghanza dismissed stale reviews from sy-records and leocavalcante via 80aeadc July 20, 2021 05:45
@waghanza
Copy link
Collaborator Author

waghanza commented Jul 24, 2021

@Jeckerson
Copy link

@waghanza This occur due missing header, will be fixed in next version.

https://github.com/phalcon/cphalcon/pull/15610/files#diff-90ad24223200fd3ab4bdd6b25c2264b1339b2f6693b8c4b3b869c0ff03cbb5a5R54

@waghanza
Copy link
Collaborator Author

@sy-records @leocavalcante @laylatichy @dominikzogg finally ... this PR is ready and CI is OK

the only think is that I use phalcon alpha @Jeckerson

thanks for your awesome contributions to all 🎉

@waghanza waghanza merged commit bfdf3cc into master Sep 10, 2021
@waghanza waghanza deleted the php/upgrade/v8 branch September 10, 2021 05:13
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.