Package management should Ignore unused opam files when pinning

[giltho]

This is feedback on the dune package management preview.

I have a file a project that pins another project, but it fails to build with Error: unexpected version because of some empty but unused opam files in the project.

It works after just adding one line to the relevant opam files. See https://github.com/rems-project/cerberus/pull/817

[rgrinberg]

Thanks. Dune can infer projects from one of two sources:

1. The project file
2. The .opam files

The project file doesn't define any packages so dune goes for 2. AFAIK, an opam file with the version incantation is invalid. So I don't think there's anything we can do here.

[rgrinberg]

Can you post your pins stanza? Maybe we can make the loading a little lazier.

[giltho]

I deleted that branch but I think it was simply

(pin
 (package
  (name cerberus-lib))
 (url
  "git+https://github.com/giltho/cerberus#6316f8f46dd8d6b207b8669c6ec082f78032e538"))

Yeah, I kind of understand that, ideally, all opam files should be correct. But empty opam files exist in many places because people don't generate them from dune-projects and dune refuses to build public modules without a corresponding opam file.

One or both of the following could be done:

• newer versions of dune could require the opam file to be valid (i.e. at least contain the version) in order to compile a public module, and if not issue at least a warning.
• If an opam file is detected to be empty, dune can just load use the default "empty" opam file that only contains 'opam-version: "2.0"'. It'd be a wrapper around the current function that loads the opam file, even if it's not pretty.

I do understand if option 2 is not desirable, and I can see that it doesn't appear, but on the issue now, but I clicked "Feature request" and not "Bug report" when I created this issue. I don't believe this is a bug, the current behaviour is acceptable, but it is worth flagging

[Leonidas-from-XIV]

AFAIK, an opam file with the version incantation is invalid.

Yes, that's true. The shortest valid OPAM file is

opam-version: "2.0"

[Alizter]

I've also come across a similar situation for Frama-C which has many empty (?!) opam files to get around having to write package stanzas in dune-project. Maybe it's not too far-fetched to default to opam-version: "2.0" ourselves in this case.

[rgrinberg]

get around having to write package stanzas in dune-project

Why would they want to get around that?

Maybe it's not too far-fetched to default to opam-version: "2.0" ourselves in this case.

It really is far fetched. We don't want to interpret these files in a way that is inconsistent from opam.

[Alizter]

Here is the repo: https://git.frama-c.com/pub/frama-c

Here is an example of an empty file https://git.frama-c.com/pub/frama-c/-/blob/master/src/libraries/qed/qed.opam?ref_type=heads

In this repo alone there are around 20 empty files. Opam seems to work fine with it presumably because they are not being picked up. Dune will however choke on these.

[Alizter]

I am not suggesting we support every quirky build. I'm just trying to understand how a user might recover from this situation.

If I remove the empty opam file:

File "src/libraries/qed/dune", line 28, characters 15-18:
28 |   (public_name qed)
                    ^^^
Error: You cannot declare items to be installed without adding a
<package>.opam file at the root of your project.
To declare elements to be installed as part of package "qed", add a
"qed.opam" file at the root of your project.
Root of the project as discovered by dune: src/libraries/qed

So this message probably prompted them to add empty opam files historically.

[rgrinberg]

Which opam command did you use?

So this message probably prompted them to add empty opam files historically.

You could tweak this message to say that packages can be declared in the project file

[Alizter]

I didn't use opam in the message above. That was just dune pkg lock followed by dune build.

The INSTALL.md file details setting up the switch with opam for which the magic command is opam pin . -n. This won't pick up the opam files in the subdirectories I believe. So that's probably why opam hasn't complained about this.

This is probably just an unfortunate misuse of opam files that can be corrected, but I think a better step would be to make that error message more informative like you siad.

[rgrinberg]

I think you should use opam's --recursive flag to make a fair comparison

[Alizter]

opam pin . -n --recursive also seems to just ignore the empty files rather than fail on them. Maybe @kit-ty-kate can confirm if opam will ignore empty opam files or not.

[kit-ty-kate]

yes, currently opam pin does ignore any opam file that fail to parse and simply prints an error message