From 5905fbe8053c5e58e77234dc1f9f81bde6e46b41 Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Mon, 25 Jan 2016 17:28:57 +0200 Subject: Ignore revision for version equality in pkg-{status,build}, constraints --- bpkg/package | 37 ++++++++++++++++++++++++++++++---- bpkg/pkg-build.cxx | 57 +++++++++++++++++++++++++++++++++++++++++------------ bpkg/pkg-fetch.cxx | 5 +++++ bpkg/pkg-status.cxx | 41 ++++++++++++++++++++------------------ 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 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 + 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 inline auto @@ -580,6 +601,14 @@ namespace bpkg + x.canonical_release + "DESC," + x.revision + "DESC"; } + + template + 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 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 ap ( db.find (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 (q); } @@ -60,29 +60,32 @@ namespace bpkg // no need to look for it in available packages. // vector> aps; - if (p == nullptr || v.empty ()) { using query = query; 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 (""), db.query (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 ap: aps) cout << ' ' << ap->version; -- cgit v1.1