tarides/opam-monorepo

If all packages are opam-provided, an invalid lockfile is created

emillon opened this issue · 1 comments

Is opam-monorepo so powerful that it can create a lockfile that it can't pull? Turns out, yes :/

 $ cat src.opam
opam-version: "2.0"
depends: ["lwt"]
x-opam-monorepo-opam-provided: ["lwt"]
 $ opam monorepo lock        
==> Using 1 locally scanned package as the target.
==> Found 16 opam dependencies for the target package.
==> Querying opam database for their metadata and Dune compatibility.
==> Calculating exact pins for each of them.
==> Wrote lockfile with 0 entries to src.opam.locked. You can now run opam monorepo pull to fetch their sources.
 $ opam monorepo pull
==> Using lockfile src.opam.locked
opam-monorepo: [ERROR] Missing x-opam-monorepo-duniverse-dirs field in opam-monorepo lockfile /tmp/bug/src.opam.locked

The reason is that the lockfile does not, indeed, contain this field.

After a light investigation, it seems that it's due to how the opam libs render empty lists:

utop # OpamFile.OPAM.empty
  |> OpamFile.OPAM.with_extensions
      (OpamStd.String.Map.singleton "x-field"
        (OpamTypesBase.nullify_pos (OpamParserTypes.FullPos.List (OpamTypesBase.nullify_pos []))))
  |> OpamFile.OPAM.write_to_string;;
- : string = "opam-version: \"2.0\"\n"

So from the point of view of opam-monorepo, I think that it's necessary to handle the absence of this field as equivalent to an empty list. A special empty value is possible too I guess but since lockfiles are versioned I don't think there can be a confusion between "no field" and "an empty list".

Good catch!

The fix is easy but it opens a broader question: is opam-monorepo useful in such situation and is silently accepting this good for users?

I guess we'll stick to the straight forward fix for now but this is still something to think about.