refactor: Remove explicit OCaml transitive dependency files

Compute transitive module dependency closures directly from anonymous
ocamldep and ocamlobjinfo actions instead of materializing .all-deps
files. This removes the merge-file rule path for internal dependency
setup while preserving module-level cycle diagnostics.

As a side-effect, this fixes an ugly "Rule not found" error.

Signed-off-by: Rudi Grinberg <me@rgrinberg.com>

Author: rgrinberg

src/dune_rules/dep_rules.ml

@@ -33,9 +33,13 @@ val read_immediate_deps_of
   -> Module.t list Action_builder.t
 
 val read_deps_of
-  :  obj_dir:Path.Build.t Obj_dir.t
+  :  sandbox:Sandbox_config.t
+  -> sctx:Super_context.t
+  -> obj_dir:Path.Build.t Obj_dir.t
   -> modules:Modules.With_vlib.t
-  -> ml_kind:Ml_kind.t
+  -> impl:Virtual_rules.t
+  -> dir:Path.Build.t
   -> for_:Compilation_mode.t
+  -> ml_kind:Ml_kind.t
   -> Module.t
   -> Module.t list Action_builder.t

src/dune_rules/dep_rules.mli

@@ -246,7 +246,16 @@ let make_same_lib_emission_deps =
       >>= function
       | true ->
         let* intf_deps =
-          Dep_rules.read_deps_of ~obj_dir ~modules ~ml_kind:Intf module_ ~for_
+          Dep_rules.read_deps_of
+            ~sandbox
+            ~sctx
+            ~obj_dir
+            ~modules
+            ~impl:Virtual_rules.no_implements
+            ~dir:(Obj_dir.dir obj_dir)
+            ~for_
+            ~ml_kind:Intf
+            module_
         in
         (* Cross-module optimization follows implementation artifacts, but the
          initial reachability also comes from the emitted module's interface
@@ -256,10 +265,10 @@ let make_same_lib_emission_deps =
         |> Action_builder.map ~f:(deps_of_xopt_closure ~obj_dir)
       | false ->
         (* Emission reads same-library implementation artifacts recursively.
-           The generic [.impl.all-deps] files collapse transitive edges through
-           interfaces when a dependency has an [.mli], which is fine for
-           compilation but insufficient for JS emission. Follow the
-           implementation dependency graph directly instead. *)
+           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_impl_closure ~obj_dir)
 ;;

src/dune_rules/melange/melange_rules.ml

@@ -1,57 +1,4 @@
 open Import
-open Memo.O
-
-module Merge_files_into = struct
-  module Spec = struct
-    type ('src, 'dst) t =
-      { transitive : 'src list
-      ; immediate : Module_name.Unique.t list
-      ; target : 'dst
-      }
-
-    let name = "merge_files_into"
-    let version = 2
-    let is_useful_to ~memoize:_ = true
-
-    let bimap t path target =
-      { t with transitive = List.map t.transitive ~f:path; target = target t.target }
-    ;;
-
-    let encode
-          (type src dst)
-          ({ transitive; immediate; target } : (src, dst) t)
-          (input : src -> Sexp.t)
-          (output : dst -> Sexp.t)
-      : Sexp.t
-      =
-      List
-        [ List (List.map transitive ~f:input)
-        ; List
-            (List.map ~f:(fun s -> Sexp.Atom (Module_name.Unique.to_string s)) immediate)
-        ; output target
-        ]
-    ;;
-
-    let action { transitive; immediate; target } ~ectx:_ ~eenv:_ =
-      Async.async (fun () ->
-        List.fold_left
-          transitive
-          ~init:(Module_name.Unique.Set.of_list immediate)
-          ~f:(fun set source_path ->
-            Io.lines_of_file source_path
-            |> Module_name.Unique.Set.of_list_map ~f:Module_name.Unique.of_string
-            |> Module_name.Unique.Set.union set)
-        |> Module_name.Unique.Set.to_list_map ~f:Module_name.Unique.to_string
-        |> Io.write_lines (Path.build target))
-    ;;
-  end
-
-  module Action = Action_ext.Make (Spec)
-
-  let action ~transitive ~immediate ~target =
-    Action.action { transitive; immediate; target }
-  ;;
-end
 
 let parse_module_names ~dir ~(unit : Module.t) ~modules words =
   List.concat_map words ~f:(fun m ->
@@ -75,14 +22,6 @@ let parse_module_names ~dir ~(unit : Module.t) ~modules words =
         ])
 ;;
 
-let parse_compilation_units ~modules =
-  let obj_map = Modules.With_vlib.obj_map modules in
-  List.filter_map ~f:(fun m ->
-    let obj_name = Module_name.Unique.of_string m in
-    Module_name.Unique.Map.find obj_map obj_name
-    |> Option.map ~f:Modules.Sourced_module.to_module)
-;;
-
 let parse_deps_exn =
   let invalid file lines =
     User_error.raise
@@ -106,19 +45,6 @@ let parse_deps_exn =
          String.extract_blank_separated_words deps)
 ;;
 
-let transitive_deps =
-  let transive_dep obj_dir m ~for_ =
-    (match Module.kind m with
-     | Root | Alias _ -> None
-     | _ -> if Module.has m ~ml_kind:Intf then Some Ml_kind.Intf else Some Impl)
-    |> Option.map ~f:(fun ml_kind ->
-      Obj_dir.Module.dep obj_dir ~for_ (Transitive (m, ml_kind))
-      |> Option.value_exn (* we already checked if it's an alias module *)
-      |> Path.build)
-  in
-  fun obj_dir modules ~for_ -> List.filter_map modules ~f:(transive_dep obj_dir ~for_)
-;;
-
 let ocamldep_action ~sandbox ~sctx ~dir ~ml_kind unit =
   let context = Super_context.context sctx in
   let flags, sandbox =
@@ -174,39 +100,3 @@ let read_immediate_deps_of ~sandbox ~sctx ~obj_dir ~modules ~ml_kind unit =
     |> Action_builder.of_memo
     |> Action_builder.memoize memo_name
 ;;
-
-let deps_of ~sandbox ~modules ~sctx ~dir ~obj_dir ~ml_kind ~for_ unit =
-  let dep = Obj_dir.Module.dep obj_dir ~for_ in
-  let all_deps_file = dep (Transitive (unit, ml_kind)) |> Option.value_exn in
-  let+ () =
-    let produce_all_deps =
-      let open Action_builder.O in
-      (let+ transitive, immediate =
-         (let+ immediate_deps =
-            read_immediate_deps_of ~sandbox ~sctx ~obj_dir ~modules ~ml_kind unit
-          in
-          let transitive_deps = transitive_deps obj_dir immediate_deps ~for_ in
-          let immediate_deps = List.map immediate_deps ~f:Module.obj_name in
-          (transitive_deps, immediate_deps), transitive_deps)
-         |> Action_builder.dyn_paths
-       in
-       Merge_files_into.action ~transitive ~immediate ~target:all_deps_file)
-      |> Action_builder.with_file_targets ~file_targets:[ all_deps_file ]
-    in
-    Action_builder.With_targets.map ~f:Action.Full.make produce_all_deps
-    |> Super_context.add_rule sctx ~dir
-  in
-  let all_deps_file = Path.build all_deps_file in
-  Action_builder.lines_of all_deps_file
-  |> Action_builder.map ~f:(parse_compilation_units ~modules)
-  |> Action_builder.memoize (Path.to_string all_deps_file)
-;;
-
-let read_deps_of ~obj_dir ~modules ~ml_kind ~for_ unit =
-  let all_deps_file =
-    Obj_dir.Module.dep obj_dir ~for_ (Transitive (unit, ml_kind)) |> Option.value_exn
-  in
-  Action_builder.lines_of (Path.build all_deps_file)
-  |> Action_builder.map ~f:(parse_compilation_units ~modules)
-  |> Action_builder.memoize (Path.Build.to_string all_deps_file)
-;;

src/dune_rules/ocamldep.ml

@@ -2,25 +2,6 @@
 
 open Import
 
-val deps_of
-  :  sandbox:Sandbox_config.t
-  -> modules:Modules.With_vlib.t
-  -> sctx:Super_context.t
-  -> dir:Path.Build.t
-  -> obj_dir:Path.Build.t Obj_dir.t
-  -> ml_kind:Ml_kind.t
-  -> for_:Compilation_mode.t
-  -> Module.t
-  -> Module.t list Action_builder.t Memo.t
-
-val read_deps_of
-  :  obj_dir:Path.Build.t Obj_dir.t
-  -> modules:Modules.With_vlib.t
-  -> ml_kind:Ml_kind.t
-  -> for_:Compilation_mode.t
-  -> Module.t
-  -> Module.t list Action_builder.t
-
 (** [read_immediate_deps_of ~obj_dir ~modules ~ml_kind unit] returns the
     immediate dependencies found in the modules of [modules] for the file with
     kind [ml_kind] of the module [unit]. If there is no such file with kind

src/dune_rules/ocamldep.mli

@@ -23,12 +23,11 @@ Reports module dependency cycles inside executables.
 
   $ dune build
   Error: Dependency cycle between:
-     _build/default/.foo.eobjs/dune__exe__Baz.impl.all-deps
-  -> _build/default/.foo.eobjs/dune__exe__Bar.impl.all-deps
-  -> _build/default/.foo.eobjs/dune__exe__Baz.impl.all-deps
-  -> required by _build/default/.foo.eobjs/dune__exe__Foo.impl.all-deps
+     transitive deps of dune__exe__Baz.impl in _build/default
+  -> transitive deps of dune__exe__Bar.impl in _build/default
+  -> transitive deps of dune__exe__Baz.impl in _build/default
+  -> required by transitive deps of dune__exe__Foo.impl in _build/default
   -> required by _build/default/foo.exe
   -> required by alias all
   -> required by alias default
   [1]
-

test/blackbox-tests/test-cases/cyclic-dep-executable.t

@@ -19,11 +19,28 @@ Test `(include_subdirs qualified)` with sandboxing
 
   $ DUNE_SANDBOX=symlink dune build
 
-Transitive deps file includes the alias module
+No transitive deps file is materialized.
 
-  $ cat _build/default/lib/.foo.objs/foo__Bar.impl.all-deps
-  foo__Sub
-  foo__Sub__Hello
+  $ find _build -name "*.all-deps" | sort
+
+Transitive alias dependencies are still available under sandboxing when a
+compiled interface exposes a qualified submodule path.
+
+  $ cat > lib/a.ml <<EOF
+  > let x = B.x
+  > EOF
+  $ cat > lib/b.mli <<EOF
+  > val x : Sub.T.t
+  > EOF
+  $ cat > lib/b.ml <<EOF
+  > let x = Sub.T.x
+  > EOF
+  $ cat > lib/sub/t.ml <<EOF
+  > type t = int
+  > let x = 0
+  > EOF
+
+  $ DUNE_SANDBOX=symlink dune build
 
   $ cat > lib/dune <<EOF
   > (include_subdirs qualified)
@@ -34,6 +51,7 @@ Transitive deps file includes the alias module
   > EOF
   $ cat > lib/sub/sub.ml <<EOF
   > module Hello = Hello
+  > module T = T
   > let world = "world"
   > EOF
   $ cat > lib/sub/hello.ml <<EOF
@@ -42,7 +60,4 @@ Transitive deps file includes the alias module
 
   $ DUNE_SANDBOX=symlink dune build
 
-  $ cat _build/default/lib/.foo.objs/foo__Bar.impl.all-deps
-  foo__Sub
-  foo__Sub__
-  foo__Sub__Hello
+  $ find _build -name "*.all-deps" | sort

test/blackbox-tests/test-cases/include-qualified/build-with-sandbox.t

@@ -23,7 +23,7 @@ We shouldn't allow foo/y/$x.ml to depend on foo/foo.ml
   X is the main module of the library and is the only module exposed outside of
   the library. Consequently, it should be the one depending on all the other
   modules in the library.
-  -> required by _build/default/.foo.objs/foo__X__Y__Z.impl.all-deps
+  -> required by transitive deps of foo__X__Y__Z.impl in _build/default
   -> required by _build/default/.foo.objs/byte/foo__X__Y__Z.cmo
   -> required by _build/default/foo.cma
   -> required by alias all

test/blackbox-tests/test-cases/include-qualified/invalid-deps/group-interface-sub-module.t

@@ -23,7 +23,7 @@ We shouldn't allow foo/$x.ml to depend on foo/foo.ml
   Baz is the main module of the library and is the only module exposed outside
   of the library. Consequently, it should be the one depending on all the other
   modules in the library.
-  -> required by _build/default/.foo.objs/foo__Baz__Bar.impl.all-deps
+  -> required by transitive deps of foo__Baz__Bar.impl in _build/default
   -> required by _build/default/.foo.objs/byte/foo__Baz__Bar.cmo
   -> required by _build/default/foo.cma
   -> required by alias all

test/blackbox-tests/test-cases/include-qualified/invalid-deps/group-interface.t

@@ -39,7 +39,7 @@ We also forbid submodules from depending on their interface modules:
   Baz is the main module of the library and is the only module exposed outside
   of the library. Consequently, it should be the one depending on all the other
   modules in the library.
-  -> required by _build/default/.foo.objs/foo__Baz__Bar.impl.all-deps
+  -> required by transitive deps of foo__Baz__Bar.impl in _build/default
   -> required by _build/default/.foo.objs/byte/foo__Baz__Bar.cmo
   -> required by _build/default/foo.cma
   -> required by alias all
@@ -60,7 +60,7 @@ Or their parent interface modules:
   Baz is the main module of the library and is the only module exposed outside
   of the library. Consequently, it should be the one depending on all the other
   modules in the library.
-  -> required by _build/default/.foo.objs/foo__Baz__Foo__Z.impl.all-deps
+  -> required by transitive deps of foo__Baz__Foo__Z.impl in _build/default
   -> required by _build/default/.foo.objs/byte/foo__Baz__Foo__Z.cmo
   -> required by _build/default/foo.cma
   -> required by alias all