reactphp / filesystem

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

Bumped child process packages and open up Windows support again #64

Open WyriHaximus opened 5 years ago

WyriHaximus commented 5 years ago

With the latest child process related packages adding support for windows again we can also open support for it again in this package

WyriHaximus commented 5 years ago

@clue Whoops this was supposed to be a draft PR. Wanted to do one last test run before opening it. In short all communication on the underlying dependencies goes over sockets rather then STD* and they all support react/child-process v0.6 by passing an list of overwritten file descriptors.

ghost commented 5 years ago

@WyriHaximus @clue 🏓 What's the current state on this PR?

clue commented 5 years ago

I agree that supporting the latest ChildProcess component makes perfect sense, but I don't see how this currently supports Windows? Perhaps split this into a separate follow-up PR?

For the reference, in case anybody's interested, here's an example how one could use socket I/O to communicate with a child process on Windows: https://github.com/clue/reactphp-sqlite/pull/13

ghost commented 4 years ago

@WyriHaximus status?

WyriHaximus commented 4 years ago

@CharlotteDunois right!

@clue the messenger pool switched to fully using sockets in that release

clue commented 4 years ago

@WyriHaximus That's great! Let's make this actionable, what makes this PR "WIP"?

WyriHaximus commented 4 years ago

@clue I can't remember 🤐 , will have a check on my windows box tomorrow

WyriHaximus commented 4 years ago

@clue Checked earlier today and it works on windows now. What do you think about adding an allowed to fail windows build on travis?

clue commented 4 years ago

@WyriHaximus That would be fantastic! See https://github.com/reactphp/child-process/pull/71 for possible Travis CI config.

WyriHaximus commented 4 years ago

@clue added it to this PR :+1:

ghost commented 4 years ago

You'll probably want to add this utility method to the tests for cross compatibility paths. https://github.com/reactphp/filesystem/pull/69/files#diff-d4c8c6dc8769324fc27cfdac19f05cafR122-R131

WyriHaximus commented 4 years ago

@CharlotteDunois yup, I'll fix all the windows build issues in this PR 🤣

ghost commented 4 years ago

@WyriHaximus status?

WyriHaximus commented 4 years ago

@CharlotteDunois just rebased and pushed it, will have a better look tomorrow