Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
emillon opened this issue Jul 4, 2022 · 1 comment · Fixed by #321
Closed

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

emillon opened this issue Jul 4, 2022 · 1 comment · Fixed by #321

Comments

@emillon
Copy link
Collaborator

emillon commented Jul 4, 2022

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".

@NathanReb
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants