reactphp / filesystem

Evented filesystem access.
MIT License
135 stars 40 forks source link

PHP 8 support #89

Closed cheeghi closed 3 years ago

cheeghi commented 3 years ago

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 commented 3 years ago

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

cheeghi commented 3 years ago

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 commented 3 years ago

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

cheeghi commented 3 years ago

@WyriHaximus Ok thanks

WyriHaximus commented 3 years ago

@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 commented 3 years ago

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

WyriHaximus commented 3 years ago

@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 commented 3 years ago

@WyriHaximus in https://github.com/reactphp/filesystem/issues/89#issuecomment-734946005 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 commented 3 years ago

@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 commented 3 years ago

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 commented 3 years ago

@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 :zipper_mouth_face: .

clue commented 3 years ago

@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 :+1: We've verified PHP 8 works fine for most components via manual tests (e.g. https://github.com/reactphp/http/pull/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 https://github.com/reactphp/reactphp/pull/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 :+1:

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 :+1:

WyriHaximus commented 3 years ago

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 commented 3 years ago

@WyriHaximus any update so far?

WyriHaximus commented 3 years ago

@isfar Almost there, this is the last piece: https://github.com/WyriHaximus/reactphp-child-process-pool/issues/57#issuecomment-855434497

WyriHaximus commented 3 years ago

FYI I'm aware and looking into which angle to fix this from: https://github.com/WyriHaximus/reactphp-child-process-pool/issues/69#issue-924484539

WyriHaximus commented 3 years ago

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