From 5b353cce5b2d78e007924f903b8b02afaf9d3975 Mon Sep 17 00:00:00 2001 From: Karen Arutyunov Date: Mon, 21 Mar 2022 13:44:48 +0300 Subject: Improve search for available package corresponding to selected package in pkg-build --- bpkg/package.cxx | 31 ++++++++--------------- bpkg/package.hxx | 18 +++++++++++++- bpkg/pkg-build.cxx | 72 +++++++++++++++++++++++++++++++++++++++++++----------- 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& c, - bool order) + bool order, + bool revision) { using query = query; @@ -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 query_available (database&, const package_name&, const optional&, - 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& c, const lazy_shared_ptr& 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& c, const vector>& 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& c, - bool prereq = true) + bool prereq = true, + bool revision = false) { for (database& db: dbs) { - auto r (filter_one (db.load (""), - query_available (db, name, c), - prereq)); + auto r ( + filter_one (db.load (""), + 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 vc ( + !system + ? version_constraint (dsp->version) + : optional ()); + // 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 ())); + // 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). -- cgit v1.1