ocaml / dune

A composable build system for OCaml.
https://dune.build/
MIT License
1.62k stars 401 forks source link

Dune 3.11 introduces regression on macOS with regarding to executable promotion #9272

Closed haochenx closed 6 months ago

haochenx commented 10 months ago

Expected Behavior

Promoted executable runs fine.

Actual Behavior

Promoted executable getting killed by OS with signal 9. Directly executing executable in _build folder or executing a manually copied version of the executable behave as expected.

Reproduction

See the minimal reproducing project at https://github.com/kxcinc/sample.bc47-dune-reproduce

Specifications

Additional information

output of `dune build --verbose`: ``` Shared cache: disabled Workspace root: /Users/hx/git/sample.bc47-dune-reproduce Auto-detected concurrency: 8 Dune context: { name = "default" ; kind = "default" ; profile = Dev ; merlin = true ; for_host = None ; fdo_target_exe = None ; build_dir = In_build_dir "default" ; ocaml_bin = External "/Users/hx/git/sample.bc47-dune-reproduce/_opam/bin" ; ocaml = Ok External "/Users/hx/git/sample.bc47-dune-reproduce/_opam/bin/ocaml" ; ocamlc = External "/Users/hx/git/sample.bc47-dune-reproduce/_opam/bin/ocamlc.opt" ; ocamlopt = Ok External "/Users/hx/git/sample.bc47-dune-reproduce/_opam/bin/ocamlopt.opt" ; ocamldep = Ok External "/Users/hx/git/sample.bc47-dune-reproduce/_opam/bin/ocamldep.opt" ; ocamlmklib = Ok External "/Users/hx/git/sample.bc47-dune-reproduce/_opam/bin/ocamlmklib" ; installed_env = map { "INSIDE_DUNE" : "/Users/hx/git/sample.bc47-dune-reproduce/_build/default" ; "OCAML_COLOR" : "always" ; "OPAMCOLOR" : "always" } ; findlib_paths = [ External "/Users/hx/git/sample.bc47-dune-reproduce/_opam/lib/ocaml" ; External "/Users/hx/git/sample.bc47-dune-reproduce/_opam/lib" ] ; ocaml_config = { version = "5.1.0" ; standard_library_default = "/Users/hx/git/sample.bc47-dune-reproduce/_opam/lib/ocaml" ; standard_library = "/Users/hx/git/sample.bc47-dune-reproduce/_opam/lib/ocaml" ; standard_runtime = "the_standard_runtime_variable_was_deleted" ; ccomp_type = "cc" ; c_compiler = "cc" ; ocamlc_cflags = [ "-O2"; "-fno-strict-aliasing"; "-fwrapv"; "-pthread" ] ; ocamlc_cppflags = [ "-D_FILE_OFFSET_BITS=64" ] ; ocamlopt_cflags = [ "-O2"; "-fno-strict-aliasing"; "-fwrapv"; "-pthread" ] ; ocamlopt_cppflags = [ "-D_FILE_OFFSET_BITS=64" ] ; bytecomp_c_compiler = [ "cc" ; "-O2" ; "-fno-strict-aliasing" ; "-fwrapv" ; "-pthread" ; "-D_FILE_OFFSET_BITS=64" ] ; bytecomp_c_libraries = [ "-L/opt/homebrew/Cellar/zstd/1.5.5/lib"; "-lzstd"; "-lpthread" ] ; native_c_compiler = [ "cc" ; "-O2" ; "-fno-strict-aliasing" ; "-fwrapv" ; "-pthread" ; "-D_FILE_OFFSET_BITS=64" ] ; native_c_libraries = [ "-L/opt/homebrew/Cellar/zstd/1.5.5/lib"; "-lzstd"; "-lpthread" ] ; native_pack_linker = [ "ld"; "-r"; "-o" ] ; cc_profile = [] ; architecture = "arm64" ; model = "default" ; int_size = 63 ; word_size = 64 ; system = "macosx" ; asm = [ "cc"; "-c"; "-Wno-trigraphs" ] ; asm_cfi_supported = true ; with_frame_pointers = false ; ext_exe = "" ; ext_obj = ".o" ; ext_asm = ".s" ; ext_lib = ".a" ; ext_dll = ".so" ; os_type = "Unix" ; default_executable_name = "a.out" ; systhread_supported = true ; host = "aarch64-apple-darwin23.1.0" ; target = "aarch64-apple-darwin23.1.0" ; profiling = false ; flambda = false ; spacetime = false ; safe_string = true ; exec_magic_number = "Caml1999X033" ; cmi_magic_number = "Caml1999I033" ; cmo_magic_number = "Caml1999O033" ; cma_magic_number = "Caml1999A033" ; cmx_magic_number = "Caml1999Y033" ; cmxa_magic_number = "Caml1999Z033" ; ast_impl_magic_number = "Caml1999M033" ; ast_intf_magic_number = "Caml1999N033" ; cmxs_magic_number = "Caml1999D033" ; cmt_magic_number = "Caml1999T033" ; natdynlink_supported = true ; supports_shared_libraries = true ; windows_unicode = false } ; instrument_with = [] } Actual targets: - alias @@default Running[1]: (cd _build/default && /Users/hx/git/sample.bc47-dune-reproduce/_opam/bin/ocamlopt.opt -I .hello.eobjs/byte -I .hello.eobjs/native -I /Users/hx/git/sample.bc47-dune-reproduce/_opam/lib/dune-site -intf-suffix .ml-gen -no-alias-deps -opaque -o .hello.eobjs/native/dune_site__Dune_site_data.cmx -c -impl .hello.eobjs/dune_site__Dune_site_data.ml-gen) Running[2]: (cd _build/default && /Users/hx/git/sample.bc47-dune-reproduce/_opam/bin/ocamlc.opt -w @1..3@5..28@30..39@43@46..47@49..57@61..62-40 -strict-sequence -strict-formats -short-paths -keep-locs -g -bin-annot -I .hello.eobjs/byte -I /Users/hx/git/sample.bc47-dune-reproduce/_opam/lib/camlp-streams -I /Users/hx/git/sample.bc47-dune-reproduce/_opam/lib/camomile -I /Users/hx/git/sample.bc47-dune-reproduce/_opam/lib/camomile/lib -I /Users/hx/git/sample.bc47-dune-reproduce/_opam/lib/dune-private-libs/dune-section -I /Users/hx/git/sample.bc47-dune-reproduce/_opam/lib/dune-site -I /Users/hx/git/sample.bc47-dune-reproduce/_opam/lib/dune-site/private -no-alias-deps -opaque -o .hello.eobjs/byte/dune__exe__Hello.cmo -c -impl hello.ml) Running[3]: (cd _build/default && /Users/hx/git/sample.bc47-dune-reproduce/_opam/bin/ocamlopt.opt -w @1..3@5..28@30..39@43@46..47@49..57@61..62-40 -strict-sequence -strict-formats -short-paths -keep-locs -g -I .hello.eobjs/byte -I .hello.eobjs/native -I /Users/hx/git/sample.bc47-dune-reproduce/_opam/lib/camlp-streams -I /Users/hx/git/sample.bc47-dune-reproduce/_opam/lib/camomile -I /Users/hx/git/sample.bc47-dune-reproduce/_opam/lib/camomile/lib -I /Users/hx/git/sample.bc47-dune-reproduce/_opam/lib/dune-private-libs/dune-section -I /Users/hx/git/sample.bc47-dune-reproduce/_opam/lib/dune-site -I /Users/hx/git/sample.bc47-dune-reproduce/_opam/lib/dune-site/private -intf-suffix .ml -no-alias-deps -opaque -o .hello.eobjs/native/dune__exe__Hello.cmx -c -impl hello.ml) Running[4]: (cd _build/default && /Users/hx/git/sample.bc47-dune-reproduce/_opam/bin/ocamlopt.opt -w @1..3@5..28@30..39@43@46..47@49..57@61..62-40 -strict-sequence -strict-formats -short-paths -keep-locs -g -o hello.exe -linkall /Users/hx/git/sample.bc47-dune-reproduce/_opam/lib/dune-private-libs/dune-section/dune_section.cmxa /Users/hx/git/sample.bc47-dune-reproduce/_opam/lib/dune-site/private/dune_site_private.cmxa .hello.eobjs/native/dune_site__Dune_site_data.cmx /Users/hx/git/sample.bc47-dune-reproduce/_opam/lib/dune-site/dune_site.cmxa /Users/hx/git/sample.bc47-dune-reproduce/_opam/lib/camlp-streams/camlp_streams.cmxa /Users/hx/git/sample.bc47-dune-reproduce/_opam/lib/camomile/lib/camomileLib.cmxa /Users/hx/git/sample.bc47-dune-reproduce/_opam/lib/camomile/camomile.cmxa .hello.eobjs/native/dune__exe__Hello.cmx) Promoting "_build/default/hello.exe" to "hello.exe" ```
emillon commented 10 months ago

@haochenx Thanks for the bug report. The fact that it's coming from camomile (presumably 2.0.0) which is the exact version fixed in #8361 points again at that PR.

@anmonteiro The opam instructions for camomile.2.0.0 have an explicit dune install step that's different from the classic dune subst / dune build -p @install one (this is because of the sites extension). In #8361, were you using buildDunePackage, or something that interprets the opam build instructions? If that's the case I think that we should try using dune install in the nix instructions instead of the fix that's merged (and presumably does not work for non-nix users).

haochenx commented 10 months ago

@emillon thanks for the triage. I hope it can be reproduced on your side. I'm happy to help running additional diagnosis if that helps.

(btw I just spot an incorrect commenting-out of the reproduce.sh in the minimal reproduction example project and fixed it (https://github.com/kxcinc/sample.bc47-dune-reproduce/commit/0550f9dcee14b5e9768d6ba311cb97177b792bc5). I hope the broken version didn't hurt your setup too much.. 🙏 )

emillon commented 10 months ago

I don't have an arm mac available at the moment, so yes I would be interested in some testing. Can you try the following versions and tell me if the issue is present:

Thanks!

haochenx commented 10 months ago

I don't have an arm mac available at the moment, so yes I would be interested in some testing. Can you try the following versions and tell me if the issue is present:

Sure! Here's the result:

emillon commented 10 months ago

Thanks. My impression for now is that the correct fix is revert-8361 and that the fix in #8361 was too specific to nix, and maybe to an issue caused by wrong build instructions. Let's wait for @anmonteiro's opinion.

emillon commented 10 months ago

Specifically, camomile.2.0.0 installs a Sites.ml which contains some placeholders, so I suppose they need to be substituted even if the file is not executable.

haochenx commented 10 months ago

Thanks!

My impression for now is that the correct fix is revert-8361

Do you think we should mark Dune 3.11.x as incompatible with macOS on opam-repository? Or if we don't want to do that we can probably just add a post-message to warn the user (I don't think it's ideal though) @emillon

emillon commented 10 months ago

Ah I didn't realize it was already broken in 3.11. This means it shouldn't block 3.12. It needs to be fixed, though, but this also means that we can fix it in a point release.

Do you think we should mark Dune 3.11.x as incompatible with macOS on opam-repository?

I'll try to write a regression test to see exactly what is wrong. I think that it would probably be dune-site being unavailable in macos arm, but I want to make sure that it's always a problem. If this only happens if there's in addition a promote rule, marking it as unavailable would prevent legitimate uses.

haochenx commented 10 months ago

We discussed this during the opam-repo maintainer's meeting a bit. The conclusion was that there's probably very little we can do to precisely disable the installation of affected versions for only the users that may experience this problem (no surprise), and it only affects a tiny percentage of users, so we don't want to mark 3.11.x unavailable on macOS for all the other users.

I will do some testing about making dune-site.3.11.x unavailable on macOS will help alleviate this issue without affecting the majority of Dune users.

emillon commented 6 months ago

I now have a M1 mac and I was able to reproduce the issue from the linked repo.

emillon commented 6 months ago

and I confirm that I bisect the issue to #8361

haochenx commented 6 months ago

Thanks for tackling this! Please know if there's anything you'd like me to help.