Skip to content

Add support for OCaml 4.12#88

Merged
hannesm merged 3 commits into
mirage:masterfrom
kit-ty-kate:412
Mar 3, 2021
Merged

Add support for OCaml 4.12#88
hannesm merged 3 commits into
mirage:masterfrom
kit-ty-kate:412

Conversation

@kit-ty-kate
Copy link
Copy Markdown
Member

mkdir and rmdir were added in ocaml/ocaml#9797

@hannesm
Copy link
Copy Markdown
Member

hannesm commented Nov 6, 2020

thanks for your patch, I approve the first commit (add mkdir & rmdir), but would for now delay the second commit until opam 2.1 semantics is settled (in respect to opam var).

@kit-ty-kate
Copy link
Copy Markdown
Member Author

My reasoning behind adding --readonly is that it is always correct to have it anyway so whether or not opam 2.1 is broken (which I would argue it is, I agree). The option itself was added in opam 2.0.0.
However if you really don't want it and want to make a new release quickly I'll remove it no problem. I'll keep it in opam-alpha-repository though as I need it for testing.

@hannesm
Copy link
Copy Markdown
Member

hannesm commented Nov 6, 2020

@kit-ty-kate my reasoning is "we use at 10 other places opam config var prefix, so if we add --readonly here, let's go the full path down and use it everywhere". it does not hurt to add it here, since opam 2.0 supports it already, as you mentioned. but it hurts a bit to have it not consistent across the packages I care about.

@kit-ty-kate
Copy link
Copy Markdown
Member Author

Fair enough, removed.

Copy link
Copy Markdown
Contributor

@mato mato left a comment

Choose a reason for hiding this comment

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

5476e34 looks good. You are still missing an update to the opam metadata, and we should do something about CI; it's still only testing with OCaml up to 4.10 (i.e. missing 4.11 and 4.12, but the latter presumably won't work until actually released?)

@hannesm hannesm force-pushed the 412 branch 2 times, most recently from 1c01f98 to 693e7da Compare November 13, 2020 17:23
@hannesm
Copy link
Copy Markdown
Member

hannesm commented Nov 13, 2020

I rebased and force-pushed. The issue with a CI for OCaml 4.12 is that we need the ocaml-src package in opam-repository first (which is debatable whether we want to have this for pre-releases).

@kit-ty-kate
Copy link
Copy Markdown
Member Author

This PR is good to go

@hannesm
Copy link
Copy Markdown
Member

hannesm commented Mar 3, 2021

The CI issue is (same as observed in mirage/mirage-tcpip#439 (comment)):

ld: /home/opam/.opam/4.08/lib/pkgconfig/../../lib/ocaml-freestanding/libasmrun.a(startup_aux_n.o):/home/opam/.opam/4.08/.opam-switch/build/ocaml-freestanding.0.6.3/ocaml/runtime/startup_aux.c:35: multiple definition of `caml_atom_table'; /home/opam/.opam/4.08/lib/pkgconfig/../../lib/ocaml-freestanding/libasmrun.a(startup_nat_n.o):/home/opam/.opam/4.08/.opam-switch/build/ocaml-freestanding.0.6.3/ocaml/runtime/startup_nat.c:47: first defined here

Interestingly, this only happens on GNU/Linux (with 4.08.1), but not on FreeBSD (see the cirrus CI run). To move forward (and not waste several days investigating build systems), I suggest to drop 4.08 support from freestanding (and MirageOS). Any objections? /cc @mato

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants