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

The embed_in_plugin_libraries stanza fails with libraries requiring link-time code #6086

Open
ivg opened this issue Aug 18, 2022 · 0 comments
Labels

Comments

@ivg
Copy link
Member

ivg commented Aug 18, 2022

Expected Behavior

Adding a module that requires link-time generated code to the embed_in_plugin_libraries shall not fail and shall include the generated code into the resulting plugin.

Actual Behavior

Error: Multiple rules generated for
_build/default/lib/bap/.bap_init_toplevel.eobjs/dune_site_plugins_data.ml-gen:
- <internal location>
- <internal location>
-> required by _build/default/lib/bap/bap_init_toplevel.cma
-> required by alias lib/bap/all

Reproduction

(executable
 (name foo)
 (modes (byte plugin))
 (modules foo)
 (embed_in_plugin_libraries dune-site)
 (libraries dune-site))

Note: you can put dune-site or a library that depends on dune-site into the libraries list.

Problem

The problem is here:

  let* link_time_code_gen = Link_time_code_gen.handle_special_libs cctx in
  ... <snip> ...
                  match Linkage.is_plugin linkage with
                  | false -> Memo.return link_time_code_gen
                  | true ->
                    let cc =
                      CC.for_plugin_executable cctx ~embed_in_plugin_libraries
                    in
                    Link_time_code_gen.handle_special_libs cc

We generate link-time code proactively, even if it is a plugin, so once we get to the plugin-specific code generation we already have the code generated and regenerate it again, which results in this obscure error message. The hot-patch is provided below. If you're interested in the fix, I can make a PR.

Patch

I am not PRing it, because it is a hot-fix, just to have a PoC for a self-contained *.cma library that could be loaded into a regular OCaml top-level for #6081. It worked as expected (though the PoC showed that the idea is flawed). Note, the patch will fail if we have several linkages, e.g., (exe plugin). And I don't know how to handle this case, since we indeed need to generate the some link-time for both targets.

diff --git a/src/dune_rules/exe.ml b/src/dune_rules/exe.ml
index eac72fdb5..ceb5c0daa 100644
--- a/src/dune_rules/exe.ml
+++ b/src/dune_rules/exe.ml
@@ -46,9 +46,6 @@ module Linkage = struct
 
   let js = { mode = Byte; ext = ".bc.js"; flags = [] }
 
-  let is_plugin t =
-    List.mem (List.map ~f:Mode.plugin_ext Mode.all) t.ext ~equal:String.equal
-
   let c_flags = [ "-output-obj" ]
 
   let o_flags = [ "-output-complete-obj" ]
@@ -161,10 +158,10 @@ let link_exe ~loc ~name ~(linkage : Linkage.t) ~cm_files ~link_time_code_gen
        While this fix works, more principled solutions should be explored:
 
        - Having foreign stubs declare dependencies on foreign libraries
-       explicitly in the dune file.
+         explicitly in the dune file.
 
        - Implicitly declaring a dependency of all foreign stubs on all foreign
-       libraries.
+         libraries.
 
        In each case, we could then pass the argument in dependency order, which
        would provide a better fix for this issue. *)
@@ -229,7 +226,13 @@ let link_many ?link_args ?o_files ?(embed_in_plugin_libraries = []) ?sandbox
     ~programs ~linkages ~promote cctx =
   let open Memo.O in
   let modules = Compilation_context.modules cctx in
-  let* link_time_code_gen = Link_time_code_gen.handle_special_libs cctx in
+  let* link_time_code_gen =
+    match embed_in_plugin_libraries with
+    | [] -> Link_time_code_gen.handle_special_libs cctx
+    | libs ->
+      let cc = CC.for_plugin_executable cctx ~embed_in_plugin_libraries:libs in
+      Link_time_code_gen.handle_special_libs cc
+  in
   let+ for_exes =
     Memo.parallel_map programs
       ~f:(fun { Program.name; main_module_name; loc } ->
@@ -250,15 +253,6 @@ let link_many ?link_args ?o_files ?(embed_in_plugin_libraries = []) ?sandbox
               if Linkage.is_js linkage then
                 link_js ~loc ~name ~cm_files ~promote cctx ~link_time_code_gen
               else
-                let* link_time_code_gen =
-                  match Linkage.is_plugin linkage with
-                  | false -> Memo.return link_time_code_gen
-                  | true ->
-                    let cc =
-                      CC.for_plugin_executable cctx ~embed_in_plugin_libraries
-                    in
-                    Link_time_code_gen.handle_special_libs cc
-                in
                 link_exe cctx ~loc ~name ~linkage ~cm_files ~link_time_code_gen
                   ~promote ?link_args ?o_files ?sandbox)
         in

cc @nojb as #3141 author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants