reftest: add '.install' exhaustive test for all fields by rjbou · Pull Request #6956 · ocaml/opam

rjbou · 2026-06-05T17:16:45Z

#6938 add new field to .install files, there was no exhaustive test.
The tests are 3 files:

a cross platform test for non executable fields
2 per platform tests for executable fields (it differs from win to nux)
Note: the windows test highlights some bugs, issues will be opened

rjbou · 2026-06-08T10:08:11Z

+### OPAMYES=1
+### # we neeed the precise tracking to have a sha in changes file instead of timestamp
+### OPAMPRECISETRACKING=1
+### OPAMDEBUGSECTIONS="TRACK ACTION SYSTEM"


I'm not sure if it worth having the SYSTEM logging

I think that we can indeed make it so that the cat.ml script provides the information we need, that would make the whole test shorter and easier to understand!

I've removed the SYSTEM log, we don't necessarily need to see all the system access

rjbou · 2026-06-08T10:09:08Z

+SYSTEM                          write ${BASEDIR}/OPAM/sw-misc/.opam-switch/build/pkg-misc.1/pkg-misc.install
+[WARNING] Errors in ${BASEDIR}/OPAM/sw-misc/.opam-switch/build/pkg-misc.1/pkg-misc.install, some fields have been ignored:
+            - At ${BASEDIR}/OPAM/sw-misc/.opam-switch/build/pkg-misc.1/pkg-misc.install:1:8-1:20::
+              expected abs-filename


Comment for issue : the error can be better here

issue #6958

rjbou · 2026-06-08T10:09:41Z

+[ERROR] In ${BASEDIR}/OPAM/sw-man/.opam-switch/build/pkg-man.1/pkg-man.install:
+        Not_found


We should fail gracefully

issue #6959

NathanReb

The current logging level makes it quite hard to read the test. Are we testing something that's only shown in the logs or are the resulting install and/or errors enough?

Can't some of those tests be merged? E.g. the basic case turned into a single pkg which successfully install a file for each field?

NathanReb · 2026-06-11T08:25:15Z

+### OPAMPRECISETRACKING=1
+### OPAMDEBUGSECTIONS="TRACK ACTION SYSTEM"
+### # Printer of `changes` file and the installed file 'finstalled' in switch (or root)
+### <cat.ml>


Don't we have a way to write this small utility script separately and use it from the reftest? We could make it a dune executable and add an explicit dependency. That would make later modification or debug of the script easier. Would also ease the review/reading of the script itself.

It might at least be worth a small comment for each function of the script!

cat.ml is specific to this test, it can not be externalised. Some of its function could. Updated with comments

I simplified cat.ml to avoid rewriting other utilities that we already have, like opam-cat

NathanReb · 2026-06-11T09:01:34Z

+  "lib|pkg-lib|finstalled" {"F:a611026bf7b99362c57473c4e1ef30a0"}
+]
+==> ${BASEDIR}/OPAM/sw-lib/lib/pkg-lib/finstalled
+### OPAMDEBUG=-5 opam remove pkg-lib | grep -v LOCK


Is this is just a post test clean up? If so how about disabling the logs in the output to avoid polluting the test?

It is not cleanup, it checks that the installed files are well removed

NathanReb · 2026-06-11T09:03:05Z

+### OPAMYES=1
+### # we neeed the precise tracking to have a sha in changes file instead of timestamp
+### OPAMPRECISETRACKING=1
+### OPAMDEBUGSECTIONS="TRACK ACTION SYSTEM"


I think that we can indeed make it so that the cat.ml script provides the information we need, that would make the whole test shorter and easier to understand!

rjbou · 2026-06-24T09:04:50Z

Can't some of those tests be merged? E.g. the basic case turned into a single pkg which successfully install a file for each field?

It was my first attempt, but special cases came quite quickly (for misc & man). Having a test case per field permit to see more specifically the changes if they are (on a given field), and helps on specific cases testing (imo).

rjbou · 2026-06-24T09:49:46Z

PR updated. There is a remaining issue with +x flag on windows i think

rjbou · 2026-07-10T18:32:49Z

Updated, main comment updated too

rjbou · 2026-07-10T18:41:14Z

+ACTION                          Removing the local metadata
+Done.
+### opam-cat OPAM/sw-bin/.opam-switch/install/pkg-bin.changes
+# OPAM/sw-bin/.opam-switch/install/pkg-bin.changes not found


On removal, the installed file is not removed. Issue #7021

rjbou · 2026-07-10T18:45:14Z

+TRACK                           before install: 1 elements scanned in 0.000s
+ACTION                          Installing pkg-libexec.1.
+
+ACTION                          creating ${BASEDIR}/OPAM/sw-libexec/lib/pkg-libexec


There is also this failure on libexec field... issue #7022

rjbou added this to the 2.6.0~alpha1 milestone Jun 5, 2026

rjbou requested a review from NathanReb June 5, 2026 17:16

rjbou added the AREA: TEST label Jun 5, 2026

rjbou mentioned this pull request Jun 5, 2026

Add root and rootexec sections to .install files #6938

Merged

2 tasks

rjbou commented Jun 8, 2026

View reviewed changes

This was referenced Jun 8, 2026

Better parsing error message for .install misc field #6958

Open

Better parsing error message for .install man field #6959

Open

NathanReb reviewed Jun 11, 2026

View reviewed changes

rjbou requested a review from NathanReb June 24, 2026 09:50

kit-ty-kate reviewed Jun 28, 2026

View reviewed changes

rjbou mentioned this pull request Jul 8, 2026

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

Merged

rjbou force-pushed the reftest-dot-install branch from d6367e4 to 3a49c44 Compare July 10, 2026 18:31

rjbou commented Jul 10, 2026

View reviewed changes

rjbou mentioned this pull request Jul 10, 2026

On Windows, some installed files via .install are not cleaned on package removal #7021

Open

rjbou commented Jul 10, 2026

View reviewed changes

rjbou mentioned this pull request Jul 10, 2026

On Windows, installing a binary via libexec field in .install fails #7022

Open

rjbou force-pushed the reftest-dot-install branch from 3a49c44 to d80b724 Compare July 10, 2026 18:50

This was referenced Jul 10, 2026

On paths display, for user output and tests #7023

Open

Add more Windows tests for .install #7025

Open

kit-ty-kate force-pushed the reftest-dot-install branch from d80b724 to 6b34ec2 Compare July 10, 2026 20:52

rjbou added 2 commits July 11, 2026 00:02

reftest: add other path replacement for dot-install tests

1988989

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

6a1eeeb

kit-ty-kate force-pushed the reftest-dot-install branch from 6b34ec2 to 6a1eeeb Compare July 10, 2026 23:02

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

View reviewed changes

kit-ty-kate merged commit ec4489f into ocaml:master Jul 11, 2026
41 checks passed

		[ERROR] In ${BASEDIR}/OPAM/sw-man/.opam-switch/build/pkg-man.1/pkg-man.install:
		Not_found

Uh oh!

Conversation

rjbou commented Jun 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

NathanReb left a comment

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!

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!

rjbou commented Jun 24, 2026

Uh oh!

rjbou commented Jun 24, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rjbou commented Jul 10, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

rjbou commented Jun 5, 2026 •

edited

Loading