Ignore revision for version equality in pkg-{status,build}, constraints

4 files changed, 104 insertions, 36 deletions

diff --git a/bpkg/package b/bpkg/package
index e46164c..8bd9546 100644
--- a/bpkg/package
+++ b/bpkg/package
@@ -503,13 +503,34 @@ namespace bpkg
   //
   template <typename T1, typename T2>
   inline auto
+  compare_version_eq (const T1& x, const T2& y, bool revision)
+    -> decltype (x.epoch == y.epoch)
+  {
+    // 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).
+    //
+    if (revision)
+      return x.epoch == y.epoch &&
+        x.canonical_upstream == y.canonical_upstream &&
+        x.canonical_release == y.canonical_release &&
+        x.revision == y.revision;
+    else
+      return x.epoch == y.epoch &&
+        x.canonical_upstream == y.canonical_upstream &&
+        x.canonical_release == y.canonical_release;
+  }
+
+  /*
+  Currently unused (and probably should stay that way).
+
+  template <typename T1, typename T2>
+  inline auto
   operator== (const T1& x, const T2& y) -> 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;
+    return compare_version_eq (x, y, true);
   }
+  */
 
   template <typename T1, typename T2>
   inline auto
@@ -580,6 +601,14 @@ namespace bpkg
       + x.canonical_release + "DESC,"
       + x.revision + "DESC";
   }
+
+  template <typename T>
+  inline auto
+  order_by_revision_desc (const T& x) -> //decltype ("ORDER BY" + x.epoch)
+                                         decltype (x.revision == 0)
+  {
+    return "ORDER BY" + x.revision + "DESC";
+  }
 }
 
 #include <bpkg/package.ixx>
diff --git a/bpkg/pkg-build.cxx b/bpkg/pkg-build.cxx
index 6202095..c3dc00e 100644
--- a/bpkg/pkg-build.cxx
+++ b/bpkg/pkg-build.cxx
@@ -66,24 +66,55 @@ namespace bpkg
     //
     if (c)
     {
-      if (c->min_version)
+      // If the constraint is equality and the revision is not explicitly
+      // specified, then compare ignoring the revision. The idea is that when
+      // the user runs 'bpkg build libfoo/1.0.0' and there is 1.0.0+1
+      // available, it should just work. The user shouldn't have to spell the
+      // revision explicitly. Similarly, when we have 'depends: libfoo ==
+      // 1.0.0', then it would be strange if 1.0.0+1 did not satisfy this
+      // constraint.
+      //
+      // Note that strictly speaking 0 doesn't mean unspecified. Which means
+      // with this implementation there is no way to say "I really mean
+      // revision 0" since 1.0.0 == 1.0.0+0. One can, in the current model, say
+      // libfoo == 1.0.0+1, though. This is probably ok since one would assume
+      // any subsequent revision of a package is only better than the ones
+      // before.
+      //
+      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 = compare_version_eq (vm, v, v.revision != 0);
 
-      if (c->max_version)
+        if (v.revision == 0)
+          q += order_by_revision_desc (vm);
+      }
+      else
       {
-        if (c->max_open)
-          q = q && vm < *c->max_version;
-        else
-          q = q && vm <= *c->max_version;
+        if (c->min_version)
+        {
+          if (c->min_open)
+            q = q && vm > *c->min_version;
+          else
+            q = q && vm >= *c->min_version;
+        }
+
+        if (c->max_version)
+        {
+          if (c->max_open)
+            q = q && vm < *c->max_version;
+          else
+            q = q && vm <= *c->max_version;
+        }
+
+        q += order_by_version_desc (vm);
       }
     }
-
-    q += order_by_version_desc (vm);
+    else
+      q += order_by_version_desc (vm);
 
     // Filter the result based on the repository to which each version
     // belongs.
diff --git a/bpkg/pkg-fetch.cxx b/bpkg/pkg-fetch.cxx
index f9c97ca..3032eda 100644
--- a/bpkg/pkg-fetch.cxx
+++ b/bpkg/pkg-fetch.cxx
@@ -186,6 +186,11 @@ namespace bpkg
       fail << "configuration " << c << " has no available packages" <<
         info << "use 'bpkg cfg-fetch' to fetch available packages list";
 
+    // Note that here we compare including the revision (unlike, say in
+    // pkg-status). Which means one cannot just specify 1.0.0 and get 1.0.0+1
+    // -- they must spell it out explicitly. This is probably ok since this is
+    // a low-level command where some extra precision doesn't hurt.
+    //
     shared_ptr<available_package> ap (
       db.find<available_package> (available_package_id (n, v)));
 
diff --git a/bpkg/pkg-status.cxx b/bpkg/pkg-status.cxx
index 26cf803..98c583a 100644
--- a/bpkg/pkg-status.cxx
+++ b/bpkg/pkg-status.cxx
@@ -50,7 +50,7 @@ namespace bpkg
       query q (query::name == n);
 
       if (!v.empty ())
-        q = q && query::version == v;
+        q = q && compare_version_eq (query::version, v, v.revision != 0);
 
       p = db.query_one<selected_package> (q);
     }
@@ -60,29 +60,32 @@ namespace bpkg
     // no need to look for it in available packages.
     //
     vector<shared_ptr<available_package>> aps;
-    if (p == nullptr || v.empty ())
     {
       using query = query<available_package>;
 
       query q (query::id.name == n);
 
-      // If the user specified the version, then only look for that
-      // specific version.
+      // If we found an existing package, then only look for versions greater
+      // than what already exists.
       //
-      if (!v.empty ())
-        q = q && query::id.version == v;
+      if (p != nullptr)
+      {
+        q = q && query::id.version > p->version;
+        q += order_by_version_desc (query::id.version);
+      }
       //
-      // Otherwise, if we found an existing package, then only look for
-      // versions greater than what already exists.
+      // Otherwise, if the user specified the version, then only look for that
+      // specific version (we still do it since there are might be other
+      // revisions).
       //
-      else if (p != nullptr)
-        q = q && query::id.version > p->version;
-
-      q += order_by_version_desc (query::id.version);
+      else if (!v.empty ())
+      {
+        q = q && compare_version_eq (query::id.version, v, v.revision != 0);
+        q += order_by_revision_desc (query::id.version);
+      }
 
-      // Only consider packages that are in repositories that were
-      // explicitly added to the configuration and their complements,
-      // recursively.
+      // Only consider packages that are in repositories that were explicitly
+      // added to the configuration and their complements, recursively.
       //
       aps = filter (db.load<repository> (""), db.query<available_package> (q));
     }
@@ -97,7 +100,7 @@ namespace bpkg
 
       // Also print the version of the package unless the user specified it.
       //
-      if (v.empty ())
+      if (v != p->version)
         cout << " " << p->version;
 
       if (p->hold_package)
@@ -113,10 +116,10 @@ namespace bpkg
     {
       cout << (found ? "; " : "") << "available";
 
-      // If the user specified the version, then there will be only one
-      // entry.
+      // If the user specified the version, then there might only be one
+      // entry in which case it is useless to repeat it.
       //
-      if (v.empty ())
+      if (v.empty () || aps.size () > 1 || aps[0]->version != v)
       {
         for (shared_ptr<available_package> ap: aps)
           cout << ' ' << ap->version;