[BUG] (implicit_transitive_deps true) conflicts with Yojson's seq dependency

[nojb]

I tried to build merlin in a standalone workspace consisting of:

• merlin
• csexp
• yojson
• cppo
• seq (https://github.com/c-cube/seq)

(no opam, no ocamlfind). Launching dune build -p merlin,merlin-lib,cppo,yojson,seq,csexp fails with

The problem can be worked around by setting the (implicit_transitive_deps true) in dune-project to false true.

[voodoos]

Maybe you can just drop the seq package if you are building Merlin for 4.14 ? The latest base version of the package is just a dummy.

[nojb]

The problem is that the dependency appears in the dune file of Yojson. If I take it out by hand then it works, but editing the file is not convenient. I guess they (Yojson) need to use (re_export seq) https://dune.readthedocs.io/en/stable/dune-files.html#implicit-transitive-deps-1?

[voodoos]

Maybe ? I am not familiar with re_export.

Also looking at Merlin's dune_project we do have (implicit_transitive_deps false), not true.

[nojb]

Also looking at Merlin's dune_project we do have (implicit_transitive_deps false), not true.

Right, sorry, I meant it could be set to true.

[voodoos]

Maybe you could try to add yojson , cppo and csexp in "vendored" directories ?

dune build -p merlin,merlin-lib worked for me after using opam monorepo to pull the three vendored dependencies and putting them in a directory with that dune file:

; This file is generated by opam-monorepo.
; Be aware that it is likely to be overwritten by your next opam monorepo pull invocation.

(vendored_dirs *)

(but my opam installation might be interfering positively with the build, my setup is very different from yours)

[nojb]

Maybe you could try to add yojson , cppo and csexp in "vendored" directories ?

That's my setup :)

(but my opam installation might be interfering positively with the build, my setup is very different from yours)

Yes, I think that in order to reproduce you need to be in an environment without opam and ocamlfind, and just the above projects in a single workspace.

Also looking at Merlin's dune_project we do have (implicit_transitive_deps false), not true.

Coming back to the more general question, is there a specific reason why Merlin is using (implicit_transitive_deps false), knowing that it is not well supported by the compiler today? Would it be possible to stop using this?

[voodoos]

I see. But also Merlin does not declare its use of seq in the server's stanza, maybe that is the problem ?

Could you try adding the seq library to the ocamlmerlin_server executable's stanza line 20 in src/frontend/ocamlmerlin/dune ? It works without it in my setup because Dune is finding the stdlib's version but that might be the issue on your side.

I don't know the historical reason for Merlin usage of (implicit_transitive_deps false), I believe it was just considered a saner default. I would prefer to keep it like that if we find a reasonable solution.

[nojb]

I see. But also Merlin does not declare its use of seq in the server's stanza, maybe that is the problem ?

I believe that would solve the problem but it would not be the "right" solution: there is no reason for Merlin to declare a dependency on this "compatibility" shim, since Merlin requires a version of OCaml that already includes the Seq module.

I don't know the historical reason for Merlin usage of (implicit_transitive_deps false), I believe it was just considered a saner default. I would prefer to keep it like that if we find a reasonable solution.

OK. I think the idea behind implicit_transitive_deps makes a lot of sense, but it necessitates compiler support in order to work well. This support is being discussed ocaml/RFCs#31, but does not exist yet. IMO it would be better not to use this feature until it is fully supported in the toolchain.

But if you prefer to keep Merlin as it is, the alternative is to modify Yojson to "re-export" the seq dependency (which has the same effect as adding a seq dependency in Merlin's Dune file, but is a bit more general and less intrusive). I will propose a PR to Yojson with this change.

[nojb]

Problem has been fixed upstream in the seq library c-cube/seq#13. Thanks!