Switch to the Dune build system and ppxlib #40
Conversation
MisterDA
force-pushed
the
dune
branch
5 times, most recently
from
f94266f to
6767420
Compare
MisterDA
force-pushed
the
dune
branch
2 times, most recently
from
740506c to
13d1ebc
Compare
|
Hello @ygrek, |
MisterDA
changed the title
|
(I didn't forget about this) |
|
Yay! I think this is it. I also have some patches to make more functions work on Windows, but I intend to rebase them on this branch once it's merged. |
| @@ -5,6 +5,19 @@ environment: | |||
| FORK_USER: ocaml | |||
| FORK_BRANCH: master | |||
| CYG_ROOT: C:\cygwin64 | |||
There was a problem hiding this comment.
| CYG_ROOT: C:\cygwin64 | |
| CYG_ROOT: C:\cygwin64 | |
| OPAM_VERSION: 2.0.8 |
Could you try this? it might help the doubling of output in opam
There was a problem hiding this comment.
I can reproduce in local with Opam 2.0.8, so this won't change it.
There was a problem hiding this comment.
ah, I see. That sounds like an opam-on-windows™ bug indeed then :/ Have you tried with opam 2.1.0~beta4?
- remove all Oasis configuration and generated files; - add Dune build files; - move discover and ppx_have to their separate directories; - rename extUnix.mlpp to extUnix.pp.ml; - generate the opam file using Dune; - use dune-release for release process; - update README documentation.
Ppxlib allows inserting a (possibly empty) list of structure_items, so this has the nice side-effect of removing the `include module ___ end` and have all the functions in the top-level of the generated modules.
|
ping @ygrek if you have some time to review this |
| #define UNUSED(x) (void)(x) | ||
|
|
||
| #include <caml/version.h> | ||
| #if OCAML_VERSION < 41200 |
There was a problem hiding this comment.
maybe better check for symbol itself
#ifndef Some_val
There was a problem hiding this comment.
Sure, the end-result is the same, but we know these macros were introduced in 4.12.
I applied the same patch for the same issue here: ocaml-opam/ocaml-mccs#30.
| @@ -16,7 +16,7 @@ array_of_value(value v) | |||
| size = Wosize_val(v); | |||
| arr = caml_stat_alloc((size + 1) * sizeof(char *)); | |||
| for (i = 0; i < size; i++) | |||
| arr[i] = String_val(Field(v, i)); | |||
| arr[i] = caml_stat_strdup(String_val(Field(v, i))); | |||
There was a problem hiding this comment.
unrelated change
original code looks correct, why?
There was a problem hiding this comment.
Because of this warning: a const string pointer is written into the non-const array.
dune build @install
gcc src/fexecve.o
fexecve.c: In function ‘array_of_value’:
fexecve.c:19:12: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
arr[i] = String_val(Field(v, i));
^
However you're right, the copy isn't necessary and I believe a cast removing the const is enough to remove the warning, as the string isn't written into.
|
@ygrek is there a release planned by any chance? Having a release with this PR in would help us immensely switch away from OMP1 and/or camlp4 (given some projects use OMP2 + extunix and only extunix < 0.2.0 is available with this configuration) |
|
@ygrek would you have time to cut a release with this PR included? Not having this is causing problems with the ocaml/opam base image builders (as they need newer ppxlib and extlib). Thanks! |
Hi!
This PR switches extunix to the Dune build system, and updates the code onto OCaml 4.11.
Dune prefers libraries and executable to live in different directories, so I had to move ppx_have and discover out of
src. There were a few updates needed to account for the changes to the language and the standard library. The switch to odoc also requires some changes to the documentation, and to move the default AST in ppx_have to a recent OCaml.Prefixing module names with the name of the library is also deprecated, so ExtUnixAll, ExtUnixSpecific, and ExtUnixConfig only exist as ExtUnix.All, ExtUnix.Specific, and ExtUnix.Config now.
Dune now generate extunix.opam automatically, and the release process is done with dune-release which reads CHANGES.txt, tags the git repo, creates then publishes a tarball.