refactor: move library file deps from Includes to per-module in build_cm (#4572)#14030
Closed
robinbb wants to merge 3 commits intoocaml:mainfrom
Closed
refactor: move library file deps from Includes to per-module in build_cm (#4572)#14030robinbb wants to merge 3 commits intoocaml:mainfrom
robinbb wants to merge 3 commits intoocaml:mainfrom
Conversation
robinbb
force-pushed
the
robinbb-issue-4572-refactor
branch
from
fc3f91e to
bd43f2d
Compare
robinbb
force-pushed
the
robinbb-issue-4572-refactor
branch
from
bd43f2d to
c38cdca
Compare
Collaborator
Author
|
@rgrinberg Here is the refactor that I think you were suggesting in PR #14021. It will be easier to read after #14017 is merged. Subsequently, #14021 will be easier to review, once rebased. |
robinbb
force-pushed
the
robinbb-issue-4572-refactor
branch
4 times, most recently
from
142f3f8 to
a79bb9d
Compare
robinbb
force-pushed
the
robinbb-issue-4572-refactor
branch
3 times, most recently
from
5b3066a to
21284b3
Compare
robinbb
force-pushed
the
robinbb-issue-4572-refactor
branch
4 times, most recently
from
4b6e10e to
817fe3f
Compare
Collaborator
Author
|
The problem WAS due to THIS Dune, which is used in the "install deps" phases of some CI runs. So, I must fix the problem. Meanwhile, this PR must convert to draft mode. |
robinbb
force-pushed
the
robinbb-issue-4572-refactor
branch
2 times, most recently
from
ee0e316 to
676d5aa
Compare
robinbb
force-pushed
the
robinbb-issue-4572-refactor
branch
3 times, most recently
from
05de02d to
34c14f5
Compare
robinbb
force-pushed
the
robinbb-issue-4572-refactor
branch
from
34c14f5 to
0ad4949
Compare
1 task
robinbb
force-pushed
the
robinbb-issue-4572-refactor
branch
2 times, most recently
from
8c3d290 to
82bf016
Compare
robinbb
force-pushed
the
robinbb-issue-4572-refactor
branch
from
82bf016 to
95b4fce
Compare
rgrinberg
pushed a commit
that referenced
this pull request
Apr 10, 2026
…lds (#4572) (#14100) ## Summary - Add baseline tests documenting existing behavior before the refactor that moves `Hidden_deps` from `Includes` to per-module in `build_cm` - `lib-deps-preserved.t`: every non-alias module declares glob deps on its library dependencies' `.cmi` files - `sandbox-lib-deps.t`: sandboxed package builds have dependency libraries' `.cmi` files available when a library depends on another library These tests pass on `main` today and establish a baseline for the refactor in #14030. Ref: #4572 Signed-off-by: Robin Bate Boerop <me@robinbb.com>
…ml#4572) Add test verifying that when a library re-exports a dependency via a module alias (module Impl = Impl), incremental builds correctly recompile consumers when the re-exported library's .cmi changes. Soft changes (implementation only, no .cmi change) can safely skip recompilation due to -opaque. Hard changes (.cmi modified) must trigger recompilation to avoid inconsistent interface assumptions. Reproduction case from @art-w. Signed-off-by: Robin Bate Boerop <me@robinbb.com>
Move Hidden_deps (library file dependencies) out of Includes.make and into per-module computation in build_cm. This is a pure refactor with no behavioral change — all modules still depend on all libraries in the stanza's requires. This separation is necessary for issue ocaml#4572: Includes carries -I flags (which must be shared across all modules) while Hidden_deps can vary per-module. With this refactor, a future change can use ocamldep output to filter Hidden_deps per-module without affecting -I flags. - Includes.make no longer takes ~opaque or bundles Hidden_deps - New deps_of_entries in lib_file_deps.ml handles opaque logic - Alias and Wrapped_compat modules skip library deps (matching their existing Includes.empty behavior) - deps_with_exts removed (sole caller was the old opaque branch) Signed-off-by: Robin Bate Boerop <me@robinbb.com>
…issue ocaml#4572) Add a new function `read_immediate_deps_raw_of` to the ocamldep module that returns raw module names (as Module_name.Set.t) from ocamldep output, without resolving them to Module.t values. This function will be used to determine which external library modules a module actually references, enabling finer-grained dependency tracking where modules are only recompiled when libraries they actually use change. Signed-off-by: Robin Bate Boerop <me@robinbb.com>
robinbb
force-pushed
the
robinbb-issue-4572-refactor
branch
from
95b4fce to
f58b334
Compare
Collaborator
Author
|
@rgrinberg I am closing this PR, as this is a refactoring that serves the branch in PR #14021. However, I am deprecating that PR in favour of PR #14116 which has a more elegant solution than what this branch was heading toward (and it works on CI tests). |
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.
Pure refactor: moves
Hidden_deps(library file dependencies) out ofIncludes.makeand into per-module computation inbuild_cm. No behavioralchange — all modules still depend on all libraries in their stanza's requires.
This separation is prerequisite work for #4572 (finer dependency analysis
between libraries).
Includescarries-Iflags which must be shared acrossall modules in a stanza.
Hidden_depscan vary per-module — once separated,a future change can use
ocamldepoutput to declare only the library depseach module actually needs.
Changes:
Includes.makeno longer takes~opaqueor bundlesHidden_depsdeps_of_entriesinlib_file_deps.mlreplaces the old inline opaquelogic;
deps_with_extsremoved (sole caller was the old opaque branch)build_cmadds alib_cm_depsblock that declares library file depsper-module; alias and wrapped-compat modules skip library deps (matching
their existing
Includes.emptybehavior)Based on #14100, #14031, #14101, and #14103 (merged via merge commits).
Depends on #14017 (merged), #14031, #14100, #14101, and #14103.
Ref: #4572