Better missing version error

CHANGES.md

@@ -5,6 +5,9 @@
 - Add opam extensions `x-opam-monorepo-opam-repositories` and
   `x-opam-monorepo-global-opam-vars` to make `lock` fully reproducible.
   (#250, #253, @NathanReb)
+- Show an error message when the solver can't find any version that satisfies
+  the requested version constraint in the user's OPAM file (#215, #248,
+  @Leonidas-from-XIV)
 
 ### Changed
 

cli/lock.ml

@@ -192,9 +192,34 @@ let display_verbose_diagnostics = function
   | None -> false
   | Some l -> l >= Logs.Info
 
+let could_not_determine_version offending_packages =
+  let f (relop, version) =
+    let pp_relop fmt = function
+      | `Eq -> Fmt.pf fmt "="
+      | `Geq -> Fmt.pf fmt ">="
+      | `Gt -> Fmt.pf fmt ">"
+      | `Leq -> Fmt.pf fmt "<="
+      | `Lt -> Fmt.pf fmt "<"
+      | `Neq -> Fmt.pf fmt "!="
+    in
+    Fmt.str "%a %a" pp_relop relop Opam.Pp.version version
+  in
+  let s = OpamFormula.string_of_formula f in
+  let pp_version_formula = Fmt.using s Fmt.string in
+  let pp_offending_package =
+    Fmt.pair ~sep:Fmt.sp Opam.Pp.package_name pp_version_formula
+  in
+  let pp_offending_packages = Fmt.list ~sep:Fmt.comma pp_offending_package in
+  Logs.err (fun l ->
+      l "There is no eligible package that matches %a." pp_offending_packages
+        offending_packages)
+
 let interpret_solver_error ~repositories solver = function
   | `Msg _ as err -> err
   | `Diagnostics d ->
+      (match Opam_solve.no_matching_versions solver d with
+      | [] -> ()
+      | offending_packages -> could_not_determine_version offending_packages);
       (match Opam_solve.not_buildable_with_dune solver d with
       | [] -> ()
       | offending_packages ->

lib/opam_solve.ml

@@ -179,6 +179,9 @@ module type OPAM_MONOREPO_SOLVER = sig
   val diagnostics_message : verbose:bool -> diagnostics -> [> `Msg of string ]
 
   val not_buildable_with_dune : diagnostics -> OpamPackage.Name.t list
+
+  val no_matching_versions :
+    diagnostics -> (OpamPackage.Name.t * OpamFormula.version_formula) list
 end
 
 module Make_solver (Context : OPAM_MONOREPO_CONTEXT) :
@@ -251,6 +254,65 @@ module Make_solver (Context : OPAM_MONOREPO_CONTEXT) :
       rolemap []
     |> List.filter_map ~f:Solver.package_name
 
+  let find_version_restriction component =
+    let open Option.O in
+    let notes = Solver.Diagnostics.Component.notes component in
+    let* restriction =
+      List.find_map
+        ~f:(function
+          | UserRequested restriction -> Some restriction
+          | Restricts (_other_role, _impl, restrictions) -> (
+              match restrictions with
+              | [] -> None
+              | restriction :: _ -> Some restriction)
+          | _ -> None)
+        notes
+    in
+    let _, version_restriction = Solver.formula restriction in
+    Some version_restriction
+
+  let no_matching_versions diagnostics =
+    let rolemap = Solver.diagnostics_rolemap diagnostics in
+    Pkg_map.fold
+      (fun pkg component acc ->
+        match Solver.Diagnostics.Component.selected_impl component with
+        | Some _ -> acc
+        | None ->
+            (* short-circuit skip of fold *)
+            let ( let* ) a f = match a with Some a -> f a | None -> acc in
+            let* pkg_name = Solver.package_name pkg in
+            let* version_restriction = find_version_restriction component in
+            let rejects, _reason =
+              Solver.Diagnostics.Component.rejects component
+            in
+            (* only looking at the model rejections that is those that we have rejected as e.g. not building with dune *)
+            let model_rejections_would_match_version =
+              List.exists
+                ~f:(fun (model, reason) ->
+                  match reason with
+                  | `Model_rejection _ -> (
+                      match Solver.version model with
+                      | None -> true
+                      | Some rejected_package ->
+                          let rejected_version =
+                            OpamPackage.version rejected_package
+                          in
+                          let would_be_eligible_otherwise =
+                            OpamFormula.check_version_formula
+                              version_restriction rejected_version
+                          in
+                          would_be_eligible_otherwise)
+                  | _ -> false)
+                rejects
+            in
+            let* failed_pkg =
+              match model_rejections_would_match_version with
+              | false -> None
+              | true -> Some (pkg_name, version_restriction)
+            in
+            failed_pkg :: acc)
+      rolemap []
+
   let get_opam_info ~context pkg =
     match Context.opam_file context pkg with
     | Ok opam_file -> Opam.Package_summary.from_opam ~pkg opam_file
@@ -424,3 +486,16 @@ let not_buildable_with_dune :
     t
   in
   Solver.not_buildable_with_dune diagnostics
+
+let no_matching_versions :
+    type context diagnostics.
+    (context, diagnostics) t ->
+    diagnostics ->
+    (OpamPackage.Name.t * OpamFormula.version_formula) list =
+ fun t diagnostics ->
+  let (module Solver : OPAM_MONOREPO_SOLVER
+        with type diagnostics = diagnostics
+         and type input = context) =
+    t
+  in
+  Solver.no_matching_versions diagnostics

lib/opam_solve.mli

@@ -40,3 +40,8 @@ val diagnostics_message :
 
 val not_buildable_with_dune :
   (_, 'diagnostics) t -> 'diagnostics -> OpamPackage.Name.t list
+
+val no_matching_versions :
+  (_, 'diagnostics) t ->
+  'diagnostics ->
+  (OpamPackage.Name.t * OpamFormula.version_formula) list

stdext/option.ml

@@ -10,6 +10,10 @@ module O = struct
   let ( >>= ) opt f = bind ~f opt
 
   let ( >>| ) opt f = map ~f opt
+
+  let ( let* ) = ( >>= )
+
+  let ( let+ ) = ( >>| )
 end
 
 let map_default ~f ~default = function None -> default | Some x -> f x

stdext/option.mli

@@ -6,6 +6,10 @@ module O : sig
   val ( >>= ) : 'a t -> ('a -> 'b t) -> 'b t
 
   val ( >>| ) : 'a t -> ('a -> 'b) -> 'b t
+
+  val ( let* ) : 'a t -> ('a -> 'b t) -> 'b t
+
+  val ( let+ ) : 'a t -> ('a -> 'b) -> 'b t
 end
 
 val value : default:'a -> 'a t -> 'a

test/bin/invalid-package-version.t/existing.opam

@@ -0,0 +1,9 @@
+opam-version: "2.0"
+depends: [
+  "dune"
+  "a"
+]
+x-opam-monorepo-opam-repositories: [
+  "file://$OPAM_MONOREPO_CWD/minimal-repo"
+  "file://$OPAM_MONOREPO_CWD/repo"
+]

test/bin/invalid-package-version.t/repo/packages/a/a.0.1/opam

@@ -0,0 +1,11 @@
+opam-version: "2.0"
+dev-repo: "git+https://a.com/a.git"
+depends: [
+  "dune"
+]
+url {
+  src: "https://a.com/a.0.1.tbz"
+  checksum: [
+    "sha256=0000000000000000000000000000000000000000000000000000000000000000"
+  ]
+}

test/bin/invalid-package-version.t/repo/packages/a/a.1.0/opam

@@ -0,0 +1,11 @@
+opam-version: "2.0"
+dev-repo: "git+https://a.com/a.git"
+depends: [
+  "ocaml"
+]
+url {
+  src: "https://a.com/a.1.0.tbz"
+  checksum: [
+    "sha256=0000000000000000000000000000000000000000000000000000000000000000"
+  ]
+}

test/bin/invalid-package-version.t/repo/repo

@@ -0,0 +1 @@
+opam-version: "2.0"

test/bin/invalid-package-version.t/run.t

@@ -0,0 +1,45 @@
+We want to make sure the error messages are sensible, and as such if the user
+picks a version that doesn't exist we want to make them aware of it.
+
+We setup the default base repository
+
+  $ gen-minimal-repo
+
+Here we define a package test that depends on a package `a`:
+
+  $ grep '\"a\"' < existing.opam
+    "a"
+
+We have a local repo that defines a package `a` that satisfies the predicate,
+with a version that is valid and can be picked.
+
+  $ cat repo/packages/a/a.0.1/opam > /dev/null
+
+opam-monorepo solver should successfully pick a.0.1:
+
+  $ opam-monorepo lock existing
+  ==> Using 1 locally scanned package as the target.
+  ==> Found 9 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 1 entries to $TESTCASE_ROOT/existing.opam.locked. You can now run opam monorepo pull to fetch their sources.
+  $ cat existing.opam.locked | grep "\"a\"\s\+{"
+    "a" {= "0.1" & vendor}
+
+Yet if we attempt to use the same package, but pick a version that doesn't
+exist in our repo:
+
+  $ grep '\"a\"' < toonew.opam
+    "a" {>= "1.0"}
+
+opam-monorepo should fail with some error code and display an error message
+that there is no version of `a` that matches the constraint.
+
+(grep appends a NUL byte at the end, hence the head call, this is not important
+to the test)
+
+  $ opam-monorepo lock toonew 2> errors
+  ==> Using 1 locally scanned package as the target.
+  [1]
+  $ grep -Pazo "(?s)opam-monorepo: \[ERROR\].*(?=opam-monorepo)" < errors | head --bytes=-1
+  opam-monorepo: [ERROR] There is no eligible package that matches a >= 1.0.

test/bin/invalid-package-version.t/toonew.opam

@@ -0,0 +1,9 @@
+opam-version: "2.0"
+depends: [
+  "dune"
+  "a" {>= "1.0"}
+]
+x-opam-monorepo-opam-repositories: [
+  "file://$OPAM_MONOREPO_CWD/minimal-repo"
+  "file://$OPAM_MONOREPO_CWD/repo"
+]