Skip to content

Fix build on 5.3.0+trunk#9733

Merged
emillon merged 2 commits into
ocaml:mainfrom
jmid:fix-build-on-trunk
Jan 15, 2024
Merged

Fix build on 5.3.0+trunk#9733
emillon merged 2 commits into
ocaml:mainfrom
jmid:fix-build-on-trunk

Conversation

@jmid

@jmid jmid commented Jan 15, 2024

Copy link
Copy Markdown
Member

After a recent change to the internal compiler headers, dune no longer compiles on trunk (named 5.3.0) on macOS.
I spotted it since it is breaking out multicoretests CI run:
https://github.com/ocaml-multicore/multicoretests/actions/runs/7502879005/job/20426416884?pr=431#step:5:128

  #=== ERROR while compiling dune.3.12.2 ========================================#
  # context     2.1.5 | macos/x86_64 | ocaml-variants.5.3.0+trunk | git+https://github.com/ocaml/opam-repository.git
  # path        ~/work/multicoretests/multicoretests/_opam/.opam-switch/build/dune.3.12.2
  # command     ~/.opam/opam-init/hooks/sandbox.sh build ocaml boot/bootstrap.ml -j 4
  # exit-code   2
  # env-file    ~/.opam/log/dune-5096-92e0aa.env
  # output-file ~/.opam/log/dune-5096-92e0aa.out
  ### output ###
  # ocamlc -output-complete-exe -w -24 -g -o .duneboot.exe -I boot -I +unix unix.cma boot/libs.ml boot/duneboot.ml
  # ./.duneboot.exe -j 4
  # cd _boot && /Users/runner/work/multicoretests/multicoretests/_opam/bin/ocamlopt.opt -c -g -I +unix -I +threads dune_digest_stubs.c
  # In file included from src/dune_digest/dune_digest_stubs.c:10:
  # In file included from /Users/runner/work/multicoretests/multicoretests/_opam/lib/ocaml/caml/md5.h:24:
  # In file included from /Users/runner/work/multicoretests/multicoretests/_opam/lib/ocaml/caml/io.h:26:
  # /Users/runner/work/multicoretests/multicoretests/_opam/lib/ocaml/caml/platform.h:79:17: error: implicit declaration of function 'atomic_load_acquire' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
  #     uintnat v = atomic_load_acquire(p);
  #                 ^
  # 1 error generated.
  # 

The fix is to #define CAML_INTERNALS before loading any ocaml headers as documented in the OCaml manual:
https://v2.ocaml.org/manual/intfc.html#ss:c-internals

"Since OCaml 4.04, it is possible to get access to every part of the internal runtime API by defining the CAML_INTERNALS macro before loading caml header files."

CC to @dra27 who co-authored the PR

Signed-off-by: Jan Midtgaard <mail@janmidtgaard.dk>
@jmid jmid force-pushed the fix-build-on-trunk branch from 0a50815 to f5c1373 Compare January 15, 2024 11:20
Comment thread src/dune_digest/dune_digest_stubs.c Outdated
@@ -1,3 +1,4 @@
#define CAML_INTERNALS

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Might be worth commenting here that it's needed to access the functions in md5.h?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Fair enough. I added a comment in 818a6c3

Signed-off-by: Jan Midtgaard <mail@janmidtgaard.dk>
@jmid jmid force-pushed the fix-build-on-trunk branch from 97a1cfa to 818a6c3 Compare January 15, 2024 11:37

@emillon emillon left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks. #undef'ing CAML_INTERNALS does not work, anyway.

@emillon

emillon commented Jan 15, 2024

Copy link
Copy Markdown
Collaborator

In terms of metadata, that probably means we'll have some versions to restrict in opam-repository but they don't claim compat with 5.3 anyway.

@emillon

emillon commented Jan 15, 2024

Copy link
Copy Markdown
Collaborator

Thanks!

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.

3 participants