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

zarith-freestanding.1.6 + zarith-xen.1.6 #10350

Merged
merged 5 commits into from
Oct 10, 2017
Merged

Conversation

hannesm
Copy link
Member

@hannesm hannesm commented Sep 27, 2017

this adds the two above mentioned packages. in order to build zarith-freestanding (at least on FreeBSD), gmp-freestanding and gmp-xen needs to pass --with-pic during configuration phase (which enables -fPIC).

//cc @mato

@camelus
Copy link
Contributor

camelus commented Sep 27, 2017

❗ opam-lint warnings b34604f
  • gmp-freestanding.6.0.0-1 has some warnings:

    • warning 37: Missing field 'dev-repo'
  • gmp-freestanding.6.0.0 has some warnings:

    • warning 37: Missing field 'dev-repo'
  • gmp-xen.6.0.0-1 has some warnings:

    • warning 36: Missing field 'bug-reports'
    • warning 37: Missing field 'dev-repo'
  • gmp-xen.6.0.0 has some warnings:

    • warning 36: Missing field 'bug-reports'
    • warning 37: Missing field 'dev-repo'
  • zarith-freestanding.1.6 has some warnings:

    • warning 36: Missing field 'bug-reports'
    • warning 37: Missing field 'dev-repo'
  • zarith-xen.1.6 has some warnings:

    • warning 36: Missing field 'bug-reports'
    • warning 37: Missing field 'dev-repo'
  • These packages passed lint tests: solo5-kernel-ukvm.0.2.2-1, solo5-kernel-virtio.0.2.2-1


✅ Installability check (7503 → 7509)
  • new installable packages (6): gmp-freestanding.6.0.0-1 gmp-xen.6.0.0-1
    solo5-kernel-ukvm.0.2.2-1 solo5-kernel-virtio.0.2.2-1
    zarith-freestanding.1.6 zarith-xen.1.6

@mato
Copy link
Contributor

mato commented Sep 27, 2017

@hannesm Didn't you want to upgrade gmp to 6.1.2 also or is that forthcoming in a separate PR? If that's the case, perhaps do the -fPIC change there only and depend on the newer gmp in the newer zarith?

(Possibly I'm being too paranoid here, but I'm not 100% sure about any side-effects of the -fPIC change and don't have the bandwidth to test right now)

@hannesm
Copy link
Member Author

hannesm commented Sep 27, 2017

@mato well, gmp.6.1.2 turns out to need more C symbols -- which also means modifications to ocaml-freestanding (already merged) and the ocaml-xen stuff.

I did a full end-to-end test using ukvm and FreeBSD with the mirage-skeleton static_website_tls unikernel, and gmp-freestanding.6.0.0 (and zarith-freestanding.1.6) -- and this works fine!

@mato
Copy link
Contributor

mato commented Sep 27, 2017

@hannesm I guess my point is that this changes the build of gmp subtly, without changing the version number. In the interests of not accidentally breaking something I'd prefer that the change include a new version of gmp. What you could do is do this for freestanding only and keep xen on 6.0.0, leaving someone else to upgrade that.

/cc @avsm @yomimono for opinions

@hannesm
Copy link
Member Author

hannesm commented Sep 29, 2017

so, are there any opinions how to move forward here? travis passes fine! has anyone a clue why opam1 and opam2 behave differently in respect to solo5 and depext?

@hannesm
Copy link
Member Author

hannesm commented Oct 2, 2017

/cc @avsm I'd be glad to get an opinion here - zarith-*.V has strict dependencies onto = zarith.V which keeps me from upgrading to latest 1.6 when I've -freestanding installed :/

@avsm
Copy link
Member

avsm commented Oct 2, 2017

@hannesm @mato - we could enable pic on a minor version bump of the package (add "a" or something to the suffix to denote it rather than changing the existing package). Then version constraints will let zarith-freestanding pick up the pic versions, but also address @mato's concern about changing an existing library without a version bump.

@hannesm
Copy link
Member Author

hannesm commented Oct 2, 2017

@avsm sounds like a good idea. I used 6.0.0-1 (which is AFAICT what Debian also does), and depend on > 6.0.0 in the zarith-*.1.6 packages.

@hannesm
Copy link
Member Author

hannesm commented Oct 2, 2017

CI failures indicate that ocamlfind is not available (502 - bad gateway)...

@yallop
Copy link
Member

yallop commented Oct 2, 2017

I've cancelled the Travis build; we can restart it once findlib/ocamlfind is available again (#10383).

…ith-pic)

depend in zarith-{xen,freestanding}.1.6 on "> gmp-{xen,freestanding}.6.0.0"
@hannesm
Copy link
Member Author

hannesm commented Oct 2, 2017

I rebased upon current master, and travis is happy again! :)

@hannesm
Copy link
Member Author

hannesm commented Oct 2, 2017

failures for debian unstable & testing are due to a fresh gcc, which are fixed upstream in solo5 (Solo5/solo5@48f8c4f)

@hannesm
Copy link
Member Author

hannesm commented Oct 3, 2017

ping. CI seems to be happy, apart from the above mentioned problem with released solo5 and gcc7.

@mato
Copy link
Contributor

mato commented Oct 3, 2017

I just ran a manual smoketest with static_website_tls using the released version of solo5-kernel-ukvm (v0.2.2) and the branch for this PR and all seems fine, so looks good to me.

Unfortunately there's not a lot we can easily do right now about the gcc7 failures, is that a problem for merging this?

@hannesm
Copy link
Member Author

hannesm commented Oct 3, 2017

@mato thx! what we could do is to include a patch into opam-repository which removes -Werror from the solo5 build system.. but I'm not sure whether it is worth it, given that there's a new solo5 release on the horizon.

@mato
Copy link
Contributor

mato commented Oct 3, 2017

@hannesm Well, "on the horizon" is more like "some time this month", so patching v0.2.2 in opam to remove -Werror is a good idea. I can look into that if you like.

@hannesm
Copy link
Member Author

hannesm commented Oct 3, 2017

@mato I removed -Werror in 59073a4

@mato
Copy link
Contributor

mato commented Oct 3, 2017

@hannesm Thanks. Nit: You should probably call those packages 0.2.2-1 in opam, since we just had that discussion in this PR :-)

…h removing -Werror to unbreak build with gcc7
@hannesm
Copy link
Member Author

hannesm commented Oct 3, 2017

@mato thx, done (0.2.2-1) in b0ae925

@mato
Copy link
Contributor

mato commented Oct 3, 2017

@hannesm Thanks, tested, LGTM!

@hannesm
Copy link
Member Author

hannesm commented Oct 3, 2017

according to https://packages.debian.org/search?searchon=contents&keywords=asm%2Fmsr-index.h&mode=exactfilename&suite=unstable&arch=any Debian unstable/testing/experimental provides the file asm/msr-index.h from linux-libc-dev and linux-headers-XX-common. I've no clue how to express this in opam atm (and am done with this PR here, take it or leave it).

@cfcs
Copy link

cfcs commented Oct 3, 2017

It looks like that header is not shipped in Debian (anymore)?
FWIW I do have this:

$ dpkg -S msr.h
linux-libc-dev:amd64: /usr/include/x86_64-linux-gnu/asm/msr.h

@mato
Copy link
Contributor

mato commented Oct 4, 2017

@cfcs asm/msr-index.h does indeed seem to be gone in Debian testing/unstable, at least it's not in the linux-libc-dev package. In fact, the entire /usr/include/asm/ symlink is also gone. ISTR there was an email about transitioning how the kernel headers are packaged, but I can't find it now :-(

Will keep looking to see if I can find a solution. Up to the opam-repository maintainers to decide if this can go in with the new distros (Debian testing, unstable, Fedora 25) failing.

@mato
Copy link
Contributor

mato commented Oct 4, 2017

@hannesm After some discussion, it's unclear what the situation with #include <asm/msr-index.h> from userspace is, but given that file is included solely to get the value of a single constant, and that code has since been fixed on ukvm master, the best plan right now would be to add the attached small patch to this PR.
ukvm-msr-index.h.patch.txt

@hannesm
Copy link
Member Author

hannesm commented Oct 4, 2017

@mato thanks, added in 36a6c1a d848e92

@hannesm
Copy link
Member Author

hannesm commented Oct 7, 2017

this is finally green again and ready for merge. to sum up the changes:

  • new packages zarith-freestanding.1.6 and zarith-xen.1.6 (which follow along zarith.1.6)
  • gmp-xen.6.0.0: fixed opam file (added authors) and dependency on conf-m4
  • gmp-xen.6.0.0-1: new package, which is gmp-xen.6.0.0 copied, the only difference is --with-pic being passed to configure (required by newer zarith)
  • gmp-freestanding.6.0.0: added dependency on conf-m4
  • gmp-freestanding.6.0.0-1: new package, same as gmp-freestanding.6.0.0, but configure with --with-pic
  • solo5-kernel-ukvm.0.2.2-1: new package, which is solo5-kernel-ukvm.0.2.2 copied, but with two additional patches: (a) remove -Werror (which fails with newer gcc), and (b) remove #include <asm/msr-index.h> which debian (unstable & testing) decided to no longer ship with linux-libc-dev
  • solo5-kernel-virtio.0.2.2-1: new package, which is solo5-kernel-virtio.0.2.2 copied, but with the same patch removing -Werror

I didn't intended to go this deep down here, but I'm glad that the resulting repository is installable on a wide variety of Linux distributions. The patches for solo5 are already upstreamed.

If someone would be so kind to merge this PR, I'd be glad (and can finally have my binaries where the stack is non-executable). Thanks!

@samoht
Copy link
Member

samoht commented Oct 10, 2017

Many thanks for leading this PR to its conclusion :-)

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.

7 participants