fix(pkg): use available local package version to satisfy =version opam constraints

[art-w]

The solver currently assumes that local packages always have version dev even when the dune-project or .opam file specifies a different version. This leads to solver failure when a local package depends on a non-local package with a {= version} constraint, since we won't find a dev version of the dependency in that case (as we should have used the locally specified version instead). cc @Leonidas-from-XIV

[Leonidas-from-XIV]

The code looks good and seems to pass all tests :)

I have a worry about this though that it would require packages to have correct version information and will fail otherwise. In my experience, nearly every time I check out a git repo and it defines a version, the version is wrong. Unless it is the commit from the version it was tagged, but asides from that (name foo) (version 0.1.0) in a git repo and foo.0.1.0 aren't necessarily compatible.

Currently all { = version} constraints succeed because the version for all local packages is dev and dune doesn't care about versions when building anyway, but if they don't match up, then we suddenly start failing to lock projects that worked before.

How does opam handle this? If you pin a package, will it use the version info from the opam files or will it pick dev by default?

[art-w]

How does opam handle this? If you pin a package, will it use the version info from the opam files or will it pick dev by default?

Opam will use the version from the opam file if specified, or the latest known version from the opam-repo otherwise (which is bonkers, as it depends on when you last ran opam update), or dev for truly unknown packages. The last two fallback behaviors together result in an ugly issue when a project repo has a new unpublished package, with {=version} constraints with its published packages, as opam pin will produce a mix of "latest known version" and dev version which are incompatibles (so the best practice is to opam pin --with-version=dev).

I would very much like to avoid this in dune pkg indeed... I'm hoping that there are two differences from how opam does things which makes this reasonable:

• If (version ...) isn't specified, then dev is the default as usual (no random "latest known version" is picked)
• If (version ...) is specified in the dune-project then it'll end up in the .opam files produced by dune, so we get the same behavior as opam pin (which doesn't use its fallbacks in that case). It's much easier to keep all local packages on the same version from the dune-project root (version ...), so unless the maintainer decided to manually override this in a (package ...) then all local packages would automatically have the same version (and things should just work).

On the other end, I found it surprising that dune pkg lock would fail to satisfy an (= version) constraint because it decided to ignore the specified (version ...) and picked dev instead. Now if a maintainer specified a wrong (version ...) in their project, such that their dependencies constraints are wrong if we use their (version ...), then I would argue it's a bug in their project configuration and not in dune pkg... I guess someone could publish a new package version to opam and forget to update their dune-project (version ...), but in that case it would have been easier not to specify that (version ...) in the first place.

[rgrinberg]

Writing a version inside the project file is a non starter for dune. One of the very first things that dune did to improve packaging over oasis and friends is relieve users of the duty to maintain these version numbers in their project configuration.

While it’s a shame we don’t support this, writing such constraints seems like a misfeature to me. Just to give one example, when does it ever make sense to bump a constraint on a dependency whenever you released a version of your package?

If this information is really needed to guide the solver, it can go in the workspace file.

[rgrinberg]

I think my original assessment of this PR was a bit harsh. We don't need to approve of this feature to support it. Existing users will remain largely unaffected by this. We should just be mindful that this is only an edge case that we don't mind supporting, and not promote it as the recommended way of interacting with the solver.

[Leonidas-from-XIV]

I guess someone could publish a new package version to opam and forget to update their dune-project (version ...), but in that case it would have been easier not to specify that (version ...) in the first place.

I fully agree, yet I recall packages specifying this (I guess because if you specify name, why not also version and then forget about it as it mostly works, as OPAM ignores it except for pinning but authors don't pin their packages) where the first PR to a project would be to remove it and explain the maintainer why this is a bad idea that looks like a good idea on the surface.

Existing users will remain largely unaffected by this

That said, could we maybe assess how many packages on opam would be affected (by checking out the dev-repo and seeing whether there's a version in .opam or dune-profile)?

If setting the version to dev always is the better solution, I fear that giving people a gun to shoot themselves in the foot might be the wrong thing to do and the better solution would be do add documentation why we are doing this and do the better thing by default instead. Or maybe introduce a warning what's disabled by --release?

So far the only usecase I know for it is when you have a project that has multiple packages that use {= :version} as a constraint and delete some of them, then the solver will fail to find a solution that matches <deleted-package>.dev on OPAM. With this fix it will lock . successfully. But even to me who produced this exact bug it seems like a rather contrived setup.

[Leonidas-from-XIV]

@art-w After discussing this today with @gridbugs it seems this feature is actually way more useful than I expected (even outside of opam-health-check which does weird stuff) so if you could rebase it, I'm in favor of merging it.