diff --git a/src/dune_rules/compilation_context.ml b/src/dune_rules/compilation_context.ml index b629d7874d3..afd2bc4e92a 100644 --- a/src/dune_rules/compilation_context.ml +++ b/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 diff --git a/src/dune_rules/lib_file_deps.ml b/src/dune_rules/lib_file_deps.ml index 0aa3440da59..94cbd04b28a 100644 --- a/src/dune_rules/lib_file_deps.ml +++ b/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 diff --git a/src/dune_rules/lib_file_deps.mli b/src/dune_rules/lib_file_deps.mli index 75a3453e64b..977a0f6346c 100644 --- a/src/dune_rules/lib_file_deps.mli +++ b/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 diff --git a/src/dune_rules/module_compilation.ml b/src/dune_rules/module_compilation.ml index 75bcea17c87..56bd7cd5e50 100644 --- a/src/dune_rules/module_compilation.ml +++ b/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,6 +537,7 @@ 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 @@ -519,6 +545,17 @@ let ocamlc_i ~deps cctx (m : Module.t) ~output = 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" diff --git a/src/dune_rules/ocamldep.ml b/src/dune_rules/ocamldep.ml index 22fb8f5a89d..3cc561570e5 100644 --- a/src/dune_rules/ocamldep.ml +++ b/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") +;; diff --git a/src/dune_rules/ocamldep.mli b/src/dune_rules/ocamldep.mli index d455459262e..83e7f41a81a 100644 --- a/src/dune_rules/ocamldep.mli +++ b/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 diff --git a/test/blackbox-tests/test-cases/per-module-lib-deps/alias-reexport.t b/test/blackbox-tests/test-cases/per-module-lib-deps/alias-reexport.t new file mode 100644 index 00000000000..14e7726f496 --- /dev/null +++ b/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 < (lang dune 3.23) + > EOF + +A library where we'll perform the changes: + + $ mkdir impl + $ cat > impl/dune < (library (name impl)) + > EOF + $ cat > impl/impl.ml < let foo = "initial build" + > EOF + +Another library which exposes an alias to impl: + + $ mkdir alias + $ cat > alias/dune < (library (name alias) (libraries impl)) + > EOF + $ cat > alias/alias.ml < 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 < (executable (name main) (libraries alias)) + > EOF + $ cat > bin/main.ml < 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 < 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 < 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