feat: per-module [-I]/[-H] include flags via [kept_libs]#14518
Draft
robinbb wants to merge 1 commit into
Draft
Conversation
This was referenced May 13, 2026
c82233e to
0373a72
Compare
c50afa4 to
38a7871
Compare
0373a72 to
a94a5c6
Compare
38a7871 to
8634a30
Compare
8655699 to
e0a6cdf
Compare
8634a30 to
323cf88
Compare
e0a6cdf to
6a96f3d
Compare
323cf88 to
b8bba4b
Compare
6a96f3d to
90b06f8
Compare
b8bba4b to
c73a510
Compare
bd0ca68 to
37cc59a
Compare
c73a510 to
d12a8e5
Compare
37cc59a to
fca9936
Compare
d12a8e5 to
9dfc6ed
Compare
a7491f0 to
929749a
Compare
9dfc6ed to
1c8148e
Compare
There was a problem hiding this comment.
Pull request overview
Adds per-module filtering of OCaml include flags so compile rules only see -I/-H for libraries actually reached by that module’s ocamldep reference set (via kept_libs), reducing unnecessary rule cache invalidations/rebuilds when unrelated sibling libraries are present in (libraries ...).
Changes:
- Introduce
Compilation_context.filtered_include_flagsto compute-I/-Hrestricted to aLib.Set.tof “kept” libraries. - Wire
Module_compilation.lib_deps_for_moduleto usekept_libsto produce per-module include args (instead of cctx-wide includes) alongside the deps set. - Promote/adjust blackbox tests to assert the new per-module include-flag behavior and the resulting no-rebuild trace when adding an unreferenced sibling lib.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| test/blackbox-tests/test-cases/per-module-lib-deps/per-module-include-flags.t | Updates expectations so only the referenced dep library’s objdir appears in -I flags. |
| test/blackbox-tests/test-cases/per-module-lib-deps/add-unreferenced-sibling-lib.t | Updates expectations to assert no rebuild occurs when adding an unreferenced library to (libraries ...). |
| src/dune_rules/module_compilation.ml | Uses kept_libs to request per-module include flags via Compilation_context.filtered_include_flags. |
| src/dune_rules/compilation_context.mli | Exposes the new filtered_include_flags API with documentation describing kept_libs semantics. |
| src/dune_rules/compilation_context.ml | Implements filtered_include_flags by filtering requires_compile/requires_hidden against kept_libs and generating -I/-H. |
[Compilation_context.filtered_include_flags]: new function returning the [-I]/[-H] flags restricted to [kept_libs]. The cctx's [requires_compile] and [requires_hidden] are each filtered by [Lib.Set.mem kept_libs]; the result is built as a single [Command.Args.t] under [Action_builder]. No caching yet — each call recomputes; a follow-up adds the cache. [Module_compilation.lib_deps_for_module]: the tight branch was already threading [kept_libs] through the classification fold (it had been unused at L4-L5). Now wired into [filtered_include_flags]; the returned pair is [(filtered_include_flags, tight_deps + glob_deps)] instead of [(cctx_includes_for_cm_kind (), …)]. Behavioural effect: a consumer module's compile command sees [-I] / [-H] only for libraries its ocamldep reference set actually reaches. Adding an unreferenced sibling to the cctx's [(libraries ...)] no longer changes the consumer module's compile command, so the rule does not re-execute. Tests: - [per-module-include-flags.t]: promoted — [-I] for the unreferenced [unrelated_lib] no longer appears in the consumer's compile rule. - [add-unreferenced-sibling-lib.t]: promoted — adding an unreferenced sibling lib produces no rebuild for consumer modules. Signed-off-by: Robin Bate Boerop <me@robinbb.com>
929749a to
de5df16
Compare
1c8148e to
3492d57
Compare
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 6 of 9 of #14492.
Compilation_context.filtered_include_flags: new function returning-I/-Hflags restricted tokept_libs. The cctx'srequires_compileandrequires_hiddenare each filtered byLib.Set.mem kept_libs. No caching yet — layer 7 adds it.Module_compilation.lib_deps_for_module: the tight branch was already producingkept_libs(unused at layers 4–5); now wired intofiltered_include_flags. Returns the filtered include args instead of the cctx-wideIncludes.Effect: a consumer's compile command sees
-I/-Honly for libraries its ocamldep reference set actually reaches. Adding an unreferenced sibling to the cctx's(libraries ...)no longer changes the compile command, so the rule does not re-execute.Promotes
per-module-include-flags.tandadd-unreferenced-sibling-lib.t— both deferred from layer 4 because their assertions depend on this filter.Stack: rebases on #14517. Next: #14519.
Part of #14492. Related to #4572.