feat: filter -I flags per-module to match dependency filtering#14186
Draft
robinbb wants to merge 12 commits intoocaml:mainfrom
Draft
feat: filter -I flags per-module to match dependency filtering#14186robinbb wants to merge 12 commits intoocaml:mainfrom
robinbb wants to merge 12 commits intoocaml:mainfrom
Conversation
robinbb
added a commit
to robinbb/dune
that referenced
this pull request
Apr 15, 2026
A module's .mli can reference different libraries than its .ml. Read ocamldep output from both and union them so the filtered set includes all cross-library references. This makes the filtering more precise and prepares for per-module -I flag filtering (ocaml#14186) where the .cmi compilation rule no longer serves as a safety net. Signed-off-by: Robin Bate Boerop <me@robinbb.com>
Transparent module aliases (e.g., `module M = OtherLib.Foo`) create cross-library .cmi reads that ocamldep does not report. This test documents the discrepancy: ocamldep only sees the direct library reference, but the compiler follows alias chains and reads .cmi files from transitive libraries at compile time. This property is critical for any future per-module inter-library dependency optimization (ocaml#4572). Signed-off-by: Robin Bate Boerop <me@robinbb.com>
…caml#4572) Use the already-computed ocamldep output to filter inter-library dependencies on a per-module basis. Each module now only depends on .cmi/.cmx files from libraries it actually imports, rather than glob deps on all files from all dependent libraries. This eliminates unnecessary recompilation when a module in a dependency changes but the current module doesn't reference that library. The implementation: - Add read_immediate_deps_raw_of to ocamldep, returning raw module names without filtering against the stanza's module set - Move Hidden_deps from Includes (library-wide) to per-module in build_cm - Add Lib_index mapping module names to libraries, computed from requires_compile and requires_hidden - In build_cm, use ocamldep output + Lib_index to determine which libraries each module actually needs; fall back to all-library glob deps when filtering is not possible (Melange, virtual library implementations, singleton stanzas, alias/root modules) - For local unwrapped libraries, use per-file deps on specific .cmi/.cmx files rather than directory-wide globs Signed-off-by: Robin Bate Boerop <me@robinbb.com> chore: add changelog entry for per-module dependency filtering (ocaml#4572) Signed-off-by: Robin Bate Boerop <me@robinbb.com>
Remove deps_of_module, cm_kinds_for, and the Module.t option from Lib_index and deps_of_entries. All entries use glob deps (no per-file deps on individual modules within libraries), so the Some m path was dead code. Simplify the types accordingly: Lib_index maps module names to Lib.t list, and deps_of_entries takes Lib.t list directly. Per-module filtering within unwrapped libraries can be revisited in the future once the dependency cone of individual modules is tracked. Signed-off-by: Robin Bate Boerop <me@robinbb.com>
Extract the per-module inter-library dependency filtering logic from build_cm into a shared filtered_lib_deps helper. Use it in both build_cm and ocamlc_i so that inferred .mli generation also benefits from per-module filtering. Signed-off-by: Robin Bate Boerop <me@robinbb.com>
Use the existing Lib.closure for transitive library dependency closure instead of a hand-rolled close_over function. Lib.closure follows the same requires path and the libraries already passed overlap checks when the compilation context was built. Signed-off-by: Robin Bate Boerop <me@robinbb.com>
Remove the main_module_name special case in lib_index construction. For wrapped libraries, entry_modules returns the wrapper module — the same name that main_module_name provided. Since all entries now use glob deps (no per-file Module.t), the two branches were equivalent. Signed-off-by: Robin Bate Boerop <me@robinbb.com>
- Extract all_libs helper to deduplicate requires_compile @ requires_hidden - Expand Module.kind wildcards to explicit variants - Reword fallback comment to not reference removed code - Drop trivial punctuation-only changes to unrelated test files Signed-off-by: Robin Bate Boerop <me@robinbb.com>
Signed-off-by: Robin Bate Boerop <me@robinbb.com>
Signed-off-by: Robin Bate Boerop <me@robinbb.com>
A module's .mli can reference different libraries than its .ml. Read ocamldep output from both and union them so the filtered set includes all cross-library references. This makes the filtering more precise and prepares for per-module -I flag filtering (ocaml#14186) where the .cmi compilation rule no longer serves as a safety net. Signed-off-by: Robin Bate Boerop <me@robinbb.com>
Document the desired behavior where -I flags are restricted to only include directories for libraries a module actually references. This test currently fails because -I flags are computed per-context, not per-module. The implementation will follow. Signed-off-by: Robin Bate Boerop <me@robinbb.com>
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>
robinbb
force-pushed
the
robinbb-issue-4572-filtered-includes
branch
from
1800860 to
7c18a92
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
1 participant
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.
Follow-up to #14116. When per-module dependency filtering determines which
libraries a module actually references, also restrict the
-I/-Hcompilerflags to only include directories for those libraries.
Previously, #14116 filtered the hidden deps (build dependencies) but left
-Iflags unchanged — all libraries in the compilation context were alwaysincluded. This meant:
.cmifiles viaoverly-broad
-Iflags, even if hidden deps were wrongNow both
-Iflags and hidden deps are computed from the same filteredlibrary set, making builds more precise and improving cache behavior.
Suggested by @art-w in
#14116 (comment)
Depends on #14116.
per-module-lib-deps/filtered-includes.tverifies per-module-Ifiltering