-
Notifications
You must be signed in to change notification settings - Fork 412
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
RFC: Multi-context library installation #10507
Comments
This would likely also solve #10362 |
The main thing that I dislike about this proposal is that it mixes low level constructs such as contexts and high level constructs as melange settings. The way things were originally, it made sense to me because contexts were expanded to cover more use cases without adding melange specific features. Now it turns that you need melange specific features anyways. In that case, we could have just started with something custom made for melange and the end result wouldn't have some of the sharp edges of this proposal. What could a custom solution could look like? Well I suppose you could just make libraries built with the melange mode completely have their own sources and compilation artifacts and add conditional source selection for melange (though this would not work for bytecode/native/jsoo modes). |
If I understand correctly, the main addition in the proposal is the However, telling Dune about Melange-specific compilation at the context level feels like the wrong place (maybe this is what @rgrinberg is also expressing) because inside the contexts, one can put a lot of things besides Melange libraries or emit stanzas. This friction can be noticed in rules like:
I assume a bunch of other things would become forbidden that are not mentioned. For example what happens if I enable some I wonder how a proposed solution would look like if we went the "mode => context" path. Meaning, we leave
That issue is mostly the same idea that I described above. |
I've been thinking about this and it looks like the most straightforward solution. Additionally, this preserves the status quo of Melange code not needing to live in a separate context -- I've been a bit unhappy about the limitation that we've self-imposed for universal libraries related to multiple contexts.
I don't understand this part, though. What wouldn't work? |
All of those modes share build artifacts. Bytecode and native share cmi's and bytecode and jsoo share cmo's. |
I added a new proposal in #10630 that would replace this one. |
@rgrinberg can you elaborate on why you think contexts are low level constructs? The appealing of contexts is that they introduce a modality (e.g. tools operate in different contexts operate differently, e.g. LSPs) and are already exposed in dune's UI (and we have added a switcher to vscode extension as well). So we can control diagnostics from melange or ocamlopt we get when viewing a source file (which is built by both). But so far, as you've noted, the extensions to contexts are all generally applicable to any contexts. Now we have few things which needs to be tweaked specifically for melange, and we have two RFCs now:
With 2. we also seem to be losing the modality I've described above. |
Contexts were added to support cross compilation. It's not clear if they are well suited for anything else because they do not compose and have poor performance characteristics. Teaching all the tools to know about contexts sounds like a whole lot of work compared to just implementing the bespoke feature in dune and making it available to everyone automatically. It made more sense when the premise was that the feature was going to work with contexts unmodified. But now that it's no longer the case, we're getting the worst of both worlds.
There are much better ways of controlling this than contexts |
Context
"Multi-context" or "Universal" libraries refer to libraries sharing the same name, but enabled in different dune build contexts (through
(enabled_if (=%{context_name} ...))
). Support for this was added in ocaml/dune#10307.ocaml/dune#10471 shows that these libraries cannot currently share the same
public_name
.Problem statement
dune install
and passing the--context
flag.--context
flag.Proposed solution
melange.emit
exclusively in their own context.(melange)
field to(context ..)
indune-workspace
and phase out(modes melange)
from the dune configuration(context ..)
field in(library ..)
that adheres to the ordered set language::standard
defines the default context where most libraries live today(context :standard melange other-context)
(context melange)
implies(modes melange)
(context :standard) (modes melange)
becomes forbiddenpublic_name
to coexist iff one lives exclusively in thedefault
context and the other lives exclusively in themelange
contextmelange
artifacts are already installed in their own directory:<lib-root>/<lib-name-segments>/melange
melange
directory mentioned above--context
isn't passed todune install
, automatically install the Melange and default contexts in the same prefixThe following example illustrates how a Melange project could be configured in this proposal:
Alternatives Considered
dune install
commands in the<project>.opam
filedune install
twice(using melange <version>)
is present in thedune-project
fileThe text was updated successfully, but these errors were encountered: