Skip to content

fix(package-deps): follow library-induced package closure - #16362

Open
Alizter wants to merge 12 commits into
ocaml:mainfrom
Alizter:push-nqsuumtwqqly
Open

fix(package-deps): follow library-induced package closure#16362
Alizter wants to merge 12 commits into
ocaml:mainfrom
Alizter:push-nqsuumtwqqly

Conversation

@Alizter

@Alizter Alizter commented Sep 10, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Include whole packages reached through library dependencies and track installed file changes. For packages in the lock directory, we have no easy way of getting the libraries directly, so instead we use the direct package closure which is already computed.

When we start loading sources directly, this distinction with the old "package universe" will no longer exist, so I think its fine for it to work like this for now.

rgrinberg
rgrinberg reviewed Sep 10, 2026
View reviewed changes
Comment thread src/dune_rules/package_db.ml Outdated
rgrinberg
rgrinberg reviewed Sep 10, 2026
View reviewed changes
Comment thread src/dune_rules/dep_conf_eval.ml Outdated
@Alizter
Alizter force-pushed the push-nqsuumtwqqly branch 13 times, most recently from ec579f2 to a64c9fc Compare September 15, 2026 15:45
@Alizter
Alizter marked this pull request as ready for review September 16, 2026 11:22
@Alizter

Alizter commented Sep 16, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator Author

This currently breaks CI because nix is compressing man pages and making the dune-package manifest incorrect. There are a few ways we can fix this:

  1. Fix the nix derivations so they don't compress the man pages of the packages we depend on. This is tedious to maintain.
  2. Ignore invalid doc entries in package deps, as requested here Please ignore doc files in (package pkg) #14364. Seems sensible but could be a footgun for consumers of docs like odoc.
  3. Support compression of manpages directly. Seems sensible but a lot of work.

1 seems the best as a temporary solution. I don't know if there is anything else we can do here.

rgrinberg
rgrinberg reviewed Sep 17, 2026
View reviewed changes
Comment thread src/dune_rules/lib.ml Outdated
rgrinberg
rgrinberg reviewed Sep 17, 2026
View reviewed changes
Comment thread src/dune_rules/lib.ml Outdated
rgrinberg
rgrinberg reviewed Sep 21, 2026
View reviewed changes
Comment thread src/dune_rules/lib_info.ml Outdated
@Alizter
fix(package-deps): follow library-induced package closure
2dfc769 
Include whole packages reached through library dependencies and track
installed file changes.

Fixes ocaml#15511

The melange closure is not traversed.

Signed-off-by: Ali Caglayan <alizter@gmail.com>
@Alizter
fix(nix): keep lambda-term manpages uncompressed
f76884d 
Library-induced package dependencies make (package utop) also track
lambda-term. Nix compression leaves its dune-package manifest pointing
to missing uncompressed manpages.

Extend the existing utop workaround to lambda-term so the CI shell keeps
the installed files consistent with the manifest.

Signed-off-by: Ali Caglayan <alizter@gmail.com>
@Alizter
refactor: move package owner field from Lib to Lib_info
477c1a8 
Signed-off-by: Ali Caglayan <alizter@gmail.com>
@Alizter
refactor(lib): use Lib_info.package for installed ownership
5ed37a7 
Use the existing package accessor for resolved package ownership instead
of keeping a separate package_owner API. Derive findlib namespaces
explicitly for callers that need library identity, preserving their
existing behavior.

Signed-off-by: Ali Caglayan <alizter@gmail.com>
@Alizter
refactor(lib): move installed package ownership into status
b42a694 
Store the optional owner in Installed and Installed_private instead of a
separate Lib_info field. Preserve ownership resolution and public/private
behavior while updating status construction and matching.

Signed-off-by: Ali Caglayan <alizter@gmail.com>
@Alizter
refactor(findlib): construct library metadata with package ownership
731930d 
Associate search paths with managed packages and resolve ownership in
findlib before passing it to META and dune-package decoding. Construct
installed library status with its owner instead of updating records later.

Keep unknown ownership in managed contexts and use the findlib package name
outside lock mode. Remove ownership setters and Lib.DB postprocessing.

Signed-off-by: Ali Caglayan <alizter@gmail.com>
@Alizter
refactor(odoc): use package ownership for workspace documentation
5839d46 
Use Lib_info.package for workspace-library documentation compilation,
linking, package labels, output directories, and markdown package membership.
Dispatch library documentation by status so installed libraries aren't
treated as workspace targets. Keep the independent workspace-package lookup
unchanged.

Use status to identify unpackaged private libraries in the new odoc index,
preserving the existing name-based layout for other libraries. Apply the new
odoc package mask using library ownership. Pass the documentation package
explicitly when constructing library indexes, retaining the classified
metadata package for installed libraries and including it in the memo key.

Record library membership in documentation packages alongside location maps,
and use it for odoc dependencies instead of re-deriving package identity.

Signed-off-by: Ali Caglayan <alizter@gmail.com>
@Alizter
refactor(rocq): select workspace packages for plugin layouts
d7904dc 
Collect public workspace ML-library packages directly from library status.
Keep private-plugin rejection and include flags for installed libraries.
Leave the Lib/Libexec-only layout dependency filtering unchanged to avoid
cycles when theories and plugins share a package.

Signed-off-by: Ali Caglayan <alizter@gmail.com>
@Alizter
refactor(lib): use package ownership for default implementations
45e645a 
Compare the package owners of a virtual library and its default
implementation. Keep implementation identity and privacy checks unchanged,
and clarify that installed package ownership may be unknown.

Signed-off-by: Ali Caglayan <alizter@gmail.com>
@Alizter
refactor(mel): use package ownership for JavaScript emission
85ced06 
Use Lib_info.package to select the package-name flag for library
JavaScript emission.

Signed-off-by: Ali Caglayan <alizter@gmail.com>
@Alizter
refactor(lib): remove the unused findlib package accessor
34352f1 
Remove Lib_info.findlib_package now that all its callers have migrated.

Signed-off-by: Ali Caglayan <alizter@gmail.com>
@Alizter
Alizter requested a review from rgrinberg September 25, 2026 12:12
@Alizter Alizter mentioned this pull request Sep 26, 2026
Open
25 tasks
rgrinberg
rgrinberg reviewed Sep 27, 2026
View reviewed changes
Comment thread src/dune_rules/pkg_rules.ml
())
let* () = Action_builder.deps (Pkg.package_deps pkg) in
let* { Install_cookie.Gen.files; _ } = (Pkg_installed.of_paths paths).cookie in
Action_builder.paths (Section.Map.values files |> List.concat))

@rgrinberg rgrinberg Sep 27, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are these extra deps doing?

@rgrinberg rgrinberg Sep 27, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To expand on the above, if this is fixing anything, I suspect it's also fixing things in a way that is not related to this PR.

@Alizter Alizter Sep 28, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've reverted this particular tracking of dependencies. Without it, we only depend on the cookie, but this isn't enough if the contents of the package change in the meantime. You can see the change of behaviour in the final commit.

@Alizter
revert(package-deps): leave lock package closure untracked
6d8ca03 
Signed-off-by: Ali Caglayan <alizter@gmail.com>

This branch has not been deployed

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

Reviewers

@rgrinberg rgrinberg rgrinberg left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

In the future release, deps does not load package dependencies transitively

2 participants

@Alizter @rgrinberg