search

mirage / ocaml-solo5

Freestanding OCaml runtime
Other
100 stars 30 forks source link

Add support for OCaml 4.10 #68

Closed kit-ty-kate closed 4 years ago

kit-ty-kate commented 4 years ago

This is a temporary PR, it works locally but requires https://github.com/ocaml/ocaml/pull/9213 and a patch to ocaml-ci-scripts to support 4.10 for the CI

dinosaure commented 4 years ago

If CI is happy, LGTM to merge, thanks!

mato commented 4 years ago

@kit-ty-kate Is this intended to stay "temporary" until 4.10 is actually released, or would you like it merged earlier? From a quick review it LGTM.

kit-ty-kate commented 4 years ago

@mato it can stay temporary but if it's merged early it's better. I think CI should be all green this time with OCaml 4.10 being tested on both Travis and Cirrus

kit-ty-kate commented 4 years ago

ah right, ocaml-src.4.10.0 is required for this.. I have it in this opam overlay (https://github.com/kit-ty-kate/opam-alpha-repository/commit/9ba1d32882ccafcfed2b55248585b34b58cd80e6) for now but I'm not sure how to make it available to CIs just yet.

kit-ty-kate commented 4 years ago

The first beta version for OCaml 4.10 is due tomorrow so maybe I can add ocaml-src.4.10.0~beta1 at this occasion. It should work then

kit-ty-kate commented 4 years ago

@mato what do you think about this? https://github.com/ocaml/opam-repository/pull/15650

mato commented 4 years ago

@kit-ty-kate To be honest, I'd just wait for OCaml 4.10 to be released, including ocaml-src to OPAM, then merge mirage/ocaml-freestanding#68 and cut an ocaml-freestanding release as normal. This is how we've done it for previous compiler releases, and is the easiest thing to do without incurring extra mental and work effort to get CI to pass.

According to this discourse thread, OCaml 4.10 should be released around the end of this month, so there's not much to be gained from pushing this earlier.

kit-ty-kate commented 4 years ago

This PR should now be ready to be merged once the CI is green.

kit-ty-kate commented 4 years ago

wait how is mirage-solo5 a dependency but mirage-solo5 also depends on ocaml-freestanding. Isn't that a recursive dependency?

In any case I suppose you'll understand your CI better than me. The PR is ready to be merged anyway. Tell me whether I should do something more

hannesm commented 4 years ago

@kit-ty-kate the ocaml-freestanding opam package does not depend on mirage-solo5, but the CI links a unikernel image to ensure that a change to ocaml-freestanding leads to a result which can be linked (i.e. without unresolved symbols etc.) into a unikernel -- see https://github.com/mirage/ocaml-freestanding/pull/55 for further discussion.

mato commented 4 years ago

@hannesm The FreeBSD CIs are failing -- 4.10.0 I can understand, since it's the same failure as I just got rid of on Travis (circular dependency), however I don't understand the failure on 4.09.0: https://cirrus-ci.com/task/4865166823129088

Any ideas? I can ignore the 4.10.0 failure since I don't know how to set environment variables per matrix entry on Cirrus to disable it, and don't want to spend unknown amounts of time figuring it out, but 4.09.0 should work?

I would like to cut a new ocaml-freestanding release today. Alternatively, if you can confirm manually that 4.09.0 does indeed work then I can just merge and proceed.

hannesm commented 4 years ago

@mato looks like a spurious error to me. This PR works fine (including the e2e unikernel) on my FreeBSD machine in a 4.09.0 switch. The CI from two commits above (https://github.com/mirage/ocaml-freestanding/runs/461911909) also reports success with 4.09.0 and FreeBSD.

mato commented 4 years ago

@hannesm Thanks for confirming, I'll just go ahead and merge this then.