Wrapped (?) module name conflict with stdlib module

yawaramin commented 4 years ago

Hello! I have created a minimal repro here: https://github.com/yawaramin/dune-nested-lib-conflict . The only requirements are an active opam switch with OCaml 4.06 or later, with the ezjsonm package installed. Maybe I'm missing something but it seems that dune is not properly wrapping a module name.

When I run: dune exec ./main.exe

Expected Behavior

There is no module name conflict between the List module defined in the main.list library, and the Stdlib.List module.

Actual Behavior

There is the following conflict:

File "_none_", line 1:
Error: Files list/list.cmxa
       and /Users/yawar/.opam/4.06.1/lib/ocaml/stdlib.cmxa
       both define a module named List

The strange thing is that this reproduces only when certain libraries are added to the libraries field of https://github.com/yawaramin/dune-nested-lib-conflict/blob/07e84ee33d5077030c34fc0f95d8b9ae1c13931c/dune#L3 , like ezjsonm, camomile, but not for others, like cmdliner, result, seq, etc.

Reproduction

See above repo

Specifications

Version of dune (output of dune --version): 1.11.1
Version of ocaml (output of ocamlc --version): 4.06.1
Operating system (distribution and version): macOS Catalina 10.15.1

rgrinberg commented 4 years ago

This is a bug in 4.06.1 I guess. Later versions do not install a list.cmi in the stdlib directory which makes it possible to avoid the top level name conflict.

fakenickels commented 4 years ago

@rgrinberg I also have the same problem while using esy, but my OCaml version is 4.8 🤔

rgrinberg commented 4 years ago

@fakenickels can you post the contents of lib/ocaml/*.cmi?

yawaramin commented 4 years ago

@rgrinberg hey sorry when I posted this issue I tried to create a simplified example, the real error is this one:

File "_none_", line 1:
Error: Files /Users/yawar/.esy/3___________________________________________________________________/i/ocaml-4.8.1000-426898cd/lib/ocaml/compiler-libs/ocamlcommon.cmxa
       and /Users/yawar/.esy/3___________________________________________________________________/i/opam__s__re_web-387ce658/lib/re-web/Z_internal_Config/Config.cmxa
       both define a module named Config
error: command failed: 'dune' 'build' '--only-packages' 'fullstack-reason' (exited with 1)

You are right though, the Stdlib.List conflict goes away with a newer OCaml. But something interesting about your earlier comment:

Later versions do not install a list.cmi in the stdlib directory which makes it possible to avoid the top level name conflict.

I'm probably missing something here but I don't understand why there would be a top level name conflict even in that case, because my repro project is using dune which is by default wrapping my modules under the main namespace, right?

rgrinberg commented 4 years ago

The name of your library is list, which means that it will be accessible via a toplevel List module. For this to work, a list.cmi must be generated by dune. This is the source of the conflict. Toplevel names must still be unique even with dune’s wrapping. On Feb 8, 2020, 4:12 PM +0100, Yawar Amin notifications@github.com, wrote:

@rgrinberg hey sorry when I posted this issue I tried to create a simplified example, the real error is this one: File "none", line 1: Error: Files /Users/yawar/.esy/3/i/ocaml-4.8.1000-426898cd/lib/ocaml/compiler-libs/ocamlcommon.cmxa and /Users/yawar/.esy/3/i/opamsre_web-387ce658/lib/re-web/Z_internal_Config/Config.cmxa both define a module named Config error: command failed: 'dune' 'build' '--only-packages' 'fullstack-reason' (exited with 1) You are right though, the Stdlib.List conflict goes away with a newer OCaml. But something interesting about your earlier comment:

Later versions do not install a list.cmi in the stdlib directory which makes it possible to avoid the top level name conflict. I'm probably missing something here but I don't understand why there would be a top level name conflict even in that case, because my repro project is using dune which is by default wrapping my modules under the main namespace, right? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

yawaramin commented 4 years ago

In dune, to my understanding, nested directories i.e. nested modules must also be libraries (unless one uses include_subdirs unqualified which is not the default). Under this 'nested-modules-are-separate-libraries' model, how would I ensure that my nested modules' names don't get exposed to the toplevel? I can't make them private libs because dune says that only public libs can be used by other public libs. Is there any other way?

rgrinberg commented 4 years ago

It’s not your manually written list.ml that is causing the conflict, but the list module that is generated by dune based on your library name. Set the name of your library to listfoo and the conflict should disappear. On Feb 8, 2020, 4:29 PM +0100, Yawar Amin notifications@github.com, wrote:

In dune, to my understanding, nested directories i.e. nested modules must also be libraries (unless one uses include_subdirs unqualified which is not the default). Under this 'nested-modules-are-separate-libraries' model, how would I ensure that my nested modules' names don't get exposed to the toplevel? I can't make them private libs because dune says that only public libs can be used by other public libs. Is there any other way? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

yawaramin commented 4 years ago

Understood. But just to be 100% clear on this: dune will generate a toplevel list.cmi for my library based on its 'private' name list, even though it has a public_name main.list? If so, that's OK and I can work around that, but just want to make sure 🙂

rgrinberg commented 4 years ago

Almost correct. There are two cases:

(wrapped true) dune will generate a toplevel module corresponding to the private library name

(wrapped false) dune will generate toplevel modules for every user written module

public names are only used for expressing dependencies between libraries On Feb 8, 2020, 4:35 PM +0100, Yawar Amin notifications@github.com, wrote:

Understood. But just to be 100% clear on this: dune will generate a toplevel list.cmi for my library based on its 'private' name list, even though it has a public_name main.list? If so, that's OK and I can work around that, but just want to make sure 🙂 — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

yawaramin commented 4 years ago

Got it, thanks! I think I can rename my modules with fairly minimal effects. E.g. from List to Main__List. Or rather for the real case, from Config to ReWeb__Config.

fakenickels commented 4 years ago

@fakenickels can you post the contents of lib/ocaml/*.cmi?

Yes! But my problem was with compiler-libs/config.cmi and a config.cmi from re-web. @yawaramin already patched re-web with the your suggested fix and I'll retry

ocaml / dune