-
Notifications
You must be signed in to change notification settings - Fork 409
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
Remove all the shady Option.value_exn in lib.ml #2090
Conversation
d1d874d
to
ac1904c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Except for my comment, it looks good to me ! Thank you for this, indeed removing that db
traveling around feels nicer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Except for my comment, it looks good to me ! Thank you for this, indeed removing that db
traveling around feels nicer.
@rgrinberg I tested with your fork and I no longer have the internal error. |
561047e
to
f34b23d
Compare
@TheLortex I've amended the fix to consider dbs recursively |
Should fix ocaml#2085 Signed-off-by: Rudi Grinberg <[email protected]>
Signed-off-by: Rudi Grinberg <[email protected]>
Signed-off-by: Rudi Grinberg <[email protected]>
f34b23d
to
bffe5ba
Compare
CHANGES: - Put back library variants in development mode. We discovered a serious unexpected issue and we might need to adjust the design of this feature before we are ready to commit to a final version. Users will need to write `(using library_variants 0.1)` in their `dune-project` file if they want to use it before the design is finalized. (ocaml/dune#2116, @diml) - Forbid to attach a variant to a library that implements a virtual library outside the current project (ocaml/dune#2104, @rgrinberg) - Fix a bug where `dune install` would install man pages to incorrect paths when compared to `opam-installer`. For example dune now installs `(foo.1 as man1/foo.1)` correctly and previously that was installed to `man1/man1/foo.1`. (ocaml/dune#2105, @aalekseyev) - Do not fail when a findlib directory doesn't exist (ocaml/dune#2101, fix ocaml/dune#2099, @diml) - [coq] Rename `(coqlib ...)` to `(coq.theory ...)`, support for `coqlib` will be dropped in the 1.0 version of the Coq language (ocaml/dune#2055, @ejgallego) - Fix crash when calculating library dependency closure (ocaml/dune#2090, fixes ocaml/dune#2085, @rgrinberg)
Can we get @TheLortex as a reviewer on this?
@vsiles could you confirm this fixes your bug? #2085