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

Incompatibility between enabled_if and foreign_stubs? #10675

Closed
shym opened this issue Jun 27, 2024 · 5 comments · Fixed by #10677
Closed

Incompatibility between enabled_if and foreign_stubs? #10675

shym opened this issue Jun 27, 2024 · 5 comments · Fixed by #10677

Comments

@shym
Copy link
Contributor

shym commented Jun 27, 2024

Using dune version 3.16.0, I’ve tried to define 2 ways to build an executable depending on the context, once adding a foreign stub (to override the default main). This makes dune crash.
Only the dune file contains something really relevant I think, but here is the full setup:

$ cat > dune << EOF
(executable
 (enabled_if
  (<> %{context_name} other))
 (name main))

(executable
 (enabled_if
  (= %{context_name} other))
 (name main)
 (foreign_stubs
  (language c)
  (names startup)))
EOF

$ cat > dune-project << EOF
(lang dune 3.0)
EOF

$ cat > dune-workspace << EOF
(lang dune 2.0)

(context (default))

(context
 (default
  (name other)))
EOF

$ cat > main.ml << EOF
let () = Printf.printf "Hello from OCaml!\n%!"
EOF

$ cat > startup.c << EOF
#include <stdio.h>
#include <caml/callback.h>

int main(int argc, char **argv) {
    printf("Hello from C!\n");
    caml_startup(argv);
    return 0;
}
EOF

With that setup, dune build crashes with:

Internal error, please report upstream including the contents of _build/log.
Description:
  ("Map.of_list_map_exn", { key = "main" })
Raised at Stdune__Code_error.raise in file
  "otherlibs/stdune/src/code_error.ml", line 10, characters 30-62
Called from Dune_rules__Foreign_sources.make in file
  "src/dune_rules/foreign_sources.ml", lines 245-247, characters 4-19
Called from Fiber__Core.O.(>>|).(fun) in file "vendor/fiber/src/core.ml",
  line 253, characters 36-41
Called from Fiber__Scheduler.exec in file "vendor/fiber/src/scheduler.ml",
  line 76, characters 8-11
-> required by ("<unnamed>", ())
-> required by ("<unnamed>", ())
-> required by ("load-dir", In_build_dir "other")
-> required by
   ("build-alias", { dir = In_build_dir "other"; name = "default" })
-> required by ("toplevel", ())

I must not crash...

with the following build log (redacted with pseudo-variables for legibility):

# dune build
# OCAMLPARAM: unset
# Shared cache: disabled
# Shared cache location: $HOME/.cache/dune/db
# Workspace root: $PWD
# Auto-detected concurrency: 16
# Dune context:
#  { name = "other"
#  ; kind = "default"
#  ; profile = Dev
#  ; merlin = false
#  ; fdo_target_exe = None
#  ; build_dir = In_build_dir "other"
#  ; instrument_with = []
#  }
# Dune context:
#  { name = "default"
#  ; kind = "default"
#  ; profile = Dev
#  ; merlin = true
#  ; fdo_target_exe = None
#  ; build_dir = In_build_dir "default"
#  ; instrument_with = []
#  }
$ $OPAM_SWITCH_PREFIX/bin/ocamlc.opt -config > /tmp/dune_f671fe_output
$ $OPAM_SWITCH_PREFIX/bin/ocamlc.opt -config > /tmp/dune_4cd267_output
$ (cd _build/default && $OPAM_SWITCH_PREFIX/bin/ocamlc.opt -w @[email protected]@30..39@[email protected]@[email protected] -strict-sequence -strict-formats -short-paths -keep-locs -g -bin-annot -bin-annot-occurrences -I .main.eobjs/byte -no-alias-deps -opaque -o .main.eobjs/byte/dune__exe__Main.cmi -c -intf main.mli)
$ (cd _build/default && $OPAM_SWITCH_PREFIX/bin/ocamlopt.opt -w @[email protected]@30..39@[email protected]@[email protected] -strict-sequence -strict-formats -short-paths -keep-locs -g -I .main.eobjs/byte -I .main.eobjs/native -intf-suffix .ml -no-alias-deps -opaque -o .main.eobjs/native/dune__exe__Main.cmx -c -impl main.ml)
$ (cd _build/default && $OPAM_SWITCH_PREFIX/bin/ocamlopt.opt -w @[email protected]@30..39@[email protected]@[email protected] -strict-sequence -strict-formats -short-paths -keep-locs -g -o main.exe .main.eobjs/native/dune__exe__Main.cmx)

FWIW, if I define the foreign_stubs in both executable stanzas, namely:

(executable
 (enabled_if
  (<> %{context_name} other))
 (name main)
 (foreign_stubs
  (language c)
  (names startup)))

(executable
 (enabled_if
  (= %{context_name} other))
 (name main)
 (foreign_stubs
  (language c)
  (names startup)))

I get:

File "dune", line 7, characters 9-16:
7 |   (names startup)))
             ^^^^^^^
Error: Multiple definitions for the same object file "startup". See another
definition at dune:15.
Hint: You can avoid the name clash by renaming one of the objects, or by
placing it into a different directory.
@emillon
Copy link
Collaborator

emillon commented Jun 27, 2024

Thanks for your report.
There are a couple things here.

  • first, dune shouldn't crash in that way, that's a bug that needs fixing (I'll try to repro this part).
  • (enabled_if) for an executable depending on the context is a bit unusual. Things exist in multiple context by default, but they're built on demand so they're not always built in every context. Concretely, if a rule producing _build/y/file.txt depends on _build/x/prog.exe (different context), dune will build prog.exe in context x, but will not build it in context y unless it has a reason to.
  • I had no idea it's possible to override main directly in C like that, but apparently ocaml supports that since 1999.
  • reusing the same name across context is not completely possible (your last example) at the moment. There's some ongoing work in the context of melange but I think that's for libraries.
  • as a workaround, you can try passing the context name as a C macro.

What are you trying to do?

emillon added a commit to emillon/dune that referenced this issue Jun 27, 2024
Signed-off-by: Etienne Millon <[email protected]>
@emillon
Copy link
Collaborator

emillon commented Jun 27, 2024

repro in #10676

@shym
Copy link
Contributor Author

shym commented Jun 27, 2024

This is a minification of a crash I encountered in the context of Mirage cross-compilation. So I wanted to override main only in the Mirage context (which might explain why I needed to override main) but the (toy) executable can still be built in the standard context, where using OCaml-provided main would be better.
I ran into this by trying to (ab)use enabled_if to give different stanzas depending on various criteria for the same executable. You unblocked me in a conversation elsewhere on how to set different link_flags according to context, namely (for the record) using (:include xyz) in link_flags and pushing the enabled_ifs to rules generating the xyz file.
So I reported this for the crash; I tried the last example to better understand the problem (because I can work with the overriden main in all contexts).

emillon added a commit to emillon/dune that referenced this issue Jun 27, 2024
Executables with same name can have different `(foreign_stubs)` attached
to them, as long as there's no name collision.

Fixes ocaml#10675

Signed-off-by: Etienne Millon <[email protected]>
@emillon
Copy link
Collaborator

emillon commented Jun 27, 2024

So I reported this for the crash; I tried the last example to better understand the problem (because I can work with the overriden main in all contexts).

Good to know thanks. To clarify, do you plan on relying on this being fixed? In other words, would you be fine if the fix is just a better error message?

@shym
Copy link
Contributor Author

shym commented Jun 28, 2024

To clarify, do you plan on relying on this being fixed?

No. I think I now know how to work around it if need be.

In other words, would you be fine if the fix is just a better error message?

Yes, that would be nice!

emillon added a commit to emillon/dune that referenced this issue Jun 28, 2024
When executables with the same name have different `(foreign_stubs)`
attached, we would crash.

Fixes ocaml#10675

Signed-off-by: Etienne Millon <[email protected]>
emillon added a commit that referenced this issue Jun 28, 2024
Signed-off-by: Etienne Millon <[email protected]>
emillon added a commit to emillon/dune that referenced this issue Jun 28, 2024
When executables with the same name have different `(foreign_stubs)`
attached, we would crash.

Fixes ocaml#10675

Signed-off-by: Etienne Millon <[email protected]>
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 a pull request may close this issue.

2 participants