fix: run ocamldep for single-module stanzas (#4572)

Dune previously skipped ocamldep for singleton stanzas as an optimization
(no intra-stanza deps to compute). This prevented per-module library
dependency filtering from working, since it relies on ocamldep output to
determine which libraries a module references.

Remove the singleton shortcut from dep_rules.ml so ocamldep runs for all
stanzas. The cost is negligible (one extra ocamldep invocation per
single-module stanza) and enables the filtering optimization for all cases.

Signed-off-by: Robin Bate Boerop <me@robinbb.com>

Author: robinbb

src/dune_rules/dep_rules.ml

@@ -131,9 +131,6 @@ let deps_of_vlib_module ~obj_dir ~vimpl ~dir ~sctx ~ml_kind ~for_ sourced_module
     Ocamldep.read_deps_of ~obj_dir:vlib_obj_dir ~modules ~ml_kind ~for_ m
 ;;
 
-(** Tests whether a set of modules is a singleton *)
-let has_single_file modules = Option.is_some @@ Modules.With_vlib.as_singleton modules
-
 let rec deps_of
           ~obj_dir
           ~modules
@@ -153,7 +150,7 @@ let rec deps_of
        | Root | Alias _ -> true
        | _ -> false)
   in
-  if is_alias_or_root || has_single_file modules
+  if is_alias_or_root
   then Memo.return (Action_builder.return [])
   else (
     let skip_if_source_absent f sourced_module =
@@ -195,18 +192,15 @@ let read_deps_of_module ~modules ~obj_dir dep ~for_ =
     in
     List.singleton interface_module |> Action_builder.return
   | _ ->
-    if has_single_file modules
-    then Action_builder.return []
-    else (
-      match dep with
-      | Immediate (unit, ml_kind) ->
-        Ocamldep.read_immediate_deps_of ~obj_dir ~modules ~ml_kind ~for_ unit
-      | Transitive (unit, ml_kind) ->
-        let open Action_builder.O in
-        let+ deps = Ocamldep.read_deps_of ~obj_dir ~modules ~ml_kind ~for_ unit in
-        (match Modules.With_vlib.alias_for modules unit with
-         | [] -> deps
-         | aliases -> aliases @ deps))
+    (match dep with
+     | Immediate (unit, ml_kind) ->
+       Ocamldep.read_immediate_deps_of ~obj_dir ~modules ~ml_kind ~for_ unit
+     | Transitive (unit, ml_kind) ->
+       let open Action_builder.O in
+       let+ deps = Ocamldep.read_deps_of ~obj_dir ~modules ~ml_kind ~for_ unit in
+       (match Modules.With_vlib.alias_for modules unit with
+        | [] -> deps
+        | aliases -> aliases @ deps))
 ;;
 
 let read_immediate_deps_of ~obj_dir ~modules ~ml_kind ~for_ m =
@@ -231,15 +225,12 @@ let for_module ~obj_dir ~modules ~sandbox ~impl ~dir ~sctx ~for_ module_ =
 ;;
 
 let rules ~obj_dir ~modules ~sandbox ~impl ~sctx ~dir ~for_ =
-  match Modules.With_vlib.as_singleton modules with
-  | Some m -> Memo.return (Dep_graph.Ml_kind.dummy m)
-  | None ->
-    dict_of_func_concurrently (fun ~ml_kind ->
-      let+ per_module =
-        Modules.With_vlib.obj_map modules
-        |> Parallel_map.parallel_map ~f:(fun _obj_name m ->
-          deps_of ~obj_dir ~modules ~sandbox ~impl ~sctx ~dir ~ml_kind ~for_ m)
-      in
-      Dep_graph.make ~dir ~per_module)
-    |> Memo.map ~f:(Dep_graph.Ml_kind.for_module_compilation ~modules)
+  dict_of_func_concurrently (fun ~ml_kind ->
+    let+ per_module =
+      Modules.With_vlib.obj_map modules
+      |> Parallel_map.parallel_map ~f:(fun _obj_name m ->
+        deps_of ~obj_dir ~modules ~sandbox ~impl ~sctx ~dir ~ml_kind ~for_ m)
+    in
+    Dep_graph.make ~dir ~per_module)
+  |> Memo.map ~f:(Dep_graph.Ml_kind.for_module_compilation ~modules)
 ;;

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

@@ -1,4 +1,4 @@
-Per-module filtering does not help single-module library consumers.
+Per-module filtering works for single-module library consumers.
 
 When a consumer library has only one module, dune skips ocamldep for that
 stanza. Without ocamldep data, the per-module filtering has nothing to work
@@ -59,8 +59,8 @@ Modify only the unused module:
   > let new_fn () = "new"
   > EOF
 
-uses_alpha is recompiled even though it only references Alpha, not Unused:
+uses_alpha is no longer recompiled because it only references Alpha, not Unused:
 
   $ dune build ./main.exe
   $ dune trace cat | jq -s 'include "dune"; [.[] | targetsMatchingFilter(test("uses_alpha"))] | length'
-  2
+  0

test/blackbox-tests/test-cases/virtual-libraries/github2896.t

@@ -23,8 +23,8 @@ where vlib is a virtual library, and impl implements this library.
   Error: Library "vlib" was pulled in.
   -> required by library "lib" in _build/default/lib
   -> required by library "impl" in _build/default/impl
-  -> required by _build/default/impl/.impl.objs/byte/vlib.cmo
-  -> required by _build/default/impl/impl.cma
+  -> required by _build/default/impl/.impl.objs/native/vlib.cmx
+  -> required by _build/default/impl/impl.a
   -> required by alias impl/all
   [1]
 
@@ -37,6 +37,5 @@ The implementation impl was built, but it's not usable:
   -> required by library "lib" in _build/default/lib
   -> required by library "impl" in _build/default/impl
   -> required by executable foo in dune:1
-  -> required by _build/default/.foo.eobjs/native/dune__exe__Foo.cmx
   -> required by _build/default/foo.exe
   [1]

test/blackbox-tests/test-cases/virtual-libraries/virtual-modules-excluded-by-modules-field.t

@@ -40,11 +40,6 @@ X is warned about:
   1 | module type F = X
                       ^
   Error: Unbound module type X
-  File "src/impl/dune", lines 1-3, characters 0-40:
-  1 | (library
-  2 |  (name impl)
-  3 |  (implements foo))
-  Error: No rule found for src/.foo.objs/y.impl.all-deps
   [1]
 
 In 3.11 onwards this warning becomes an error
@@ -76,6 +71,5 @@ This should be ignored if we are in vendored_dirs
   Error: No implementation found for virtual library "foo" in
   _build/default/src.
   -> required by executable bar in dune:3
-  -> required by _build/default/.bar.eobjs/native/dune__exe__Bar.cmx
   -> required by _build/default/bar.exe
   [1]

test/blackbox-tests/test-cases/virtual-libraries/vlib-wrong-default-impl.t/run.t

@@ -4,7 +4,6 @@ library is not actually an implementation of the virtual library.
   $ dune build @default
   Error: "not_an_implem" is not an implementation of "vlibfoo".
   -> required by executable exe in exe/dune:2
-  -> required by _build/default/exe/.exe.eobjs/native/dune__exe__Exe.cmx
   -> required by _build/default/exe/exe.exe
   -> required by alias exe/default in exe/dune:5
   [1]