Skip to content

Commit 1800860

Browse files
committed
feat: filter -I flags per-module to match dependency filtering
When per-module dependency filtering determines which libraries a module actually references, also restrict the -I/-H flags to only include directories for those libraries. Previously, hidden deps were filtered but -I flags included all libraries, allowing accidental compilation success on clean builds while incremental rebuilds could fail. This also improves caching: adding a new dependency no longer invalidates the compilation cache for unrelated modules. Both build_cm and ocamlc_i now compute -I flags and hidden deps together from the filtered library set. Signed-off-by: Robin Bate Boerop <me@robinbb.com>
1 parent 7a47080 commit 1800860

2 files changed

Lines changed: 86 additions & 60 deletions

File tree

src/dune_rules/module_compilation.ml

Lines changed: 85 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -14,14 +14,17 @@ let all_libs cctx =
1414
d @ h
1515
;;
1616

17-
let filtered_lib_deps ~cctx ~obj_dir ~ml_kind ~for_ ~dep_graph ~opaque ~cm_kind ~mode m =
17+
(* Compute the filtered set of libraries that a module actually references.
18+
Returns the library list, which callers use for both -I flags and hidden
19+
deps. Falls back to all [libs] when virtual implementations are present. *)
20+
let filtered_libs_for_module ~cctx ~obj_dir ~ml_kind ~for_ ~dep_graph ~mode m =
1821
let open Action_builder.O in
1922
let* libs = Resolve.Memo.read (all_libs cctx) in
2023
let has_virtual_impl =
2124
List.exists libs ~f:(fun lib -> Option.is_some (Lib.implements lib))
2225
in
2326
if has_virtual_impl
24-
then Action_builder.return ((), Lib_file_deps.deps_of_entries ~opaque ~cm_kind libs)
27+
then Action_builder.return libs
2528
else
2629
let* lib_index = Resolve.Memo.read (Compilation_context.lib_index cctx) in
2730
let* raw_deps_m = Ocamldep.read_immediate_deps_raw_of ~obj_dir ~ml_kind ~for_ m in
@@ -67,8 +70,7 @@ let filtered_lib_deps ~cctx ~obj_dir ~ml_kind ~for_ ~dep_graph ~opaque ~cm_kind
6770
let libs_set = Table.create (module Lib) (List.length libs) in
6871
List.iter libs ~f:(fun lib -> Table.set libs_set lib ());
6972
let+ closed = Resolve.Memo.read (Lib.closure filtered_libs ~linking:false ~for_) in
70-
let all_libs = List.filter closed ~f:(Table.mem libs_set) in
71-
(), Lib_file_deps.deps_of_entries ~opaque ~cm_kind all_libs
73+
List.filter closed ~f:(Table.mem libs_set)
7274
;;
7375

7476
(* Arguments for the compiler to prevent it from being too clever.
@@ -391,34 +393,50 @@ let build_cm
391393
&& Module.has m ~ml_kind
392394
&& not (Virtual_rules.is_implementation (Compilation_context.implements cctx))
393395
in
394-
(* Fallback lib deps: when per-module filtering is not possible, depend
395-
on all .cmi/.cmx files from all required libraries. *)
396-
let lib_cm_deps_args =
397-
if skip_lib_deps || can_filter
398-
then Command.Args.empty
399-
else
400-
(let open Resolve.Memo.O in
401-
let+ libs = all_libs cctx in
402-
Command.Args.Hidden_deps (Lib_file_deps.deps_of_entries ~opaque ~cm_kind libs))
403-
|> Resolve.Memo.args
404-
|> Command.Args.memo
405-
in
406-
(* Dynamic lib deps: used when per-module filtering is possible. *)
407-
let lib_cm_deps_filtered =
408-
if not can_filter
409-
then Action_builder.return ()
396+
(* Per-module library deps and includes. When filtering is possible, both
397+
hidden deps and -I flags are restricted to the libraries the module
398+
actually references. Otherwise, fall back to all required libraries. *)
399+
let lib_deps_and_includes =
400+
if skip_lib_deps
401+
then Action_builder.return (Command.Args.empty, Command.Args.empty)
402+
else if can_filter
403+
then
404+
let open Action_builder.O in
405+
let+ filtered =
406+
filtered_libs_for_module ~cctx ~obj_dir ~ml_kind ~for_ ~dep_graph ~mode m
407+
in
408+
let project = Compilation_context.scope cctx |> Scope.project in
409+
let lib_config = (Compilation_context.ocaml cctx).lib_config in
410+
let includes =
411+
Lib_flags.L.include_flags
412+
~project
413+
~direct_libs:filtered
414+
~hidden_libs:[]
415+
(Lib_mode.of_cm_kind cm_kind)
416+
lib_config
417+
in
418+
let deps =
419+
Command.Args.Hidden_deps
420+
(Lib_file_deps.deps_of_entries ~opaque ~cm_kind filtered)
421+
in
422+
includes, deps
410423
else
411-
Action_builder.dyn_deps
412-
(filtered_lib_deps
413-
~cctx
414-
~obj_dir
415-
~ml_kind
416-
~for_
417-
~dep_graph
418-
~opaque
419-
~cm_kind
420-
~mode
421-
m)
424+
let open Action_builder.O in
425+
let+ libs = Resolve.Memo.read (all_libs cctx) in
426+
let project = Compilation_context.scope cctx |> Scope.project in
427+
let lib_config = (Compilation_context.ocaml cctx).lib_config in
428+
let includes =
429+
Lib_flags.L.include_flags
430+
~project
431+
~direct_libs:libs
432+
~hidden_libs:[]
433+
(Lib_mode.of_cm_kind cm_kind)
434+
lib_config
435+
in
436+
let deps =
437+
Command.Args.Hidden_deps (Lib_file_deps.deps_of_entries ~opaque ~cm_kind libs)
438+
in
439+
includes, deps
422440
in
423441
let other_cm_files =
424442
let dep_graph = Ml_kind.Dict.get (Compilation_context.dep_graphs cctx) ml_kind in
@@ -538,7 +556,6 @@ let build_cm
538556
?loc:(Compilation_context.loc cctx)
539557
(let open Action_builder.With_targets.O in
540558
Action_builder.with_no_targets other_cm_files
541-
>>> Action_builder.with_no_targets lib_cm_deps_filtered
542559
>>> Command.run
543560
~dir:(Path.build (Context.build_dir ctx))
544561
compiler
@@ -547,9 +564,9 @@ let build_cm
547564
; cmt_args
548565
; cms_args
549566
; Command.Args.S obj_dirs
550-
; Command.Args.as_any
551-
(Lib_mode.Cm_kind.Map.get (Compilation_context.includes cctx) cm_kind)
552-
; Command.Args.as_any lib_cm_deps_args
567+
; Dyn
568+
(Action_builder.map lib_deps_and_includes ~f:(fun (includes, deps) ->
569+
Command.Args.S [ includes; deps ]))
553570
; extra_args
554571
; As as_parameter_arg
555572
; as_argument_for
@@ -655,7 +672,7 @@ let ocamlc_i ~deps cctx (m : Module.t) ~output =
655672
List.concat_map deps ~f:(fun m ->
656673
[ Path.build (Obj_dir.Module.cm_file_exn obj_dir m ~kind:(Ocaml Cmi)) ]))
657674
in
658-
let lib_cm_deps =
675+
let lib_deps_and_includes =
659676
let opaque = Compilation_context.opaque cctx in
660677
let for_ = Compilation_context.for_ cctx in
661678
let ml_kind = Ml_kind.Impl in
@@ -669,24 +686,35 @@ let ocamlc_i ~deps cctx (m : Module.t) ~output =
669686
&& Module.has m ~ml_kind
670687
&& not (Virtual_rules.is_implementation (Compilation_context.implements cctx))
671688
in
672-
if can_filter
673-
then
674-
Action_builder.dyn_deps
675-
(filtered_lib_deps
676-
~cctx
677-
~obj_dir
678-
~ml_kind
679-
~for_
680-
~dep_graph
681-
~opaque
682-
~cm_kind:(Ocaml Cmo)
683-
~mode:(Ocaml Byte)
684-
m)
685-
else
686-
Action_builder.dyn_deps
687-
(let open Action_builder.O in
688-
let+ libs = Resolve.Memo.read (all_libs cctx) in
689-
(), Lib_file_deps.deps_of_entries ~opaque ~cm_kind:(Ocaml Cmo) libs)
689+
let open Action_builder.O in
690+
let+ libs =
691+
if can_filter
692+
then
693+
filtered_libs_for_module
694+
~cctx
695+
~obj_dir
696+
~ml_kind
697+
~for_
698+
~dep_graph
699+
~mode:(Ocaml Byte)
700+
m
701+
else Resolve.Memo.read (all_libs cctx)
702+
in
703+
let project = Compilation_context.scope cctx |> Scope.project in
704+
let lib_config = (Compilation_context.ocaml cctx).lib_config in
705+
let includes =
706+
Lib_flags.L.include_flags
707+
~project
708+
~direct_libs:libs
709+
~hidden_libs:[]
710+
(Ocaml Byte)
711+
lib_config
712+
in
713+
let deps =
714+
Command.Args.Hidden_deps
715+
(Lib_file_deps.deps_of_entries ~opaque ~cm_kind:(Ocaml Cmo) libs)
716+
in
717+
includes, deps
690718
in
691719
let ocaml_flags = Ocaml_flags.get (Compilation_context.flags cctx) (Ocaml Byte) in
692720
let modules = Compilation_context.modules cctx in
@@ -698,18 +726,16 @@ let ocamlc_i ~deps cctx (m : Module.t) ~output =
698726
~file_targets:[ output ]
699727
(let open Action_builder.With_targets.O in
700728
Action_builder.with_no_targets cm_deps
701-
>>> Action_builder.with_no_targets lib_cm_deps
702729
>>> Command.run
703730
(Ok ocaml.ocamlc)
704731
~dir:(Path.build (Context.build_dir ctx))
705732
~stdout_to:output
706733
[ Command.Args.dyn ocaml_flags
707734
; A "-I"
708735
; Path (Path.build (Obj_dir.byte_dir obj_dir))
709-
; Command.Args.as_any
710-
(Lib_mode.Cm_kind.Map.get
711-
(Compilation_context.includes cctx)
712-
(Ocaml Cmo))
736+
; Dyn
737+
(Action_builder.map lib_deps_and_includes ~f:(fun (includes, deps) ->
738+
Command.Args.S [ includes; deps ]))
713739
; opens modules m
714740
; A "-short-paths"
715741
; A "-i"

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ module's own obj dir):
5757

5858
$ extract_includes () {
5959
> dune rules "$1" 2>&1 | tr -s '() \n' '\n' | \
60-
> awk '/^-I$/{getline; print}' | grep -v '\.main\.' | sort -u
60+
> awk '/^-I$/{getline; print}' | grep -v '\.main\.' | sort -u || true
6161
> }
6262

6363
The module uses_mylib references Mylib, so its -I flags should include

0 commit comments

Comments
 (0)