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

Add base-bytes for OCaml 5.0 support (bytes is not a dune builtin anymore since OCaml 5.0) #175

Merged
merged 1 commit into from
Oct 25, 2022

Conversation

kit-ty-kate
Copy link
Contributor

In opam-repository, base-bytes depends on ocamlfind which installs a dummy META file for the bytes package.
With OCaml 5.0 dune does not have any builtin packages anymore and thus relies on the META files given by ocamlfind for bytes.

However in mirage context ocamlfind is not installed, thus we need a dummy package providing bytes otherwise we are unable to compile any mirage projects in OCaml 5.0

@Leonidas-from-XIV
Copy link
Contributor

Ah yes, I can see how this makes sense. I'm slightly worried about packages implicitly depending on ocamlfind via base-bytes but frankly these packages (if they exist) have wrong metadata anyway and should be fixed elsewhere.

Can you make a tarball with the release? That way it'll be more universal and faster to install since that way the git client is not required and the tarball can be checksummed easily.

@kit-ty-kate
Copy link
Contributor Author

Done

@Leonidas-from-XIV Leonidas-from-XIV merged commit 83aaba4 into dune-universe:master Oct 25, 2022
@Leonidas-from-XIV
Copy link
Contributor

Thanks, that seems good!

@avsm
Copy link
Contributor

avsm commented Jan 10, 2023

This appears to break any compiler < 5.0.0, e.g. realworldocaml/book#3656 with a duplicate rule for the bytes META file.

Error: Multiple rules generated for _build/install/default/lib/bytes/META:
2924	
- duniverse/lib-findlib/site-lib-src/dune:6
2925	
- _build/default/duniverse/bytes/META.bytes:1
2926	
-> required by alias duniverse/lib-findlib/.findlib-files
2927	
-> required by alias duniverse/lib-findlib/.ocamlfind-files
2928	
-> required by
2929	
   _build/default/book/files-modules-and-programs/.mdx/README.md.corrected
2930	
-> required by alias book/files-modules-and-programs/runtest in
2931	
   book/files-modules-and-programs/dune:1
2932	

@Leonidas-from-XIV
Copy link
Contributor

@avsm Thanks for the report! I've submitted a PR to fix up opam-overlays: #179

@avsm
Copy link
Contributor

avsm commented Jan 12, 2023

It concerns me that all compilers < 5.0 have been broken since November. Is there a CI test in place that would test your fix?

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