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

New order #703

Merged
merged 8 commits into from Nov 29, 2016

Conversation

Projects
None yet
4 participants
@hannesm
Member

hannesm commented Nov 25, 2016

requires mirage/functoria#84, on top of #691

relies on Solo5/solo5#137 which emits ukvm build products into _build/ukvm instead of _build-ukvm

The generated Makefile is much more simplicistic. There is a new build subcommand, which calls out to ocamlbuild, and in a link step to ld (apart from Unix target).

~~~As mentioned in mirage/functoria#84, clean is somehow broken, but configure and build are tested on unix.~~~

Superseeds #691 fixes #696 (we use opam, use its available and ocaml-version!) #700 #596 superseeds #586

Feedback welcome.

@hannesm

This comment has been minimized.

Member

hannesm commented Nov 26, 2016

There is a working mirage-skeleton travis job at https://travis-ci.org/hannesm/mirage-dev/builds/179095513

Depends on the following PRs:

and is ready to. as @mato mentioned in the previous PR, ukvm make (multiple times) looks a bit flaky, not sure what should be done there.

as mentioned by @Drup, the info stuff needs some more thoughts (now that we have packages, libraries, and version constraints). since there's now a build phase, that should be straightforward to integrate.

it might be worth for a future PR to have another subcommand, distclean, which behaves as the current clean, and clean only removes the _build (and _build-ukvm) subdirs.

@Drup

Ok, so, what's the story about user customisation here ?
The user can write a Makefile.user, and a _tags file, is that right ? I may have missed it, but I'm pretty sure it's undocumented.

Not sure why we don't also generate a _tags file. Maybe use the same trick as oasis (mirage touches only in the mirage section, leaves the rest untouched). It would solve various other annoyance too.

I would be curious to know how that would lead to an improvement for mirage-www, like having some jsoo stuff in the unikernel.

@hannesm Since the build system story is starting to get a bit clearer, maybe we should start thinking about moving some bits back into functoria. I didn't do it originally because it was quite unclear how to parametrize things, but it should be easier now.

include Functoria
let get_target i = Key.(get (Info.context i) target)
let target i = Key.(get (Info.context i) target)

This comment has been minimized.

@Drup

Drup Nov 26, 2016

Member

Please keep get_target for that one, it overlaps with the key otherwise. :)

This comment has been minimized.

@hannesm
let clean_makefile () = Bos.OS.File.delete Fpath.(v "Makefile")
let configure_myocamlbuild () = Bos.OS.File.write Fpath.(v "myocamlbuild.ml") ""

This comment has been minimized.

@Drup

Drup Nov 26, 2016

Member

Why create an empty myocamlbuild.ml ? It prevents the user from having one.

This comment has been minimized.

@hannesm

hannesm Nov 26, 2016

Member

to avoid getting ocamlbuild error that it might be not an OCaml project and called with -r (see https://github.com/ocaml/ocamlbuild/blob/756fe48e76a874e5d7e20de637024c0379355d1b/src/options.ml#L373), discussion at #537 and #691 (comment). But sure, that should be guarded.

@hannesm

This comment has been minimized.

Member

hannesm commented Nov 26, 2016

customisation:

Which code to move? since functoria doesn't have a concept of targets, and both compile and link steps are target specific, I don't know which pieces you'd want to move over. I've the feeling that we might want to phase configure out (since no user-devices should need to specify it, they'll much more likely want to use the build phase). But certainly mirage generates a lot of files during configure (which have mirage-specific stuff in there), opam, Makefile, xe, xl, xl.in, libvirt.xml, ...

@Drup

This comment has been minimized.

Member

Drup commented Nov 26, 2016

Which code to move?

The concept of makefile that you have currently it's mostly generic. make depend, opam stuff, etc. That's not dependent on the target. Anyway, as long as nobody else uses functoria, it's a bit abstract. :)

@hannesm hannesm referenced this pull request Nov 27, 2016

Closed

enforce the right minimum OCaml version #696

0 of 3 tasks complete

@hannesm hannesm added this to the mirage 3.0.0 milestone Nov 27, 2016

@mato

This comment has been minimized.

Member

mato commented Nov 28, 2016

I have a possible fix for the problem with ukvm not building the 2nd time round. Adding the following to _tags makes it work for me:

<_build-ukvm>: -traverse

Perhaps we can generate a _tags file as part of mirage configure? If we want to make it user-customizable then we can include _tags.user from it.

One nit: With the current implementation I don't see the final link command ld ... output when running make, is this intentional?

@mato

This comment has been minimized.

Member

mato commented Nov 28, 2016

Another nit -- running make multiple times seems to always rebuild everything?

@hannesm

This comment has been minimized.

Member

hannesm commented Nov 28, 2016

@mato

This comment has been minimized.

Member

mato commented Nov 28, 2016

ah, nice. I added this (unfortunately it seems to have to be in both functoria and mirage ocamlbuild invocation, is there another solution?)

Doesn't seem to have had any effect here...did you try with _tags removed? None gets generated.

did you try mirage build -v -v ?

I did not. Works, thanks. Possible improvement would be to display the commands executed in a more cut-and-paste format, would help with debugging/tweaking.

fixed in functoria (only delete config.*, not entire _build)

Confirmed fixed.

Minor nit: ukvm-configure gets invoked on every mirage build, which regenerates Makefile.ukvm, which recompiles ukvm. It might be a better idea to only invoke ukvm-configure if Makefile.ukvm is not present? However, if you reinstall ukvm then you probably want to reinvoke it but that seems an edge case...?

Also, how hard would it be for mirage build (and indeed any other commands) take the default value of -t from that passed to a previous run of mirage configure? Right now -t always defaults to unix.

@mato

This comment has been minimized.

Member

mato commented Nov 28, 2016

Clarification: If I ran mirage configure -t X, does it make any sense for mirage build -t Y to run at all? Shouln't it just error out?

@hannesm

This comment has been minimized.

Member

hannesm commented Nov 28, 2016

@mato

This comment has been minimized.

Member

mato commented Nov 28, 2016

@hannesm

This comment has been minimized.

Member

hannesm commented Nov 28, 2016

@mato

This comment has been minimized.

Member

mato commented Nov 28, 2016

@hannesm

This comment has been minimized.

Member

hannesm commented Nov 29, 2016

@mato the ocamlbuild problem needs further investigation and should be reported upstream.

what I still don't understand is why we don't encounter this issue with mirage&functoria master (or do we?).

what the actual question for me is how to work around the ocamlbuild bug (since I wouldn't want to wait for an ocamlbuild bugfix and release). the options I can see so far are:

  • emit a _tags file -- which would to be generated by functoria (since mirage configure leads to functoria calling ocamlbuild on config.ml)
  • switch for ukvm to a different build directory below _build

I prefer the second option. I don't think the complexity to set the output directory explicitly via command-line argument is needed.

@hannesm

This comment has been minimized.

Member

hannesm commented Nov 29, 2016

@mato I went ahead and opened Solo5/solo5#137 which puts build products into _build/ukvm. if this is fine for you, it'd make me happy (and this PR mergeable)

@hannesm

This comment has been minimized.

Member

hannesm commented Nov 29, 2016

(travis is happy according to https://travis-ci.org/hannesm/mirage-dev/builds/179629868 with the most recent changes here and in solo5)

@yomimono

This comment has been minimized.

Member

yomimono commented Nov 29, 2016

I want to take this for a test drive before I click any green buttons, but feel free to click for yourself if you'd like. This set of PRs is great! :D

@hannesm

This comment has been minimized.

Member

hannesm commented Nov 29, 2016

I'm waiting for @mato approval for the _build-ukvm to _build/ukvm change

@mato

This comment has been minimized.

Member

mato commented Nov 29, 2016

@hannesm I'm not 100% happy with using _build/ukvm -- thinking of other users of Solo5 and ukvm especially, _build could be used by other build systems which is why I suggested adding an option to specify the build directory.

Regarding the ocamlbuild problem, why don't you want to emit a _tags file?

@hannesm

This comment has been minimized.

Member

hannesm commented Nov 29, 2016

@mato an option to ukvm-configure is fine as well.

Emitting a _tags file would need to be done in functoria (since this is the first caller to ocamlbuild). I dislike the idea of hardcoding something special about solo5-ukvm in functoria. Users should be able to tweak the builds by writing their own _tags files if needed, and I rather dislike how e.g. oasis handles this.

I still don't know whether the behaviour in this branch is different from master (and if so, what is the difference in master?)

@mato

This comment has been minimized.

Member

mato commented Nov 29, 2016

@hannesm I think I have a working solution without the need for _tags or a change to the ukvm build directory. Invoking ocamlbuild with -X _build-ukvm makes it correctly ignore the directory and everything in it. Where do I need to add it to try this with your changes? (My OCaml is not up to the task...)

As for why it's not causing a problem on master, I had a look, no idea ...

@hannesm

This comment has been minimized.

Member

hannesm commented Nov 29, 2016

updated those

@mato

This comment has been minimized.

Member

mato commented Nov 29, 2016

Seems to work, hooray!

@mato

This comment has been minimized.

Member

mato commented Nov 29, 2016

@hannesm One more ukvm-specific bug which got lost somewhere in this change: ukvm-configure needs to be called with net and/or blk, depending on whether or not the unikernel intends to use a network/block device.

@mato

This comment has been minimized.

hannesm added some commits Nov 19, 2016

Updates for Functoria
- package type
- instead of lists of packages and libraries, use lists of packages
- Cmd and Log got replaced with Bos and Logs
- re-add build subcommand
- don't generate a complex Makefile, instead call out to ocamlbuild and pkg-config manually

@mato mato merged commit 85b6932 into mirage:master Nov 29, 2016

0 of 6 checks passed

ci/datakit/All Distros depfail
Details
ci/datakit/Alpine Build exit 4
Details
ci/datakit/Common Distros depfail
Details
ci/datakit/Compilers depfail
Details
ci/datakit/Revdeps depfail
Details
continuous-integration/travis-ci/pr The Travis CI build failed
Details
@mato

This comment has been minimized.

Member

mato commented Nov 29, 2016

(acked by @yomimono, merging this and dependents)

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