Merged
Conversation
51a5715 to
1b77ccb
Compare
Member
Author
|
The ocaml-ci failure is just ocurrent/opam-dune-lint#12 |
10 tasks
rjbou
approved these changes
Jan 7, 2021
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
2.1.1 added
OpamParserTypes.FullPos. Unfortunately, this means thatOpamParserTypesis no longer an mli-only module. TheMakefilebuild system attests this:and indeed we get link errors like this:
Dune avoids this problem because
OpamParseris compiled with-no-alias-depswhich removes the link dependency on an implementation forOpamParserTypes. #36 essentially replicates this in theMakefile, but this still doesn't fix the underlying problem:which with both #36 and with Dune leads to:
unless
broken.mlitself is compiled with-no-alias-deps. Note that these modules have no business being compiled with-no-alias-deps(they don't really in Dune either) and compiling everything with-opaquekills cross-module inlining.So this PR converts
OpamParserTypes.mlitoOpamParserTypes.mlwhich should fix the problem in both build systems.