Skip to content
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

Add note about nested libraries #3111

Closed
wants to merge 1 commit into from
Closed

Conversation

yawaramin
Copy link
Contributor

As explained in #3102

As explained in ocaml#3102

Signed-off-by: Yawar Amin <[email protected]>
@@ -327,6 +327,12 @@ OCaml/Reason modules only consume modules from the directory where the
stanza appear. In order to declare a multi-directory library, you need
to use the :ref:`include_subdirs` stanza.

Note also that a nested library in a project is not automatically namespaced
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The term "nested library" does not exist in the documentation, so I'm not sure what you're referring to?

Copy link
Contributor Author

@yawaramin yawaramin Feb 10, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm referring to libraries in subdirectories of libraries. Is there a more standard terminology used for that? If so I can easily change to that.

under its parent library. E.g. if you have a public library ``foo`` which nests
another public library ``bar``, it will not be exposed as ``Foo.Bar`` outside
the project, but as ``Bar``. Take care to give unique names to nested libraries
to avoid conflicts. E.g., ``foo__Bar``.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

double underscores in names are a (private) artifact of how dune builds modules, I don't think it's a good idea to reuse this kind of names.

Copy link
Contributor Author

@yawaramin yawaramin Feb 10, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Disagree, double underscores are a widespread convention in the OCaml community which dune adopted. Dune doesn't 'own' the double underscores naming convention.

[EDIT: to clarify, double underscores are certainly the only relatively unambiguous way to disambiguate toplevel module names in OCaml. Dune can't claim exclusive ownership of that :-)]

@ghost
Copy link

ghost commented Feb 12, 2020

From the point of view of Dune, double underscores are an implementation detail. In particular, they are not mentioned in the documentation. If a future version of OCaml allows to hide them, Dune will hide them.

@yawaramin
Copy link
Contributor Author

Hi @diml , what concretely would happen to such modules e.g. Foo__Bar defined by (library (name foo__Bar))? Would Dune stop publishing the Foo__Bar.cmi and they would become effectively private?

What about if I alias the nested module like module Bar = Foo__Bar in foo.ml? I assume in that case the module would still be available as Foo.Bar?

@ghost
Copy link

ghost commented Feb 12, 2020

Well, if you explicitly call your module or library Foo__bar, things will continue to work the same way. There would be no reason for Dune to change anything there.

However, if you write:

(library
 (name foo)
 (modules bar))

the fact that dune chooses to compile bar to foo__bar.cmx is an undocumented implementation detail and we reserve the freedom to change it in the future if a better mechanism is provided by the compiler. In any case, that would only change when using a newer compiler. We will keep producing the double underscore names for older compilers given that we can't do better.

@yawaramin
Copy link
Contributor Author

I may have misunderstood something. In which case this PR may be moot. Let me try to verify :-)

  • If I have a library like (library (name foo) (modules bar)) in foo/dune
  • If I have a library like (library (name bar)) in foo/bar/dune

Then does Dune still make bar.cmi visible at the toplevel? Or does it namespace it? My main concern is to use nested libraries i.e. subdirectories in my projects, without exposing them to the toplevel. If Dune can already do that with the above, then excellent.

I realize this is something I can check for myself, and I will when I get home tonight. I suppose I just wanted to capture it somewhere :-)

@yawaramin
Copy link
Contributor Author

yawaramin commented Feb 13, 2020

@diml so my suggestion above doesn't work. Dune seems to not recognize Bar as a valid module of the foo library if it's auto-generated from the bar library name. Anyway, that aside, your comment:

if you explicitly call your module or library Foo__bar, things will continue to work the same way. There would be no reason for Dune to change anything there.

That is exactly what I am suggesting in this PR. Key line of the added doc:

Take care to give unique names to nested libraries to avoid conflicts. E.g., foo__Bar.

@emillon
Copy link
Collaborator

emillon commented Feb 13, 2020

There's an important point with wrapped libraries: users are never supposed to refer in their code to modules with double underscores in them. It's not possible to completely hide that today, but a user is supposed to never see double underscores. This is why the suggestion to mention foo__Bar as a way to name a library seems weird to me.

@ghost
Copy link

ghost commented Feb 13, 2020

I agree with @emillon, we should definitely not encourage users to use __ in their library or even module names. Indeed, you cannot use together a library named foo__bar and a library named foo with a module bar precisely because of the name mangling dune does to support wrapped libraries.

BTW, note that the two following setups are exactly the same from the point of view of Dune:

Setup 1:

  • (library (name foo) (modules bar)) in foo/dune
  • (library (name bar)) in foo/bar/dune

Setup 2:

  • (library (name foo) (modules bar)) in x/dune
  • (library (name bar)) in y/dune

In other terms, where you decide to put your libraries doesn't matter to Dune. A library is simply the collection of the modules present in the same directory as the dune file containing the library stanza. Unless you use (include_subdirs unqualified), in which case the library will include the modules present in sub-directories as well. They will all be at the same level because of the unqualified. If you write (include_subdirs unqualified) in foo/dune in setup 1, Dune will report an error as this case is not allowed.

Then does Dune still make bar.cmi visible at the toplevel? Or does it namespace it? My main concern is to use nested libraries i.e. subdirectories in my projects, without exposing them to the toplevel. If Dune can already do that with the above, then excellent.

In practice yes, but users of Dune shouldn't reason at the level of cm files or make assumption about how they are organised. What you should know is that:

  • module Bar of library foo is accessible as Foo.Bar for users of foo
  • unless Foo has (wrapped false) in which case it is accessible as Bar
  • or library foo has an explicit module named Foo, in which case users of foo only have access to module Foo

@yawaramin
Copy link
Contributor Author

yawaramin commented Feb 14, 2020

Lots to digest in both your replies :-) Let me try to clarify a couple of things.

@emillon I agree that it's less than optimal :-) A better solution would have been that Dune did not expose nested libraries as top-level-accessible modules. I am simply proposing a layout (which is working quite well for me and I thought that others would want the same benefit) that will mitigate that to an extent.

@diml

Indeed, you cannot use together a library named foo__bar and a library named foo with a module bar precisely because of the name mangling dune does to support wrapped libraries.

That is totally reasonable but I am proposing that a library should be named foo__Bar only if it is a nested library inside a library named foo, and not in other scenarios. So you can see that in this case, the name conflict does not apply.

Setup 1:

It's strange, I literally last night tried this setup and got the following error:

$ dune --version
2.1.3
$ dune build
File "foo/dune", line 4, characters 11-14:
4 |   (modules bar))
               ^^^
Error: Module Bar doesn't exist.

I've uploaded an example repo here, would appreciate if you could let me know what I'm doing wrong!

users of Dune shouldn't reason at the level of cm files or make assumption about how they are organised.

Agreed, I was just talking about .cmi files as a shorthand way of asking, 'Will this nested library's module be visible at the top-level i.e. will it conflict with other top-level modules'.

What you should know is that:

This all makes sense; my question is specifically about the other case, that is about a library bar nested inside a library foo. From what I can tell now (based on #3102 (comment)), bar is published by Dune as a top-level module.

@emillon
Copy link
Collaborator

emillon commented Feb 14, 2020

To clarify the situation, it seems to me that what you're after is a way to reflect the nesting of directories as a nesting of modules, so that a/b/c.ml is exposed as A.B.C without having to define a library B (and make it visible). Is that the case?

If it is, the codename for that feature is (include_subdirs qualified), which we'd like to support but which we do not at the moment.

@yawaramin
Copy link
Contributor Author

@emillon indeed, that's exactly what I want :-) in the absence of built-in support, I'm using nested libraries which seems to be the recommended approach to doing that. However, I also want there to be an awareness that nested library names are globally namespaced, because people may be using them unaware of that and publishing potentially any number of clashing top-level modules. I hope that makes sense :-)

@ghost
Copy link

ghost commented Feb 17, 2020

That makes a lot more sense to me too 😄

If you really want to this nesting now, calling the sub-library foo__bar won't work as it will conflict with module Bar of library foo. You'd need to call it something like foo_bar_ and then add a bar.ml file in library foo containing include Foo_bar_ to re-export the toplevel Foo_bar_ module as Foo.Bar.

From the point of view of Dune, the best we can recommend is to wait for (include_subdirs qualified), or better help implementing it!

I've uploaded an example repo here, would appreciate if you could let me know what I'm doing wrong!

Unless you use (include_subdirs unqualified), the only modules you can refer to in the modules field are the modules of the current directory. In your example, you are trying to refer to bar which is in a sub-directory. This is the problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants