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 support for (include_subdirs qualified) #1084

Open
jeremiedimino opened this issue Aug 1, 2018 · 12 comments
Open

Add support for (include_subdirs qualified) #1084

jeremiedimino opened this issue Aug 1, 2018 · 12 comments
Assignees

Comments

@jeremiedimino
Copy link
Member

@jeremiedimino jeremiedimino commented Aug 1, 2018

With the following project:

./src:
dune       # containing (include_subdirs qualified)
x.ml

./src/a:
y.ml

./src/a/b:
z.ml

x.ml would see y.ml as A.Y and z.ml as A.B.Z. Then y.ml would see z.ml as B.Z but couldn't see x.ml.

This should allow to organize the modules using the FS layout.

I'm interested in this as it would allow to port Camlp4 to dune, which would simplify it's maintenance, and it's also a nice feature to have. However, I'm not planning to work on this in the near future, so anyone should feel free to assign themselves this feature if they are interested in it.

@rgrinberg rgrinberg self-assigned this Aug 6, 2018
@rgrinberg rgrinberg added this to the 1.2.0 milestone Aug 6, 2018
@rgrinberg rgrinberg removed this from the 1.2.0 milestone Feb 12, 2019
@rgrinberg
Copy link
Member

@rgrinberg rgrinberg commented Feb 12, 2019

There's a few complications that have kept this feature on the back burner. I'll just list them here to note that I haven't forgotten about this feature:

  • Our current code relies on passing around a lot of maps keyed by the module name. Naturally, this doesn't quite work anymore since the module names are no longer unique.

  • I'd like to it be possible to define a lib interface module for every subdirectory. Our codebase currently assumes there's only one such module in many places.

@Lupus
Copy link

@Lupus Lupus commented Mar 5, 2020

Will it still be possible to add some custom rules in dune files within the FS hierarchy that form one dune lib? Or define some aliases? I.e. will dune files within such hierarchy be treated as normal dune files with all features available?

Also will it be possible to customize compiler flags for certain submodules? Like I want to have -open Base, but it won't work with atdgen modules, currently I'm moving them to separate lib so that I can tune the compiler flags there.

May be ability for tuning ppx-es that are used will be nice to have. When using ppx_deriving with protobuf plug-in, sexp derivers stop working, I had to move protobuf types to separate library to just change ppx set used.

@jeremiedimino
Copy link
Member Author

@jeremiedimino jeremiedimino commented Mar 5, 2020

Will it still be possible to add some custom rules in dune files within the FS hierarchy that form one dune lib? Or define some aliases? I.e. will dune files within such hierarchy be treated as normal dune files with all features available?

It will be the same as for (include_subdirs unqualified). You'll be able to do everything except defining libraries or executables.

Also will it be possible to customize compiler flags for certain submodules? Like I want to have -open Base, but it won't work with atdgen modules, currently I'm moving them to separate lib so that I can tune the compiler flags there.

Probably not in the first version. But in the future, why not.

May be ability for tuning ppx-es that are used will be nice to have. When using ppx_deriving with protobuf plug-in, sexp derivers stop working, I had to move protobuf types to separate library to just change ppx set used.

You can already do that: https://dune.readthedocs.io/en/stable/concepts.html#per-module-preprocessing-specification

@rgrinberg
Copy link
Member

@rgrinberg rgrinberg commented Mar 5, 2020

Just so I don't forget this later on, here's quick brain dump that's blocking
this issue. The main problem is the layout of the alias module. Consider this
directory structure:

$ ls -R .
  dune
  y.ml
  x/
    foo.ml
    bar.ml
    x.ml
$ cat dune
  (include_subdirs qualified)
  (library (name lib))

In this setup, it's important to note that x/x.ml is a lib interface module
and that y.ml may not access foo.ml and bar.ml. We might consider the
following alias map that works for such a file:

$ cat lib__.ml
module X__ = struct
  module Foo = Lib__X__Foo
  module Bar = Lib__X__Bar
end
module X = Lib__X
module Y = Lib__Y

When compiling {foo,bar}.ml, we use the flags -open Lib__ -open X__.
When compiling y.ml, we use the flags -open Lib__.

This looks great except for one little blemish: now Y can see this ugly X__
name. I can't think of a way to avoid this downside without making the alising a
lot more complicated and using multiple alias files.

@diml, @aalekseyev, perhaps you guys have a suggestion?

@aalekseyev
Copy link
Collaborator

@aalekseyev aalekseyev commented Mar 6, 2020

Will X__ also appear in module type of Lib?
My first reaction is that multiple alias modules seems reasonable and having X__ in the module type of Lib is not reasonable.

(we use multiple alias modules when we have multiple libraries right now so I'm not surprised if we need multiple alias modules when we have multiple include_qualified directories (I think about them roughly as libraries))

@aalekseyev
Copy link
Collaborator

@aalekseyev aalekseyev commented Mar 6, 2020

Wait, doesn't something like this work?

module Lib__ = struct
  module To_open = struct
    module X = Lib__X
    module Y = Lib__Y
  end
  module X = struct
    module To_open = struct
      module Foo = Lib__X__Foo
      module Bar = Lib__X__Bar
    end
  end
end

When compiling {foo,bar}.ml, we use the flags -open Lib__.To_open -open Lib__.X.To_open.
When compiling y.ml, we use the flags -open Lib__.To_open.

(instead of multiple opens you can put a statement include To_open on top of X.To_open, which I think should also work if you've got the compiler that allows include shadowing)

@aalekseyev

This comment has been hidden.

@rgrinberg
Copy link
Member

@rgrinberg rgrinberg commented Mar 6, 2020

By the way, in your original message you probably wanted to say "y.ml may not access foo.ml and bar.ml", I suggest you edit that before others are confused.

Corrected. Thanks.

Wait, doesn't something like this work?

That seems like it's worth trying, but I'm not confident that these To_open identifiers aren't going to leak when doing type inference, completion in merlin. At least with the __, merlin knows to hide these symbols from the user.

I suppose there's no harm trying this out. Generating the alias module is a fairly small piece of the work anyway.

@aalekseyev
Copy link
Collaborator

@aalekseyev aalekseyev commented Mar 6, 2020

[To_open does not contain __]

We could call it O__, or something like that. Thinking again, it seems we don't really need the surrounding modules at all:

module To_open_in_Lib__ = struct
  module X = Lib__X
  module Y = Lib__Y
end
module To_open_in_Lib__X__ = struct
  module Foo = Lib__X__Foo
  module Bar = Lib__X__Bar
end

Or maybe I'm missing something?

@rgrinberg
Copy link
Member

@rgrinberg rgrinberg commented Mar 6, 2020

@OndrejPopp
Copy link

@OndrejPopp OndrejPopp commented Nov 12, 2020

Ok cool... I would like to have this feature too! 👨‍💻
In the meantime I have settled for this,

  • leave include_subdirs as default no, so each sub directory will be compiled into a seperate library, I figured better this than to have the whole FS hierarchy flattened into the root dir.
  • then dune file for the toplevel lib : (library (name lib) (libraries x))
  • dune file for the x subdir : (library (name x))
  • move the x.ml file one level up, containing,
    module Foo = X__Foo
    module Bar = X__Bar
  • drop the lib__.ml file, not needed in this setup, because of the moved x.ml file

now Lib.X.{Foo,Bar} are visible, the only drawback is that X.{Foo, Bar} is visible as well and so on for every subdirectory, library, in the FS hierarchy. But I guess just ignore those until this feature gets available?

Or is there still a better way to achieve this in the meantime?
Thanks ☺️

@OndrejPopp
Copy link

@OndrejPopp OndrejPopp commented Nov 12, 2020

👨‍💻 Ok... I just checked whether this approach still works for next subdirectory levels, if X would have have a subdirectory Z with modules Mx and My. And it does! To add Z to X you need to do,

  • create the subdirectory Z and inside it the M{x,y}.ml files
  • the dune file for Z : (library (name z))
  • add this to the dune file for X : (libraries z)
  • create a Z.ml file inside X and alias the modules of Z, :
    module Mx = Z__Mx
    module My = Z__My
  • add this to the X.ml file inside lib : module Z = X__Z
  • done! Now Lib.X.Z.M{x,y} are visible.

So this approach works, and you can write a script to automate this. The only drawback is again that you have all these sub libraries created, but aside from that it could maybe serve as a basis to implement this feature in dune?

Tx. 😃

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

Successfully merging a pull request may close this issue.

None yet
5 participants