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

Don't use private libraries. #2

Merged
merged 1 commit into from
Dec 7, 2021
Merged

Conversation

toots
Copy link

@toots toots commented Nov 25, 2021

This PR removes the use of private libraries.

At the moment, dune does not generate valid META files when using private libraries: ocaml/dune#4839. The fix has been implemented in ocaml/dune#4841 but is currently waiting for a release: https://github.com/ocaml/dune/blob/main/CHANGES.md#unreleased

@toots
Copy link
Author

toots commented Nov 25, 2021

Example of the issue:

#=== ERROR while compiling liquidsoap.2.1.0 ===================================#
# context              2.0.8 | linux/arm64 | ocaml-base-compiler.4.12.1 | pinned(git+file:///tmp/liquidsoap-full/liquidsoap#main#948278f1)
# path                 ~/.opam/4.12.1/.opam-switch/build/liquidsoap.2.1.0
# command              /usr/bin/make
# exit-code            2
# env-file             ~/.opam/log/liquidsoap-12-014b3e.env
# output-file          ~/.opam/log/liquidsoap-12-014b3e.out
### output ###
# [...]
# make[2]: Leaving directory '/home/opam/.opam/4.12.1/.opam-switch/build/liquidsoap.2.1.0/src/encoder_formats'
# make[2]: Entering directory '/home/opam/.opam/4.12.1/.opam-switch/build/liquidsoap.2.1.0/src'
# MENHIR tools/json_parser.mly
# MENHIR lang/parser.mly
# OCAMLDEP
# OCAMLOPT -c tools/log.mli
# ocamlfind: Package `errno_bindings' not found - required by `unix-errno.unix'
# make[2]: *** [../Makefile.rules:125: tools/log.cmi] Error 2

Signed-off-by: Romain Beauxis <toots@rastageeks.org>
@psafont
Copy link
Member

psafont commented Nov 25, 2021

So that's what made some failures appear in the CI when merging the change to opam-repo.

Thanks for this, I'll cut a release with this changes and once it's fixed on a released dune I can bump the minimal version and make the libraries private again to reduce the chance of using them externally.

Do you think there are other worthwhile changes from your port to dune? I'd be happy to integrate them as well

@toots
Copy link
Author

toots commented Nov 25, 2021

Thanks for the quick response! I'm not sure haha. I think this looks fine as is, except for this unfortunate bug.

@lindig lindig merged commit 08ed358 into xapi-project:master Dec 7, 2021
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