From 88dec81102d2209a88c6c404285ebb263b02d922 Mon Sep 17 00:00:00 2001 From: Antonio Nuno Monteiro Date: Sun, 5 Jan 2025 21:57:13 -0800 Subject: [PATCH] fix(melange): disallow private implementations of public virtual libs (#11253) * test(melange): show wrong require for private impl of public virtual lib Signed-off-by: Antonio Nuno Monteiro * fix(melange): disallow private implementations of public virtual libs Signed-off-by: Antonio Nuno Monteiro * changelog Signed-off-by: Antonio Nuno Monteiro --------- Signed-off-by: Antonio Nuno Monteiro --- doc/changes/11253.md | 2 + src/dune_rules/melange/melange_rules.ml | 77 +++++++++++++++---------- 2 files changed, 48 insertions(+), 31 deletions(-) create mode 100644 doc/changes/11253.md diff --git a/doc/changes/11253.md b/doc/changes/11253.md new file mode 100644 index 00000000000..5c29c108aa1 --- /dev/null +++ b/doc/changes/11253.md @@ -0,0 +1,2 @@ +- Disallow private implementations of public virtual libs in melange mode. + (@amonteiro, #11253) diff --git a/src/dune_rules/melange/melange_rules.ml b/src/dune_rules/melange/melange_rules.ml index cb046e02d2e..efc431b49c5 100644 --- a/src/dune_rules/melange/melange_rules.ml +++ b/src/dune_rules/melange/melange_rules.ml @@ -577,38 +577,53 @@ let setup_js_rules_libraries = | None -> Memo.return () | Some vlib -> let* vlib = Resolve.Memo.read_memo vlib in - let* includes = - let+ requires_link = - let+ requires_link = - Lib.Compile.for_lib - ~allow_overlaps:mel.allow_overlapping_dependencies - (Scope.libs scope) - vlib - |> Lib.Compile.requires_link - |> Memo.Lazy.force - in - let open Resolve.O in - let+ requires_link = requires_link in - (* Whenever a `concrete_lib` implementation contains a field - `(implements virt_lib)`, we also set up the JS targets for the - modules defined in `virt_lib`. + let vlib_output = output_of_lib ~target_dir vlib in + (match vlib_output, output with + | `Public_library _, `Private_library_or_emit _ -> + let info = Lib.info lib in + User_error.raise + ~loc:(Lib_info.loc info) + [ Pp.text + "Dune doesn't currently support building private implementations of \ + virtual public libaries for `(modes melange)`" + ] + ~hints: + [ Pp.textf + "Add a `public_name` to the library `%s'." + (Lib_name.to_string (Lib_info.name info)) + ] + | `Public_library _, `Public_library _ | `Private_library_or_emit _, _ -> + let* includes = + let+ requires_link = + let+ requires_link = + Lib.Compile.for_lib + ~allow_overlaps:mel.allow_overlapping_dependencies + (Scope.libs scope) + vlib + |> Lib.Compile.requires_link + |> Memo.Lazy.force + in + let open Resolve.O in + let+ requires_link = requires_link in + (* Whenever a `concrete_lib` implementation contains a field + `(implements virt_lib)`, we also set up the JS targets for the + modules defined in `virt_lib`. - In the cases where `virt_lib` (concrete) modules depend on any - virtual modules (i.e. programming against the interface), we - need to make sure that the JS rules that dune emits for - `virt_lib` depend on `concrete_lib`, such that Melange can find - the correct `.cmj` file, which is needed to emit the correct - path in `import` / `require`. *) - lib :: requires_link - in - cmj_includes ~requires_link ~scope lib_config - in - let output = output_of_lib ~target_dir vlib in - parallel_build_source_modules - ~sctx - ~scope - vlib - ~f:(build_js ~dir ~output ~includes ~compile_flags) + In the cases where `virt_lib` (concrete) modules depend on any + virtual modules (i.e. programming against the interface), we + need to make sure that the JS rules that dune emits for + `virt_lib` depend on `concrete_lib`, such that Melange can find + the correct `.cmj` file, which is needed to emit the correct + path in `import` / `require`. *) + lib :: requires_link + in + cmj_includes ~requires_link ~scope lib_config + in + parallel_build_source_modules + ~sctx + ~scope + vlib + ~f:(build_js ~dir ~output:vlib_output ~includes ~compile_flags)) and+ () = parallel_build_source_modules ~sctx