feature: auto_open field

[rgrinberg]

When vendoring code, it's quite useful to change the names of libraries to prevent issues with linking. Unfortunately, it requires polluting one's code with what amounts to undoing the renaming. This feature aims to automate this tedious aspect. In particular,

(library
 (auto_open foo))

Should automatically insert -open Foo for every explicit users of [foo]. One example where this will come in handy is removing all the module Re = Dune_re noise in our codebase.

Signed-off-by: Rudi Grinberg [email protected]

• [x] installed libraries
• [x] implicit_transitive_deps
• [x] re_exports

[Alizter]

We can also handle open Import this way too.

[nojb]

Nit, but why not just (open ...)?

[rgrinberg]

Nit, but why not just (open ...)?

I think that can be confused with modules opened when building the library. Although now that I look at it, auto_open isn't much better. Something like exported_open_modules perhaps?

[emillon]

the semantics makes me think of (re_export). Do we expect these to be used together?

[rgrinberg]

the semantics makes me think of (re_export). Do we expect these to be used together?

Quite possibly. I think the desired semantics is that if x re-exports y, then it should also re-export the automatic open flags.

[rgrinberg]

ping @nojb any more suggestions for the name?

[rgrinberg]

@emillon @anmonteiro suggestions from you are welcome as well

[nojb]

ping @nojb any more suggestions for the name?

I'm afraid that actually I haven't had time to look at this PR in detail so I don't understand what it is doing, so don't have any suggestions for the name :) I can try to review, but I'll need some more time (a bit underwater at the moment).

[rgrinberg]

Another friendly ping @nojb

[nojb]

Another friendly ping @nojb

Sorry for the lag. I read the issue description again but I'm afraid I still do not understand what this PR is about. You mention Re = Dune_re as an example. How is this PR supposed to be used in that case? Which -open flag will be added to clients of this library? Thanks.

[rgrinberg]

Sure, I can clarify:

1. We rename our vendored library re to dune_re
2. Add (auto_open dune_re) to the exported list of flags
3. Write dune_re.ml as module Re = Re

The end result is that any user of dune_re will have the Re top level name visible instead of Dune_re.

[nojb]

Sure, I can clarify:

1. We rename our vendored library re to dune_re
2. Add (auto_open dune_re) to the exported list of flags
3. Write dune_re.ml as module Re = Re

The end result is that any user of dune_re will have the Re top level name visible instead of Dune_re.

Thanks for the explanation. The scope of the (auto_open ...) field seem overly wide (eg one could use it to "auto-open" arbitrary modules). What about removing the argument and just having (auto_open) add the -open flag for the entry module of the library (maybe also restrict it to wrapped libraries)?

[rgrinberg]

eg one could use it to "auto-open" arbitrary modules

I imagined this capability would be useful to people. For example, i might not want to open every single toplevel module in unwrapped mode. Also, I might want to use a module path like Foo.Exported.

[nojb]

eg one could use it to "auto-open" arbitrary modules

I imagined this capability would be useful to people. For example, i might not want to open every single toplevel module in unwrapped mode. Also, I might want to use a module path like Foo.Exported.

OK, but I think at a minimum the feature should be limited to opening module paths in the library that declares (auto_open).

More generally, this feature has the potential to complicate making sense of code because it is going to become harder to know which modules are in scope. Before this feature opened modules had to appear, either in the source code (open ...) or in the compilation flags of your buildable. Now, -open flags may be added to your compilation flags from arbitrarily far.

[nojb]

Another issue I can see: if more than one library uses (auto_open) there is no clear way to choose an ordering for the -open flags.

Taking a step back: is there a sense of how useful this will be to the users of Dune? Or is this a feature being added exclusively to simplify Dune's development? If the only user of this feature is Dune we could hide the feature behind some switch, not document it, etc. I am afraid that it may have unintended consequences if it starts being used by Dune users at large. At the very least, we should consider how it would work in a couple of examples outside of Dune before making it generally available.

Alternatively, maybe we should consider some higher-level "vendoring" construct that would do everything at once (remaing a library, creating a dummy entry module with the new name, add -open flags to clients of the library, etc). Such a construct would be both more limited (it would be limited to the vendoring use-case) and perhaps more generally useful.

[rgrinberg]

Okay, I can mark this feature as experimental for now and we can play around with it in dune in the beginning.

I agree with your general concerns that this feature isn't to be used lightly and has the potential to confuse people. On the other hand, it's a much lesser evil than what it's going to replace.

Alternatively, maybe we should consider some higher-level "vendoring" construct that would do everything at once (remaing a library, creating a dummy entry module with the new name, add -open flags to clients of the library, etc). Such a construct would be both more limited (it would be limited to the vendoring use-case) and perhaps more generally useful.

There was some discussion about that before. I'm not sure if you were there, but we ack'd that there are two schools of thought for dune features:

1. Low level building blocks that do the minimum to allow you to accomplish a task.
2. High level feature that encapsulate some logical feature.

In general, we've done 2. and it hasn't always worked well. Take the (vendored_dirs ..) stanza for example. It tries to do a lot of things at once (tests, warnings, @check alias) that many people don't want and there's no way to select/configure the subset people are looking for. Therefore I tried 1. as the template for this particular feature. So the way I see it, this feature is the absolute minimum that is required to solve the problem at hand and is easy to understand (even if the semantics aren't very desirable).

[nojb]

So the way I see it, this feature is the absolute minimum that is required to solve the problem at hand and is easy to understand (even if the semantics aren't very desirable).

Thanks for the explanation about your thought process. I agree; but I think it would be prudent to play with it a little bit before we open it up to the general public.

• installed libraries

Incidentally, does it make sense to use this feature with installed libraries? (I know it makes sense technically, but I am wondering if it is a mechanism we would want to encourage people to use in the "installed" world.)