refactor: move library file deps from Includes to per-module in build_cm (#4572)#14030
Draft
robinbb wants to merge 7 commits intoocaml:mainfrom
Draft
refactor: move library file deps from Includes to per-module in build_cm (#4572)#14030robinbb wants to merge 7 commits intoocaml:mainfrom
robinbb wants to merge 7 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
Contributor
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
Contributor
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
When an internal module name shadows an unwrapped library's module name, the internal module takes precedence and the library's module is inaccessible. This means ocamldep will report the internal module, not the library's, so dependency filtering that treats stanza-internal names as non-library references is correct. Signed-off-by: Robin Bate Boerop <me@robinbb.com>
…lds (ocaml#4572) Add baseline tests documenting existing behavior: - lib-deps-preserved: every non-alias module declares glob deps on its library dependencies' .cmi files - sandbox-lib-deps: sandboxed package builds have dependency libraries' .cmi files available when a library depends on another library Signed-off-by: Robin Bate Boerop <me@robinbb.com>
robinbb
force-pushed
the
robinbb-issue-4572-refactor
branch
from
34c14f5 to
0ad4949
Compare
1 task
Add baseline test verifying that incremental builds succeed when a module re-exports a library via a transparent alias (module M = Mylib) and the library's interface changes. Regression reported by @Alizter. 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>
robinbb
force-pushed
the
robinbb-issue-4572-refactor
branch
from
0ad4949 to
8c3d290
Compare
…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
8c3d290 to
82bf016
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.
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)Depends on #14017 (merged), #14031 (merged), #14100, and #14101.
Ref: #4572