fix(melange): track immediate .cmj deps as dependencies of JS rules

bin/describe/describe_workspace.ml

@@ -337,7 +337,9 @@ module Crawl = struct
     | { with_deps = false; _ } ->
       Action_builder.return { Ocaml.Ml_kind.Dict.intf = []; impl = [] }
     | { with_deps = true; _ } ->
-      let deps = Dune_rules.Dep_rules.immediate_deps_of unit modules obj_dir in
+      let deps ml_kind =
+        Dune_rules.Dep_rules.immediate_deps_of unit modules ~obj_dir ~ml_kind
+      in
       let open Action_builder.O in
       let+ intf, impl = Action_builder.both (deps Intf) (deps Impl) in
       { Ocaml.Ml_kind.Dict.intf; impl }

doc/changes/10286.md

@@ -0,0 +1,3 @@
+- melange: fix a bug that would cause stale `import` paths to be emitted when
+  moving source files within `(include_subdirs ..)`
+  (#10286, fixes #9190, @anmonteiro)

src/dune_rules/dep_rules.ml

@@ -129,7 +129,7 @@ let rec deps_of md ~ml_kind (m : Modules.Sourced_module.t) =
 (** Tests whether a set of modules is a singleton *)
 let has_single_file modules = Option.is_some @@ Modules.as_singleton modules
 
-let immediate_deps_of unit modules obj_dir ml_kind =
+let immediate_deps_of unit modules ~obj_dir ~ml_kind =
   match Module.kind unit with
   | Alias _ -> Action_builder.return []
   | Wrapped_compat ->

src/dune_rules/dep_rules.mli

@@ -10,8 +10,8 @@ val for_module
 val immediate_deps_of
   :  Module.t
   -> Modules.t
-  -> Path.Build.t Obj_dir.t
-  -> Ml_kind.t
+  -> obj_dir:Path.Build.t Obj_dir.t
+  -> ml_kind:Ml_kind.t
   -> Module.t list Action_builder.t
 
 val rules : Ocamldep.Modules_data.t -> Dep_graph.t Ml_kind.Dict.t Memo.t

src/dune_rules/melange/melange_rules.ml

@@ -152,37 +152,62 @@ let js_targets_of_libs sctx libs ~module_systems ~target_dir =
         List.rev_append for_vlib base))
 ;;
 
-let build_js ~loc ~dir ~pkg_name ~mode ~module_systems ~output ~obj_dir ~sctx ~includes m =
+let build_js
+  ~loc
+  ~dir
+  ~pkg_name
+  ~mode
+  ~module_systems
+  ~output
+  ~obj_dir
+  ~sctx
+  ~includes
+  ~local_modules
+  m
+  =
   let open Memo.O in
   let* compiler = Melange_binary.melc sctx ~loc:(Some loc) ~dir in
   Memo.parallel_iter module_systems ~f:(fun (module_system, js_ext) ->
-    let src = Obj_dir.Module.cm_file_exn obj_dir m ~kind:(Melange Cmj) in
-    let output = make_js_name ~output ~js_ext m in
-    let obj_dir = [ Command.Args.A "-I"; Path (Obj_dir.melange_dir obj_dir) ] in
-    let melange_package_args =
-      let pkg_name_args =
-        match pkg_name with
-        | None -> []
-        | Some pkg_name -> [ "--bs-package-name"; Package.Name.to_string pkg_name ]
+    let build =
+      let command =
+        let src = Obj_dir.Module.cm_file_exn obj_dir m ~kind:(Melange Cmj) in
+        let output = make_js_name ~output ~js_ext m in
+        let obj_dir = [ Command.Args.A "-I"; Path (Obj_dir.melange_dir obj_dir) ] in
+        let melange_package_args =
+          let pkg_name_args =
+            match pkg_name with
+            | None -> []
+            | Some pkg_name -> [ "--bs-package-name"; Package.Name.to_string pkg_name ]
+          in
+          let js_modules_str = Melange.Module_system.to_string module_system in
+          "--bs-module-type" :: js_modules_str :: pkg_name_args
+        in
+        Command.run
+          ~dir:(Super_context.context sctx |> Context.build_dir |> Path.build)
+          compiler
+          [ Command.Args.S obj_dir
+          ; Command.Args.as_any includes
+          ; As melange_package_args
+          ; A "-o"
+          ; Target output
+          ; Dep src
+          ]
       in
-      let js_modules_str = Melange.Module_system.to_string module_system in
-      "--bs-module-type" :: js_modules_str :: pkg_name_args
+      With_targets.map_build command ~f:(fun command ->
+        let open Action_builder.O in
+        let local_library_paths =
+          match local_modules with
+          | Some (modules, obj_dir) ->
+            let+ module_deps =
+              Dep_rules.immediate_deps_of m modules ~obj_dir ~ml_kind:Impl
+            in
+            List.map module_deps ~f:(fun dep_m ->
+              Obj_dir.Module.cm_file_exn obj_dir dep_m ~kind:(Melange Cmj) |> Path.build)
+          | None -> Action_builder.return []
+        in
+        Action_builder.dyn_paths_unit local_library_paths >>> command)
     in
-    Super_context.add_rule
-      sctx
-      ~dir
-      ~loc
-      ~mode
-      (Command.run
-         ~dir:(Super_context.context sctx |> Context.build_dir |> Path.build)
-         compiler
-         [ Command.Args.S obj_dir
-         ; Command.Args.as_any includes
-         ; As melange_package_args
-         ; A "-o"
-         ; Target output
-         ; Dep src
-         ]))
+    Super_context.add_rule sctx ~dir ~loc ~mode build)
 ;;
 
 (* attach [deps] to the specified [alias] AND the (dune default) [all] alias.
@@ -394,7 +419,7 @@ let modules_for_js_and_obj_dir ~sctx ~dir_contents ~scope (mel : Melange_stanzas
     Modules.fold_no_vlib modules ~init:[] ~f:(fun x acc ->
       if Module.has x ~ml_kind:Impl then x :: acc else acc)
   in
-  modules_for_js, obj_dir
+  modules, modules_for_js, obj_dir
 ;;
 
 let setup_entries_js
@@ -408,7 +433,7 @@ let setup_entries_js
   (mel : Melange_stanzas.Emit.t)
   =
   let open Memo.O in
-  let* modules_for_js, obj_dir =
+  let* local_modules, modules_for_js, local_obj_dir =
     modules_for_js_and_obj_dir ~sctx ~dir_contents ~scope mel
   in
   let requires_link = Lib.Compile.requires_link compile_info in
@@ -418,12 +443,24 @@ let setup_entries_js
   let* requires_link = Memo.Lazy.force requires_link in
   let includes = cmj_includes ~requires_link ~scope in
   let output = `Private_library_or_emit target_dir in
-  let obj_dir = Obj_dir.of_local obj_dir in
+  let obj_dir = Obj_dir.of_local local_obj_dir in
   let* () =
     setup_runtime_assets_rules sctx ~dir ~target_dir ~mode ~output ~for_:`Emit mel
   in
   Memo.parallel_iter modules_for_js ~f:(fun m ->
-    build_js ~dir ~loc ~pkg_name ~mode ~module_systems ~output ~obj_dir ~sctx ~includes m)
+    let local_modules = Some (local_modules, local_obj_dir) in
+    build_js
+      ~dir
+      ~loc
+      ~pkg_name
+      ~mode
+      ~module_systems
+      ~output
+      ~obj_dir
+      ~sctx
+      ~includes
+      ~local_modules
+      m)
 ;;
 
 let setup_js_rules_libraries
@@ -451,12 +488,18 @@ let setup_js_rules_libraries
       let pkg_name = Lib_info.package info in
       build_js ~loc ~pkg_name ~obj_dir
     in
+    let output = output_of_lib ~target_dir lib in
     let* includes =
       let+ requires_link = Memo.Lazy.force (Lib.Compile.requires_link lib_compile_info) in
       cmj_includes ~requires_link ~scope
-    in
-    let output = output_of_lib ~target_dir lib in
-    let* () =
+    and* local_modules =
+      match Lib.Local.of_lib lib with
+      | Some lib ->
+        let+ modules = Dir_contents.modules_of_local_lib sctx lib in
+        let obj_dir = Lib.Local.obj_dir lib in
+        Some (modules, obj_dir)
+      | None -> Memo.return None
+    and* () =
       setup_runtime_assets_rules
         sctx
         ~dir
@@ -498,10 +541,10 @@ let setup_js_rules_libraries
           cmj_includes ~requires_link ~scope
         in
         impl_only_modules_defined_in_this_lib sctx vlib
-        >>= Memo.parallel_iter ~f:(build_js ~dir ~output ~includes)
+        >>= Memo.parallel_iter ~f:(build_js ~dir ~output ~includes ~local_modules)
     in
     let* source_modules = impl_only_modules_defined_in_this_lib sctx lib in
-    Memo.parallel_iter source_modules ~f:(build_js ~dir ~output ~includes))
+    Memo.parallel_iter source_modules ~f:(build_js ~dir ~output ~local_modules ~includes))
 ;;
 
 let setup_js_rules_libraries_and_entries
@@ -557,7 +600,7 @@ let setup_emit_js_rules ~dir_contents ~dir ~scope ~sctx mel =
        package). When resolution fails, we replace the JS entries with the
        resolution error inside [Action_builder.fail] to give Dune a chance to
        fail if any of the targets end up attached to a package installation. *)
-    let* modules_for_js, _obj_dir =
+    let* _local_modules, modules_for_js, _obj_dir =
       modules_for_js_and_obj_dir ~sctx ~dir_contents ~scope mel
     in
     let module_systems = mel.module_systems in

src/dune_rules/virtual_rules.ml

@@ -23,6 +23,14 @@ let setup_copy_rules_for_impl ~sctx ~dir vimpl =
     let dst = Obj_dir.Module.cm_file_exn impl_obj_dir m ~kind in
     copy_to_obj_dir ~src ~dst
   in
+  let copy_ocamldep_file m =
+    match Obj_dir.to_local vlib_obj_dir with
+    | None -> Memo.return ()
+    | Some vlib_obj_dir ->
+      let src = Obj_dir.Module.dep vlib_obj_dir (Immediate (m, Impl)) |> Path.build in
+      let dst = Obj_dir.Module.dep impl_obj_dir (Immediate (m, Impl)) in
+      copy_to_obj_dir ~src ~dst
+  in
   let copy_interface_to_impl ~src kind () =
     let dst = Obj_dir.Module.cm_public_file_exn impl_obj_dir src ~kind in
     let src = Obj_dir.Module.cm_public_file_exn vlib_obj_dir src ~kind in
@@ -39,7 +47,8 @@ let setup_copy_rules_for_impl ~sctx ~dir vimpl =
             >>> Memo.when_ melange (copy_interface_to_impl ~src (Melange Cmi)))
     >>> Memo.when_ (Module.has src ~ml_kind:Impl) (fun () ->
       Memo.when_ byte (fun () -> copy_obj_file src (Ocaml Cmo))
-      >>> Memo.when_ melange (fun () -> copy_obj_file src (Melange Cmj))
+      >>> Memo.when_ melange (fun () ->
+        copy_obj_file src (Melange Cmj) >>> copy_ocamldep_file src)
       >>> Memo.when_ native (fun () ->
         copy_obj_file src (Ocaml Cmx)
         >>>

test/blackbox-tests/test-cases/melange/library-include-subdirs.t

@@ -38,24 +38,62 @@ Melange shows the proper path to `bar.js`
   let Foo__Bar = require("./init/bar.js");
 
   $ mv lib/init lib/end
-
   $ dune build @mel
 
-The import in `foo.js` still shows the initial path to `bar.js`, but the file is not there anymore
+The import in `foo.js` has been updated to the new bar.js target
 
   $ cat _build/default/output/lib/foo.js | grep bar.js
-  let Foo__Bar = require("./init/bar.js");
+  let Foo__Bar = require("./end/bar.js");
+
+The initial file is not there anymore
 
   $ test -f _build/default/output/lib/init/bar.js
   [1]
 
+But the new one is
+
   $ test -f _build/default/output/lib/end/bar.js
 
-After removal of the js artifact, the path in `bar.js` import is correct
+Now try the same thing with `melange.emit`
 
-  $ rm _build/default/output/lib/foo.js
+  $ rm -rf lib
+  $ cat > dune <<EOF
+  > (include_subdirs unqualified)
+  > (melange.emit
+  >  (target output)
+  >  (alias mel)
+  >  (emit_stdlib false)
+  >  (preprocess (pps melange.ppx)))
+  > EOF
+
+  $ mkdir init
+  $ cat > init/bar.ml <<EOF
+  > let name = "Zoe"
+  > EOF
+  $ cat > foo.ml <<EOF
+  > let name = Bar.name
+  > EOF
 
   $ dune build @mel
 
-  $ cat _build/default/output/lib/foo.js | grep bar.js
-  let Foo__Bar = require("./end/bar.js");
+Melange shows the proper path to `bar.js`
+
+  $ cat _build/default/output/foo.js | grep bar.js
+  let Melange__Bar = require("./init/bar.js");
+
+  $ mv init end
+  $ dune build @mel
+
+The import in `foo.js` has been updated to the new bar.js target
+
+  $ cat _build/default/output/foo.js | grep bar.js
+  let Melange__Bar = require("./end/bar.js");
+
+The initial file is not there anymore
+
+  $ test -f _build/default/output/init/bar.js
+  [1]
+
+But the new one is
+
+  $ test -f _build/default/output/end/bar.js