From be903a2a162852059b1e5249681657ce8ff07cf0 Mon Sep 17 00:00:00 2001
From: Marek Kubica <marek@tarides.com>
Date: Sat, 15 Jan 2022 17:43:07 +0100
Subject: [PATCH 01/19] Display error message when none of the versions matches

Closes #215
---
 cli/lock.ml        | 27 ++++++++++++++++
 lib/opam_solve.ml  | 78 ++++++++++++++++++++++++++++++++++++++++++++++
 lib/opam_solve.mli |  5 +++
 3 files changed, 110 insertions(+)

diff --git a/cli/lock.ml b/cli/lock.ml
index f748dc32b..165cf4b21 100644
--- a/cli/lock.ml
+++ b/cli/lock.ml
@@ -192,9 +192,36 @@ 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
+        "Could not find any package that would satisfy the version constrants: \
+         %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 ->
diff --git a/lib/opam_solve.ml b/lib/opam_solve.ml
index e216aacae..5eadd57ef 100644
--- a/lib/opam_solve.ml
+++ b/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,68 @@ module Make_solver (Context : OPAM_MONOREPO_CONTEXT) :
       rolemap []
     |> List.filter_map ~f:Solver.package_name
 
+  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 -> (
+            match Solver.package_name pkg with
+            | None -> acc
+            | Some pkg_name -> (
+                let rejects, _reason =
+                  Solver.Diagnostics.Component.rejects component
+                in
+                (* In the rejected candidates, try to find one where it had failed a version restriction and extract that one *)
+                let version_restriction =
+                  List.find_map
+                    ~f:(fun (_model, reason) ->
+                      match reason with
+                      | `FailsRestriction restriction ->
+                          let _, version_restriction =
+                            Solver.formula restriction
+                          in
+                          Some version_restriction
+                      | _ -> None)
+                    rejects
+                in
+                match version_restriction with
+                | None -> acc
+                | Some version_restriction -> (
+                    (* find only the model rejections that is those that we have rejected as e.g. not building with dune *)
+                    let model_rejected =
+                      List.filter_map
+                        ~f:(fun (model, reason) ->
+                          match reason with
+                          | `Model_rejection _ -> Some model
+                          | _ -> None)
+                        rejects
+                    in
+                    let all_failures_due_to_version =
+                      List.for_all
+                        ~f:(fun model ->
+                          match Solver.version model with
+                          | None -> true
+                          | Some rejected_package ->
+                              let rejected_version =
+                                OpamPackage.version rejected_package
+                              in
+                              let failed_due_to_version =
+                                OpamFormula.check_version_formula
+                                  version_restriction rejected_version
+                                |> not
+                              in
+                              failed_due_to_version)
+                        model_rejected
+                    in
+                    match
+                      (List.length model_rejected, all_failures_due_to_version)
+                    with
+                    | 0, _ | _, false -> acc
+                    | _, true -> (pkg_name, version_restriction) :: 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 +489,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
diff --git a/lib/opam_solve.mli b/lib/opam_solve.mli
index aab23228d..333cde47d 100644
--- a/lib/opam_solve.mli
+++ b/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

From b925c7fcf74ea1aec5e86726506097c12a940438 Mon Sep 17 00:00:00 2001
From: Marek Kubica <marek@tarides.com>
Date: Tue, 18 Jan 2022 12:18:07 +0100
Subject: [PATCH 02/19] Simplify skipping the rest of the fold using `let`
 operator

---
 lib/opam_solve.ml | 107 +++++++++++++++++++++++-----------------------
 1 file changed, 53 insertions(+), 54 deletions(-)

diff --git a/lib/opam_solve.ml b/lib/opam_solve.ml
index 5eadd57ef..722307c6b 100644
--- a/lib/opam_solve.ml
+++ b/lib/opam_solve.ml
@@ -260,60 +260,59 @@ module Make_solver (Context : OPAM_MONOREPO_CONTEXT) :
       (fun pkg component acc ->
         match Solver.Diagnostics.Component.selected_impl component with
         | Some _ -> acc
-        | None -> (
-            match Solver.package_name pkg with
-            | None -> acc
-            | Some pkg_name -> (
-                let rejects, _reason =
-                  Solver.Diagnostics.Component.rejects component
-                in
-                (* In the rejected candidates, try to find one where it had failed a version restriction and extract that one *)
-                let version_restriction =
-                  List.find_map
-                    ~f:(fun (_model, reason) ->
-                      match reason with
-                      | `FailsRestriction restriction ->
-                          let _, version_restriction =
-                            Solver.formula restriction
-                          in
-                          Some version_restriction
-                      | _ -> None)
-                    rejects
-                in
-                match version_restriction with
-                | None -> acc
-                | Some version_restriction -> (
-                    (* find only the model rejections that is those that we have rejected as e.g. not building with dune *)
-                    let model_rejected =
-                      List.filter_map
-                        ~f:(fun (model, reason) ->
-                          match reason with
-                          | `Model_rejection _ -> Some model
-                          | _ -> None)
-                        rejects
-                    in
-                    let all_failures_due_to_version =
-                      List.for_all
-                        ~f:(fun model ->
-                          match Solver.version model with
-                          | None -> true
-                          | Some rejected_package ->
-                              let rejected_version =
-                                OpamPackage.version rejected_package
-                              in
-                              let failed_due_to_version =
-                                OpamFormula.check_version_formula
-                                  version_restriction rejected_version
-                                |> not
-                              in
-                              failed_due_to_version)
-                        model_rejected
-                    in
-                    match
-                      (List.length model_rejected, all_failures_due_to_version)
-                    with
-                    | 0, _ | _, false -> acc
-                    | _, true -> (pkg_name, version_restriction) :: 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 rejects, _reason =
+              Solver.Diagnostics.Component.rejects component
+            in
+            (* In the rejected candidates, try to find one where it had failed a version restriction and extract that one *)
+            let* version_restriction =
+              List.find_map
+                ~f:(fun (_model, reason) ->
+                  match reason with
+                  | `FailsRestriction restriction ->
+                      let _, version_restriction = Solver.formula restriction in
+                      Some version_restriction
+                  | _ -> None)
+                rejects
+            in
+            (* find only the model rejections that is those that we have rejected as e.g. not building with dune *)
+            let model_rejected =
+              List.filter_map
+                ~f:(fun (model, reason) ->
+                  match reason with
+                  | `Model_rejection _ -> Some model
+                  | _ -> None)
+                rejects
+            in
+            (* make sure the list is not empty *)
+            let* _ = List.nth_opt model_rejected 0 in
+            let all_failures_due_to_version =
+              List.for_all
+                ~f:(fun model ->
+                  match Solver.version model with
+                  | None -> true
+                  | Some rejected_package ->
+                      let rejected_version =
+                        OpamPackage.version rejected_package
+                      in
+                      let failed_due_to_version =
+                        OpamFormula.check_version_formula version_restriction
+                          rejected_version
+                        |> not
+                      in
+                      failed_due_to_version)
+                model_rejected
+            in
+            let* failed_pkg =
+              match all_failures_due_to_version with
+              | false -> None
+              | true -> Some (pkg_name, version_restriction)
+            in
+            failed_pkg :: acc)
       rolemap []
 
   let get_opam_info ~context pkg =

From d506432d42961fbf1a2e325d7f02b943a7545200 Mon Sep 17 00:00:00 2001
From: Marek Kubica <marek@tarides.com>
Date: Tue, 18 Jan 2022 12:23:38 +0100
Subject: [PATCH 03/19] Add changelog entry

---
 CHANGES.md | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/CHANGES.md b/CHANGES.md
index 255f2f845..4ba13a177 100644
--- a/CHANGES.md
+++ b/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
 

From eea63345ceb15b36812a218f093391f3ea4f14b3 Mon Sep 17 00:00:00 2001
From: Marek Kubica <marek@tarides.com>
Date: Wed, 19 Jan 2022 14:52:22 +0100
Subject: [PATCH 04/19] Read the version_restriction from the notes

This has the advantage that it will not fail when there aren't any
FailsRestriction rejections.
---
 lib/opam_solve.ml | 24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/lib/opam_solve.ml b/lib/opam_solve.ml
index 722307c6b..7eea5e080 100644
--- a/lib/opam_solve.ml
+++ b/lib/opam_solve.ml
@@ -263,21 +263,25 @@ module Make_solver (Context : OPAM_MONOREPO_CONTEXT) :
         | 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 rejects, _reason =
-              Solver.Diagnostics.Component.rejects component
-            in
+            let notes = Solver.Diagnostics.Component.notes component in
             (* In the rejected candidates, try to find one where it had failed a version restriction and extract that one *)
             let* version_restriction =
               List.find_map
-                ~f:(fun (_model, reason) ->
-                  match reason with
-                  | `FailsRestriction restriction ->
-                      let _, version_restriction = Solver.formula restriction in
-                      Some version_restriction
+                ~f:(function
+                  | Restricts (_other_role, _impl, restrictions) -> (
+                      match restrictions with
+                      | [] -> None
+                      | restriction :: _ ->
+                          let _, version_restriction =
+                            Solver.formula restriction
+                          in
+                          Some version_restriction)
                   | _ -> None)
-                rejects
+                notes
+            in
+            let rejects, _reason =
+              Solver.Diagnostics.Component.rejects component
             in
             (* find only the model rejections that is those that we have rejected as e.g. not building with dune *)
             let model_rejected =

From a0cf396a62ac4f2ba50fd2c5ba1a5d8168ebba42 Mon Sep 17 00:00:00 2001
From: Marek Kubica <marek@tarides.com>
Date: Wed, 19 Jan 2022 15:02:22 +0100
Subject: [PATCH 05/19] Also load restriction from UserRestriction

---
 lib/opam_solve.ml | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/lib/opam_solve.ml b/lib/opam_solve.ml
index 7eea5e080..299a5d396 100644
--- a/lib/opam_solve.ml
+++ b/lib/opam_solve.ml
@@ -266,19 +266,17 @@ module Make_solver (Context : OPAM_MONOREPO_CONTEXT) :
             let* pkg_name = Solver.package_name pkg in
             let notes = Solver.Diagnostics.Component.notes component in
             (* In the rejected candidates, try to find one where it had failed a version restriction and extract that one *)
-            let* version_restriction =
+            let* _, version_restriction =
               List.find_map
                 ~f:(function
+                  | UserRequested restriction -> Some restriction
                   | Restricts (_other_role, _impl, restrictions) -> (
                       match restrictions with
                       | [] -> None
-                      | restriction :: _ ->
-                          let _, version_restriction =
-                            Solver.formula restriction
-                          in
-                          Some version_restriction)
+                      | restriction :: _ -> Some restriction)
                   | _ -> None)
                 notes
+              |> Option.map ~f:Solver.formula
             in
             let rejects, _reason =
               Solver.Diagnostics.Component.rejects component

From 09a0c06bccece56a2cfcfc267c6465dc2b743a00 Mon Sep 17 00:00:00 2001
From: Marek Kubica <marek@tarides.com>
Date: Fri, 4 Feb 2022 15:10:53 +0100
Subject: [PATCH 06/19] Add test and fix behaviour of error message

---
 cli/lock.ml                                   |  4 +-
 lib/opam_solve.ml                             | 14 +++----
 .../invalid-package-version.t/existing.opam   |  9 ++++
 .../repo/packages/a/a.0.1/opam                | 11 +++++
 .../repo/packages/a/a.1.0/opam                | 11 +++++
 test/bin/invalid-package-version.t/repo/repo  |  1 +
 test/bin/invalid-package-version.t/run.t      | 41 +++++++++++++++++++
 .../bin/invalid-package-version.t/toonew.opam |  9 ++++
 8 files changed, 89 insertions(+), 11 deletions(-)
 create mode 100644 test/bin/invalid-package-version.t/existing.opam
 create mode 100644 test/bin/invalid-package-version.t/repo/packages/a/a.0.1/opam
 create mode 100644 test/bin/invalid-package-version.t/repo/packages/a/a.1.0/opam
 create mode 100644 test/bin/invalid-package-version.t/repo/repo
 create mode 100644 test/bin/invalid-package-version.t/run.t
 create mode 100644 test/bin/invalid-package-version.t/toonew.opam

diff --git a/cli/lock.ml b/cli/lock.ml
index 165cf4b21..d76cc0dc1 100644
--- a/cli/lock.ml
+++ b/cli/lock.ml
@@ -212,8 +212,8 @@ let could_not_determine_version offending_packages =
   let pp_offending_packages = Fmt.list ~sep:Fmt.comma pp_offending_package in
   Logs.err (fun l ->
       l
-        "Could not find any package that would satisfy the version constrants: \
-         %a"
+        "There is no eligible package that matches %a. Make sure a dune port \
+         exists."
         pp_offending_packages offending_packages)
 
 let interpret_solver_error ~repositories solver = function
diff --git a/lib/opam_solve.ml b/lib/opam_solve.ml
index 299a5d396..75fd5536d 100644
--- a/lib/opam_solve.ml
+++ b/lib/opam_solve.ml
@@ -265,7 +265,6 @@ module Make_solver (Context : OPAM_MONOREPO_CONTEXT) :
             let ( let* ) a f = match a with Some a -> f a | None -> acc in
             let* pkg_name = Solver.package_name pkg in
             let notes = Solver.Diagnostics.Component.notes component in
-            (* In the rejected candidates, try to find one where it had failed a version restriction and extract that one *)
             let* _, version_restriction =
               List.find_map
                 ~f:(function
@@ -290,10 +289,8 @@ module Make_solver (Context : OPAM_MONOREPO_CONTEXT) :
                   | _ -> None)
                 rejects
             in
-            (* make sure the list is not empty *)
-            let* _ = List.nth_opt model_rejected 0 in
-            let all_failures_due_to_version =
-              List.for_all
+            let model_rejections_would_match_version =
+              List.exists
                 ~f:(fun model ->
                   match Solver.version model with
                   | None -> true
@@ -301,16 +298,15 @@ module Make_solver (Context : OPAM_MONOREPO_CONTEXT) :
                       let rejected_version =
                         OpamPackage.version rejected_package
                       in
-                      let failed_due_to_version =
+                      let would_be_eligible_otherwise =
                         OpamFormula.check_version_formula version_restriction
                           rejected_version
-                        |> not
                       in
-                      failed_due_to_version)
+                      would_be_eligible_otherwise)
                 model_rejected
             in
             let* failed_pkg =
-              match all_failures_due_to_version with
+              match model_rejections_would_match_version with
               | false -> None
               | true -> Some (pkg_name, version_restriction)
             in
diff --git a/test/bin/invalid-package-version.t/existing.opam b/test/bin/invalid-package-version.t/existing.opam
new file mode 100644
index 000000000..fff3d6558
--- /dev/null
+++ b/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"
+]
diff --git a/test/bin/invalid-package-version.t/repo/packages/a/a.0.1/opam b/test/bin/invalid-package-version.t/repo/packages/a/a.0.1/opam
new file mode 100644
index 000000000..fbfb7c47f
--- /dev/null
+++ b/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"
+  ]
+}
diff --git a/test/bin/invalid-package-version.t/repo/packages/a/a.1.0/opam b/test/bin/invalid-package-version.t/repo/packages/a/a.1.0/opam
new file mode 100644
index 000000000..6338acd17
--- /dev/null
+++ b/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"
+  ]
+}
diff --git a/test/bin/invalid-package-version.t/repo/repo b/test/bin/invalid-package-version.t/repo/repo
new file mode 100644
index 000000000..013b84db6
--- /dev/null
+++ b/test/bin/invalid-package-version.t/repo/repo
@@ -0,0 +1 @@
+opam-version: "2.0"
diff --git a/test/bin/invalid-package-version.t/run.t b/test/bin/invalid-package-version.t/run.t
new file mode 100644
index 000000000..b83cdd7af
--- /dev/null
+++ b/test/bin/invalid-package-version.t/run.t
@@ -0,0 +1,41 @@
+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 display an error message that there is no version of `a` that matches the constraint.
+
+  $ opam-monorepo lock toonew 2> errors
+  ==> Using 1 locally scanned package as the target.
+  [1]
+  $ grep -Pzo "(?s)opam-monorepo: \[ERROR\].*(?=opam-monorepo)" < errors
+  opam-monorepo: [ERROR] There is no eligible package that matches a
+                         >= 1.0. Make sure a dune port exists.
diff --git a/test/bin/invalid-package-version.t/toonew.opam b/test/bin/invalid-package-version.t/toonew.opam
new file mode 100644
index 000000000..bda63c257
--- /dev/null
+++ b/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"
+]

From 3eb46ff741d5dbd2eff64396f5e263b8055ae334 Mon Sep 17 00:00:00 2001
From: Marek Kubica <marek@tarides.com>
Date: Mon, 7 Feb 2022 16:46:36 +0100
Subject: [PATCH 07/19] Fix the test

Thanks to @emillon for helping me debug this behavior of grep. Which is
somewhat understandable (given grep adds a \n and with -z it is a \0)
but extremely unhelpful.
---
 test/bin/invalid-package-version.t/run.t | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/test/bin/invalid-package-version.t/run.t b/test/bin/invalid-package-version.t/run.t
index b83cdd7af..120601fc2 100644
--- a/test/bin/invalid-package-version.t/run.t
+++ b/test/bin/invalid-package-version.t/run.t
@@ -26,16 +26,21 @@ opam-monorepo solver should successfully pick a.0.1:
   $ 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:
+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 display an error message that there is no version of `a` that matches the constraint.
+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 -Pzo "(?s)opam-monorepo: \[ERROR\].*(?=opam-monorepo)" < errors
+  $ 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. Make sure a dune port exists.

From 4f0897421311e03c8b9992bb0cda219c36180b43 Mon Sep 17 00:00:00 2001
From: Marek Kubica <marek@tarides.com>
Date: Tue, 8 Feb 2022 17:05:48 +0100
Subject: [PATCH 08/19] Revise error message

---
 cli/lock.ml                              | 3 +--
 test/bin/invalid-package-version.t/run.t | 3 +--
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/cli/lock.ml b/cli/lock.ml
index d76cc0dc1..49d7f9d0a 100644
--- a/cli/lock.ml
+++ b/cli/lock.ml
@@ -212,8 +212,7 @@ let could_not_determine_version offending_packages =
   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. Make sure a dune port \
-         exists."
+        "There is no eligible package that matches %a."
         pp_offending_packages offending_packages)
 
 let interpret_solver_error ~repositories solver = function
diff --git a/test/bin/invalid-package-version.t/run.t b/test/bin/invalid-package-version.t/run.t
index 120601fc2..9de615cf3 100644
--- a/test/bin/invalid-package-version.t/run.t
+++ b/test/bin/invalid-package-version.t/run.t
@@ -42,5 +42,4 @@ to the test)
   ==> 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. Make sure a dune port exists.
+  opam-monorepo: [ERROR] There is no eligible package that matches a >= 1.0.

From 3eecdc0bc4272bba94ed524bfaeffb92ee290021 Mon Sep 17 00:00:00 2001
From: Marek Kubica <marek@tarides.com>
Date: Tue, 8 Feb 2022 17:10:57 +0100
Subject: [PATCH 09/19] Only pass over the model rejections once

---
 cli/lock.ml       |  5 ++---
 lib/opam_solve.ml | 37 ++++++++++++++++---------------------
 2 files changed, 18 insertions(+), 24 deletions(-)

diff --git a/cli/lock.ml b/cli/lock.ml
index 49d7f9d0a..0e8b8065e 100644
--- a/cli/lock.ml
+++ b/cli/lock.ml
@@ -211,9 +211,8 @@ let could_not_determine_version offending_packages =
   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)
+      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
diff --git a/lib/opam_solve.ml b/lib/opam_solve.ml
index 75fd5536d..54816b864 100644
--- a/lib/opam_solve.ml
+++ b/lib/opam_solve.ml
@@ -280,31 +280,26 @@ module Make_solver (Context : OPAM_MONOREPO_CONTEXT) :
             let rejects, _reason =
               Solver.Diagnostics.Component.rejects component
             in
-            (* find only the model rejections that is those that we have rejected as e.g. not building with dune *)
-            let model_rejected =
-              List.filter_map
+            (* 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 _ -> Some model
-                  | _ -> None)
+                  | `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 model_rejections_would_match_version =
-              List.exists
-                ~f:(fun model ->
-                  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)
-                model_rejected
-            in
             let* failed_pkg =
               match model_rejections_would_match_version with
               | false -> None

From eddddf5f17cf2069e252247ce2fbdab9f8354d1f Mon Sep 17 00:00:00 2001
From: Marek Kubica <marek@tarides.com>
Date: Tue, 8 Feb 2022 17:23:28 +0100
Subject: [PATCH 10/19] Factor out code for getting the version restriction

---
 lib/opam_solve.ml | 31 ++++++++++++++++++-------------
 stdext/option.ml  |  4 ++++
 stdext/option.mli |  4 ++++
 3 files changed, 26 insertions(+), 13 deletions(-)

diff --git a/lib/opam_solve.ml b/lib/opam_solve.ml
index 54816b864..90c8321ac 100644
--- a/lib/opam_solve.ml
+++ b/lib/opam_solve.ml
@@ -254,6 +254,23 @@ 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
@@ -264,19 +281,7 @@ module Make_solver (Context : OPAM_MONOREPO_CONTEXT) :
             (* 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 notes = Solver.Diagnostics.Component.notes component in
-            let* _, version_restriction =
-              List.find_map
-                ~f:(function
-                  | UserRequested restriction -> Some restriction
-                  | Restricts (_other_role, _impl, restrictions) -> (
-                      match restrictions with
-                      | [] -> None
-                      | restriction :: _ -> Some restriction)
-                  | _ -> None)
-                notes
-              |> Option.map ~f:Solver.formula
-            in
+            let* version_restriction = find_version_restriction component in
             let rejects, _reason =
               Solver.Diagnostics.Component.rejects component
             in
diff --git a/stdext/option.ml b/stdext/option.ml
index e442c87f0..e9ada62e7 100644
--- a/stdext/option.ml
+++ b/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
diff --git a/stdext/option.mli b/stdext/option.mli
index b9b5955ae..09aa2b250 100644
--- a/stdext/option.mli
+++ b/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

From d0ab13ee02eb53d5de1c6ab4626a0b6af0a640d9 Mon Sep 17 00:00:00 2001
From: Marek Kubica <marek@tarides.com>
Date: Wed, 9 Feb 2022 14:59:33 +0100
Subject: [PATCH 11/19] Much better version error message

---
 cli/lock.ml                              | 13 ++++++-------
 test/bin/invalid-package-version.t/run.t |  2 +-
 2 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/cli/lock.ml b/cli/lock.ml
index 0e8b8065e..8a876aa07 100644
--- a/cli/lock.ml
+++ b/cli/lock.ml
@@ -206,13 +206,12 @@ let could_not_determine_version offending_packages =
   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)
+  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
diff --git a/test/bin/invalid-package-version.t/run.t b/test/bin/invalid-package-version.t/run.t
index 9de615cf3..5d7d492c8 100644
--- a/test/bin/invalid-package-version.t/run.t
+++ b/test/bin/invalid-package-version.t/run.t
@@ -42,4 +42,4 @@ to the test)
   ==> 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.
+  opam-monorepo: [ERROR] There is no eligible version of a that matches >= 1.0

From 5d8d795496680958e51c8a8b37c8fad6cd2e448c Mon Sep 17 00:00:00 2001
From: Marek Kubica <marek@tarides.com>
Date: Mon, 14 Feb 2022 14:28:06 +0100
Subject: [PATCH 12/19] Rename `no_matching_versions` to a better name

---
 cli/lock.ml        | 2 +-
 lib/opam_solve.ml  | 8 ++++----
 lib/opam_solve.mli | 2 +-
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/cli/lock.ml b/cli/lock.ml
index 8a876aa07..96d58d900 100644
--- a/cli/lock.ml
+++ b/cli/lock.ml
@@ -216,7 +216,7 @@ let could_not_determine_version offending_packages =
 let interpret_solver_error ~repositories solver = function
   | `Msg _ as err -> err
   | `Diagnostics d ->
-      (match Opam_solve.no_matching_versions solver d with
+      (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
diff --git a/lib/opam_solve.ml b/lib/opam_solve.ml
index 90c8321ac..c3c8e55bd 100644
--- a/lib/opam_solve.ml
+++ b/lib/opam_solve.ml
@@ -180,7 +180,7 @@ module type OPAM_MONOREPO_SOLVER = sig
 
   val not_buildable_with_dune : diagnostics -> OpamPackage.Name.t list
 
-  val no_matching_versions :
+  val unavailable_versions_due_to_constraints :
     diagnostics -> (OpamPackage.Name.t * OpamFormula.version_formula) list
 end
 
@@ -271,7 +271,7 @@ module Make_solver (Context : OPAM_MONOREPO_CONTEXT) :
     let _, version_restriction = Solver.formula restriction in
     Some version_restriction
 
-  let no_matching_versions diagnostics =
+  let unavailable_versions_due_to_constraints diagnostics =
     let rolemap = Solver.diagnostics_rolemap diagnostics in
     Pkg_map.fold
       (fun pkg component acc ->
@@ -487,7 +487,7 @@ let not_buildable_with_dune :
   in
   Solver.not_buildable_with_dune diagnostics
 
-let no_matching_versions :
+let unavailable_versions_due_to_constraints :
     type context diagnostics.
     (context, diagnostics) t ->
     diagnostics ->
@@ -498,4 +498,4 @@ let no_matching_versions :
          and type input = context) =
     t
   in
-  Solver.no_matching_versions diagnostics
+  Solver.unavailable_versions_due_to_constraints diagnostics
diff --git a/lib/opam_solve.mli b/lib/opam_solve.mli
index 333cde47d..2da01d422 100644
--- a/lib/opam_solve.mli
+++ b/lib/opam_solve.mli
@@ -41,7 +41,7 @@ val diagnostics_message :
 val not_buildable_with_dune :
   (_, 'diagnostics) t -> 'diagnostics -> OpamPackage.Name.t list
 
-val no_matching_versions :
+val unavailable_versions_due_to_constraints :
   (_, 'diagnostics) t ->
   'diagnostics ->
   (OpamPackage.Name.t * OpamFormula.version_formula) list

From a76fb856ca1e9b2260e97f0884250395a41cef15 Mon Sep 17 00:00:00 2001
From: Marek Kubica <marek@tarides.com>
Date: Mon, 14 Feb 2022 14:39:44 +0100
Subject: [PATCH 13/19] Factor out named function for better readability

---
 lib/opam_solve.ml | 47 ++++++++++++++++++++++++++---------------------
 1 file changed, 26 insertions(+), 21 deletions(-)

diff --git a/lib/opam_solve.ml b/lib/opam_solve.ml
index c3c8e55bd..cebc23475 100644
--- a/lib/opam_solve.ml
+++ b/lib/opam_solve.ml
@@ -271,46 +271,51 @@ module Make_solver (Context : OPAM_MONOREPO_CONTEXT) :
     let _, version_restriction = Solver.formula restriction in
     Some version_restriction
 
+  (* [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 acc ->
+      (fun pkg component unavailable ->
         match Solver.Diagnostics.Component.selected_impl component with
-        | Some _ -> acc
+        | Some _ -> unavailable
         | None ->
             (* short-circuit skip of fold *)
-            let ( let* ) a f = match a with Some a -> f a | None -> acc in
+            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 = 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 *)
+            (* check if any packages would have had matching versions if they weren't model-rejected *)
             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)
+                ~f:(model_rejection_of_eligible_version version_restriction)
                 rejects
             in
-            let* failed_pkg =
+            (* 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
-            failed_pkg :: acc)
+            unavailable_pkg :: unavailable)
       rolemap []
 
   let get_opam_info ~context pkg =

From 5ca892d18420a75c8a229d1675a642e6b2987465 Mon Sep 17 00:00:00 2001
From: Marek Kubica <marek@tarides.com>
Date: Mon, 14 Feb 2022 15:55:59 +0100
Subject: [PATCH 14/19] Add test for multiple constraints

---
 .../multiple-constraint.opam                          |  9 +++++++++
 .../repo/packages/a/a.1.1/opam                        | 11 +++++++++++
 test/bin/invalid-package-version.t/run.t              | 10 ++++++++++
 3 files changed, 30 insertions(+)
 create mode 100644 test/bin/invalid-package-version.t/multiple-constraint.opam
 create mode 100644 test/bin/invalid-package-version.t/repo/packages/a/a.1.1/opam

diff --git a/test/bin/invalid-package-version.t/multiple-constraint.opam b/test/bin/invalid-package-version.t/multiple-constraint.opam
new file mode 100644
index 000000000..f42495931
--- /dev/null
+++ b/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"
+]
diff --git a/test/bin/invalid-package-version.t/repo/packages/a/a.1.1/opam b/test/bin/invalid-package-version.t/repo/packages/a/a.1.1/opam
new file mode 100644
index 000000000..6610c9e11
--- /dev/null
+++ b/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"
+  ]
+}
diff --git a/test/bin/invalid-package-version.t/run.t b/test/bin/invalid-package-version.t/run.t
index 5d7d492c8..264ed0cd1 100644
--- a/test/bin/invalid-package-version.t/run.t
+++ b/test/bin/invalid-package-version.t/run.t
@@ -43,3 +43,13 @@ to the test)
   [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

From 4184f27dde8ba9e2a20cd89b600dcd0dbadffffa Mon Sep 17 00:00:00 2001
From: Marek Kubica <marek@tarides.com>
Date: Mon, 14 Feb 2022 15:58:33 +0100
Subject: [PATCH 15/19] Collect all version constrains and `And` then together

---
 lib/opam_solve.ml | 44 +++++++++++++++++++++++++++++---------------
 1 file changed, 29 insertions(+), 15 deletions(-)

diff --git a/lib/opam_solve.ml b/lib/opam_solve.ml
index cebc23475..b18e55e56 100644
--- a/lib/opam_solve.ml
+++ b/lib/opam_solve.ml
@@ -254,22 +254,34 @@ module Make_solver (Context : OPAM_MONOREPO_CONTEXT) :
       rolemap []
     |> List.filter_map ~f:Solver.package_name
 
-  let find_version_restriction component =
+  let determine_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
+    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_restriction = Solver.formula restriction in
-    Some version_restriction
+    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] *)
@@ -299,7 +311,9 @@ module Make_solver (Context : OPAM_MONOREPO_CONTEXT) :
               match a with Some a -> f a | None -> unavailable
             in
             let* pkg_name = Solver.package_name pkg in
-            let* version_restriction = find_version_restriction component in
+            let* version_restriction =
+              determine_version_restriction component
+            in
             let rejects, _reason =
               Solver.Diagnostics.Component.rejects component
             in

From 3ed8cf098866f6f98ecc88f986605383751ef3d7 Mon Sep 17 00:00:00 2001
From: Marek Kubica <marek@tarides.com>
Date: Mon, 14 Feb 2022 16:00:30 +0100
Subject: [PATCH 16/19] Use `opam` to query opam file contents

---
 test/bin/invalid-package-version.t/run.t | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/test/bin/invalid-package-version.t/run.t b/test/bin/invalid-package-version.t/run.t
index 264ed0cd1..813fe70b9 100644
--- a/test/bin/invalid-package-version.t/run.t
+++ b/test/bin/invalid-package-version.t/run.t
@@ -7,8 +7,8 @@ We setup the default base repository
 
 Here we define a package test that depends on a package `a`:
 
-  $ grep '\"a\"' < existing.opam
-    "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.
@@ -29,8 +29,8 @@ opam-monorepo solver should successfully pick a.0.1:
 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 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.

From 67f882f1df783f28ef543665f62bf4640cd071b2 Mon Sep 17 00:00:00 2001
From: Marek Kubica <marek@xivilization.net>
Date: Mon, 14 Feb 2022 16:59:02 +0100
Subject: [PATCH 17/19] Eliminate useless use of cat

Co-authored-by: Nathan Rebours <nathan.p.rebours@gmail.com>
---
 test/bin/invalid-package-version.t/run.t | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/bin/invalid-package-version.t/run.t b/test/bin/invalid-package-version.t/run.t
index 813fe70b9..aa9cfaba3 100644
--- a/test/bin/invalid-package-version.t/run.t
+++ b/test/bin/invalid-package-version.t/run.t
@@ -23,7 +23,7 @@ opam-monorepo solver should successfully pick a.0.1:
   ==> 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\+{"
+  $ grep "\"a\"\s\+{" existing.opam.locked 
     "a" {= "0.1" & vendor}
 
 Yet if we attempt to use the same package, but pick a version that doesn't

From da4c711f152dbdfd95b0ea3fa86dccfe2cea6c6d Mon Sep 17 00:00:00 2001
From: Marek Kubica <marek@xivilization.net>
Date: Tue, 15 Feb 2022 10:08:58 +0100
Subject: [PATCH 18/19] Simplify let block

Co-authored-by: Nathan Rebours <nathan.p.rebours@gmail.com>
---
 lib/opam_solve.ml | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/lib/opam_solve.ml b/lib/opam_solve.ml
index b18e55e56..4178f61dc 100644
--- a/lib/opam_solve.ml
+++ b/lib/opam_solve.ml
@@ -305,7 +305,7 @@ module Make_solver (Context : OPAM_MONOREPO_CONTEXT) :
       (fun pkg component unavailable ->
         match Solver.Diagnostics.Component.selected_impl component with
         | Some _ -> unavailable
-        | None ->
+        | None -> (
             (* short-circuit skip of fold *)
             let ( let* ) a f =
               match a with Some a -> f a | None -> unavailable
@@ -324,12 +324,9 @@ module Make_solver (Context : OPAM_MONOREPO_CONTEXT) :
                 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)
+            match model_rejections_would_match_version with
+            | false -> unavailable
+            | true -> (pkg_name, version_restriction) :: unavailable))
       rolemap []
 
   let get_opam_info ~context pkg =

From 4a9621c592aa0bc07bcc01e58f2a01f1597c4c94 Mon Sep 17 00:00:00 2001
From: Marek Kubica <marek@tarides.com>
Date: Tue, 15 Feb 2022 10:26:27 +0100
Subject: [PATCH 19/19] Add test with multiple restrictions

---
 .../multiple-constraint.opam                         |  3 ++-
 .../depends-on-min-a/depends-on-min-a.1/opam         | 12 ++++++++++++
 test/bin/invalid-package-version.t/run.t             |  4 ++--
 3 files changed, 16 insertions(+), 3 deletions(-)
 create mode 100644 test/bin/invalid-package-version.t/repo/packages/depends-on-min-a/depends-on-min-a.1/opam

diff --git a/test/bin/invalid-package-version.t/multiple-constraint.opam b/test/bin/invalid-package-version.t/multiple-constraint.opam
index f42495931..6860ac74f 100644
--- a/test/bin/invalid-package-version.t/multiple-constraint.opam
+++ b/test/bin/invalid-package-version.t/multiple-constraint.opam
@@ -1,7 +1,8 @@
 opam-version: "2.0"
 depends: [
   "dune"
-  "a" {>= "1.1" & < "2.0"}
+  "depends-on-min-a"
+  "a" {< "2.0"}
 ]
 x-opam-monorepo-opam-repositories: [
   "file://$OPAM_MONOREPO_CWD/minimal-repo"
diff --git a/test/bin/invalid-package-version.t/repo/packages/depends-on-min-a/depends-on-min-a.1/opam b/test/bin/invalid-package-version.t/repo/packages/depends-on-min-a/depends-on-min-a.1/opam
new file mode 100644
index 000000000..9fb9ccfbc
--- /dev/null
+++ b/test/bin/invalid-package-version.t/repo/packages/depends-on-min-a/depends-on-min-a.1/opam
@@ -0,0 +1,12 @@
+opam-version: "2.0"
+dev-repo: "git+https://a.com/depends-on-min-a.git"
+depends: [
+  "dune"
+  "a" {>= "1.0"}
+]
+url {
+  src: "https://a.com/depends-on-min-a.0.1.tbz"
+  checksum: [
+    "sha256=0000000000000000000000000000000000000000000000000000000000000000"
+  ]
+}
diff --git a/test/bin/invalid-package-version.t/run.t b/test/bin/invalid-package-version.t/run.t
index aa9cfaba3..004900cb8 100644
--- a/test/bin/invalid-package-version.t/run.t
+++ b/test/bin/invalid-package-version.t/run.t
@@ -47,9 +47,9 @@ to the test)
 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"}
+  "dune" "depends-on-min-a" "a" {< "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
+  opam-monorepo: [ERROR] There is no eligible version of a that matches >= 1.0