infra: Lib_index, deps_of_entries, deps_of_entry_modules, raw ocamldep#14513
Conversation
robinbb
force-pushed
the
robinbb-14492-l1-lib-index-infra
branch
from
093b939 to
c22c3d9
Compare
robinbb
force-pushed
the
robinbb-14492-l1-lib-index-infra
branch
2 times, most recently
from
a6bf06f to
d273159
Compare
|
@robinbb Should I wait a bit longer before continuing to review if you want to improve it? I'm basically happy with this part, its not too controversial. After L9 we can always rethink anything that's not up to par. |
There was a problem hiding this comment.
Pull request overview
Layer 1 of a 9-PR stack reconstructing #14492. This PR introduces dataflow infrastructure for per-module inter-library dependency filtering (#4572) without yet wiring any consumers: a Lib_file_deps.Lib_index module that classifies entry-module references as tight/non-tight per library, helpers deps_of_entries / deps_of_entry_modules to build glob-based or per-module file deps, and a refactor of Ocamldep.read_immediate_deps_of that factors out a shared read_immediate_deps_words (with a top-level cache keyed on (source path, ml_kind)) and adds a new read_immediate_deps_raw_of returning raw module-name sets without filtering against the stanza's module set.
Changes:
- Add
Lib_index,deps_of_entries, anddeps_of_entry_modulestoLib_file_depsfor per-module/per-library dep computation. - Refactor
Ocamldepto share a top-level cache of ocamldep word output and exposeread_immediate_deps_raw_offor cross-library use. - Extract the ocamldep error helper
invalid_ocamldep_outputand uncurryparse_deps_exn.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/dune_rules/lib_file_deps.mli | Public API for deps_of_entries, deps_of_entry_modules, and Lib_index. |
| src/dune_rules/lib_file_deps.ml | Implements the new helpers and the Lib_index classification logic. |
| src/dune_rules/ocamldep.mli | Adds the new read_immediate_deps_raw_of signature. |
| src/dune_rules/ocamldep.ml | Splits ocamldep parsing/error helpers and introduces a process-global cache shared by read_immediate_deps_of and the new raw API. |
robinbb
force-pushed
the
robinbb-14492-l1-lib-index-infra
branch
2 times, most recently
from
07c064f to
a3ab9c8
Compare
…mldep Pure-additive groundwork for #4572's per-module inter-library dependency filter. No consumer yet; the cross-library walker and per-module filter land in a follow-up. [Lib_file_deps]: - [Lib_index]: a per-cctx index mapping module-name → (lib, Some entry Module.t / None) entries plus a [tight_eligible] set of libs whose modules can serve specific-file deps and a [no_ocamldep] set of libs whose [.d] files don't exist (short-circuited by [Dep_rules.skip_ocamldep]). [create] / [filter_libs_with_modules] / [lookup_tight_entries] / [is_tight_eligible] / [wrapped_libs_referenced]. - [deps_of_entries ~opaque ~cm_kind libs]: opaque-aware glob deps over the given libraries. Mirrors the implicit semantics previously inlined in [Compilation_context.Includes.make]. - [deps_of_entry_modules ~opaque ~cm_kind lib modules]: specific-file cmi/cmx deps on the given local-lib modules, via [cm_public_file] so the produce-public-cmi rule is triggered. [Ocamldep]: - Extract [invalid_ocamldep_output] so [parse_deps_exn] can be called independently of its [Action_builder.t] caller. - [read_immediate_deps_words]: top-level cache keyed on (source path, ml_kind) holding the [Action_builder.t]. Without it, future per-module filter callers each create a distinct [Action_builder.memoize] cell (because [pp_flags] closure identity varies), causing ocamldep to run multiply per source. - [read_immediate_deps_of]: refactored to read words through the cache, then resolve against the stanza's module set; semantics unchanged. - [read_immediate_deps_raw_of]: new entry point returning raw [Module_name.Set.t] for the cross-library walker. Signed-off-by: Robin Bate Boerop <me@robinbb.com>
robinbb
force-pushed
the
robinbb-14492-l1-lib-index-infra
branch
from
a3ab9c8 to
55c7882
Compare
@Alizter Okay, but, if you approve, do you want it to merge, or do you want all 9 PRs to be approved before the first merges? (I'll keep making changes as long as you keep making comments.) |
|
@robinbb I was suggesting reviewing and merging as we go along. I don't see any point in keeping the entire series in limbo, that's just more work for everybody. |
Alizter
left a comment
Alizter
left a comment
There was a problem hiding this comment.
I don't see anything that's obviously wrong. Bench results also don't change as expected.
|
A note for observers, not for @Alizter.
This is an interesting case where the comma, or lack of comma, changes the meaning 100%. Here, @Alizter is expressing that the bench results are unchanged, and that the lack of change is what was expected. He is not expressing: the bench results failed to have an expected change. |
Layer 1 of 9 stacked PRs reconstructing #14492 in reviewable layers.
Adds the dataflow infrastructure subsequent layers consume —
Lib_file_deps.Lib_index,deps_of_entries,deps_of_entry_modules,Ocamldep.read_immediate_deps_words(top-level cache keyed on (source path, ml_kind)), andOcamldep.read_immediate_deps_raw_of. No consumer yet.Stack: this PR → main. Next: #14514.
Part of #14492. Related to #4572.