Uh oh!
There was an error while loading. Please reload this page.
Bumped child process packages and open up Windows support again - #64
Bumped child process packages and open up Windows support again#64WyriHaximus wants to merge 2 commits into
Conversation
clue
left a comment
There was a problem hiding this comment.
@WyriHaximus Thanks for looking into this! Can you provide some instructions on how to best reproduce this on Windows? Is this covered by just running the test suite on Windows? I couldn't find any references in the updated dependencies, that's why I'm wondering what changes where required upstream.
WyriHaximus
commented
Apr 5, 2019
@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 |
165e223 to
68b50a5Compareghost
commented
Jul 9, 2019
@WyriHaximus@clue 🏓 What's the current state on this PR? |
clue
commented
Jul 10, 2019
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: clue/reactphp-sqlite#13 |
ghost
commented
Oct 5, 2019
@WyriHaximus status? |
WyriHaximus
commented
Oct 5, 2019
@CharlotteDunois right! @clue the messenger pool switched to fully using sockets in that release |
clue
commented
Oct 5, 2019
@WyriHaximus That's great! Let's make this actionable, what makes this PR "WIP"? |
WyriHaximus
commented
Oct 5, 2019
@clue I can't remember 🤐 , will have a check on my windows box tomorrow |
WyriHaximus
commented
Oct 7, 2019
@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
Oct 7, 2019
@WyriHaximus That would be fantastic! See reactphp/child-process#71 for possible Travis CI config. |
WyriHaximus
commented
Oct 7, 2019
@clue added it to this PR 👍 |
| - name: "Windows" | ||
| os: windows | ||
| language: shell # no built-in php support | ||
| before_install: |
There was a problem hiding this comment.
Looks like you may want to also update or overwrite the install and/or script instructions for Windows 👍
There was a problem hiding this comment.
yeah I missed that, will fix it later tonight 👍
871cf7d to
b06b908Compareghost
commented
Oct 7, 2019
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
Oct 7, 2019
@CharlotteDunois yup, I'll fix all the windows build issues in this PR 🤣 |
ghost
commented
Apr 29, 2020
@WyriHaximus status? |
b06b908 to
0a8e671CompareWyriHaximus
commented
Apr 29, 2020
@CharlotteDunois just rebased and pushed it, will have a better look tomorrow |
With the latest child process related packages adding support for windows again we can also open support for it again in this package