Ignore version revision in package dependencies resolution

6 files changed, 127 insertions, 64 deletions

diff --git a/brep/package b/brep/package
index 2700e8a..9812feb 100644
--- a/brep/package
+++ b/brep/package
@@ -635,74 +635,116 @@ namespace brep
   // Version comparison operators.
   //
   // They allow comparing objects that have epoch, canonical_upstream,
-  // canonical_release, and revision data members.
+  // canonical_release, and revision data members. The idea is that this
+  // works for both query members of types version and canonical_version
+  // as well as for comparing canonical_version to version.
   //
   template <typename T1, typename T2>
   inline auto
-  operator== (const T1& x, const T2& y) -> decltype (x.epoch == y.epoch)
+  compare_version_eq (const T1& x, const T2& y, bool revision)
+    -> decltype (x.epoch == y.epoch)
   {
-    return x.epoch == y.epoch &&
-      x.canonical_upstream == y.canonical_upstream &&
-      x.canonical_release == y.canonical_release &&
-      x.revision == y.revision;
+    // Since we don't quite know what T1 and T2 are (and where the resulting
+    // expression will run), let's not push our luck with something like
+    // (!revision || x.revision == y.revision).
+    //
+    auto r (x.epoch == y.epoch &&
+            x.canonical_upstream == y.canonical_upstream &&
+            x.canonical_release == y.canonical_release);
+
+    return revision
+      ? r && x.revision == y.revision
+      : r;
   }
 
   template <typename T1, typename T2>
   inline auto
-  operator!= (const T1& x, const T2& y) -> decltype (x.epoch != y.epoch)
+  compare_version_ne (const T1& x, const T2& y, bool revision)
+    -> decltype (x.epoch == y.epoch)
   {
-    return x.epoch != y.epoch ||
-      x.canonical_upstream != y.canonical_upstream ||
-      x.canonical_release != y.canonical_release ||
-      x.revision != y.revision;
+    auto r (x.epoch != y.epoch ||
+            x.canonical_upstream != y.canonical_upstream ||
+            x.canonical_release != y.canonical_release);
+
+    return revision
+      ? r || x.revision != y.revision
+      : r;
   }
 
   template <typename T1, typename T2>
   inline auto
-  operator< (const T1& x, const T2& y) -> decltype (x.epoch < y.epoch)
+  compare_version_lt (const T1& x, const T2& y, bool revision)
+    -> decltype (x.epoch == y.epoch)
   {
-    return x.epoch < y.epoch ||
+    auto r (
+      x.epoch < y.epoch ||
       (x.epoch == y.epoch && x.canonical_upstream < y.canonical_upstream) ||
       (x.epoch == y.epoch && x.canonical_upstream == y.canonical_upstream &&
-       x.canonical_release < y.canonical_release) ||
+       x.canonical_release < y.canonical_release));
+
+    return revision
+      ? r ||
       (x.epoch == y.epoch && x.canonical_upstream == y.canonical_upstream &&
-       x.canonical_release == y.canonical_release && x.revision < y.revision);
+       x.canonical_release == y.canonical_release && x.revision < y.revision)
+      : r;
   }
 
   template <typename T1, typename T2>
   inline auto
-  operator<= (const T1& x, const T2& y) -> decltype (x.epoch <= y.epoch)
+  compare_version_le (const T1& x, const T2& y, bool revision)
+    -> decltype (x.epoch == y.epoch)
   {
-    return x.epoch < y.epoch ||
-      (x.epoch == y.epoch && x.canonical_upstream < y.canonical_upstream) ||
+    auto r (
+      x.epoch < y.epoch ||
+      (x.epoch == y.epoch && x.canonical_upstream < y.canonical_upstream));
+
+    return revision
+      ? r ||
       (x.epoch == y.epoch && x.canonical_upstream == y.canonical_upstream &&
        x.canonical_release < y.canonical_release) ||
       (x.epoch == y.epoch && x.canonical_upstream == y.canonical_upstream &&
-       x.canonical_release == y.canonical_release && x.revision <= y.revision);
+       x.canonical_release == y.canonical_release && x.revision <= y.revision)
+      : r ||
+      (x.epoch == y.epoch && x.canonical_upstream == y.canonical_upstream &&
+       x.canonical_release <= y.canonical_release);
   }
 
   template <typename T1, typename T2>
   inline auto
-  operator> (const T1& x, const T2& y) -> decltype (x.epoch > y.epoch)
+  compare_version_gt (const T1& x, const T2& y, bool revision)
+    -> decltype (x.epoch == y.epoch)
   {
-    return x.epoch > y.epoch ||
+    auto r (
+      x.epoch > y.epoch ||
       (x.epoch == y.epoch && x.canonical_upstream > y.canonical_upstream) ||
       (x.epoch == y.epoch && x.canonical_upstream == y.canonical_upstream &&
-       x.canonical_release > y.canonical_release) ||
+       x.canonical_release > y.canonical_release));
+
+    return revision
+      ? r ||
       (x.epoch == y.epoch && x.canonical_upstream == y.canonical_upstream &&
-       x.canonical_release == y.canonical_release && x.revision > y.revision);
+       x.canonical_release == y.canonical_release && x.revision > y.revision)
+      : r;
   }
 
   template <typename T1, typename T2>
   inline auto
-  operator>= (const T1& x, const T2& y) -> decltype (x.epoch >= y.epoch)
+  compare_version_ge (const T1& x, const T2& y, bool revision)
+    -> decltype (x.epoch == y.epoch)
   {
-    return x.epoch > y.epoch ||
-      (x.epoch == y.epoch && x.canonical_upstream > y.canonical_upstream) ||
+    auto r (
+      x.epoch > y.epoch ||
+      (x.epoch == y.epoch && x.canonical_upstream > y.canonical_upstream));
+
+    return revision
+      ? r ||
       (x.epoch == y.epoch && x.canonical_upstream == y.canonical_upstream &&
        x.canonical_release > y.canonical_release) ||
       (x.epoch == y.epoch && x.canonical_upstream == y.canonical_upstream &&
-       x.canonical_release == y.canonical_release && x.revision >= y.revision);
+       x.canonical_release == y.canonical_release && x.revision >= y.revision)
+      : r ||
+      (x.epoch == y.epoch && x.canonical_upstream == y.canonical_upstream &&
+       x.canonical_release >= y.canonical_release);
   }
 
   template <typename T>
@@ -723,19 +765,8 @@ namespace brep
     if (int r = x.name.compare (y.name))
       return r < 0;
 
-    return x.version < y.version;
+    return compare_version_lt (x.version, y.version, true);
   }
 }
 
-namespace odb
-{
-  using ::brep::operator==;
-  using ::brep::operator!=;
-  using ::brep::operator<;
-  using ::brep::operator<=;
-  using ::brep::operator>;
-  using ::brep::operator>=;
-  using ::brep::order_by_version_desc;
-}
-
 #endif // BREP_PACKAGE
diff --git a/load/load.cxx b/load/load.cxx
index 9e6ee5d..e2a318c 100644
--- a/load/load.cxx
+++ b/load/load.cxx
@@ -591,20 +591,32 @@ resolve_dependencies (package& p, database& db)
       {
         auto c (*d.constraint);
 
-        if (c.min_version)
+        if (c.min_version && c.max_version && *c.min_version == *c.max_version)
         {
-          if (c.min_open)
-            q = q && vm > *c.min_version;
-          else
-            q = q && vm >= *c.min_version;
+          const version& v (*c.min_version);
+          q = q && compare_version_eq (vm, v, v.revision != 0);
         }
-
-        if (c.max_version)
+        else
         {
-          if (c.max_open)
-            q = q && vm < *c.max_version;
-          else
-            q = q && vm <= *c.max_version;
+          if (c.min_version)
+          {
+            const version& v (*c.min_version);
+
+            if (c.min_open)
+              q = q && compare_version_gt (vm, v, v.revision != 0);
+            else
+              q = q && compare_version_ge (vm, v, v.revision != 0);
+          }
+
+          if (c.max_version)
+          {
+            const version& v (*c.max_version);
+
+            if (c.max_open)
+              q = q && compare_version_lt (vm, v, v.revision != 0);
+            else
+              q = q && compare_version_le (vm, v, v.revision != 0);
+          }
         }
       }
 
diff --git a/tests/load/1/math/packages b/tests/load/1/math/packages
index dbeda89..9c8bfea 100644
--- a/tests/load/1/math/packages
+++ b/tests/load/1/math/packages
@@ -18,7 +18,7 @@ location: libstudxml-1.0.0+1.tar.gz
 sha256sum: 05ccba3da34dd0296866027a26b6bacf08cacc80f54516d3b8d8eeccbe31ab93
 :
 name: libexp
-version: 1~1.2
+version: 1~1.2+1
 summary: The exponent
 description: The exponent math function.
 license: MIT
@@ -26,7 +26,7 @@ tags: c++, exponent
 url: http://www.exp.com
 email: users@exp.com
 depends: libmisc
-location: libexp-1~1.2.tar.gz
+location: libexp-1~1.2+1.tar.gz
 sha256sum: 15ccba3da34dd0296866027a26b6bacf08cacc80f54516d3b8d8eeccbe31ab93
 :
 name: libfoo
diff --git a/tests/load/1/misc/packages b/tests/load/1/misc/packages
index b5bb275..9cffca5 100644
--- a/tests/load/1/misc/packages
+++ b/tests/load/1/misc/packages
@@ -2,7 +2,7 @@
 sha256sum: 2f2b297be1eafd70fe55f179a0cf062baf8405e08b3854600801420132a206b1
 :
 name: libbar
-version: 2.3.5
+version: 2.4.0+3
 priority: security; Very important to install.
 summary: The Bar library
 description: very very good library.
@@ -14,7 +14,7 @@ depends: libfoo
 depends: libmath >= 2.0.0
 requires: linux | windows | macosx
 changes: some changes
-location: libbar-2.3.5.tar.gz
+location: libbar-2.4.0+3.tar.gz
 sha256sum: 70ccba3da34dd0296866027a26b6bacf08cacc80f54516d3b8d8eeccbe31ab93
 :
 name: libfoo
diff --git a/tests/load/1/testing/packages b/tests/load/1/testing/packages
index 82fcc90..a903878 100644
--- a/tests/load/1/testing/packages
+++ b/tests/load/1/testing/packages
@@ -10,3 +10,13 @@ email: misc-users@example.com
 depends: libexpat >= 5.0
 location: libmisc-2.4.0.tar.gz
 sha256sum: 75ccba3da34dd0296866027a26b6bacf08cacc80f54516d3b8d8eeccbe31ab94
+:
+name: libmisc
+version: 2.3.0+1
+summary: The Misc Library
+license: MIT
+url: http://www.example.com/misc/
+email: misc-users@example.com
+depends: libexpat >= 5.0
+location: libmisc-2.3.0+1.tar.gz
+sha256sum: 11ccba3da34dd0296866027a26b6bacf08cacc80f54516d3b8d8eeccbe31ab94
diff --git a/tests/load/driver.cxx b/tests/load/driver.cxx
index 7a70ff5..992ce2d 100644
--- a/tests/load/driver.cxx
+++ b/tests/load/driver.cxx
@@ -117,7 +117,7 @@ main (int argc, char* argv[])
       transaction t (db.begin ());
 
       assert (db.query<repository> ().size () == 5);
-      assert (db.query<package> ().size () == 15);
+      assert (db.query<package> ().size () == 16);
 
       shared_ptr<repository> sr (db.load<repository> ("cppget.org/stable"));
       shared_ptr<repository> mr (db.load<repository> ("cppget.org/math"));
@@ -388,7 +388,7 @@ main (int argc, char* argv[])
       assert (mr->internal);
 
       shared_ptr<package> epv (
-        db.load<package> (package_id ("libexp", version ("1~1.2"))));
+        db.load<package> (package_id ("libexp", version ("1~1.2+1"))));
       assert (check_location (epv));
 
       shared_ptr<package> fpv5 (
@@ -593,7 +593,7 @@ main (int argc, char* argv[])
       assert (!cr->internal);
 
       shared_ptr<package> bpv (
-        db.load<package> (package_id ("libbar", version ("2.3.5"))));
+        db.load<package> (package_id ("libbar", version ("2.4.0+3"))));
       assert (check_location (bpv));
 
       shared_ptr<package> fpv0 (
@@ -610,7 +610,7 @@ main (int argc, char* argv[])
 
       // Verify libbar package version.
       //
-      // libbar-2.3.5
+      // libbar-2.4.0+3
       //
       assert (check_external (*bpv));
       assert (bpv->other_repositories.size () == 1);
@@ -655,6 +655,10 @@ main (int argc, char* argv[])
         db.load<package> (package_id ("libmisc", version ("2.4.0"))));
       assert (check_location (mpv0));
 
+      shared_ptr<package> mpv1 (
+        db.load<package> (package_id ("libmisc", version ("2.3.0+1"))));
+      assert (check_location (mpv1));
+
       assert (tr->prerequisites.empty ());
       assert (tr->complements.size () == 1);
       assert (tr->complements[0].load () == gr);
@@ -667,6 +671,12 @@ main (int argc, char* argv[])
       assert (mpv0->other_repositories.size () == 1);
       assert (mpv0->other_repositories[0].load () == tr);
 
+      // libmisc-2.3.0+1
+      //
+      assert (check_external (*mpv1));
+      assert (mpv1->other_repositories.size () == 1);
+      assert (mpv1->other_repositories[0].load () == tr);
+
       // Verify 'staging' repository.
       //
       assert (gr->location.canonical_name () == "cppget.org/staging");
@@ -696,9 +706,9 @@ main (int argc, char* argv[])
         db.load<package> (package_id ("libgenx", version ("1.0"))));
       assert (check_location (gpv));
 
-      shared_ptr<package> mpv1 (
+      shared_ptr<package> mpv2 (
         db.load<package> (package_id ("libmisc", version ("1.0"))));
-      assert (check_location (mpv1));
+      assert (check_location (mpv2));
 
       assert (gr->prerequisites.empty ());
       assert (gr->complements.empty ());
@@ -723,9 +733,9 @@ main (int argc, char* argv[])
       //
       // libmisc-1.0
       //
-      assert (check_external (*mpv1));
-      assert (mpv1->other_repositories.size () == 1);
-      assert (mpv1->other_repositories[0].load () == gr);
+      assert (check_external (*mpv2));
+      assert (mpv2->other_repositories.size () == 1);
+      assert (mpv2->other_repositories[0].load () == gr);
 
       // Change package summary, update the object persistent state, rerun
       // loader and ensure the model were not rebuilt.
@@ -740,7 +750,7 @@ main (int argc, char* argv[])
     transaction t (db.begin ());
 
     shared_ptr<package> bpv (
-      db.load<package> (package_id ("libbar", version ("2.3.5"))));
+      db.load<package> (package_id ("libbar", version ("2.4.0+3"))));
 
     assert (bpv->summary == "test");