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

Use virtual_modules in Dune 1.7 #59

Closed
wants to merge 3 commits into from
Closed

Use virtual_modules in Dune 1.7 #59

wants to merge 3 commits into from

Conversation

avsm
Copy link
Member

@avsm avsm commented Jun 26, 2019

This allows use of the interface file separately from the implementations. We now provide separate implementations that use dune variants so that they can be found via a variants tag search:

  • unix (Unix or Windows)
  • js (JavaScript)
  • xen (Xen)

This cleans up the utop toplevel support as well to work by default now. (@avsm, fixes #50)
Also use implicit transitive dependencies, which will become the default in dune 2.0

I am noticing some local problems with opam installations though:

#=== ERROR while compiling io-page-unix.2.2.0 =================================#
# context     2.0.4 | macos/x86_64 | ocaml-base-compiler.4.07.1 | pinned(git+file:///Users/avsm/src/git/mirage/io-page#vmodules#a58f53d0)
# path        ~/.opam/ocaml-base-compiler.4.07.1/.opam-switch/build/io-page-unix.2.2.0
# command     ~/.opam/opam-init/hooks/sandbox.sh build dune build -p io-page-unix -j 27
# exit-code   1
# env-file    ~/.opam/log/io-page-unix-37329-03b1a4.env
# output-file ~/.opam/log/io-page-unix-37329-03b1a4.out
### output ###
# File "lib/unix/dune", line 7, characters 14-21:
# 7 |   (implements io-page)
#                   ^^^^^^^
# Error: Library implementation io-page for variant "unix" implements a library outside the project. This is forbidden.

which is why I am marking this as a draft PR to work through the CI first.

This allows use of the interface file separately from the
implementations.

We now provide separate implementations that use dune variants
so that they can be found via a variants tag search:

- `unix` (Unix or Windows)
- `js` (JavaScript)
- `xen` (Xen)

This cleans up the utop toplevel support as well to work by
default now. (@avsm, fixes mirage#50)

Use implicit transitive dependencies, which will become the
default in dune 2.0
- `xen` (Xen)

This cleans up the utop toplevel support as well to work by
default now. (@avsm, fixes #50)
Copy link
Member

Choose a reason for hiding this comment

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

awesome!

@dinosaure
Copy link
Member

Should related to mirage/irmin#338

@hannesm
Copy link
Member

hannesm commented Jun 27, 2019

I still don't fully understand variants, but in the above list, what is happening with freestanding? or is this a non-issue? AFAIR the C stubs are provided by mirage-solo5 atm, but what does dune do when asked to use a variant that does not exist for a certain package? take the "default" one (is there a default?)?

and what is the extensibility story - do all variants now need to end up in this io-page repository, or are they free to be defined elsewhere? (how will people/programs know about the existance elsewhere, what happens if there are multiple implementations of a single package for the same variant?)

thanks for working on this! and sorry for asking questions, but I could not discover where/whether they've been answered previously (please point me to a comment if this is the case)

@samoht
Copy link
Member

samoht commented Aug 3, 2019

Do we really need variants when we could just rely on dune doing the correct cross-compilation of C bindings? e.g. in #60 I've simply removed the unix and xen subpackages and everything seems much cleaner/simpler :-)

@samoht samoht closed this Aug 6, 2019
@samoht
Copy link
Member

samoht commented Aug 6, 2019

Closing in favour of #60

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.

Doesn't work from utop : function `mirage_alloc_pages' is not available
5 participants