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

Copy over META files during ocaml-windows64.5.3.0 install #352

Merged
merged 1 commit into from
Feb 6, 2025

Conversation

WardBrian
Copy link
Collaborator

Closes #350

@WardBrian WardBrian requested a review from toots February 6, 2025 15:50
Copy link
Member

@toots toots left a comment

Choose a reason for hiding this comment

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

Looks good to me!

In general, if the CI passes and the changes are straight forward like this, I like to think that we can trust each other and merge with the understanding that this would be okay for everyone.

Just to help alleviate some of the potential pain points if someone else is too late to review.. 🙂

@WardBrian WardBrian merged commit 9fc2ad1 into main Feb 6, 2025
4 of 5 checks passed
@WardBrian WardBrian deleted the ocaml530-add-METAs branch February 6, 2025 16:33
@pirbo
Copy link
Collaborator

pirbo commented Feb 6, 2025

I was investigating the issue today but didn't had the time to share my conclusion in time. Still, sorry but according to me this PR is not correct.
The install rule of ocaml-windows64 does install the META files of bigarray bytes compiler-libs dynlink stdlib str threads unix (in windows-sysroot/lib/ocaml/*/META which is) in findlib path as shown by the output (after the installation of ocaml-windows without this PR) of

$ ocamlfind -toolchain windows list
compiler-libs       (version: 5.3.0)
compiler-libs.bytecomp (version: 5.3.0)
compiler-libs.common (version: 5.3.0)
compiler-libs.native-toplevel (version: 5.3.0)
compiler-libs.optcomp (version: 5.3.0)
compiler-libs.toplevel (version: 5.3.0)
dynlink             (version: 5.3.0)
optint              (version: 0.0.4)
result              (version: 1.5)
runtime_events      (version: 5.3.0)
seq                 (version: [distributed with OCaml 4.07 or above])
stdlib              (version: 5.3.0)
str                 (version: 5.3.0)
threads             (version: 5.3.0)
threads.posix       (version: [internal])
unix                (version: 5.3.0)

It does not intall findlib (and graphics) META as it is not its concern (anymore :)) !

The question then is why ocamlfind-windows does not intall findlib META file and my understanding of that is: because in ocamlfind-windows.1.9.6 would install if it existed src/findlib/META but it is not generated (from src/findlib/META.in) because it would be by the ./configure script which is not called in the opam file (I guess because the configure would overwrite the Makefile.config provided by the package...)

"What is the proper fix to ocamlfind-windows" was what I was hammering on but didn't sort out early enough to answer you in time :D (I was on the path of trying to call the configure script with enough option so that it generates the Makefile.config we want but also the other file it generates ;). An other option would be to call configure which would generate a wrong Makefile.config and then as an extra build step overwrite the generated Makefile.config by our own. Adding src/findlib/META.in to the subst of the opam file is sadly not an option as it does not follow the same replacement pattern :/

@WardBrian
Copy link
Collaborator Author

I’m happy if we come up with a more satisfying or correct response!

I think it would be reasonable to manually copy the sed command used to turn META.in into META out of the configure script and have that as a separate part of the build command?

pirbo added a commit to pirbo/opam-cross-windows that referenced this pull request Feb 6, 2025
@pirbo pirbo mentioned this pull request Feb 6, 2025
github-merge-queue bot pushed a commit that referenced this pull request Feb 7, 2025
* Revert "Copy over META files during ocaml-windows64.5.3.0 install (#352)"

This reverts commit 9fc2ad1.

* ocamlfind-windows installs its META files
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

ocamlfind-windows not populating META in 5.3.0 switches
3 participants