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

Initial depexts for native Windows #25442

Merged
merged 4 commits into from
Mar 12, 2024
Merged

Conversation

dra27
Copy link
Member

@dra27 dra27 commented Mar 9, 2024

opam 2.2 provides integrated depext support for the Windows. This will be primarily of use for the mingw-w64 port of OCaml, rather than the MSVC port.

The main PR updating the compiler packages will demonstrate how these are integrated into the main conf- packages, but it seems worth having a separate PR where naming conventions, etc., can be discussed, and it reduces the diff of what is going to be quite a large PR...

Cygwin and MSYS2 expose the mingw-w64 compilers and libraries in a cross-compilation-like way. In Cygwin, the compilers are actually cross-compilers; in MSYS2, the packagement is cross-compiler-like, but the compilers themselves are host-native.

This creates an issue for opam-repository packaging which at present has been possible to ignore, given the general lack of first class cross-compilation support in OCaml. The fundamental issue is that the precise depext required depends on the compiler being installed in the switch. How that works will be addressed in the Windows PR, but it means that where each depext presently requires just one package, they now require additional packages for each architecture, and it is these architecture-specific packages which are in this PR.

I propose switching all of this to be done with a generator over the coming months, but I don't want to hold things up further while that's being developed/agreed/sorted, and having a PR which shows how depexts are generally handled will hopefully mean that others can be contributed manually in the meantime.

For naming convention, I've gone for conf-mingw-w64- (which should only be used for these packages) followed by the name of the "master" conf package (e.g. gcc, g++ or zstd here - so conf-gcc, conf-g++ and conf-zstd all should exist), followed by the name of the architecture as used by GCC (i.e. i686 / x86_64, not x86_32 / amd64, etc.).

It would be possible simply to install all of these architectures in the conf- package. I have two issues with that:

  • I think it's morally repugnant 😃
  • Certainly on Cygwin, many of these packages de facto pull in GCC - so it makes the size of the Cygwin / MSYS2 installation much bigger

I care quite a bit about the moral argument (if we're using an x86_64 compiler, why should opam start installing i686 dependencies?), but I think the space argument (especially for caching - e.g. in GitHub Actions runners, etc.) is significant.

@dra27
Copy link
Member Author

dra27 commented Mar 9, 2024

All review/comment/discussion very much welcomed now, though I'm not expecting this PR actually to be merged until fractionally before the main Windows PR has been (opened and) approved.

@dra27
Copy link
Member Author

dra27 commented Mar 10, 2024

opam-ci — Failed - Everything was skipped

Excellent, this one's at ERROR_SUCCESS too 🙂

@mseri mseri merged commit 035f72c into ocaml:master Mar 12, 2024
1 of 2 checks passed
@mseri
Copy link
Member

mseri commented Mar 12, 2024

We cannot really test the windows packages for now and they are restricted to windows, so they don't break anything if we merge them. I think we can revisit them once we have a windows CI and we can check how things are working. So far I check that what I see make sense in the limits of my understanding of windows and merge

@dra27
Copy link
Member Author

dra27 commented Mar 12, 2024

Thank you, @mseri!

@dra27 dra27 deleted the windows-native-depexts branch March 12, 2024 09:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants