refactor: route lib file deps through [lib_deps_for_module]#14515
Merged
robinbb merged 1 commit intoMay 23, 2026
Merged
Conversation
This was referenced May 13, 2026
robinbb
force-pushed
the
robinbb-14492-l2-cctx-fields
branch
from
c804917 to
d37e419
Compare
robinbb
force-pushed
the
robinbb-14492-l3-includes-refactor
branch
from
84710e9 to
d197872
Compare
There was a problem hiding this comment.
Pull request overview
Refactors how module compilation rules obtain library include flags and library file dependencies by routing lib-file deps through Module_compilation.lib_deps_for_module/lib_cm_deps, while simplifying Compilation_context.Includes to carry only -I/-H flags (dropping Hidden_deps emission). This is part of the staged work toward per-module library dependency filtering (#14492 / #4572) and is intended to be behavior-equivalent.
Changes:
- Added
Module_compilation.lib_deps_for_module(scaffold “always-glob” form) pluslib_cm_deps, and switchedbuild_cm/ocamlc_ito consume dynamic lib deps viaCommand.Args.Dyn. - Simplified
Compilation_context.Includes.maketo no longer thread~opaqueor emitHidden_deps; it now produces only include flags. - Removed the unused
Lib_file_deps.deps_with_extshelper (previously only used by the oldCmxbranch inIncludes.make).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/dune_rules/module_compilation.ml | Introduces per-module lib deps plumbing (lib_deps_for_module/lib_cm_deps) and updates compilation actions to use them. |
| src/dune_rules/compilation_context.ml | Refactors Includes.make to produce only -I/-H flags (no Hidden_deps, no ~opaque). |
| src/dune_rules/lib_file_deps.mli | Removes the deps_with_exts declaration. |
| src/dune_rules/lib_file_deps.ml | Removes the deps_with_exts implementation. |
robinbb
force-pushed
the
robinbb-14492-l2-cctx-fields
branch
3 times, most recently
from
7459ded to
22fcd5f
Compare
robinbb
force-pushed
the
robinbb-14492-l3-includes-refactor
branch
2 times, most recently
from
d3418da to
246a914
Compare
robinbb
force-pushed
the
robinbb-14492-l2-cctx-fields
branch
3 times, most recently
from
2d4b53c to
87a2380
Compare
robinbb
force-pushed
the
robinbb-14492-l3-includes-refactor
branch
from
246a914 to
d88da28
Compare
robinbb
force-pushed
the
robinbb-14492-l2-cctx-fields
branch
2 times, most recently
from
82bc32c to
7f288f7
Compare
robinbb
force-pushed
the
robinbb-14492-l2-cctx-fields
branch
5 times, most recently
from
b5c7e42 to
c0de663
Compare
robinbb
force-pushed
the
robinbb-14492-l3-includes-refactor
branch
from
d88da28 to
8dcb05b
Compare
robinbb
force-pushed
the
robinbb-14492-l2-cctx-fields
branch
2 times, most recently
from
693843e to
87f8132
Compare
robinbb
force-pushed
the
robinbb-14492-l3-includes-refactor
branch
from
8dcb05b to
60b92c8
Compare
robinbb
force-pushed
the
robinbb-14492-l2-cctx-fields
branch
from
87f8132 to
6863ae5
Compare
robinbb
force-pushed
the
robinbb-14492-l3-includes-refactor
branch
from
60b92c8 to
dde100d
Compare
robinbb
force-pushed
the
robinbb-14492-l2-cctx-fields
branch
from
6863ae5 to
4298335
Compare
robinbb
force-pushed
the
robinbb-14492-l3-includes-refactor
branch
from
dde100d to
63c3651
Compare
robinbb
force-pushed
the
robinbb-14492-l2-cctx-fields
branch
2 times, most recently
from
59744cf to
ff3952d
Compare
robinbb
force-pushed
the
robinbb-14492-l3-includes-refactor
branch
from
63c3651 to
416e701
Compare
Alizter
added a commit
that referenced
this pull request
May 22, 2026
…h cctx (#14514) Layer 2 of 9 of #14492. Threads `lib_index`, `has_virtual_impl`, and `pps_runtime_libs` through `Compilation_context.t`. Adds `build_lib_index` (constructs the `Lib_index` from the cctx's libs, indexing on post-pp modules for non-staged-ppx local libs). Wires `~pps_runtime_libs` at all nine `Compilation_context.create` call sites: computed from `Lib.Compile.pps + Lib.ppx_runtime_deps` at `Lib_rules`, `Exe_rules`, `Cinaps`, and `Toplevel`; `Resolve.Memo.return []` at `Ppx_driver`, `Utop`, `Mdx`, `Inline_tests`, and `Melange_rules`. Exposes `Dep_graph.dir` / `mem` for layer 4's `can_filter` guard. Adds `Modules.as_singleton`. `pps_runtime_libs` is a required parameter of `Compilation_context.create` — no implicit default — so a new caller cannot silently drop ppx runtime libs from the dep set under narrowing (per @Alizter's review feedback). Fields are populated but never consumed by L2's own logic; consumers arrive in layer 4. Stack: rebases on `main` (L1 merged in #14513). Next: #14515. Part of #14492. Related to #4572.
Base automatically changed from
robinbb-14492-l2-cctx-fields
to
robinbb-14492-test-stack-integration
May 22, 2026 19:17
Behavior-equivalent restructuring that opens the lib-deps computation to per-module filtering in a follow-up. No test promotions. [Compilation_context.Includes.make] previously emitted both [-I]/[-H] include flags AND [Hidden_deps] for the cctx's libs in a single [Command.Args.t]. The opaque-aware [Cmx] case duplicated the deps logic that already exists in [Lib_file_deps.deps_of_entries]. Simplified: [Includes.t] now carries only the include flags; the [~opaque] parameter (and the [for_module_generated_at_link_time] call site) drops out. [Module_compilation]: - [lib_deps_for_module]: scaffold form, returns [(cctx_includes, deps_of_entries libs)] where [libs = requires_compile @ requires_hidden]. The per-module tight filter activates in a follow-up; arguments [obj_dir], [for_], [dep_graph], [ml_kind], [mode] are threaded but ignored here. - [lib_cm_deps]: wraps [lib_deps_for_module] with [Action_builder.dyn_deps], yielding the include args and registering the lib file deps. - [build_cm]: gated route — [Alias _] (non-stdlib) and [Wrapped_compat] modules short-circuit to the cctx's now-flag-only [Includes] (no lib deps, matching prior behavior since [Includes.empty] was used for these); all other module kinds call [lib_cm_deps]. Replace the in-line [Includes] lookup at the [Command.run] site with [Command.Args.Dyn lib_cm_deps]. - [ocamlc_i]: same swap. Combined: every consumer that previously read [-I]/[-H] + [Hidden_deps] from [Includes] now reads [-I]/[-H] from [Includes] and the deps from [deps_of_entries] (via [lib_cm_deps]). Same flags, same deps. The [Alias]/[Wrapped_compat] short-circuit preserves the prior "no-lib-deps" behavior for those module kinds. Signed-off-by: Robin Bate Boerop <me@robinbb.com>
robinbb
force-pushed
the
robinbb-14492-test-stack-integration
branch
from
580fc34 to
e0f2a6d
Compare
robinbb
force-pushed
the
robinbb-14492-l3-includes-refactor
branch
from
416e701 to
1c1953d
Compare
Alizter
approved these changes
May 23, 2026
robinbb
merged commit c8ab405
into
robinbb-14492-test-stack-integration
31 checks passed
robinbb
added a commit
that referenced
this pull request
May 23, 2026
Layer 3 of 9 of #14492. Behaviour-equivalent refactor. `Compilation_context.Includes.make` drops `~opaque` and stops emitting `Hidden_deps`; `Includes.t` carries only `-I`/`-H` flags. `Module_compilation.lib_deps_for_module` (scaffold form: always-glob) and `lib_cm_deps` wire the dep set back in via `deps_of_entries`. `build_cm` and `ocamlc_i` are switched to consume `lib_cm_deps`; `Alias` / `Wrapped_compat` short-circuit to the cctx's now-flag-only `Includes`. `Lib_file_deps.deps_with_exts` removed — only used by the old `Cmx` arm of `Includes.make`. Combined effect: same `-I` / `-H`, same lib file deps. Stack: rebases on #14514. Next: #14516. Part of #14492. Related to #4572. Signed-off-by: Robin Bate Boerop <me@robinbb.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Layer 3 of 9 of #14492. Behaviour-equivalent refactor.
Compilation_context.Includes.makedrops~opaqueand stops emittingHidden_deps;Includes.tcarries only-I/-Hflags.Module_compilation.lib_deps_for_module(scaffold form: always-glob) andlib_cm_depswire the dep set back in viadeps_of_entries.build_cmandocamlc_iare switched to consumelib_cm_deps;Alias/Wrapped_compatshort-circuit to the cctx's now-flag-onlyIncludes.Lib_file_deps.deps_with_extsremoved — only used by the oldCmxarm ofIncludes.make.Combined effect: same
-I/-H, same lib file deps.Stack: rebases on #14514. Next: #14516.
Part of #14492. Related to #4572.