Add `root` and `rootexec` sections to .install files by WardBrian · Pull Request #6938 · ocaml/opam

WardBrian · 2026-05-18T15:26:56Z

Would close #6919

dbuenzli · 2026-05-18T15:41:48Z

I'm currently unable to run the tests locally due to an issue that seems related to #5968

I recently got that aswell on a machine. It disappeared once I used the provided https://opam.ocaml.org/install.sh install script rather than download a binary and copy it to ~/.local/bin/opam. (I was in a hurry so did not try to understand but I suspect the install script does some bubblewrap magic).

WardBrian · 2026-05-18T15:50:30Z

Using install.sh (and installing in /usr/local instead of /usr) is what eventually resolved my bwrap issue as a user, but not when running development versions it seems. Oddly enough, I seem to recall the tests running fine before when I was regularly running into #5968 as a user

rjbou

The PR itself looks good, I've put some comments on the code. It is missing a test where we see the new field (i realise that there is no test that checks all fields, it could be the moment to add, i can do it).

rjbou · 2026-06-04T15:35:07Z

+         Pp.singleton -| Pp.V.string -| pp_check_relative
+           -| pp_check_not_absolute
+           -| pp_check ~name:"rel-filename"
+               ~f:(fun s -> List.exists


The opam switch internal directory is only at the root of the switch prefix, there is nothing preventing users to have internal .opam-switch directories inside their packages file. It is possible to check the first element, but there is a need to handle the case where the filename begins with one or several ./././path (it is allowed)

Yes, this is a conservative over-approximation. I believe the same is currently true for the might_escape check, since in theory a path like ./foo/bar/../bar should be fine, but I believe it will be prevented at the moment

The odds of someone having a legitimate use case for a .opam-switch path seem low enough that being conservative seems like it would be fine in this case

WardBrian · 2026-06-05T15:07:55Z

Thanks for the review @rjbou. I believe I addressed all your comments other than the desire for more testing

WardBrian · 2026-06-08T14:01:35Z

@rjbou at this point, would you consider this feature safe to assume for the next opam release (i.e., it would be reasonable for dune to assume its existence and change what it generates in .install files)?

Do let me know if there is anything else you’d like me to do on the PR!

rjbou · 2026-07-10T19:08:36Z

Updated with the new tests. The main todo is to add prefix check for opam internals (other fields), and an update of the windows test, and it'll be ready.

rjbou · 2026-07-13T10:23:17Z

Added the new checks

kit-ty-kate

the branch is a bit messy with all the fixup commits. I would suggest rebasing it with 3 commits (in order):

OpamFile.Install.fields refactoring
The addition of the root and rootexec fields
Additions to the reftests

kit-ty-kate · 2026-07-13T12:40:50Z

+                Pp.warn ~pos
+                  "Path '%s' begins with %s directory in 'root' field. \
+                   Use '%s' field instead."
+                  str root root


I don't think it's worth adding a warning at this point and there are legitimate cases where using rootexec: ["file" {"share/blah"}] will trigger the warning. Could we split it to a separate PR and think about it later?

Do you have an example of a legitimate case ?

Could we split it to a separate PR and think about it later?

opened #7039

kit-ty-kate · 2026-07-13T12:51:02Z

+           let paths = List.map OpamTypesBase.string_of_std_path paths in
+           (match OpamFilename.split ~sep:`Unspecified str with
+            | root::_::_ ->
+              if OpamStd.List.mem String.equal root paths then


this check is not the right one. The name of the field and the name of the directory it points to can be different (e.g. the *exec fields, stublibs, toplevel, …)

We are not checking regarding the field name, but the root dir of the installation path (if given) versus all defined directories that will/would be created on the switch prefix given the .install file.

rjbou · 2026-07-13T13:34:15Z

+           let paths = List.map OpamTypesBase.string_of_std_path paths in
+           (match OpamFilename.split ~sep:`Unspecified str with
+            | root::_::_ ->
+              if OpamStd.List.mem String.equal root paths then


We are not checking regarding the field name, but the root dir of the installation path (if given) versus all defined directories that will/would be created on the switch prefix given the .install file.

rjbou · 2026-07-13T13:34:37Z

+                Pp.warn ~pos
+                  "Path '%s' begins with %s directory in 'root' field. \
+                   Use '%s' field instead."
+                  str root root


Do you have an example of a legitimate case ?

rjbou · 2026-07-13T13:52:34Z

the branch is a bit messy with all the fixup commits.

It is intended, to highlight my changes to @WardBrian and reciprocally

WardBrian · 2026-07-13T14:47:24Z

the branch is a bit messy with all the fixup commits.

It is intended, to highlight my changes to @WardBrian and reciprocally

This has definitely been helpful so far but at this point I’d be okay with you rebasing it and erasing this distinction

kit-ty-kate

lgtm modulo the minor issue i raised above

…ystem

…y paths target without using '..' Co-authored-by: Raja Boujbel <raja.boujbel@ocamlpro.com>

rjbou · 2026-07-15T17:47:48Z

Updated accordingly, and extracted #7039 specific tests in that PR

kit-ty-kate · 2026-07-15T21:01:25Z

Thanks!

WardBrian changed the title ~~Feature/dot install prefix~~ Add prefix and prefixexec sections to .install files May 18, 2026

WardBrian mentioned this pull request May 18, 2026

Cross compilation: Also generate 'root'/'rootexec' sections for installation ocaml/dune#14600

Open

rjbou self-requested a review May 21, 2026 13:38

rjbou added this to the 2.6.0~alpha1 milestone May 30, 2026

rjbou reviewed Jun 4, 2026

View reviewed changes

rjbou mentioned this pull request Jun 5, 2026

reftest: add '.install' exhaustive test for all fields #6956

Merged

rjbou self-requested a review June 8, 2026 10:01

WardBrian changed the title ~~Add prefix and prefixexec sections to .install files~~ Add root and rootexec sections to .install files Jun 18, 2026

rjbou mentioned this pull request Jul 7, 2026

[2.5] Re-allow .install files containing '..' #7009

Merged

rjbou force-pushed the feature/dot-install-prefix branch from 54a7476 to 8ad3d2f Compare July 10, 2026 19:07

rjbou force-pushed the feature/dot-install-prefix branch from 8ad3d2f to 93ee4a1 Compare July 13, 2026 10:23

kit-ty-kate added the AREA: FORMAT label Jul 13, 2026

kit-ty-kate reviewed Jul 13, 2026

View reviewed changes

rjbou reviewed Jul 13, 2026

View reviewed changes

Comment thread src/format/opamFile.ml Outdated

This was referenced Jul 15, 2026

Re-allow .install files containing '..' #7008

Merged

Add some checks on .install root and rootexec fields #7039

Draft

rjbou requested a review from kit-ty-kate July 15, 2026 16:37

rjbou force-pushed the feature/dot-install-prefix branch from 81eaad9 to b0af601 Compare July 15, 2026 16:49

kit-ty-kate reviewed Jul 15, 2026

View reviewed changes

Comment thread tests/reftests/dot-install-non-exe-fields.test Outdated

kit-ty-kate reviewed Jul 15, 2026

View reviewed changes

Comment thread tests/reftests/run.ml Outdated

kit-ty-kate approved these changes Jul 15, 2026

View reviewed changes

rjbou force-pushed the feature/dot-install-prefix branch from b0af601 to 53ab983 Compare July 15, 2026 17:24

kit-ty-kate force-pushed the feature/dot-install-prefix branch from 53ab983 to b6d7fee Compare July 15, 2026 17:27

kit-ty-kate added the STATE: READY TO MERGE label Jul 15, 2026

rjbou and others added 3 commits July 15, 2026 19:46

Add OpamFilename.split to split a string path, non dependent from s…

1c9dcfd

…ystem

Add '.opam-switch' name in 'OpamPathName'

dfda171

Add 'root' and 'rootexec' fiels in '.install' files to allow arbitrar…

4de6a6e

…y paths target without using '..' Co-authored-by: Raja Boujbel <raja.boujbel@ocamlpro.com>

rjbou force-pushed the feature/dot-install-prefix branch from b6d7fee to 77536e3 Compare July 15, 2026 17:46

reftests: add 'root' and 'rootexec' test in dot install fields tests

d3852d5

kit-ty-kate force-pushed the feature/dot-install-prefix branch from 77536e3 to d3852d5 Compare July 15, 2026 19:30

kit-ty-kate merged commit 8c142ef into ocaml:master Jul 15, 2026
40 of 41 checks passed

kit-ty-kate removed the STATE: READY TO MERGE label Jul 15, 2026

WardBrian deleted the feature/dot-install-prefix branch July 23, 2026 20:42

Uh oh!

Conversation

WardBrian commented May 18, 2026 • edited by kit-ty-kate Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dbuenzli commented May 18, 2026

Uh oh!

WardBrian commented May 18, 2026

Uh oh!

rjbou left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

WardBrian commented Jun 5, 2026

Uh oh!

WardBrian commented Jun 8, 2026

Uh oh!

rjbou commented Jul 10, 2026

Uh oh!

rjbou commented Jul 13, 2026

Uh oh!

kit-ty-kate left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rjbou commented Jul 13, 2026

Uh oh!

WardBrian commented Jul 13, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kit-ty-kate left a comment

Choose a reason for hiding this comment

Uh oh!

rjbou commented Jul 15, 2026

Uh oh!

kit-ty-kate commented Jul 15, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

WardBrian commented May 18, 2026 •

edited by kit-ty-kate

Loading

kit-ty-kate left a comment •

edited

Loading