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

compilation fixes #4

Merged
merged 15 commits into from
Feb 12, 2020
Merged

compilation fixes #4

merged 15 commits into from
Feb 12, 2020

Conversation

hannesm
Copy link
Member

@hannesm hannesm commented Feb 10, 2020

best viewed commit by commit. this is supposed to achieve the following (by cherry-picking and polishing commits from a multitude of forks):

  • use dune instead of ocamlbuild/topkg
  • revert the "remove sexplib serialisers"
  • use dune-configurator and bigarray-compat
  • do not use -Wno-implicit-fallthrough, since this is a reasonable warning. instead, mark the case statements explicitly where a fallthrough is expected (use a C comment instead of annotation to make gcc<7 and clang not complain)
  • fix "unused C function" warnings
  • cross-compilation for freestanding and xen (by flattening the src/native directory and copying around)
  • be more precise why warning -3 is in nocrypto_lwt_entropy

the freestanding and xen support is not yet properly tested (and may need some more work) works fine (a static_website_tls unikernel links, and the freestanding/hvt one also runs smoothly), I'll follow up on that. once this is good to go, the renaming could happen.

dune-project Outdated Show resolved Hide resolved
| "false" -> []
| _ -> auto
| exception Not_found -> auto in
Format.(printf "(@[%a@])%!" (fun ppf -> List.iter (fprintf ppf "%s@ ")) fs)
Copy link
Collaborator

Choose a reason for hiding this comment

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

You can use Configurator.V1.Flags.write_sexp here.

Copy link
Member Author

Choose a reason for hiding this comment

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

ok, I do not know how to do this. I am unable to find the documentation / API of Configurator.V1.Flags. this cfg.exe is used by src/dune (with a rule (rule (with-stdout-to c_flags.sexp (run ../config/cfg.exe)))) as well as xen/dune and freestanding/dune.

From https://dune.readthedocs.io/en/stable/quick-start.html#defining-a-library-with-c-stubs I gather that your proposed write_sexp takes two arguments, a file name and an OCaml value - but it is rather unclear to me where the output is written to, then (in terms of relative path).

.gitignore Outdated Show resolved Hide resolved
.ocamlinit Outdated Show resolved Hide resolved
.travis.yml Outdated Show resolved Hide resolved
lwt/nocrypto_entropy_lwt.ml Outdated Show resolved Hide resolved
nocrypto.opam Outdated Show resolved Hide resolved
TheLortex and others added 5 commits February 12, 2020 12:45
Adapted by @hannesm to use the comment /* fall through */ instead of the
__attribute__((fallthrough)); to silence clang and gcc<7 warnings
"declaration does not declare anything [-Wmissing-declarations]"
this prepares support for xen and freestanding cross-compilation
@hannesm
Copy link
Member Author

hannesm commented Feb 12, 2020

thanks for your reviews.

I force-pushed to add a lower bound of OCaml 4.07.0 -- and then removed the bigarray-compat commit (since this is only needed < 4.07.0 -- in 4.07.0+ we have Stdlib.Bigarray)

@hannesm
Copy link
Member Author

hannesm commented Feb 12, 2020

hmm, now when I run dune utop, I get:

File "xen/dune", line 7, characters 12-28:
7 |  (libraries mirage-xen-posix)
                ^^^^^^^^^^^^^^^^
Error: Library "mirage-xen-posix" not found.
Hint: try: dune external-lib-deps --missing .utop/utop.exe

but in xen/dune the stanza optional is included -- any dune hints how to get dune utop to work in such a scenario (dune 2.1.3 is what I use)?

another question about CI: is it possible that it runs with optional dependencies? (in case the answer is no (what I expect), I guess an appropriate solution is to have separate packages (though I'm not sure how this would work with the META.in, but I can try to figure that out).

@hannesm hannesm mentioned this pull request Feb 12, 2020
9 tasks
@talex5
Copy link
Contributor

talex5 commented Feb 12, 2020

another question about CI: is it possible that it runs with optional dependencies? (in case the answer is no (what I expect), I guess an appropriate solution is to have separate packages (though I'm not sure how this would work with the META.in, but I can try to figure that out).

Yeah, separate packages would be great!

If you need to force optional deps to be installed in CI as a quick fix though you could always add a dummy opam package that depends on all the optional stuff. Or you could make the optional dependencies required for tests.

@hannesm
Copy link
Member Author

hannesm commented Feb 12, 2020

@talex5 thanks for your quick and valuable response. Since I'm not eager to accumulate hacks in this repository, but will subsequently revise the package layout (not in this PR).

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.

None yet

8 participants