Skip to content

fix(melange): stage wrapped root deps for JS emission in the sandbox #15376

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.

Sign up for GitHub

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

Open
anmonteiro wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(melange): stage wrapped root deps for JS emission in the sandbox #15376

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 14 additions & 5 deletions src/dune_rules/melange/melange_rules.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,14 @@ let make_same_lib_emission_deps =
|> Action_builder.map ~f:melange_cross_module_opt_enabled
in
let dep_graph = impl_dep_graph ~sctx ~obj_dir ~modules in
let lib_interface_deps =
(* The Melange emitter loads the library interface/root artifact when
emitting wrapped units such as [Stdlib__Char], even if the source
dependency graph does not mention [Stdlib] explicitly. *)
Modules.With_vlib.lib_interface modules
|> Option.map ~f:(fun m -> deps_of_xopt_closure ~obj_dir [ m ])
|> Option.value ~default:Dep.Set.empty
in
fun module_ ->
let open Action_builder.O in
xopt_enabled
Expand All @@ -264,19 +272,20 @@ let make_same_lib_emission_deps =
module_
in
(* Cross-module optimization follows implementation artifacts, but the
initial reachability also comes from the emitted module's interface
dependencies. Seed the implementation graph with that interface
closure so wrappers such as [Stdlib] stay visible to the emitter. *)
initial reachability also comes from the emitted module's interface
dependencies. *)
Dep_graph.top_closed_implementations dep_graph (module_ :: intf_deps)
|> Action_builder.map ~f:(deps_of_xopt_closure ~obj_dir)
|> Action_builder.map ~f:(fun deps ->
Dep.Set.union lib_interface_deps (deps_of_xopt_closure ~obj_dir deps))
| false ->
(* Emission reads same-library implementation artifacts recursively.
Compilation dependencies collapse transitive edges through interfaces
when a dependency has an [.mli], which is insufficient for JS
emission. Follow the implementation dependency graph directly
instead. *)
Dep_graph.top_closed_implementations dep_graph [ module_ ]
|> Action_builder.map ~f:(deps_of_closure ~obj_dir ~kind:Cmj)
|> Action_builder.map ~f:(fun deps ->
Dep.Set.union lib_interface_deps (deps_of_closure ~obj_dir ~kind:Cmj deps))
;;

let make_external_lib_emission_deps =
Expand Down
6 changes: 2 additions & 4 deletions test/blackbox-tests/test-cases/melange/cross-module-opt-deps.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,10 +76,8 @@ repeat the library's `--mel-cross-module-opt` compile flag.

$ dune describe rules --display=quiet --profile=release dist-default-flags/node_modules/foo/char.js > char-default-rules.sexp
$ grep -c 'stdlib/.stdlib.objs/melange/stdlib.cmi' char-default-rules.sexp
0
[1]
1
$ grep -c 'stdlib/.stdlib.objs/melange/stdlib.cmj' char-default-rules.sexp
0
[1]
1

$ dune build dist-default-flags/node_modules/foo/char.js
1 change: 1 addition & 0 deletions test/blackbox-tests/test-cases/melange/emit-local-transitive-private-module.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ transitive implementation closure, even across `.mli` boundaries.

$ dune rules --root . --format=json --deps app/dist/node_modules/repro.foo/foo_map.js |
> jq_dune -r '.[] | depsFilePaths | select(test("lib/\\.foo\\.objs/melange/.*\\.cmj$")) | sub("^_build/default/"; "")'
lib/.foo.objs/melange/foo.cmj
lib/.foo.objs/melange/foo__Foo_internalAVLtree.cmj
lib/.foo.objs/melange/foo__Foo_internalMapInt.cmj
lib/.foo.objs/melange/foo__Foo_map.cmj
Expand Down
Loading