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 8 support #89

Closed
cheeghi opened this issue Nov 4, 2020 · 17 comments
Closed

PHP 8 support #89

cheeghi opened this issue Nov 4, 2020 · 17 comments

Comments

@cheeghi
Copy link

cheeghi commented Nov 4, 2020

I'm trying to use react/filesystem with PHP 8:

PHP 8.0.0RC3 (cli) (built: Oct 29 2020 21:25:02) ( NTS )
Copyright (c) The PHP Group
Zend Engine v4.0.0-dev, Copyright (c) Zend Technologies

But on composer install I got:

 Problem 1
    - wyrihaximus/react-child-process-pool[1.3.0, ..., 1.4.2] require php ^5.4||^7.0 -> your php version (8.0.0RC3) does not satisfy that requirement.
    - wyrihaximus/react-child-process-pool[1.5.0, ..., 1.6.0] require php ^7.0 || ^5.4 -> your php version (8.0.0RC3) does not satisfy that requirement.
    - react/filesystem v0.1.2 requires wyrihaximus/react-child-process-pool ^1.3 -> satisfiable by wyrihaximus/react-child-process-pool[1.3.0, ..., 1.6.0].
    - Root composer.json requires react/filesystem ^0.1.2 -> satisfiable by react/filesystem[v0.1.2].

It seems that the transitive dependency wyrihaximus/react-child-process-pool can't be used with PHP 8.

Do you have any plan on supporting PHP 8? Thanks.

@xtrime-ru
Copy link

Have you tried composer install --ignore-platform-reqs ?

@cheeghi
Copy link
Author

cheeghi commented Nov 5, 2020

Thank you for the response, of course ignoring the php version it works, but I think:

  1. having to specify the parameter is annoying (if I develop a package that depends on react/filesystem anyone who wants to use my package has to specify it)
  2. if actually a package can run on PHP 8, it shouldn't force the php version to be 5 or 7

@WyriHaximus
Copy link
Member

@cheeghi since those are my packages I'll have a look at adding PHP8 support

@cheeghi
Copy link
Author

cheeghi commented Nov 5, 2020

@WyriHaximus Ok thanks

@WyriHaximus
Copy link
Member

@cheeghi FYI I haven't forgotten about this and updated a lot of the underlying packages to support PHP 8. There are 3 left to do, two of them do all the messaging and pooling, I'll tag a release that adds the most bare PHP 8 support possible, and do a major release that fully supports PHP 7 features and also works on PHP 8.

@cheeghi
Copy link
Author

cheeghi commented Nov 27, 2020

@WyriHaximus Thanks for the info, I can imagine there's some work to do, hope can be released soon!

@WyriHaximus
Copy link
Member

@cheeghi The packages done so far are significantly smaller, 2/3 of what is left is a lot bigger so using a different short term strategy for them.

@holtkamp
Copy link

@WyriHaximus in #89 (comment) you mention a "bare minimum version that works on PHP 8", is that something I can find here: https://github.com/WyriHaximus/reactphp-child-process-pool ?

@WyriHaximus
Copy link
Member

@holtkamp So https://github.com/WyriHaximus/reactphp-child-process-pool relies on https://github.com/WyriHaximus/reactphp-child-process-messenger and that doesn't work on PHP 8 yet

@valzargaming
Copy link

valzargaming commented Jan 22, 2021

I was able to install this into DiscordPHP by using --ignore-platform-reqs in composer and was able to get some functionality working, but most things will either fail silently or throw a 500 internal server error. Most notably with PHP 8 changes, Windows stills seems to have issues with pipes but can use stdio sockets to achieve similar results.

@WyriHaximus
Copy link
Member

@valzargaming Yeah which is why https://github.com/WyriHaximus/reactphp-child-process-messenger uses TCP sockets to communicate with the child processes.

Anyway, about to round off work around a set of my ReactPHP packages today. Picking up related packages for this after that, the messenger package is going to be fun 🤐 .

@clue
Copy link
Member

clue commented Jan 25, 2021

@cheeghi Thanks for bringing this up!

I'm currently in the process of updating the test suites for all ReactPHP component and will add PHP 8 as a test target along the way 👍 We've verified PHP 8 works fine for most components via manual tests (e.g. reactphp/http#391), but couldn't add this to our CI setup due to a lack of support in Travis. I'll address this as soon as reactphp/reactphp#442 is merged, so this is definitely in the works and shouldn't take too long before everything is in :shipit:

On top of this, this project currently depends on a few external libraries maintained by others. @WyriHaximus already pointed out he's working on an update these, looking forward to an update 👍

Additionally, we're in the process of reworking this project's API (#46 and #90), so there's hope we will be able to simplify this somewhat and also support more platforms in the future 👍

@WyriHaximus
Copy link
Member

An update on this, since this weekend I've been spending an hour a day or more working on updating https://github.com/WyriHaximus/reactphp-child-process-messenger to PHP 7.4 and to get it working on PHP 8. FYI releases will be done in such a way there is no new release required on react/filesystem to get PHP 8 support out.

@isfar
Copy link

isfar commented Jun 2, 2021

@WyriHaximus any update so far?

@WyriHaximus
Copy link
Member

@isfar Almost there, this is the last piece: WyriHaximus/reactphp-child-process-pool#57 (comment)

@WyriHaximus
Copy link
Member

FYI I'm aware and looking into which angle to fix this from: WyriHaximus/reactphp-child-process-pool#69 (comment)

@WyriHaximus
Copy link
Member

Just did a few releases resolving this issue. Please reopen this issue if it somehow persists for you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants