Skip to content
This repository has been archived by the owner on Oct 18, 2020. It is now read-only.

Port to dune and set as a virtual library #7

Closed
wants to merge 6 commits into from

Conversation

TheLortex
Copy link
Member

@TheLortex TheLortex commented May 22, 2019

Using dune features mirage-os-shim can become a virtual library implemented by mirage-xen, mirage-unix and mirage-freestanding.

We need to figure out the correct conflicts field in the opam file.

Main post: mirage/mirage#969

mirage-os-shim.opam Outdated Show resolved Hide resolved
@hannesm
Copy link
Member

hannesm commented May 25, 2019

I'd be in favour of renaming the Mirage_OS module to OS (to avoid the (only-forward-declaring) Mirage_OS modules in implementations hereof); or am I missing something?

@TheLortex
Copy link
Member Author

For now there are two limitations:
(1) an implementation cannot expose other modules than what is specified by the virtual library interface.
(2) a module cannot be used for two separated libraries.

(1) is why I need to separate mirage-solo5 in two libraries:

  • mirage-solo5 is the implementation of mirage-os-shim and only that mirage-os-shim's interface.
  • mirage-solo5.internals is the underlying solo5 interface that solo5-specific packages can still use (mirage-net-solo5, ...)

(2) is why it's not possible to rename Mirage_OS to OS as long as both mirage-solo5 and mirage-solo5.internals need to expose this module.

@hannesm
Copy link
Member

hannesm commented May 27, 2019

@TheLortex thanks for your explanation, though tbh I'm still not convinced that introducing OS and Mirage_OS as modules is the right approach. Are (1) and (2) inherent to OCaml, or technical issues in this new build system? I'm really not a fan of layering up hacks and workarounds just because of deficiencies in the supply chain (and I remember that (2) bit me in logs-syslog, where now a copy rule is present (see https://github.com/hannesm/logs-syslog/blob/master/src/lwt/dune) to "solve" this issue). sorry for being picky.

@TheLortex
Copy link
Member Author

TheLortex commented May 28, 2019

@hannesm I can't really tell if it's an ocaml or a dune issue. However, the current design is already a bit hacky because for example mirage-xen's OS.mli doesn't match mirage-os-shim's OS.mli (mirage-xen also exposes internals). So it might be a good thing to enforce a bit more discipline.

So instead of introducing a Mirage_OS module, let's keep it under the name OS and update mirage-solo5 and mirage-xen to not expose more than what's required in their implementation of OS ? That means we have to expose internals in an other module. What do you think ?

@TheLortex
Copy link
Member Author

So the scheme I think is good to apply is the following:

  • mirage-os-shim exposes an OS module as a virtual interface: this defines a clear common interface to implement.
  • mirage-<target> implements this interface.
  • mirage-<target>.internals is a subpackage that exposes all the other modules wrapped in an OS_<target> module. Thus there is minimal changes to do on revdeps such as mirage-net-<target>.

@avsm @hannesm What's your feedback on this ? I updated PRs accordingly. There's a lot of interdependencies so I'm not sure how this should be done.

@avsm
Copy link
Member

avsm commented Jun 2, 2019

The scheme above sounds good. It would be good to release the various mirage-target.internals targets before the mirage-os-shim release to break the dependency cycle and get the individual implementations all adjusted

@hannesm
Copy link
Member

hannesm commented Jun 12, 2019

ok, this is fine with me, though I don't understand the naming of modules here and in mirage-xen/mirage-solo5... earlier we had filenames oS.ml{,i} (i.e. first character lower case, other upper case) -- now there's OS.ml{,i} -- what's the reason behind this?

while at it, the Time submodule looks to me like legacy -- there's a mirage-time repository and module which does exactly what time provides (a sleep_ns function) -- is this really needed to be exposed here?

about the Lifecycle module, is this actually used (maybe in the xen cases, i.e. on qubesos)!?

@avsm
Copy link
Member

avsm commented Jun 12, 2019

I think these modules like Lifecycle and Time can be deprecated, but lets do that in a separate step to this PR which adapts the existing build strategy. I'm going to start merging/releasing them soon without module changes, so we can spot any build regressions independently of type definition evolution.

@hannesm
Copy link
Member

hannesm commented Jun 12, 2019

@avsm uhm, iiuc this change and related work requires mirage+dune -- or am I wrong and these changes also work with mirage using ocamlbuild for building?? which seems to be slightly stalled (or at least I can't evaluate whether there's progress or not since mirage/mirage#969 didn't get updated -- NB: still missing a concrete developer-workflow changes document (and still don't understand how your "duniverse" fits into the picture))

@TheLortex
Copy link
Member Author

@hannesm

ok, this is fine with me, though I don't understand the naming of modules here and in mirage-xen/mirage-solo5... earlier we had filenames oS.ml{,i} (i.e. first character lower case, other upper case) -- now there's OS.ml{,i} -- what's the reason behind this?

I don't know why the o was lowercase so I renamed it to OS.ml{,i} to make it look better but if it doesn't make sense to you we can keep it as before.
This is meant to be used with dune's virtual libraries feature (several implementations for the same .mli) and I'm not sure it's compatible with ocamlfind.

On mirage+dune the work is mostly done, it's just a matter of reviewing and releasing packages. I've published a recap on mirage/mirage#969, is that was you wanted ?

@@ -1 +1 @@
src/Mirage_OS
src/OS
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you don't need to keep the doc/ folder anymore

bug-reports: "https://github.com/mirage/mirage-os-shim/issues"
tags: []
build: [
["dune" "subst" ] {pinned}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is not needed anymore with the latest versions of dune (I think)

@hannesm
Copy link
Member

hannesm commented Aug 5, 2019

Thinking again about the split into multiple modules (OS vs OS_xen / OS_solo5) / sublibraries / ... of the implementations, I'm really against this (as mentioned to @samoht) the other day. When programming OCaml, I find it intuitive to have an interface A and an interface B which extends A, and an implementation Bimpl that satisfies the interface A. Now, for some technical reason I neither understand nor find documentation (all I find is the sentence here): It is not possible for implementations to introduce new public modules; and then further down the line Virtual libraries must be defined using dune which makes me afraid of vendor lock-in and extensibility. The same page mentions Some of these are temporary -- so is this the case for the two limitations mentioned above? If no, why not? If yes, can they be leveraged before we use this approach in mirage?

@dinosaure
Copy link
Member

This PR seems the real bottleneck of the dunification of MirageOS so we need to precise what is really going on in a future about mirage-os-shim and implications with the current status of MirageOS.

First, we need to separate two details about variants and dune and do an explanation about what we currently do (not specially here) and what we want to do.

Virtual libraries

Virtual libraries is already known with ptime, digestif or checkseum. The idea of a virtual library is to provide only a *.mli/*.cmi. By this way, we are able to compile others libraries which want to use this specific interface.

As @TheLortex said, by this interface, an implementation can not be extended from it (in others words, it can not provide new values/functions) and an implementation must respect the given interface.

In your cases, we have 2 problems about that:

  • mirage-solo5/mirage-xen want to extend OS.mli with some others stuffs
  • an implementation, even if it does not need to have a function foo expected by the given interface must implement it (and/or fake it)

digestif and checkseum are not the best example due to C stubs. ptime is less complex about that. Virtual libraries are already used by MirageOS in some ways (ocaml-git, irmin). What we know about them is:

  • a library should never choose an implementation

A segfault can be replicated if 2 orthogonal libraries choose different implementations. At the end, ocaml with ocamlfind will link both and, if you are lucky, the compiler will observe that you want to link same symbols two times or, in the case of digestif (due to C stubs again), one implementation will defines (already existing by the other implementation) symbols at the runtime - and we get the segfault.

  • implementation must be before any others libraries (or a topological sort is needed between libraries)

If it's not the case, at the end, you will have a linking error.

About virtual libraries and mirage-os-shim

mirage-os-shim is particular because the current dependency graph on OPAM says that mirage-os-shim depends on mirage-solo5 | mirage-xen | mirage-unix. This PR interverts this dependency where mirage-solo5 | mirage-xen | mirage-unix will depends on mirage-os-shims to get the common interface OS. Then, these packages will provide an implementation of this interface.

So at the end, the update of this PR is much more deeper than a simple dunification when MirageOS will trust only on mirage-os-shim and, at the link time, we will choose which implementation we want according to the target.

My current status of Mirage + dune works on this PR and finally uses virtual libraries feature but, as I said, produced dune file by the mirage tool will explicitly provide the implementation (in your cases, mirage-solo5 | mirage-xen | mirage-unix).

About compatibility

Virtual libraries are already used by MirageOS with the constraint to explicit give the right implementation at the link-time. So, even if we use dune, this PR should be compatible with the current status of MirageOS when dune provide the right META file (with implementation) - however, it adds the constraint to take care about the link time (explicit the dependency and take care about the order).

dune is currently able to take care about the order but it go furthermore with variants.

Variants

First, I would like to say that this PR does not introduce the variant feature here!

Variant is a feature on top of virtual libraries when a virtual library can_ provide a variant name (like solo5 or xen). Then, at the end, if an executable says (variants xen), dune will take all implementations/*.cmx which are described as a xen variant to plug them with right interfaces/*cmi.

About the scope of this feature

I need a confirmation but it seems that this feature can exist only with dune and is not compatible in any way with ocamlfind/ocamlbuild.

About mirage-os-shim

As I said, this PR does not integrate the variant feature and, due to the incompatibility, should not. However, the question still is open. But it seems too far from what we have and what we want.

Difference with current status of Mirage and this PR

The real difference on this PR if we compare with the current status of MirageOS is: who leads on OS.mli? Currently, each implementation leads their own interface and this PR try to change that and constraint us under a common interface which will be used by mirage then.

I'm in favor on this PR but we really need to figure out if it's the right direction for us and implications on mirage-solo5 | mirage-xen | mirage-unix. This change mostly say that extension of implementations according the target will be not available then for unikernel.ml.

From what I know, I reached a situation where pasteur is compatible with MirageOS 3.6 and MirageOS + dune.

However, if I go to details, mirage-net-solo5 according multiple devices of Solo5 0.6 needs mirage-solo5.internals (and specially wait_for_work_on_handle) which is not the part of the common interface OS.mli.

From pasteur, occurence of OS.* are mostly found with OS.Time (for mirage-entropy and arp). Then, only the generated main.ml seems to use the OS module.

I hope all is clear for people (@mirage/core) now 🎉 .

@hannesm
Copy link
Member

hannesm commented Oct 17, 2019

thanks @dinosaure for your explanation. I guess mirage-os-shim is a sore thumb in MirageOS atm (due to its reversed dependencies -- e.g. mirage-net only defines the network interface, which is then implemented by mirage-net-xen/-solo5/-unix/...). I'd propose to:

  1. change mirage-os-shim to provide the interface only (i.e. no dependencies on mirage-xen/-solo5/-unix) -- this should be pretty independent of other things, and since the mirage tool already registers dependencies onto mirage-solo5/mirage-xen/..., it should magically work -- this should as well work fine with ocamlbuild-based mirage 3 -- the only user of mirage-os-shim is mirage-entropy (uses ocamlbuild + ocb-stubblr), best to ensure this still works
  2. switch build system to dune for mirage-os-shim
  3. maybe it should follow the common naming scheme, and be called mirage-os, or while at it being split into (a) mirage-time(-lwt) -- that is already present, so the module Time can be dropped and (b) module Main, so maybe mirage-main is the right library name then?
  4. release this dependency-inverted mirage-os-shim^W mirage-main, adjust and release mirage-entropy
  5. I suspect mirage-entropy should then be functorized over the Main implementation (and the boilerplate in mirage which applies the functor)
  6. "set as virtual library" / "variants" -- this is sth to be done for other mirageos signature packages (mirage-net) as well, and likely needs a dunified mirage (and some guideline/information how to package these things up, where to store the information "for solo5 you need the mirage-net-solo5 package" -- atm this is in mirage, but it'd be nice to not hardcode this for extensibility to new targets etc.) -- a way around this may be to push all mirage-net implementations into the mirage-net package (using sublibraries)

The mentioned c vs ocaml variants in checkseum/digestif are rather different (it is unclear to me who should decide which to use) from target-specific variants (where mirage knows the target and decides which variant is suitable to be linked into a unikernel).

On a side note, I'd appreciate if (4) could be avoided -- some resources are always available (once and only once) in a MirageOS unikernel (at least in my opinion): Random, Mclock, Pclock, Main, Time - it'd be nice to not have to functorize over these (that AFAIU was the original intention of mirage-os-shim).

@samoht
Copy link
Member

samoht commented Oct 18, 2019

I haven't look much into the details but my gut feeling is that we don't need mirage-os-shim at all and that we should try to have the smaller OS.ml as possible (and if possible not at all).

@hannesm
Copy link
Member

hannesm commented Oct 18, 2019

@samoht yes as mentioned earlier -- I guess the Main is the only leftover (the user I can find is mirage-entropy calling OS.Main.at_enter_iter), the Lifecycle I haven't seen in action, and Time is provided by Mirage_time.S.

@dinosaure dinosaure mentioned this pull request Oct 19, 2019
@dinosaure dinosaure closed this Oct 30, 2019
@mato mato mentioned this pull request Oct 30, 2019
46 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants