Skip to content
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] No opam, constrain package versions in unikernels #691

Closed
wants to merge 3 commits into from

Conversation

Projects
None yet
5 participants
@hannesm
Copy link
Member

commented Nov 19, 2016

see mirage/functoria#82 ... very much WIP (but compiles + runs :)

@hannesm hannesm force-pushed the hannesm:no-opam branch from 258a02e to 97c583a Nov 19, 2016

@yomimono yomimono referenced this pull request Nov 19, 2016

Closed

pending items for consideration for Mirage 3.0 #571

9 of 9 tasks complete

@hannesm hannesm force-pushed the hannesm:no-opam branch from ef78404 to ffdc1cd Nov 19, 2016

@hannesm

This comment has been minimized.

Copy link
Member Author

commented Nov 19, 2016

there's some stuff which now needs (in contrast to what I thought in #627) to be delayed until build time, such as nocrypto check (NB: the only user of OCamlfind.query/installed from Functoria, which can thus go away as well -- shouldn't be a concern to functoria)... at configuration time we cannot promise that all dependent packages are installed (thus calling out to ocamlfind is not very robust).

I'd suggest to leave the fat and crunch at the configuration step for now (requires the user to have crunch/fat around when configuring the unikernel). This has at least the property that there are not more source files generated at build time (and mirage really needs to run only once).

But stuff like pkg-config should be defered to later (into the makefile for when build is called). also, the depend target should be updated to do sth along the lines opam install --deps-only <myopam>.

see mirage/mirage-skeleton#198

@hannesm

This comment has been minimized.

Copy link
Member Author

commented Nov 19, 2016

Version constraints of a unikernel: mirage configure emits automatically constraints into the opam file for mirage-types and mirage-runtime (always: >= current-version & < next-major). When you change a types or runtime API, if it is backwards compatible (extension of the current things), just bump the minor version. For non-backwards compatible changes, we can just bump the major version. old configured unikernels will not even been tried to compile with the new types or runtime.

If you like, you can also specify constraints in your very own config.ml on mirage-types or mirage-runtime. Version constraints get merged: maximum version of all mins, minimum version of all max versions.

Thus, package ~min:"3.0.5" ~max:"3.5.0" "mirage-types" and package ~min:"3.2.0" ~max:"4.0.0" "mirage-types" will result into package ~min:"3.2.0" ~max:"3.5.0" "mirage-types".

Maybe foreign should get a(n optional) parameter version, and functoria emits a >= version & < major_succ version constraint? Otherwise, taking a bare unikernel you never know which version it was intended for, since mirage configure only emits the current release. -- OTOH this is manually doable, pass package ~min:"3.0.0" ~max:"4.0.0" "mirage-types" via config.ml / foreign. A ?version argument would need to be handled in functoria, where we better don't know anything about mirage-types.

@hannesm hannesm changed the title [WIP] No opam, constrain package versions in unikernels [RFC] No opam, constrain package versions in unikernels Nov 19, 2016

@yomimono

This comment has been minimized.

Copy link
Member

commented Nov 19, 2016

First off, this is excellent and I really appreciate this PR. Thank you very much for putting this (very badly needed) set of patches together!

I think there's some room for improvement in the user experience, though. On a fresh switch:

o-opam🐫  ((detached from hannesm/no-opam)) mirageos:~/mirage-skeleton/console$ opam pin
functoria.dev~mirage            git  /home/user/functoria#no-opam
functoria-runtime.dev~mirage    git  /home/user/functoria#no-opam
mirage.dev~mirage               git  /home/user/mirage#no-opam   
mirage-runtime.dev~mirage       git  /home/user/mirage#no-opam   
mirage-types.dev~mirage         git  /home/user/mirage#no-opam   
mirage-types-lwt.dev~mirage     git  /home/user/mirage#no-opam   
no-opam🐫  ((detached from hannesm/no-opam)) mirageos:~/mirage-skeleton/console$ mirage configure -t virtio
Fatal error: exception (Failure
   "command terminated with exit code 1\
  \nstderr: Package solo5-kernel-virtio was not found in the pkg-config search path.\
  \nPerhaps you should add the directory containing `solo5-kernel-virtio.pc'\
  \nto the PKG_CONFIG_PATH environment variable\
  \nNo package 'solo5-kernel-virtio' found\
  \n")

I get a mirage-console.opam but an incomplete main.ml and Makefile. Installing the set of packages in mirage-console.opam then rerunning mirage configure works as expected.

The automatic upper bound interacts poorly with pins that specify a high package number (coincidentally, anything pinned in a switch that knows about mirage-dev at the moment, as those version numbers are all dev~mirage which is higher than any number).

Basic opam question -- is opam install --deps-only <file> supposed to work? For a generated console-virtio.opam, I get a complaint that there's no such package and failure when trying to run that command. I skimmed TFM for opam install but maybe missed what would've helped me.

opam pin add console-virtio . in the directory containing the opam file works, but then I have a console-virtio package installed locally. It doesn't hurt anything, but it's rather nonsensical IMO, and this introduces the potential for namespace collisions between the generated name of the opam file and the opam package universe. (I see this concern is addressed over in the Functoria PR though.)

@samoht

This comment has been minimized.

Copy link
Member

commented Nov 19, 2016

The pb with crunch/fat is that it is actually a configuration that you want to make available at deployment time (e.g. if you want to embed some secrets). So it makes sense that they don't fit very well in the refactoring, as we used to put together stuff which were supposed to happen a configuration/build/deployment time.

Not sure how to solve that properly, maybe we need to had an other stage in the compilation pipeline, but we can see later. The current simplification is way better than it was before ;-) Maybe just generate the scripts to generate the fat/crunch images so that they can be run later?

@hannesm

This comment has been minimized.

Copy link
Member Author

commented Nov 20, 2016

@yomimono thanks for your feedback. sorry for pushing an unfinished changeset which doesn't completely work yet (and all I tested was opam lint on the generated opam file)

  • incomplete main.ml/Makefile: as mentioned in this PR, we'll need to defer pkg-config calls again to the Makefile (rather than doing them in mirage during configure time) - will update
  • version constraints: there is no need for mirage to emit any (upper bound) version constraints. reasonable unikernels will specify their mirage-types lower and upper bounds - will update
  • opam troubles: I haven't tried, but my idea was to do (in make depend) opam pin -n add <name> <name.opam>; opam install --deps-only <name>; opam pin remove <name> - will update
  • crunch/fat (@samoht): I have two workflows in mind: dev where it would be nice as it is atm (generate the stuff during mirage configure, leave it there forever, requires fat/crunch being available at configure time); deployment where the build host does not have access to secret data. there should be a vm image produced which is postprocessed (using ld?) to add arbitrary binary data (as an elf section). while mirage currently uses dev, we've to find a good solution for deployment (while keeping the dev working for development) -- I guess best is to discuss this in another issue or on the mailing list (and since we revise processes for Mirage3, we should decide whether this is important enough to be considered for Mirage3)

@hannesm hannesm force-pushed the hannesm:no-opam branch from 2955a64 to 1bb9880 Nov 20, 2016

@hannesm

This comment has been minimized.

Copy link
Member Author

commented Nov 20, 2016

I updated this PR:

  • no more upper bound emitted by mirage
  • the name for the opam file is now mirage-unikernel--`
  • depend got tweaked: opam pin add <name> . ; opam install --deps-only <name> ; opam pin remove <name>
  • the pkg_config calls went back into the Makefile (instead of having them in OCaml code).

This works pretty much for me. I don't have any clue whether something (and what and how) is supposed to call opam depext... but others (@avsm?) will shed some light into that.

@yomimono if you have time and energy, this should be much smoother for testing.

With BSD make, the sed -e s|@|$(OPREFIX)/lib/|g does not work (it does with GNU make), where OPREFIX=$(shell opam config var prefix). The reason is that sed does not evaluate the opam config var prefix expression (somehow does with GNU make)... Changing it to OPREFIX=$$(..) does work for neither make. is there a make-agnostic shell command which evaluates directly instead of at use site? maybe @mato knows something?

@hannesm

This comment has been minimized.

Copy link
Member Author

commented Nov 21, 2016

and this lets me run mirage configure -t xen on my FreeBSD laptop, which will never be able to compile and execute such unikernels ;)

@hannesm hannesm force-pushed the hannesm:no-opam branch from 110028d to 08a57b2 Nov 21, 2016

@@ -1758,6 +1693,10 @@ let check_ocaml_version () =
) else
R.ok ()

let unikernel_name target name =
let target = Fmt.strf "%a" Key.pp_target target in

This comment has been minimized.

Copy link
@Drup

Drup Nov 21, 2016

Member

Fmt.to_to_string

(* XXX: use %%VERSION_NUM%% here instead of hardcoding a version? *)
package "lwt";
package ~ocamlfind:[] "mirage-types-lwt";
package ~sublibs:["lwt"] ~min:"3.0.0" "mirage-types";

This comment has been minimized.

Copy link
@Drup

Drup Nov 21, 2016

Member

Both mirage-types-lwt and mirages-types.lwt ?

This comment has been minimized.

Copy link
@hannesm

hannesm Nov 21, 2016

Author Member

yes. no. soon to be changed... atm there is mirage-types which provides V1 and V1_LWT in its lwt subpackage, and mirage-types-lwt is a virtual package which gathers all deps to get mirage-types.lwt. pretty insane atm, should be better soon...

@Drup

This comment has been minimized.

Copy link
Member

commented Nov 21, 2016

The opam file generation should be in functoria directly. It's not mirage specific and it's not doing any extra work.

there's some stuff which now needs (in contrast to what I thought in #627) to be delayed until build time, such as nocrypto check (NB: the only user of OCamlfind.query/installed from Functoria, which can thus go away as well -- shouldn't be a concern to functoria)... at configuration time we cannot promise that all dependent packages are installed (thus calling out to ocamlfind is not very robust).

Yes++. You have all my support on that one. :D

@hannesm

This comment has been minimized.

Copy link
Member Author

commented Nov 21, 2016

the ocamlfind code from functoria is already removed in functoria's PR...

@hannesm

This comment has been minimized.

Copy link
Member Author

commented Nov 21, 2016

I don't know when to merge (rough times ahead), an example unikernel over at logs-syslog works https://travis-ci.org/hannesm/logs-syslog/jobs/177822764 ci-script: https://github.com/hannesm/logs-syslog/blob/dev/.travis-test.sh (basically mirage configure -t XX followed by make depend and make) -- I'm pretty certain mirage, mirage-dev, mirage-skeleton need updates in their ci scripts

unfortunately I won't be able to spend much time on this in the upcoming weeks, so please go ahead and do what is good.

hannesm added some commits Nov 19, 2016

Updates for Functoria's introduction of the package type
- instead of lists of packages and libraries, use lists of packages
@hannesm

This comment has been minimized.

Copy link
Member Author

commented Nov 22, 2016

There's a full travis build at https://travis-ci.org/hannesm/mirage-dev/builds/177981102 which has some changes:

Unix/Xen/virtio works fine, ~~~ukvm does not~~~ ukvm as well with additional patches

good to go from my point of view!

defer ukvm-configure to build time, and don't include Makefile.ukvm (…
…to allow

make depend without ukvm-configure)

@hannesm hannesm force-pushed the hannesm:no-opam branch from 6f65a13 to 9340522 Nov 22, 2016

@hannesm

This comment has been minimized.

Copy link
Member Author

commented Nov 22, 2016

update: travis is happy, see comment above

@yomimono

This comment has been minimized.

Copy link
Member

commented Nov 22, 2016

I probably don't have time to review this properly today but I'm very excited to see it. Thanks a ton for this, @hannesm! :D

@mato

This comment has been minimized.

Copy link
Member

commented Nov 22, 2016

Same here, this is great but still need to test and review it fully. Hopefully tomorrow, fingers crossed. Thanks @hannesm!

@mato

This comment has been minimized.

Copy link
Member

commented Nov 23, 2016

Initial comments from a first try of this, using ukvm as a target:

  1. The pinning command in make depend asks for interactive confirmation ("Package mirage-unikernel-... does not exist, create as a NEW package?"). Can we remove this check?
  2. One of the better things about generating all of the Makefile before this change was that it was obvious what the actual commands invoked are. Now we're back to stuff like this:
ld $(PKG_CONFIG_PATH=/home/mato/.opam/mirage-dev-4.04.0/share/pkgconfig:/home/mato/.opam/mirage-dev-4.04.0/lib/pkgconfig pkg-config --variable=ldflags solo5-kernel-ukvm) \
_build/main.native.o \
$(ocamlfind query -r -format '-L%d %(freestanding_linkopts)' -predicates native conduit conduit.mirage functoria-runtime io-page lwt magic-mime mirage-bootvar-solo5 mirage-clock-freestanding mirage-http mirage-logs mirage-net-solo5 mirage-runtime mirage-solo5 mirage-stdlib-random mirage-types mirage-types.lwt nocrypto nocrypto.mirage tcpip tcpip.arpv4 tcpip.ethif tcpip.icmpv4 tcpip.ipv4 tcpip.stack-direct tcpip.tcp tcpip.udp tls tls.mirage uri | sort -u | sed -e 's|@|/home/mato/.opam/mirage-dev-4.04.0/lib/|g') $(PKG_CONFIG_PATH=/home/mato/.opam/mirage-dev-4.04.0/share/pkgconfig:/home/mato/.opam/mirage-dev-4.04.0/lib/pkgconfig pkg-config --static --libs mirage-solo5) \
-o mir-https.ukvm

which IMO makes it much harder to understand what is going on.
3. Running make a second time after a successful build gives me the following errors:

SANITIZE: a total of 2 files that should probably not be in your source tree
  has been found. A script shell file
  "/home/mato/solo5/mirage-skeleton/static_website_tls/_build/sanitize.sh" is
  being created. Check this script and run it to remove unwanted files or use
  other options (such as defining hygiene exceptions or using the -no-hygiene
  option).
IMPORTANT: I cannot work with leftover compiled files.
ERROR: Leftover object files:
  File ukvm-net.o in _build-ukvm has suffix .o
  File ukvm-core.o in _build-ukvm has suffix .o
Exiting due to hygiene violations.
Compilation unsuccessful after building 0 targets (0 cached) in 00:00:00.
Makefile:40: recipe for target 'main.native.o' failed
make: *** [main.native.o] Error 1

@hannesm

This comment has been minimized.

Copy link
Member Author

commented Nov 23, 2016

thanks for testing and your feedback, @mato

  1. we can add the -y parameter to the calls to opam (or export OPAMYES=1) in the generated Makefile.
  2. I don't have a solution for this (apart from doing #700). we do agree that configure -> install dependencies -> build is the right workflow (in contrast to earlier times), don't we?
  3. this is only relevant for ukvm afaics. I can see three possible solutions (a mix of them is possible):
  • a) call ocamlbuild without the recursive flag (see #537) -- I don't agree with the argument in there (and if the warning is misleading, we should find another way to disable it: judging from the source we can influence the heuristics by having an (empty) _tags or myocamlbuild.ml file around)
  • b) require a -$(MAKE) clean in the build rule in the Makefile
  • c) build ukvm stuff in _build as well

but trying 3) out, it is unclear to me whether a second make in a ukvm target works atm. it does not really look like... FWIW removal of make clean and instead using mirage clean is in hannesm@0a75cb1

@hannesm hannesm referenced this pull request Nov 25, 2016

Merged

New order #703

@hannesm

This comment has been minimized.

Copy link
Member Author

commented Nov 26, 2016

superseeded by #703

@hannesm hannesm closed this Nov 26, 2016

@hannesm hannesm deleted the hannesm:no-opam branch Jan 26, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.