Improve search for available package corresponding to selected package in pkg-build

3 files changed, 85 insertions, 36 deletions

diff --git a/bpkg/package.cxx b/bpkg/package.cxx
index da560f2..4b9cdc3 100644
--- a/bpkg/package.cxx
+++ b/bpkg/package.cxx
@@ -120,7 +120,8 @@ namespace bpkg
   query_available (database& db,
                    const package_name& name,
                    const optional<version_constraint>& c,
-                   bool order)
+                   bool order,
+                   bool revision)
   {
     using query = query<available_package>;
 
@@ -134,18 +135,6 @@ namespace bpkg
     {
       assert (c->complete ());
 
-      // If the revision is not explicitly specified, then compare ignoring the
-      // revision. The idea is that when the user runs 'bpkg build libfoo/1'
-      // and there is 1+1 available, it should just work. The user shouldn't
-      // have to spell the revision explicitly. Similarly, when we have
-      // 'depends: libfoo == 1', then it would be strange if 1+1 did not
-      // satisfy this constraint. The same for libfoo <= 1 -- 1+1 should
-      // satisfy.
-      //
-      // Note that we always compare ignoring the iteration, as it can not be
-      // specified in the manifest/command line. This way the latest iteration
-      // will always be picked up.
-      //
       query qs (compare_version_eq (vm,
                                     canonical_version (wildcard_version),
                                     false /* revision */,
@@ -160,8 +149,8 @@ namespace bpkg
         q = q &&
             (compare_version_eq (vm,
                                  canonical_version (v),
-                                 v.revision.has_value (),
-                                 false /* iteration */) ||
+                                 revision || v.revision.has_value (),
+                                 revision /* iteration */) ||
              qs);
       }
       else
@@ -172,24 +161,24 @@ namespace bpkg
         {
           const version& v (*c->min_version);
           canonical_version cv (v);
-          bool rv (v.revision);
+          bool rv (revision || v.revision);
 
           if (c->min_open)
-            qr = compare_version_gt (vm, cv, rv, false /* iteration */);
+            qr = compare_version_gt (vm, cv, rv, revision /* iteration */);
           else
-            qr = compare_version_ge (vm, cv, rv, false /* iteration */);
+            qr = compare_version_ge (vm, cv, rv, revision /* iteration */);
         }
 
         if (c->max_version)
         {
           const version& v (*c->max_version);
           canonical_version cv (v);
-          bool rv (v.revision);
+          bool rv (revision || v.revision);
 
           if (c->max_open)
-            qr = qr && compare_version_lt (vm, cv, rv, false /* iteration */);
+            qr = qr && compare_version_lt (vm, cv, rv, revision);
           else
-            qr = qr && compare_version_le (vm, cv, rv, false /* iteration */);
+            qr = qr && compare_version_le (vm, cv, rv, revision);
         }
 
         q = q && (qr || qs);
diff --git a/bpkg/package.hxx b/bpkg/package.hxx
index 26eb34b..fbf8643 100644
--- a/bpkg/package.hxx
+++ b/bpkg/package.hxx
@@ -876,11 +876,27 @@ namespace bpkg
   // version constraint and return them in the version descending order, by
   // default. Note that a stub satisfies any constraint.
   //
+  // By default if the revision is not explicitly specified for the version
+  // constraint, then compare ignoring the revision. The idea is that when the
+  // user runs 'bpkg build libfoo/1' and there is 1+1 available, it should
+  // just work. The user shouldn't have to spell the revision
+  // explicitly. Similarly, when we have 'depends: libfoo == 1', then it would
+  // be strange if 1+1 did not satisfy this constraint. The same for libfoo <=
+  // 1 -- 1+1 should satisfy.
+  //
+  // Note that by default we compare ignoring the iteration, as it can not be
+  // specified in the manifest/command line. This way the latest iteration
+  // will always be picked up.
+  //
+  // Pass true as the revision argument to query the exact available package
+  // version, also comparing the version revision and iteration.
+  //
   odb::result<available_package>
   query_available (database&,
                    const package_name&,
                    const optional<version_constraint>&,
-                   bool order = true);
+                   bool order = true,
+                   bool revision = false);
 
   // Only return packages that are in the specified repository fragments, their
   // complements or prerequisites (if prereq is true), recursively. While you
diff --git a/bpkg/pkg-build.cxx b/bpkg/pkg-build.cxx
index 3d0c47f..8f56fc8 100644
--- a/bpkg/pkg-build.cxx
+++ b/bpkg/pkg-build.cxx
@@ -310,13 +310,17 @@ namespace bpkg
   find_available_one (const package_name& name,
                       const optional<version_constraint>& c,
                       const lazy_shared_ptr<repository_fragment>& rf,
-                      bool prereq = true)
+                      bool prereq = true,
+                      bool revision = false)
   {
     // Filter the result based on the repository fragment to which each
     // version belongs.
     //
     database& db (rf.database ());
-    auto r (filter_one (rf.load (), query_available (db, name, c), prereq));
+    auto r (
+      filter_one (rf.load (),
+                  query_available (db, name, c, true /* order */, revision),
+                  prereq));
 
     if (r.first == nullptr)
       r.first = find_imaginary_stub (name);
@@ -336,12 +340,16 @@ namespace bpkg
                       const package_name& name,
                       const optional<version_constraint>& c,
                       const vector<shared_ptr<repository_fragment>>& rfs,
-                      bool prereq = true)
+                      bool prereq = true,
+                      bool revision = false)
   {
     // Filter the result based on the repository fragments to which each
     // version belongs.
     //
-    auto r (filter_one (rfs, query_available (db, name, c), prereq));
+    auto r (
+      filter_one (rfs,
+                  query_available (db, name, c, true /* order */, revision),
+                  prereq));
 
     if (r.first == nullptr)
       r.first = find_imaginary_stub (name);
@@ -357,13 +365,15 @@ namespace bpkg
   find_available_one (const linked_databases& dbs,
                       const package_name& name,
                       const optional<version_constraint>& c,
-                      bool prereq = true)
+                      bool prereq = true,
+                      bool revision = false)
   {
     for (database& db: dbs)
     {
-      auto r (filter_one (db.load<repository_fragment> (""),
-                          query_available (db, name, c),
-                          prereq));
+      auto r (
+        filter_one (db.load<repository_fragment> (""),
+                    query_available (db, name, c, true /* order */, revision),
+                    prereq));
 
       if (r.first != nullptr)
         return make_pair (
@@ -1650,17 +1660,51 @@ namespace bpkg
               {
                 system = dsp->system ();
 
+                optional<version_constraint> vc (
+                  !system
+                  ? version_constraint (dsp->version)
+                  : optional<version_constraint> ());
+
                 // First try to find an available package for this exact
-                // version. In particular, this handles the case where a
+                // version, falling back to ignoring version revision and
+                // iteration. In particular, this handles the case where a
                 // package moves from one repository to another (e.g., from
                 // testing to stable). For a system package we pick the latest
                 // one (its exact version doesn't really matter).
                 //
-                rp = find_available_one (dependent_repo_configs (*ddb),
-                                         dn,
-                                         (!system
-                                          ? version_constraint (dsp->version)
-                                          : optional<version_constraint> ()));
+                // It seems reasonable to search for the package in the
+                // repositories explicitly added by the user if the selected
+                // package was explicitly specified on command line, and in
+                // the repository (and its complements/prerequisites) of the
+                // dependent being currently built otherwise.
+                //
+                if (dsp->hold_package)
+                {
+                  linked_databases dbs (dependent_repo_configs (*ddb));
+
+                  rp = find_available_one (dbs,
+                                           dn,
+                                           vc,
+                                           true /* prereq */,
+                                           true /* revision */);
+
+                  // Note: constraint is not present for the system package,
+                  // so there is no sense to repeat the attempt.
+                  //
+                  if (dap == nullptr && !system)
+                    rp = find_available_one (dbs, dn, vc);
+                }
+                else if (af != nullptr)
+                {
+                  rp = find_available_one (dn,
+                                           vc,
+                                           af,
+                                           true /* prereq */,
+                                           true /* revision */);
+
+                  if (dap == nullptr && !system)
+                    rp = find_available_one (dn, vc, af);
+                }
 
                 // A stub satisfies any version constraint so we weed them out
                 // (returning stub as an available package feels wrong).