Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Windows vendor stubs #530

Merged
merged 5 commits into from
May 28, 2023
Merged

Conversation

patricoferris
Copy link
Collaborator

Building on #529

This PR handles most of the external pins that the windows backend needed:

  • The small number of cstruct stubs we needed are now vendored directly in eio_windows. We should probably also bring the send and recv stubs along but eio_posix could uses those, so perhaps a follow-up PR for that would be nice.
  • Turns out most of our woes stemmed back to not having a patch for ocamlbuild from opam-repository-mingw which I think is the essence of this PR quick windows fixes for 4.03 ocaml/ocamlbuild#70 -- though not equal. I have the patch applied to a fork of ocamlbuild and that is now the only pin in the eio_windows package. The patch either needs upstreaming to ocamlbuild or ocaml/opam-repository.

avsm added a commit to avsm/opam-repository that referenced this pull request May 28, 2023
This patch has been lingering externally since 2016
(ocaml/ocamlbuild#70), and Windows builds
don't get very far without it any more, and it's blocking development
of other upstream Windows ports (ocaml-multicore/eio#530).

So barring strong objections, I'd like to pull this into opam-repository
mainline. No version bump required since it only patches Windows, which is
broken anyway without this fix.

Source patch from https://github.com/fdopen/opam-repository-mingw/blob/opam2/packages/ocamlbuild/ocamlbuild.0.14.2/opam
@avsm
Copy link
Contributor

avsm commented May 28, 2023

Time to ditch that last pin-depends! ocaml/opam-repository#23832

Copy link
Collaborator

@talex5 talex5 left a comment

Choose a reason for hiding this comment

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

Thanks! Should we merge this now, or wait for ocaml/opam-repository#23832?

@avsm
Copy link
Contributor

avsm commented May 28, 2023

Merge away I say, can do a followup for the last pin-depends later. Need to wait a few days for feedback on the opam-repo one.

@avsm avsm merged commit bf25e24 into ocaml-multicore:main May 28, 2023
avsm added a commit to avsm/opam-repository that referenced this pull request May 29, 2023
A variant of this patch has been lingering externally since 2016
(ocaml/ocamlbuild#70), and Windows builds don't get very far without it
any more, and it's blocking development of other upstream Windows ports
(ocaml-multicore/eio#530).

So barring strong objections, I'd like to pull this into opam-repository
mainline.  This package is only available on Windows.

Source patch from https://github.com/fdopen/opam-repository-mingw/blob/opam2/packages/ocamlbuild/ocamlbuild.0.14.2/opam

Reviewed by @dra27
avsm added a commit to avsm/opam-repository that referenced this pull request May 29, 2023
A variant of this patch has been lingering externally since 2016
(ocaml/ocamlbuild#70), and Windows builds don't get very far without it
any more, and it's blocking development of other upstream Windows ports
(ocaml-multicore/eio#530).

So barring strong objections, I'd like to pull this into opam-repository
mainline.  This package is only available on Windows.

Source patch from https://github.com/fdopen/opam-repository-mingw/blob/opam2/packages/ocamlbuild/ocamlbuild.0.14.2/opam

Reviewed by @dra27 and @patricoferris
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.

3 participants