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

assertion failed when using computed versions #3716

Open
avsm opened this issue Dec 27, 2018 · 4 comments
Open

assertion failed when using computed versions #3716

avsm opened this issue Dec 27, 2018 · 4 comments

Comments

@avsm
Copy link
Member

avsm commented Dec 27, 2018

I'm trying to use computed versions in opam 2.0.2 to make multi-repository packages easier, but it leads to an assertion failure in opam:

+opam install depext
Fatal error:
File "src/solver/opamSolver.ml", line 231, characters 18-24: Assertion failed
'unset TESTS; OPAMYES=1 export OPAMYES; set -uex; opam install depext' exited 99. Terminating with 99

Full log: https://travis-ci.com/avsm/mirage-clock/jobs/167225256

Example of an opam file causing this:

opam-version: "2.0"
maintainer: "anil@recoil.org"
authors: ["Anil Madhavapeddy" "Daniel C. Bünzli" "Matthew Gray"]
license: "ISC"
tags: "org:mirage"
homepage: "https://github.com/mirage/mirage-clock"
doc: "https://mirage.github.io/mirage-clock/"
bug-reports: "https://github.com/mirage/mirage-clock/issues"
synopsis: "Unix-based implementation for the MirageOS Clock interface"
description: """
The Unix implementation of the MirageOS Clock interface uses
`gettimeofday` or `clock_gettime`, depending on
which OS is in use (see [clock_stubs.c](https://github.com/mirage/mirage-clock/blob/master/unix/clock_stubs.c)).
"""
depends: [
  "ocaml" {>= "4.04.2"}
  "dune" {build}
  "mirage-clock" {>= version}
  "mirage-clock-lwt" {>= versionn}
  "lwt"
]
build: [
  ["dune" "subst"] {pinned}
  ["dune" "build" "-p" name "-j" jobs]
  ["dune" "runtest" "-p" name] {with-test}
]
dev-repo: "git+https://github.com/mirage/mirage-clock.git"

And the full tree that has the offending versions: https://github.com/mirage/mirage-clock/tree/d57102ff19e43b02791ec67070f3ee315df3b147

avsm added a commit to avsm/mirage-clock that referenced this issue Dec 27, 2018
@avsm
Copy link
Member Author

avsm commented Dec 27, 2018

It looks like this is due to a reference to an undefined variable (versionn). See workaround in avsm/mirage-clock@24f2f42

@AltGr
Copy link
Member

AltGr commented Jan 3, 2019

Indeed, that seems to be the error ; still, assert failing is not the best handing of this case ^ ^

@AltGr
Copy link
Member

AltGr commented Jan 3, 2019

To handle this case, we could hard fail with a better message, or just print a warning and ignore the constraint. However, the general policy is to avoid polluting users with errors from package definition files.

What actually happens in most places is that undefined variable errors are silently ignored. This is consistent in a way, since we have well-defined semantics for handling undefined variables (at least in the boolean case), but has led to confusion and time lost in debugging mistakes quite a few times already.

This is not easy to solve, though: packages are allowed to depend on user-defined variables. The best compromise is probably to have a list of variable names known to be normally defined, and output a warning on linting for other names. This would be a proper solution for global variables, but less so for package variables, which are more commonly custom (using conf files) : we could however trust packagers to ignore these warnings (after double-checking) when they know what they are doing.

@avsm
Copy link
Member Author

avsm commented Jan 30, 2019

I'm not a fan of having lint issue unfixable warnings. One observation is that I've seen many instances of typoed variables (e.g. test instead of with-test), but very few packages use custom user-defined variables. We could just rev the opam-version and add a field to declare the use of those custom variables.

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

No branches or pull requests

2 participants