Skip to content

Conversation

@Mistuke
Copy link
Contributor

@Mistuke Mistuke commented May 20, 2020

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
Copy link
Contributor Author

Mistuke commented May 20, 2020

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

Copy link

@AndreasPK AndreasPK left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will take a closer look soon.

@Mistuke Mistuke force-pushed the wip/new-io-manager-support branch 5 times, most recently from 3320a25 to d73130a Compare May 25, 2020 10:49
@bgamari bgamari mentioned this pull request Jun 1, 2020
@Mistuke
Copy link
Contributor Author

Mistuke commented Jun 2, 2020

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 Mistuke force-pushed the wip/new-io-manager-support branch from d73130a to 3731a2f Compare June 10, 2020 18:50
@Mistuke 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
Copy link
Contributor Author

Mistuke commented Jun 18, 2020

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 ?

Copy link
Contributor

@bgamari bgamari left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks pretty good to me

@Mistuke 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 Mistuke changed the title WIP: Add support for WINIO to process. Add support for WinIO to process. Jun 25, 2020
@Mistuke Mistuke force-pushed the wip/new-io-manager-support branch from 9dac4b3 to 54a004c Compare June 25, 2020 07:59
@Mistuke Mistuke force-pushed the wip/new-io-manager-support branch from 54a004c to b790744 Compare June 25, 2020 21:24
@Mistuke
Copy link
Contributor Author

Mistuke commented Jun 25, 2020

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

@snoyberg
Copy link
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
Copy link
Contributor Author

Mistuke commented Jun 30, 2020

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

Copy link
Contributor

@bgamari bgamari left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me. Thanks @Mistuke!

@bgamari bgamari merged commit cb1d1a6 into haskell:master Jul 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants