Add support for WinIO to process. - #177

Merged

bgamari merged 1 commit into

haskell:masterfrom

Mistuke:wip/new-io-manager-support

Jul 2, 2020

Mistuke commented May 20, 2020

Contributor

This adds support for the new Asynchronous I/O manager in GHC for Windows[1] to process.

This is required because process uses internal GHC APIs.

Do not merge yet, however goal for this is for GHC 8.12. It is ready for review.

[1] https://gitlab.haskell.org/ghc/ghc/-/merge_requests/1224

Mistuke commented May 20, 2020

Contributor Author

/cc @bgamari (I don't know AndreasK github handle).

Mistuke commented

View reviewed changes

process.cabal Outdated

Mistuke commented

View reviewed changes

include/runProcess.h

Mistuke commented

View reviewed changes

System/Process/Windows.hsc Outdated

Mistuke commented

View reviewed changes

cbits/win32/runProcess.c

Mistuke commented

View reviewed changes

cbits/win32/runProcess.c

Mistuke commented

View reviewed changes

cbits/win32/runProcess.c

AndreasPK reviewed

View reviewed changes

AndreasPK left a comment

I will take a closer look soon.

System/Process/Common.hs Outdated

System/Process/Common.hs

cbits/win32/runProcess.c

Mistuke force-pushed the wip/new-io-manager-support branch 5 times, most recently from 3320a25 to d73130a Compare

May 25, 2020 10:49

phadej mentioned this pull request

An ability to process to share created stdout and stderr Handle. #180

Closed

bgamari mentioned this pull request

Allow base-4.15 #178

Merged

Mistuke commented Jun 2, 2020

Contributor Author

Arg... looks like createPipe and createPipeFd are exposed. So will have to change those too. createPipeFd will just error out. I wonder though... what's the use case of createPipeFd?

Mistuke force-pushed the wip/new-io-manager-support branch from d73130a to 3731a2f Compare

June 10, 2020 18:50

Mistuke force-pushed the wip/new-io-manager-support branch 2 times, most recently from 4645ca5 to 76b157e Compare

June 18, 2020 17:35

Mistuke commented Jun 18, 2020

Contributor Author

Looks like I have to drop the test using the new manager explicitly.
But this needs a small fix for older GHCs and should be done.

I will push changes for createPipe in a different pr. Any comments here @AndreasPK @snoyberg ?

bgamari approved these changes

View reviewed changes

bgamari left a comment

Contributor

Looks pretty good to me

cbits/win32/runProcess.c Outdated

cbits/win32/runProcess.c Outdated

cbits/win32/runProcess.c

System/Process/Common.hs

Mistuke mentioned this pull request

re-evaluate the fix for GHC #3231 in Process #186

Open

Mistuke force-pushed the wip/new-io-manager-support branch 3 times, most recently from 4711f0e to 9dac4b3 Compare

June 25, 2020 06:47

Mistuke changed the title ~~WIP: Add support for WINIO to process.~~ Add support for WinIO to process.

Mistuke force-pushed the wip/new-io-manager-support branch from 9dac4b3 to 54a004c Compare

June 25, 2020 07:59


          winio: Add support for WINIO to process.

b790744

Mistuke force-pushed the wip/new-io-manager-support branch from 54a004c to b790744 Compare

June 25, 2020 21:24

Mistuke commented Jun 25, 2020

Contributor Author

This should be the final commit. Everything passes with it.

snoyberg commented Jun 26, 2020

Collaborator

I'm definitely out of my depth on this one. I'm OK moving ahead with merging this if both @Mistuke and @bgamari think this is ready.

Mistuke commented Jun 30, 2020

Contributor Author

Thanks @snoyberg, I don't have anything else outstanding on this. @bgamari ?

bgamari reviewed

View reviewed changes

bgamari left a comment

Contributor

This looks good to me. Thanks @Mistuke!

cbits/win32/runProcess.c

bgamari approved these changes

View reviewed changes

bgamari merged commit cb1d1a6 into haskell:master

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

Labels

None yet