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

Switch to a single package (step 1) #60

Merged
merged 5 commits into from
Aug 6, 2019
Merged

Conversation

TheLortex
Copy link
Member

In the mirage+dune series of PR (mirage/mirage#969).

Workspace flags

Use :standard to get the flags from the build workspace. This means xen/unix flags are automatically set up and we don't need to know about backends anymore.

Compatibility stubs

We keep io-page-xen and io-page-unix in order not to have to modify every revdep.

lib/dune Outdated Show resolved Hide resolved
@TheLortex
Copy link
Member Author

@samoht rebased according to your review !

lib/dune Outdated Show resolved Hide resolved
@samoht
Copy link
Member

samoht commented Jul 31, 2019

Just to fully understand: the only thing that this PR is doing is moving the compilation of C stubs into the main package, so that's dune could cross-compile everything more easily. But this doesn't change anything for users of the library, right?

@TheLortex
Copy link
Member Author

Yes, instead of having io-page to know about the different backends (xen, unix), the task is delegated to dune which will automatically compile the C stubs with the correct flags.

@samoht
Copy link
Member

samoht commented Jul 31, 2019

But then, I am not sure to understand why we need to keep io-page-unix and io-page-xen : they will not work properly if people didn't configure their workspace to do cross-compilation.

I think it's just better to remove these and add bounds to the rev-dependencies. I'm fine doing this myself.

@TheLortex
Copy link
Member Author

TheLortex commented Jul 31, 2019

It's just because revdeps still depend on io-page-xen|unix and I haven't taken the time to look at them. For xen these are: mirage-block-xen, mirage-console-xen, mirage-net-xen, mirage-profile-xen, mirage-tcpip-xen, mirage-xen, netchannel. For unix there is even more..

@samoht
Copy link
Member

samoht commented Jul 31, 2019

I can help fixing the revdeps. I think it's better to just remove these packages if they are not useful (and if they won't be able to link with existing unikernels using ocamlbuild anyway as they will not be cross-compiled correctly).

@samoht
Copy link
Member

samoht commented Aug 1, 2019

See ocaml/opam-repository#14621 for the revdeps fixes

the cross-compilation will be done directly by dune
@samoht
Copy link
Member

samoht commented Aug 3, 2019

Now that ocaml/opam-repository#14621 is merged, I've pushed to the PR to remove io-page-unix and io-page-xen completely.

@samoht samoht merged commit 062e5f5 into mirage:master Aug 6, 2019
@avsm
Copy link
Member

avsm commented Aug 8, 2019

This is fine, but let's please document clearly in the CHANGES file that the reason it works is that existing ocamlbuild setups will pickup io-page-xen/unix, whereas mirage4 won't need them at all. Otherwise it's not clear what happens to old packages.

@hannesm
Copy link
Member

hannesm commented Aug 13, 2019

existing ocamlbuild setups will pickup io-page-xen/unix

I'd suggest to add a conflict in the opam file of the to-be-released io-page of the form:
conflicts: [ "mirage-runtime" {< "4.0.0"} ] (I guess mirage-runtime is the most sensible package (since it is depended on by every unikernel), but of course mirage-types or mirage-types-lwt could be used as well).

dinosaure added a commit to dinosaure/opam-repository that referenced this pull request Mar 22, 2021
CHANGES:

* Use workspace flags (@TheLortex, mirage/io-page#60)
* Use ocamlformat (@TheLortex, mirage/io-page#60)
* **breaking changes** Remove io-page-xen and io-page-unix split (@samoht, mirage/io-page#60)
* Update CI scripts (@samoht, mirage/io-page#60)
* **breaking changes** Update the C layout of io-page according MirageOS 3/4 (@dinosaure, mirage/io-page#62)
  JS/C functions are renamed from `mirage_*` to `caml_mirage_iopage_*`
* Add ocaml-freestanding and pkg-config as dependencies of `io-page`
  To be able to 'cross'-compile to Solo5
dinosaure added a commit to dinosaure/opam-repository that referenced this pull request Mar 22, 2021
CHANGES:

* Use workspace flags (@TheLortex, mirage/io-page#60)
* Use ocamlformat (@TheLortex, mirage/io-page#60)
* **breaking changes** Remove io-page-xen and io-page-unix split (@samoht, mirage/io-page#60)
* Update CI scripts (@samoht, mirage/io-page#60)
* **breaking changes** Update the C layout of io-page according MirageOS 3/4 (@dinosaure, mirage/io-page#62)
  JS/C functions are renamed from `mirage_*` to `caml_mirage_iopage_*`
* Add ocaml-freestanding and pkg-config as dependencies of `io-page`
  To be able to 'cross'-compile to Solo5
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.

4 participants