-
Notifications
You must be signed in to change notification settings - Fork 103
Upstream OxCaml #1399
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
base: master
Are you sure you want to change the base?
Upstream OxCaml #1399
Conversation
Signed-off-by: Arthur Wendling <arthur@tarides.com>
| (chdir | ||
| %{workspace_root} | ||
| (run %{bin:cppo} -V OCAML:%{ocaml_version} -D "OXCAML" %{x} -o %{targets})))) | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The rules duplication, here and in src/loader/dune, are rather inconvenient but I didn't find how to avoid it: cppo only accepts variables with integer values in conditionals, but dune %{ocaml-config:ox} is a boolean with no way to cast it to an int (?). I tried cppo -D "OXCAML %{ocaml-config:ox} but then the variable can't be used in #if. Please let me know if you know a better way!
80bf1f1 to
bb20ded
Compare
(builds on top of #1398 which provides
ocaml-config:oxto detect OxCaml)This PR is a rebase of @lukemaurer 's patches from oxcaml/odoc (+ small fixes from #22). I hope this will make it easier to keep everything uptodate, and that it isn't too much of a stretch as Odoc already supports multiple versions of the OCaml AST using
cppo. (Note that the preprocessor was originally usingOCAML_VERSION=(5,2,0)as a heuristic to check for OxCaml, but since dune 3.21 we can be exact.)