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,33 @@ 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
+  List.iter
+    ~f:(fun (name, formula) ->
+      Logs.err (fun l ->
+          l "There is no eligible version of %a that matches %a"
+            Opam.Pp.package_name name pp_version_formula formula))
+    offending_packages
+
 let interpret_solver_error ~repositories solver = function
   | `Msg _ as err -> err
   | `Diagnostics d ->
+      (match Opam_solve.unavailable_versions_due_to_constraints 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 unavailable_versions_due_to_constraints :
+    diagnostics -> (OpamPackage.Name.t * OpamFormula.version_formula) list
 end
 
 module Make_solver (Context : OPAM_MONOREPO_CONTEXT) :
@@ -251,6 +254,84 @@ module Make_solver (Context : OPAM_MONOREPO_CONTEXT) :
       rolemap []
     |> List.filter_map ~f:Solver.package_name
 
+  let determine_version_restriction component =
+    let open Option.O in
+    let restrictions =
+      component |> Solver.Diagnostics.Component.notes
+      |> List.map ~f:(function
+           | Solver.Diagnostics.Note.UserRequested restriction ->
+               [ restriction ]
+           | Restricts (_other_role, _impl, restrictions) -> restrictions
+           | _ -> [])
+      |> List.flatten
+    in
+    let* version_restrictions =
+      match restrictions with
+      | [] -> None
+      | restrictions ->
+          restrictions
+          |> List.map ~f:(fun restriction ->
+                 let _, version_restriction = Solver.formula restriction in
+                 version_restriction)
+          |> Option.some
+    in
+    match version_restrictions with
+    | [] -> None
+    | init :: rest ->
+        List.fold_left
+          ~f:(fun formula restriction -> OpamFormula.And (formula, restriction))
+          ~init rest
+        |> Option.some
+
+  (* [true] if the rejected package would've satisified the version constraint if it
+     wouldn't have been a [Model_rejection] *)
+  let model_rejection_of_eligible_version version_restriction (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
+
+  let unavailable_versions_due_to_constraints diagnostics =
+    let rolemap = Solver.diagnostics_rolemap diagnostics in
+    Pkg_map.fold
+      (fun pkg component unavailable ->
+        match Solver.Diagnostics.Component.selected_impl component with
+        | Some _ -> unavailable
+        | None ->
+            (* short-circuit skip of fold *)
+            let ( let* ) a f =
+              match a with Some a -> f a | None -> unavailable
+            in
+            let* pkg_name = Solver.package_name pkg in
+            let* version_restriction =
+              determine_version_restriction component
+            in
+            let rejects, _reason =
+              Solver.Diagnostics.Component.rejects component
+            in
+            (* check if any packages would have had matching versions if they weren't model-rejected *)
+            let model_rejections_would_match_version =
+              List.exists
+                ~f:(model_rejection_of_eligible_version version_restriction)
+                rejects
+            in
+            (* if it is unavailable, construct info on why *)
+            let* unavailable_pkg =
+              match model_rejections_would_match_version with
+              | false -> None
+              | true -> Some (pkg_name, version_restriction)
+            in
+            unavailable_pkg :: unavailable)
+      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 +505,16 @@ let not_buildable_with_dune :
     t
   in
   Solver.not_buildable_with_dune diagnostics
+
+let unavailable_versions_due_to_constraints :
+    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.unavailable_versions_due_to_constraints 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 unavailable_versions_due_to_constraints :
+  (_, '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/multiple-constraint.opam

@@ -0,0 +1,9 @@
+opam-version: "2.0"
+depends: [
+  "dune"
+  "a" {>= "1.1" & < "2.0"}
+]
+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/packages/a/a.1.1/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.1.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,55 @@
+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`:
+
+  $ opam show --no-lint --raw -fdepends ./existing.opam
+  "dune" "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:
+
+  $ opam show --no-lint --raw -fdepends ./toonew.opam
+  "dune" "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 version of a that matches >= 1.0
+
+We should also produce the right error message with all the constraints when we have multiple constaints
+
+  $ opam show --no-lint --raw -fdepends ./multiple-constraint.opam
+  "dune" "a" {>= "1.1" & < "2.0"}
+  $ opam-monorepo lock multiple-constraint 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 version of a that matches >= 1.1 & < 2.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"
+]