Closed TheLortex closed 5 years ago
@TheLortex I'm super excited about this since I think it can pave the way for external device types/implementations. I cannot get this to build though, can you link to the corresponding mirage
branch?
The PR currently breaks API with Mirage, but it's not necessary, I'm waiting for comments on this.
There are two branches of https://github.com/TheLortex/mirage that implement the change of this PR:
functoria-fix
(https://github.com/TheLortex/mirage/tree/functoria-fix):
full-dune+functoria-fix
(https://github.com/TheLortex/mirage/tree/full-dune+functoria-fix):
Looks good, merging!
Hi, this is quite a heavy PR. I can try to minimize the diff if you want.
Why
The dynlink method to load
config.ml
feels quite fragile, as it requiresmirage
andconfig.ml
to be built with the same code / options. When compiling usingdune
, these assumptions can be broken:config.ml
can be built with a local instance of the mirage source code.mirage
but are enabled forconfig.ml
.Proposal
Instead of dynlinking
config.ml
, we can perform a 2-stage build of the configuration ! (1) Locally buildconfig.ml
(2) Executeconfig.ml
, which calls some entry function inFunctoria_app
There are several ways of doing it:
config.ml
andmirage
:Functoria_app.Make
in two functors, one that builds theconfig.ml
and one that is called by the builtconfig.ml
register ...
to a more appropriate namemirage
:Make
and haveregister ...
as the entry point of (2)Make
and hack with the names that are currently used.I've implemented the intermediate solution. This is probably far from perfect but I'd like to have feedback on that :)