Skip to content

feat: %{pkg:foo:lib} pforms for package install layouts#14200

Open
Alizter wants to merge 2 commits intoocaml:mainfrom
Alizter:push-yqyzlrlpmltq
Open

feat: %{pkg:foo:lib} pforms for package install layouts#14200
Alizter wants to merge 2 commits intoocaml:mainfrom
Alizter:push-yqyzlrlpmltq

Conversation

@Alizter
Copy link
Copy Markdown
Collaborator

@Alizter Alizter commented Apr 16, 2026

We make the %{pkg:foo:lib} macros available outside of lock files in dune. This allows users to inspect installed package contents directly. We support all the install sections that we understand in dune.

Depending on such a path automatically introduces dependencies on the package.

This works with ocamlfind packages, dune package management and workspace packages.

The goal is to ultimately hide _build/install as an implementation detail and allow users to properly depend on packages' installed contents without having to inspect them by hand.

@Alizter Alizter force-pushed the push-yqyzlrlpmltq branch from d8cffa1 to 917dfbc Compare April 17, 2026 12:56
@Alizter Alizter force-pushed the push-yqyzlrlpmltq branch from 917dfbc to 12bd59e Compare April 17, 2026 15:00
Alizter added a commit that referenced this pull request Apr 17, 2026
These fields can appear in opam files, however we haven't encountered
them yet. It might be worth supporting them since we use these install
sections in dune anyway.

This test simply checks how we are interpreting the fields, I will
motivate adding them in a later PR. This is related to #14200.
@Alizter Alizter force-pushed the push-yqyzlrlpmltq branch from 12bd59e to 3600fb2 Compare April 17, 2026 16:11
@Alizter Alizter force-pushed the push-yqyzlrlpmltq branch 2 times, most recently from 36db8d6 to 0e048e5 Compare April 17, 2026 17:02
@Alizter Alizter requested a review from rgrinberg April 17, 2026 17:02
@Alizter Alizter marked this pull request as ready for review April 17, 2026 17:02
@Alizter Alizter force-pushed the push-yqyzlrlpmltq branch from 0e048e5 to 3a12c1b Compare April 17, 2026 17:10
Comment thread src/dune_rules/expander.ml Outdated
Need_full_expander
(fun t ->
With
(let pkg_name = Package.Name.of_string pkg_name_str in
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 happens when the package name is invalid?

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.

Oh yes, this is wrong.

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.

At the moment we do no validation so it should just say it couldn't find the package.

Comment thread src/dune_rules/package_db.ml Outdated
Comment thread test/blackbox-tests/test-cases/pkg-pform/errors.t Outdated
@rgrinberg
Copy link
Copy Markdown
Member

Thinking about this again, I think this form should take the actual file that it needs as another argument. Otherwise, we need to build everything in the install directory and that is going to cause incredibly inconvenient cycles for folks.

@Alizter
Copy link
Copy Markdown
Collaborator Author

Alizter commented Apr 17, 2026

I think making it be (package foo) is better because then later we can improve that to avoid cycles by materialising install directories with anonymous actions. Having the directory in the form is convenient because it allows users to create paths with it and we don't have to care about supplying the full install tree as syntax.

@rgrinberg
Copy link
Copy Markdown
Member

There are some advantages to both approaches. One advantage of allowing the user to specify a file is that it is actually solving the ticket we've been provided. That is analogous to %{lib:..} which does take a file. Another advantage is that it allows us to refer to installed paths from within the same package and the same section without cycles. There's no way you can workaround this with or without anonymous actions. Finally, it gives us validation to see if the path under the section actually exists.

So in summary, both approaches have their cons. But we should do the direct thing to solve the issue the user is asking for.

By the way, I'm not so hot about allowing the _build/install directory to be reachable via relative paths anymore. I think these paths (whether constructed through an anonymous action or not) should only be accessible through environment variables.

@Alizter Alizter force-pushed the push-yqyzlrlpmltq branch 3 times, most recently from 7670a82 to 8528323 Compare April 19, 2026 23:46
Signed-off-by: Ali Caglayan <alizter@gmail.com>
@Alizter Alizter force-pushed the push-yqyzlrlpmltq branch from 8528323 to dda4a63 Compare April 20, 2026 15:19
Signed-off-by: Ali Caglayan <alizter@gmail.com>
@Alizter Alizter force-pushed the push-yqyzlrlpmltq branch from dda4a63 to b8dc435 Compare April 20, 2026 17:25
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.

Support %{pkg:foo:lib} variables more generally Library variable expansion needs a library installed to work

2 participants