Skip to content

feat: per-module inter-library dependency filtering (#4572)#14116

Open
robinbb wants to merge 2 commits intoocaml:mainfrom
robinbb:robinbb-issue-4572-combined-tests
Open

feat: per-module inter-library dependency filtering (#4572)#14116
robinbb wants to merge 2 commits intoocaml:mainfrom
robinbb:robinbb-issue-4572-combined-tests

Conversation

@robinbb
Copy link
Copy Markdown
Contributor

@robinbb robinbb commented Apr 10, 2026

Summary

Use the already-computed ocamldep output to filter inter-library
dependencies on a per-module basis.

Previously, when libA depends on libB, every module of libA got a
glob dependency on all .cmi files in libB. Now each module only
depends on .cmi/.cmx files from libraries it actually imports,
eliminating unnecessary recompilation.

  • Modules that don't reference a changed library: 0 recompilations
    (was 2)
  • Unwrapped library consumers referencing only specific modules: 0
    recompilations
    (was 2), via per-file deps on individual .cmi files
  • Cross-library transparent aliases (module M = Mylib): correctly
    handled by including transitive library dependencies of referenced
    libraries
  • Sandboxed builds: unaffected

Fallback cases (no change from current behavior)

  • Single-module stanzas: ocamldep is skipped (singleton
    optimization), so filtering is not possible
  • Virtual library implementations and parameterised modules: parameter
    libraries are not in requires_compile, so filtering falls back
  • Melange mode: filtering is OCaml-only

This PR builds on prerequisite PRs:

#14017 — baseline tests documenting current recompilation behavior
#14031 — test documenting module name shadowing between stanzas and libraries
#14100 — test verifying library file deps in compilation rules and sandboxed builds
#14101 — safety test for incremental builds with transparent aliases
#14129 — test verifying incremental builds with alias re-exported libraries

Fixes #4572

@robinbb robinbb force-pushed the robinbb-issue-4572-combined-tests branch 3 times, most recently from 15c3919 to 2b4ab03 Compare April 10, 2026 07:20
@robinbb robinbb force-pushed the robinbb-issue-4572-combined-tests branch 3 times, most recently from 31ab29b to 5660079 Compare April 10, 2026 17:30
…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>
@robinbb robinbb force-pushed the robinbb-issue-4572-combined-tests branch 2 times, most recently from 4d1dce9 to 1776c6e Compare April 10, 2026 20:47
…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>
@robinbb robinbb force-pushed the robinbb-issue-4572-combined-tests branch from 1776c6e to c6d39e7 Compare April 10, 2026 21:19
@robinbb robinbb marked this pull request as ready for review April 10, 2026 21:19
@robinbb
Copy link
Copy Markdown
Contributor Author

robinbb commented Apr 10, 2026

Tagging folks who were interested in the now-deprecated PR 14021. This one supersedes that one, with a more elegant solution, and tests that actually pass. It is out of draft mode as of now.
@rgrinberg @Leonidas-from-XIV @Khady @nojb @Alizter @art-w

@robinbb
Copy link
Copy Markdown
Contributor Author

robinbb commented Apr 10, 2026

I need help running the so-called revdeps tests, as I have no credentials on this repo and therefore cannot initiate CI runs manually. I also would like to be instructed on how I can compare the results of the revdeps tests on this branch versus those on 'main'.

@robinbb
Copy link
Copy Markdown
Contributor Author

robinbb commented Apr 10, 2026

Note: reduced package-level cycle detection

This PR removes the @package-cycle test case from reporting-of-cycles.t. The underlying cycle still exists (library a.1 declares (libraries b), library b declares (libraries a.2)), but the modules in those libraries are empty — they don't reference any library in their source code. With per-module filtering, the compilation graph no longer has inter-library edges for those modules, so the package-level cycle is no longer detected.

This is an inherent trade-off of the approach: package cycle detection previously piggybacked on compilation-level glob deps as a proxy for declared library dependencies. More precise per-module deps mean that proxy no longer exists for modules that don't actually import their declared libraries.

This only affects the case where a module declares a library dependency but never references it in source code. In practice, this is uncommon — but it does mean some package cycles will only be caught at opam-level resolution rather than at build time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Finer dependency analysis between libraries

1 participant