refactor: move library file deps from Includes to per-module in build_cm (#4572)

src/dune_rules/compilation_context.ml

@@ -4,55 +4,32 @@ open Memo.O
 module Includes = struct
   type t = Command.Args.without_targets Command.Args.t Lib_mode.Cm_kind.Map.t
 
-  let make ~project ~opaque ~direct_requires ~hidden_requires lib_config
+  (* Library file dependencies (Hidden_deps) are added per-module in
+     module_compilation.ml rather than here.
+     TODO: some of the requires can be filtered out using [ocamldep] info.
+     See issue #4572. *)
+  let make ~project ~direct_requires ~hidden_requires lib_config
     : _ Lib_mode.Cm_kind.Map.t
     =
-    (* TODO: some of the requires can filtered out using [ocamldep] info *)
     let open Resolve.Memo.O in
     let iflags direct_libs hidden_libs mode =
       Lib_flags.L.include_flags ~project ~direct_libs ~hidden_libs mode lib_config
     in
-    let make_includes_args ~mode groups =
+    let make_includes_args ~mode =
       (let+ direct_libs = direct_requires
        and+ hidden_libs = hidden_requires in
-       Command.Args.S
-         [ iflags direct_libs hidden_libs mode
-         ; Hidden_deps (Lib_file_deps.deps (direct_libs @ hidden_libs) ~groups)
-         ])
+       iflags direct_libs hidden_libs mode)
       |> Resolve.Memo.args
       |> Command.Args.memo
     in
     { ocaml =
-        (let cmi_includes = make_includes_args ~mode:(Ocaml Byte) [ Ocaml Cmi ] in
+        (let cmi_includes = make_includes_args ~mode:(Ocaml Byte) in
          { cmi = cmi_includes
          ; cmo = cmi_includes
-         ; cmx =
-             (let+ direct_libs = direct_requires
-              and+ hidden_libs = hidden_requires in
-              Command.Args.S
-                [ iflags direct_libs hidden_libs (Ocaml Native)
-                ; Hidden_deps
-                    (let libs = direct_libs @ hidden_libs in
-                     if opaque
-                     then
-                       List.map libs ~f:(fun lib ->
-                         ( lib
-                         , if Lib.is_local lib
-                           then [ Lib_file_deps.Group.Ocaml Cmi ]
-                           else [ Ocaml Cmi; Ocaml Cmx ] ))
-                       |> Lib_file_deps.deps_with_exts
-                     else
-                       Lib_file_deps.deps
-                         libs
-                         ~groups:[ Lib_file_deps.Group.Ocaml Cmi; Ocaml Cmx ])
-                ])
-             |> Resolve.Memo.args
-             |> Command.Args.memo
+         ; cmx = make_includes_args ~mode:(Ocaml Native)
          })
     ; melange =
-        { cmi = make_includes_args ~mode:Melange [ Melange Cmi ]
-        ; cmj = make_includes_args ~mode:Melange [ Melange Cmi; Melange Cmj ]
-        }
+        { cmi = make_includes_args ~mode:Melange; cmj = make_includes_args ~mode:Melange }
     }
   ;;
 
@@ -240,8 +217,7 @@ let create
   ; requires_link
   ; implements
   ; parameters
-  ; includes =
-      Includes.make ~project ~opaque ~direct_requires ~hidden_requires ocaml.lib_config
+  ; includes = Includes.make ~project ~direct_requires ~hidden_requires ocaml.lib_config
   ; preprocessing
   ; opaque
   ; js_of_ocaml
@@ -333,7 +309,6 @@ let for_module_generated_at_link_time cctx ~requires ~module_ =
     let direct_requires = requires in
     Includes.make
       ~project:(Scope.project cctx.scope)
-      ~opaque
       ~direct_requires
       ~hidden_requires
       cctx.ocaml.lib_config

src/dune_rules/lib_file_deps.ml

@@ -48,9 +48,25 @@ let deps_of_lib (lib : Lib.t) ~groups =
   |> Dep.Set.of_list
 ;;
 
-let deps_with_exts = Dep.Set.union_map ~f:(fun (lib, groups) -> deps_of_lib lib ~groups)
 let deps libs ~groups = Dep.Set.union_map libs ~f:(deps_of_lib ~groups)
 
+let deps_of_entries ~opaque ~(cm_kind : Lib_mode.Cm_kind.t) (libs : Lib.t list) =
+  let groups =
+    match cm_kind with
+    | Ocaml Cmi | Ocaml Cmo -> [ Group.Ocaml Cmi ]
+    | Melange Cmi -> [ Group.Melange Cmi ]
+    | Melange Cmj -> [ Group.Melange Cmi; Melange Cmj ]
+    | Ocaml Cmx -> [ Group.Ocaml Cmi; Ocaml Cmx ]
+  in
+  Dep.Set.union_map libs ~f:(fun lib ->
+    let groups =
+      match cm_kind with
+      | Ocaml Cmx when opaque && Lib.is_local lib -> [ Group.Ocaml Cmi ]
+      | _ -> groups
+    in
+    deps_of_lib lib ~groups)
+;;
+
 type path_specification =
   | Allow_all
   | Disallow_external of Lib_name.t

src/dune_rules/lib_file_deps.mli

@@ -15,7 +15,9 @@ end
     with extension [files] of libraries [libs]. *)
 val deps : Lib.t list -> groups:Group.t list -> Dep.Set.t
 
-val deps_with_exts : (Lib.t * Group.t list) list -> Dep.Set.t
+(** Compute library file dependencies for all [libs] for the given [cm_kind].
+    When [opaque] is true, local libraries only depend on .cmi (not .cmx). *)
+val deps_of_entries : opaque:bool -> cm_kind:Lib_mode.Cm_kind.t -> Lib.t list -> Dep.Set.t
 
 type path_specification =
   | Allow_all

src/dune_rules/module_compilation.ml

@@ -286,6 +286,30 @@ let build_cm
         | Some All | None -> Hidden_targets [ obj ])
    in
    let opaque = Compilation_context.opaque cctx in
+   (* Library file dependencies, added per-module. Non-stdlib alias modules
+      and Wrapped_compat modules are compiled with Includes.empty and need no
+      library file deps. Stdlib aliases need full deps because they contain
+      code referencing CamlintternalXXX modules. All other modules depend on
+      all libraries in the stanza's requires. *)
+   let lib_cm_deps : _ Command.Args.t =
+     let stanza_modules = Compilation_context.modules cctx in
+     let skip_lib_deps =
+       match Module.kind m with
+       | Alias _ -> not (Modules.With_vlib.is_stdlib_alias stanza_modules m)
+       | Wrapped_compat -> true
+       | _ -> false
+     in
+     if skip_lib_deps
+     then Command.Args.empty
+     else
+       (let open Resolve.Memo.O in
+        let+ direct_libs = Compilation_context.requires_compile cctx
+        and+ hidden_libs = Compilation_context.requires_hidden cctx in
+        Command.Args.Hidden_deps
+          (Lib_file_deps.deps_of_entries ~opaque ~cm_kind (direct_libs @ hidden_libs)))
+       |> Resolve.Memo.args
+       |> Command.Args.memo
+   in
    let other_cm_files =
      let dep_graph = Ml_kind.Dict.get (Compilation_context.dep_graphs cctx) ml_kind in
      let module_deps = Dep_graph.deps_of dep_graph m in
@@ -414,6 +438,7 @@ let build_cm
             ; Command.Args.S obj_dirs
             ; Command.Args.as_any
                 (Lib_mode.Cm_kind.Map.get (Compilation_context.includes cctx) cm_kind)
+            ; lib_cm_deps
             ; extra_args
             ; As as_parameter_arg
             ; as_argument_for
@@ -512,13 +537,25 @@ let ocamlc_i ~deps cctx (m : Module.t) ~output =
   let ctx = Super_context.context sctx in
   let src = Option.value_exn (Module.file m ~ml_kind:Impl) in
   let sandbox = Compilation_context.sandbox cctx in
+  let opaque = Compilation_context.opaque cctx in
   let cm_deps =
     Action_builder.dyn_paths_unit
       (let open Action_builder.O in
        let+ deps = Ml_kind.Dict.get deps Impl in
        List.concat_map deps ~f:(fun m ->
          [ Path.build (Obj_dir.Module.cm_file_exn obj_dir m ~kind:(Ocaml Cmi)) ]))
   in
+  let lib_cm_deps : _ Command.Args.t =
+    (let open Resolve.Memo.O in
+     let+ direct_libs = Compilation_context.requires_compile cctx
+     and+ hidden_libs = Compilation_context.requires_hidden cctx in
+     Command.Args.Hidden_deps
+       (Lib_file_deps.deps_of_entries
+          ~opaque
+          ~cm_kind:(Ocaml Cmo)
+          (direct_libs @ hidden_libs)))
+    |> Resolve.Memo.args
+  in
   let ocaml_flags = Ocaml_flags.get (Compilation_context.flags cctx) (Ocaml Byte) in
   let modules = Compilation_context.modules cctx in
   let ocaml = Compilation_context.ocaml cctx in
@@ -540,6 +577,7 @@ let ocamlc_i ~deps cctx (m : Module.t) ~output =
                   (Lib_mode.Cm_kind.Map.get
                      (Compilation_context.includes cctx)
                      (Ocaml Cmo))
+              ; lib_cm_deps
               ; opens modules m
               ; A "-short-paths"
               ; A "-i"

src/dune_rules/ocamldep.ml

@@ -199,3 +199,18 @@ let read_immediate_deps_of ~obj_dir ~modules ~ml_kind ~for_ unit =
       |> parse_module_names ~dir:(Obj_dir.dir obj_dir) ~unit ~modules)
     |> Action_builder.memoize (Path.Build.to_string ocamldep_output)
 ;;
+
+let read_immediate_deps_raw_of ~obj_dir ~ml_kind ~for_ unit =
+  match Module.source ~ml_kind unit with
+  | None -> Action_builder.return Module_name.Set.empty
+  | Some source ->
+    let ocamldep_output =
+      Obj_dir.Module.dep obj_dir ~for_ (Immediate (unit, ml_kind)) |> Option.value_exn
+    in
+    Action_builder.lines_of (Path.build ocamldep_output)
+    |> Action_builder.map ~f:(fun lines ->
+      parse_deps_exn ~file:(Module.File.path source) lines
+      |> List.map ~f:Module_name.of_checked_string
+      |> Module_name.Set.of_list)
+    |> Action_builder.memoize (Path.Build.to_string ocamldep_output ^ "-raw")
+;;

src/dune_rules/ocamldep.mli

@@ -32,3 +32,15 @@ val read_immediate_deps_of
   -> for_:Compilation_mode.t
   -> Module.t
   -> Module.t list Action_builder.t
+
+(** [read_immediate_deps_raw_of ~obj_dir ~ml_kind ~for_ unit] returns the raw
+    module names (unresolved) from ocamldep output for the file with kind
+    [ml_kind] of the module [unit]. This includes ALL module references, both
+    intra-stanza and external library modules. If there is no such file with
+    kind [ml_kind], an empty set is returned. *)
+val read_immediate_deps_raw_of
+  :  obj_dir:Path.Build.t Obj_dir.t
+  -> ml_kind:Ml_kind.t
+  -> for_:Compilation_mode.t
+  -> Module.t
+  -> Module_name.Set.t Action_builder.t

test/blackbox-tests/test-cases/per-module-lib-deps/alias-reexport.t

@@ -0,0 +1,94 @@
+Incremental builds with library re-exporting a dependency via module alias.
+
+When library "alias" re-exports library "impl" via (module Impl = Impl),
+a consumer that accesses Impl through Alias must be recompiled when
+impl.cmi changes. The -opaque flag means soft changes (implementation
+only, no cmi change) can safely skip recompilation, but cmi changes
+must always trigger it.
+
+See: https://github.com/ocaml/dune/issues/4572
+
+  $ cat > dune-project <<EOF
+  > (lang dune 3.23)
+  > EOF
+
+A library where we'll perform the changes:
+
+  $ mkdir impl
+  $ cat > impl/dune <<EOF
+  > (library (name impl))
+  > EOF
+  $ cat > impl/impl.ml <<EOF
+  > let foo = "initial build"
+  > EOF
+
+Another library which exposes an alias to impl:
+
+  $ mkdir alias
+  $ cat > alias/dune <<EOF
+  > (library (name alias) (libraries impl))
+  > EOF
+  $ cat > alias/alias.ml <<EOF
+  > module Impl = Impl
+  > EOF
+
+A binary which depends on Alias to access Impl. An empty unused file
+makes this a multi-module executable:
+
+  $ mkdir bin
+  $ cat > bin/dune <<EOF
+  > (executable (name main) (libraries alias))
+  > EOF
+  $ cat > bin/main.ml <<EOF
+  > let () = print_endline Alias.Impl.foo
+  > EOF
+  $ touch bin/unused.ml
+
+The first build succeeds:
+
+  $ dune exec ./bin/main.exe
+  initial build
+
+Soft update — impl.cmi is NOT modified (only implementation changes).
+With -opaque, skipping recompilation of main.ml is correct because
+main.ml doesn't depend on impl's implementation, only its interface:
+
+  $ cat > impl/impl.ml <<EOF
+  > let foo = "second build, no change to cmi"
+  > EOF
+
+  $ dune exec ./bin/main.exe
+  second build, no change to cmi
+
+main.cmx is NOT rebuilt (correct — only impl changed, and -opaque
+means we don't track impl's implementation):
+
+  $ dune trace cat | jq -s 'include "dune"; [.[] | targetsMatchingFilter(test("Main"))]'
+  []
+
+unused.cmx is also NOT rebuilt (correct — it references nothing):
+
+  $ dune trace cat | jq -s 'include "dune"; [.[] | targetsMatchingFilter(test("Unused"))]'
+  []
+
+Hard update — impl.cmi IS modified (new value added). main.ml must
+be recompiled because Alias re-exports Impl and the interface changed:
+
+  $ cat > impl/impl.ml <<EOF
+  > let new_value = 42
+  > let foo = "third build, forced a cmi update"
+  > EOF
+
+  $ dune exec ./bin/main.exe
+  third build, forced a cmi update
+
+Main is rebuilt (necessary — impl.cmi changed and main.ml uses
+Impl through the Alias re-export):
+
+  $ dune trace cat | jq -s 'include "dune"; [.[] | targetsMatchingFilter(test("Main"))] | length | . > 0'
+  true
+
+Unused is NOT rebuilt (correct — it doesn't reference impl):
+
+  $ dune trace cat | jq -s 'include "dune"; [.[] | targetsMatchingFilter(test("Unused"))] | length'
+  0