-
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
Inconsistent behaviour for virtual libraries using melange mode #7104
Comments
Signed-off-by: Antonio Nuno Monteiro <anmonteiro@gmail.com>
Signed-off-by: Antonio Nuno Monteiro <anmonteiro@gmail.com>
From the test:
I am trying to figure out if the solution for this is something that can be solved on dune side, on melange side, or a combination of both 😅 On Melange side:
On Dune side:
@anmonteiro @rgrinberg wdyt? |
Adding a note here from @anmonteiro: solving this issue requires implementing "delayed cmj optimization" (melange-re/melange#464), so in terms of timing in won't be fixed as part of dune 3.8. |
* main: (56 commits) feature: add terminal ui backend based on NoTTY (ocaml#6996) doc(coq): update documentation about coqdep fix(rules): don't descend into automatic subdirs infinitely (ocaml#7208) benchmark: add warm run (ocaml#7198) test: vendored and public libs (ocaml#7197) test: use sh in concurrent test (ocaml#7205) fix: custom log file path (ocaml#7200) test(melange): add test exercising ocaml#7104 (ocaml#7204) test(melange): add a test that introduces rules in the target dir (ocaml#7196) test: duplicate packages in vendor dir (ocaml#7194) melange: interpret `melc --where` as a list of `:`-separated paths (ocaml#7176) perf: add synthetic benchmark (ocaml#7189) Test case for bug report (ocaml#6725) Add test illustrating ocaml#6575 (ocaml#6576) chore: add rule streaming proposal (ocaml#7195) test(stdlib): merge wrapped/unwrapped tests test: move all stdlib tests fix: allow unwrapped libraries with `(stdlib ..)` test: demonstrate crash in modules.ml when `(stdlib .. )` used with `(wrapped false)` fix(install): respect display options (ocaml#7116) ...
I'm fixing this over in #10051. @rizo if you're still interested in this fix and would like to take it for a spin, I'd appreciate your feedback. In terms of setup, you'll need:
|
CHANGES: ### Added - Add link flags to to `ocamlmklib` for ctypes stubs (ocaml/dune#8784, @frejsoya) - Remove some unnecessary limitations in the expansions of percent forms in install stanza. For example, the `%{env:..}` form can be used to select files to be installed. (ocaml/dune#10160, @rgrinberg) - Allow artifact expansion percent forms (`%{cma:..}`, `%{cmo:..}`, etc.) in more contexts. Previously, they would be randomly forbidden in some fields. (ocaml/dune#10169, @rgrinberg) - Allow `%{inline_tests}` in more contexts (ocaml/dune#10191, @rgrinberg) - Remove limitations on percent forms in the `(enabled_if ..)` field of libraries (ocaml/dune#10250, @rgrinberg) - Support dialects in `dune describe pp` (ocaml/dune#10283, @emillon) - Allow defining executables or melange emit stanzas with the same name in the same folder under different contexts. (ocaml/dune#10220, @rgrinberg, @jchavarri) ### Fixed - coq: Delay Coq rule setup checks so OCaml-only packages can build in hybrid Coq/OCaml projects when `coqc` is not present. Thanks to @vzaliva for the test case and report (ocaml/dune#9845, fixes ocaml/dune#9818, @rgrinberg, @ejgallego) - Fix conditional source selection with `select` on `bigarray` in OCaml 5 (ocaml/dune#10011, @moyodiallo) - melange: fix inconsistency in virtual library implementation. Concrete modules within a virtual library can now refer to its virtual modules too (ocaml/dune#10051, fixes ocaml/dune#7104, @anmonteiro) - melange: fix a bug that would cause stale `import` paths to be emitted when moving source files within `(include_subdirs ..)` (ocaml/dune#10286, fixes ocaml/dune#9190, @anmonteiro) - Dune file formatting: output utf8 if input is correctly encoded (ocaml/dune#10113, fixes ocaml/dune#9728, @moyodiallo) - Fix expanding dependencies and locks specified in the cram stanza. Previously, they would be installed in the context of the cram test, rather than the cram stanza itself (ocaml/dune#10165, @rgrinberg) - Fix bug with `dune exec --watch` where the working directory would always be set to the project root rather than the directory where the command was run (ocaml/dune#10262, @gridbugs) - Regression fix: sign executables that are promoted into the source tree (ocaml/dune#10263, fixes ocaml/dune#9272, @emillon) - Fix crash when decoding dune-package for libraries with `(include_subdirs qualified)` (ocaml/dune#10269, fixes ocaml/dune#10264, @emillon) ### Changed - Remove the `--react-to-insignificant-changes` option. (ocaml/dune#10083, @rgrinberg)
CHANGES: ### Added - Add link flags to to `ocamlmklib` for ctypes stubs (ocaml/dune#8784, @frejsoya) - Remove some unnecessary limitations in the expansions of percent forms in install stanza. For example, the `%{env:..}` form can be used to select files to be installed. (ocaml/dune#10160, @rgrinberg) - Allow artifact expansion percent forms (`%{cma:..}`, `%{cmo:..}`, etc.) in more contexts. Previously, they would be randomly forbidden in some fields. (ocaml/dune#10169, @rgrinberg) - Allow `%{inline_tests}` in more contexts (ocaml/dune#10191, @rgrinberg) - Remove limitations on percent forms in the `(enabled_if ..)` field of libraries (ocaml/dune#10250, @rgrinberg) - Support dialects in `dune describe pp` (ocaml/dune#10283, @emillon) - Allow defining executables or melange emit stanzas with the same name in the same folder under different contexts. (ocaml/dune#10220, @rgrinberg, @jchavarri) ### Fixed - coq: Delay Coq rule setup checks so OCaml-only packages can build in hybrid Coq/OCaml projects when `coqc` is not present. Thanks to @vzaliva for the test case and report (ocaml/dune#9845, fixes ocaml/dune#9818, @rgrinberg, @ejgallego) - Fix conditional source selection with `select` on `bigarray` in OCaml 5 (ocaml/dune#10011, @moyodiallo) - melange: fix inconsistency in virtual library implementation. Concrete modules within a virtual library can now refer to its virtual modules too (ocaml/dune#10051, fixes ocaml/dune#7104, @anmonteiro) - melange: fix a bug that would cause stale `import` paths to be emitted when moving source files within `(include_subdirs ..)` (ocaml/dune#10286, fixes ocaml/dune#9190, @anmonteiro) - Dune file formatting: output utf8 if input is correctly encoded (ocaml/dune#10113, fixes ocaml/dune#9728, @moyodiallo) - Fix expanding dependencies and locks specified in the cram stanza. Previously, they would be installed in the context of the cram test, rather than the cram stanza itself (ocaml/dune#10165, @rgrinberg) - Fix bug with `dune exec --watch` where the working directory would always be set to the project root rather than the directory where the command was run (ocaml/dune#10262, @gridbugs) - Regression fix: sign executables that are promoted into the source tree (ocaml/dune#10263, fixes ocaml/dune#9272, @emillon) - Fix crash when decoding dune-package for libraries with `(include_subdirs qualified)` (ocaml/dune#10269, fixes ocaml/dune#10264, @emillon) ### Changed - Remove the `--react-to-insignificant-changes` option. (ocaml/dune#10083, @rgrinberg)
Expected Behavior
When creating a virtual library in native or byte modes, it is possible to assign a module to be virtual and add an implementation and interface that wraps around the virtual module in the virtual library.
Specifically the following structure illustrates this:
This works as expected in native and byte modes and allows providing multiple implementations for
virt.mli
while offering a high-level interface invlib.mli
.This doesn't work in melange mode.
Actual Behavior
This doesn't work when
(modes melange)
is used. Not only it is not possible to replicate the described workflow, but having(modes melange)
also prevents the non-melange implementation to be compiled.Error when compiling a melange target:
Additionally, if
(modes melange)
is present invlib
, the native compilation fails with:Reproduction
The following repository fully replicates the above scenario based on the blackbox test included in dune:
Specifications
dune
: ~3.705797cd3f505beca4d6d62c576506d7608a6d29d
melange
:f4ddebbc7755af5e983c4df2ac51498f286e1074
ocaml
: 4.14.0The text was updated successfully, but these errors were encountered: