Skip to content
This repository has been archived by the owner on Oct 3, 2024. It is now read-only.

Commit

Permalink
Differentiate between missing dune dependency vs lower bound
Browse files Browse the repository at this point in the history
Previously, we incorrectly showed a missing dune dependency error, when
a version lower bound constraint was missing. This commit improves the
error messages to differentiate between missing lower bound constraint
vs missing the dune dependency entirely.
  • Loading branch information
punchagan committed Sep 4, 2024
1 parent 4c2ff93 commit 8af2b49
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 6 deletions.
5 changes: 3 additions & 2 deletions lib/lint.ml
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ module Checks = struct
| OpamFormula.Atom (pkg, constr) ->
if is_dune pkg then
let v = get_lower_bound constr in
Some (Option.default "1.0" v)
Some (Option.default "" v)
else None
| Empty -> None
| Block x -> aux x
Expand All @@ -249,10 +249,11 @@ module Checks = struct
match (dune_constraint, dune_version) with
| _, Error msg -> [ (pkg, FailedToDownload msg) ]
| None, Ok None -> []
| Some "", _ -> [ (pkg, DuneLowerBoundMissing) ]
| Some _, Ok None -> [ (pkg, DuneProjectMissing) ]
| None, Ok (Some _) ->
if is_dune (OpamPackage.name pkg) then []
else [ (pkg, DuneConstraintMissing) ]
else [ (pkg, DuneDependencyMissing) ]
| Some dep, Ok (Some ver) ->
if OpamVersionCompare.compare dep ver >= 0 then []
else [ (pkg, BadDuneConstraint (dep, ver)) ]
Expand Down
10 changes: 8 additions & 2 deletions lib/lint_error.ml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ type error =
| UnmatchedVersion of OpamPackage.Version.t
| DubiousDuneSubst
| DuneProjectMissing
| DuneConstraintMissing
| DuneDependencyMissing
| DuneLowerBoundMissing
| DuneIsBuild
| BadDuneConstraint of string * string
| UnexpectedFile of string
Expand Down Expand Up @@ -99,11 +100,16 @@ let msg_of_error (package, (err : error)) =
"Warning in %s: The package seems to use dune but the dune-project \
file is missing."
pkg
| DuneConstraintMissing ->
| DuneDependencyMissing ->
Printf.sprintf
"Warning in %s: The package has a dune-project file but no explicit \
dependency on dune was found."
pkg
| DuneLowerBoundMissing ->
Printf.sprintf
"Warning in %s: The package has a dune dependency without a lower \
bound."
pkg
| BadDuneConstraint (dep, ver) ->
Printf.sprintf
"Error in %s: Your dune-project file indicates that this package \
Expand Down
6 changes: 4 additions & 2 deletions test/lint.t
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ Setup repo for incorrect b package tests
$ git apply "patches/b-incorrect-opam.patch"
$ git add packages/
$ echo "(lang dune 3.16)" > dune-project
$ sh "scripts/setup_sources.sh" b 0.0.2 dune-project
Created tarball b.0.0.2.tgz
Updated checksum for b.0.0.2.tgz in b.0.0.2's opam file
$ sh "scripts/setup_sources.sh" b 0.0.3 dune-project
Created tarball b.0.0.3.tgz
Updated checksum for b.0.0.3.tgz in b.0.0.3's opam file
Expand Down Expand Up @@ -63,9 +66,8 @@ Test the following:
Linting opam-repository at $TESTCASE_ROOT/. ...
Warning in b.0.0.2: Dubious use of 'dune subst'. 'dune subst' should always only be called with {dev} (i.e. ["dune" "subst"] {dev}) If your opam file has been autogenerated by dune, you need to upgrade your dune-project to at least (lang dune 2.7).
Warning in b.0.0.2: The package tagged dune as a build dependency. Due to a bug in dune (https://github.com/ocaml/dune/issues/2147) this should never be the case. Please remove the {build} tag from its filter.
Error in b.0.0.2: Failed to download the archive. Details: $TESTCASE_ROOT/b.0.0.2.tgz
Warning in b.0.0.2: The package has a dune dependency without a lower bound.
Error in b.0.0.2: error 3: File format error in 'unknown-field' at line 11, column 0: Invalid field unknown-field
Error in b.0.0.2: error 60: Upstream check failed: "Source not found: $TESTCASE_ROOT/b.0.0.2.tgz"
[1]
$ opam-ci-check lint -r . -c b.0.0.3
Linting opam-repository at $TESTCASE_ROOT/. ...
Expand Down

0 comments on commit 8af2b49

Please sign in to comment.