RFC: single-context Universal Libraries

anmonteiro commented 3 weeks ago

Context

(using melange 0.1) allows writing Melange / OCaml code together with limitations
Recent work on "universal libraries" proposes using different contexts and enabled_if

Problems

Multi-context libraries introduce a few problems:
- library stanzas end up duplicated for any non-trivial use cases, and (enabled_if (= %{context} melange)) is infectious (melange libraries only work within the melange context)
- it's not trivial to install multi-context libraries in the same context as native (for consumption within the same OPAM switch).
- Editor support becomes more painful, and switching between files in different contexts still requires restarting the LSP.

Requirements:

Installing multi-mode libraries alongside each other
vary module sources according to the library mode
vary preprocessing according to the library mode
vary library dependencies according to the library mode

Proposed solution

The proposed solution is 2-fold:
- new conditional fields libraries, preprocess, flags, modules in (library ..)
  - prior art: melange.compile_flags
  - in their absence, a library with (modes melange) falls back to the fields already supported before this change.
- the ability to choose module sources based on the mode e.g. foo.%{mode}.ml
  - we can solve the "copy_files problem" by having shared module sources and mode-specific sources in the same directory, with different extensions. Example:

- src
  |_ dune
  |_ common.ml
  |_ foo.melange.ml
  |_ foo.ocaml.ml

Slightly unrelated: this proposal is likely backwards-compatible with (using melange 0.1); after adopting it successfully in universal library projects such as Ahrefs, I propose we mark the Melange extensions stable in dune (1.0).

Implementation details

we can keep module sources in the same directories in the build folder
for Melange, we'll have to copy them to e.g. a .melange-sources folder after setting up rules for the implementation-specific module sources
Installation keeps working since we already copy melange artifacts to <lib-root>/<pkg-name>/melange. We can install the Melange sources there, too.

Caveats / further work

It's likely we'll have to change Merlin to map a module name to its source file. This is desirable in general even without this proposal: Merlin currently guesses what module source corresponds to a module, and implementing this mapping would be a benefit for most OCaml projects and editor tooling.

andreypopp commented 3 weeks ago

new conditional fields libraries, preprocess, flags, modules in (library ..)

Am I correct that this is going to look as

(library 
    (name ...)
    (libraries ...)
    (melange.libraries ...) ; only needed if differs from (libraries ...)
    (preprocess ...)
    (melange.preprocess ...) ; only needed if differs from (preprocess ...)
    ...)

(just want to clarify that I understand the proposal correctly).

May I also request to modify the preprocessing pipeline for Melange to communicate to preprocessors, that they are running for the melange sources?

This would be useful so that the same preprocessor could automatically adjust its behavior based on if it runs over the "melange sources" or not.

It's likely we'll have to change Merlin to map a module name to its source file. This is desirable in general even without this proposal: Merlin currently guesses what module source corresponds to a module, and implementing this mapping would be a benefit for most OCaml projects and editor tooling.

How that part is going to be implemented?

One thing I like about the multicontext proposal is that the context is user level/visible feature in dune toolchain and thus we can build on that - expose in editor tooling, etc.

anmonteiro commented 3 weeks ago

Am I correct that this is going to look as

Yup. this is what it'd look like.

May I also request to modify the preprocessing pipeline for Melange to communicate to preprocessors, that they are running for the melange sources?

I think this makes total sense. Thanks for suggesting.

How that part is going to be implemented?

I'm not entirely sure. I think the idea is to communicate to Merlin what the sources are for a given module, rather than just the prefix and the S configuration.

jchavarri commented 3 weeks ago

I wonder about how modes will interfere with new fields e.g. melange.libraries. For example, if i understand correctly, one can define melange.libraries with modes :standard, which doesn't make a lot of sense.

Maybe there could be a new version of modes that allows to expand preprocess and libraries from inside to make impossible states impossible, e.g.:

(modes :standard (melange (libraries foo bar) (preprocess ...))

melange.compile_flags could also be moved to be a part of that.

May I also request to modify the preprocessing pipeline for Melange to communicate to preprocessors, that they are running for the melange sources?

Is this strictly necessary? I was thinking one could use different flags depending if it's preprocess or melange.preprocess, like we have been doing until now with e.g. browser_only in server-reason-react.

I am not saying we shouldn't add this feature, just that Dune already has a lot of complexity, so everything introduced needs to be well evaluated.

I'm not entirely sure. I think the idea is to communicate to Merlin what the sources are for a given module, rather than just the prefix and the S configuration.

Currently we have this logic to decide if we should point merlin to either melange or ocaml sources. iiuc it works like this:

If only melange mode is defined, use melange
In any other case, use ocaml

Maybe we could add a flag to let the users decide how it works.

davesnx commented 3 weeks ago

Is this strictly necessary? I was thinking one could use different flags depending if it's preprocess or melange.preprocess, like we have been doing until now with e.g. browser_only in server-reason-react.

I would say it will be a big pro of not having to pass the -js or other flags into ppxes, and not only have different ppx for similar things: melange.ppx vs server-reason-react.melange_ppx

jchavarri commented 3 weeks ago

Some thoughts about this RFC after giving it some time.

The issues we found with the current approach to "universal libs" using multi-context are (this kind of repeats what's been mentioned in issue description, but for the sake of completion):

a) have to use enabled_if everywhere b) editor integration can only support a single context at a given time. If I choose the Melange context, then I don't get feedback for native-only libraries, and the other way around c) it's not possible to publish libs that use same public name (like this test shows)

Issues a) and b) have been the main blockers to adopt multi-context at Ahrefs.

As far as I understand, this proposal would solve all the issues above:

a) No more need to use enabled_if. A library becomes "universal" as soon as it has (modes :standard melange) b) editor integration would keep working as it does now, except universal libraries would work: melange-only libs and native-only libs would work always out of the box, and the only thing users would need to set is some setting for the "preferred platform" to choose artifacts from for universal libraries. c) publishing universal libraries becomes possible because multi-mode libs would be installed next to each other.

I don't see at the moment which disadvantages the proposal would bring, besides the extra complexity added in Dune codebase?

andreypopp commented 3 weeks ago

... and the only thing users would need to set is some setting for the "preferred platform" to choose artifacts from for universal libraries.

my understanding is that this is the biggest downside of this proposal, as such setting is going to cascade through many tools (dune, ocamllsp, editor extensions) and at the same time it is melange specific

regarding a) and b), are we sure that the amount/kind of work required for this proposal is not similar to what's required to specialise part of multi context mechanism to melange and solve a) and b) there?

I still see multi context proposal superior even if it is required to introduce a specialisation for melange (e.g. implicit melange context controlled by (modes melange)):

contexts are already exposed to users and are not melange specific
having support for switching contexts in editor might be useful not just for melange but to switch between different opam switches
contexts can have own opam switches so in general FE and BE can have different OCaml/Melange versions (I think such flexibility might be useful)

jchavarri commented 3 weeks ago

as such setting is going to cascade through many tools (dune, ocamllsp, editor extensions) and at the same time it is melange specific

Could you expand on what changes would be required? Besides those in Dune that are being commented here.

The way I see it, things for ocamllsp and editor extensions would keep working as they do now. If you define a library with (modes :standard melange) today, how does Dune tell Merlin where to read the artifacts from? There's no way to choose this setting as a user, the way artifacts paths are chosen for Merlin is hard-coded in Dune as mentioned above in the following way:

if (modes melange) is selected, Dune will point Merlin to Melange folders
in any other case (like universal libraries), Dune will point Merlin to the OCaml folders

Worst case, if this RFC was to be implemented, we could leave that setting as is, and Dune would always point merlin to OCaml artifacts for universal libraries. This would require zero changes in tooling upstream and everything would keep working normally.

We could optionally add some flag (just in Dune) to allow users to select which artifacts are picked for universal libraries, e.g. in dune-project:

(mode_for_merlin melange)

jchavarri commented 3 weeks ago

Another thing this RFC might have to consider is error handling. In the multi-context approach, we ended up adding some custom error handling for alt contexts (#10414), but in this RFC proposal it would have to be reimplemented as well, somehow.

ocaml / dune