From 2d907a525ab169f1cb97b87550e3646fde003733 Mon Sep 17 00:00:00 2001 From: Karen Arutyunov Date: Tue, 3 Sep 2019 00:18:20 +0300 Subject: Adapt to optional package revision --- libbrep/build.hxx | 3 +- libbrep/common.cxx | 2 +- libbrep/common.hxx | 83 +++++++++++++++++++++++--------- libbrep/package.hxx | 2 +- libbrep/package.xml | 2 + load/load.cxx | 22 ++++++--- migrate/migrate.cxx | 21 ++++++++ mod/mod-build-task.cxx | 2 +- mod/mod-builds.cxx | 31 ++++++++++-- tests/load/1/math/libfoo-1.2.4+1.tar.gz | Bin 990 -> 993 bytes tests/load/1/math/packages.manifest | 4 +- tests/load/driver.cxx | 38 +++++++++------ 12 files changed, 153 insertions(+), 57 deletions(-) diff --git a/libbrep/build.hxx b/libbrep/build.hxx index a8139a8..625a0ca 100644 --- a/libbrep/build.hxx +++ b/libbrep/build.hxx @@ -52,8 +52,7 @@ namespace brep : package (move (p)), configuration (move (c)), toolchain_name (move (n)), - toolchain_version { - v.epoch, v.canonical_upstream, v.canonical_release, v.revision} {} + toolchain_version (v) {} }; inline bool diff --git a/libbrep/common.cxx b/libbrep/common.cxx index fcd3a14..b2429a6 100644 --- a/libbrep/common.cxx +++ b/libbrep/common.cxx @@ -6,5 +6,5 @@ namespace brep { - const version wildcard_version (0, "0", nullopt, 0, 0); + const version wildcard_version (0, "0", nullopt, nullopt, 0); } diff --git a/libbrep/common.hxx b/libbrep/common.hxx index 0f92d17..6dc4870 100644 --- a/libbrep/common.hxx +++ b/libbrep/common.hxx @@ -30,7 +30,7 @@ namespace brep uint16_t epoch; string canonical_upstream; string canonical_release; - uint16_t revision; + optional revision; string upstream; optional release; }; @@ -126,6 +126,22 @@ namespace brep // using bpkg::version; + // Sometimes we need to split the version into two parts: the part + // that goes into the object id (epoch, canonical upstream, canonical + // release, revision) and the original upstream and release. This is what + // the canonical_version and upstream_version value types are for. Note that + // upstream_version derives from version and uses it as storage. The idea + // here is this: when we split the version, we often still want to have the + // "whole" version object readily accessible and that's exactly what this + // strange contraption is for. See package for an example on how everything + // fits together. + // + // Note that the object id cannot contain an optional member which is why we + // make the revision type uint16_t and represent nullopt as zero. This + // should be ok for package object ids referencing the package manifest + // version values because an absent revision and zero revision mean the + // same thing. + // #pragma db value struct canonical_version { @@ -134,6 +150,15 @@ namespace brep string canonical_release; uint16_t revision; + canonical_version () = default; + + explicit + canonical_version (const version& v) + : epoch (v.epoch), + canonical_upstream (v.canonical_upstream), + canonical_release (v.canonical_release), + revision (v.effective_revision ()) {} + bool empty () const noexcept { @@ -142,8 +167,10 @@ namespace brep // rightmost digit-only zero components? So non-empty version("0") has // an empty canonical_upstream. // - return epoch == 0 && canonical_upstream.empty () && - canonical_release.empty () && revision == 0; + return epoch == 0 && + canonical_upstream.empty () && + canonical_release.empty () && + revision == 0; } // Change collation to ensure the proper comparison of the "absent" release @@ -161,14 +188,15 @@ namespace brep #pragma db value transient struct upstream_version: version { - #pragma db member(upstream_) virtual(string) \ - get(this.upstream) \ - set(this = brep::version (0, std::move (?), std::string (), 0, 0)) + #pragma db member(upstream_) virtual(string) \ + get(this.upstream) \ + set(this = brep::version ( \ + 0, std::move (?), std::string (), brep::nullopt, 0)) - #pragma db member(release_) virtual(optional_string) \ - get(this.release) \ - set(this = brep::version ( \ - 0, std::move (this.upstream), std::move (?), 0, 0)) + #pragma db member(release_) virtual(optional_string) \ + get(this.release) \ + set(this = brep::version ( \ + 0, std::move (this.upstream), std::move (?), brep::nullopt, 0)) upstream_version () = default; upstream_version (version v): version (move (v)) {} @@ -178,7 +206,16 @@ namespace brep void init (const canonical_version& cv, const upstream_version& uv) { - *this = version (cv.epoch, uv.upstream, uv.release, cv.revision, 0); + // Note: revert the zero revision mapping (see above). + // + *this = version (cv.epoch, + uv.upstream, + uv.release, + (cv.revision != 0 + ? optional (cv.revision) + : nullopt), + 0); + assert (cv.canonical_upstream == canonical_upstream && cv.canonical_release == canonical_release); } @@ -211,10 +248,7 @@ namespace brep package_id (string t, package_name n, const brep::version& v) : tenant (move (t)), name (move (n)), - version { - v.epoch, v.canonical_upstream, v.canonical_release, v.revision} - { - } + version (v) {} }; // repository_type @@ -292,13 +326,16 @@ namespace brep // // They allow comparing objects that have epoch, canonical_upstream, // 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. + // works for both query members of types version and canonical_version. + // Note, though, that the object revisions should be comparable (both + // optional, numeric, etc), so to compare version to query member or + // canonical_version you may need to explicitly convert the version object + // to canonical_version. // template inline auto compare_version_eq (const T1& x, const T2& y, bool revision) - -> decltype (x.epoch == y.epoch) + -> decltype (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 @@ -316,7 +353,7 @@ namespace brep template inline auto compare_version_ne (const T1& x, const T2& y, bool revision) - -> decltype (x.epoch == y.epoch) + -> decltype (x.revision == y.revision) { auto r (x.epoch != y.epoch || x.canonical_upstream != y.canonical_upstream || @@ -330,7 +367,7 @@ namespace brep template inline auto compare_version_lt (const T1& x, const T2& y, bool revision) - -> decltype (x.epoch == y.epoch) + -> decltype (x.revision == y.revision) { auto r ( x.epoch < y.epoch || @@ -348,7 +385,7 @@ namespace brep template inline auto compare_version_le (const T1& x, const T2& y, bool revision) - -> decltype (x.epoch == y.epoch) + -> decltype (x.revision == y.revision) { auto r ( x.epoch < y.epoch || @@ -368,7 +405,7 @@ namespace brep template inline auto compare_version_gt (const T1& x, const T2& y, bool revision) - -> decltype (x.epoch == y.epoch) + -> decltype (x.revision == y.revision) { auto r ( x.epoch > y.epoch || @@ -386,7 +423,7 @@ namespace brep template inline auto compare_version_ge (const T1& x, const T2& y, bool revision) - -> decltype (x.epoch == y.epoch) + -> decltype (x.revision == y.revision) { auto r ( x.epoch > y.epoch || diff --git a/libbrep/package.hxx b/libbrep/package.hxx index 69b25b8..9733374 100644 --- a/libbrep/package.hxx +++ b/libbrep/package.hxx @@ -21,7 +21,7 @@ // #define LIBBREP_PACKAGE_SCHEMA_VERSION_BASE 14 -#pragma db model version(LIBBREP_PACKAGE_SCHEMA_VERSION_BASE, 15, closed) +#pragma db model version(LIBBREP_PACKAGE_SCHEMA_VERSION_BASE, 16, closed) namespace brep { diff --git a/libbrep/package.xml b/libbrep/package.xml index 4af583e..2acba62 100644 --- a/libbrep/package.xml +++ b/libbrep/package.xml @@ -1,4 +1,6 @@ + + diff --git a/load/load.cxx b/load/load.cxx index 57acb3a..c786551 100644 --- a/load/load.cxx +++ b/load/load.cxx @@ -860,13 +860,19 @@ resolve_dependencies (package& p, database& db) assert (c.complete ()); - query qs (compare_version_eq (vm, wildcard_version, false)); + query qs (compare_version_eq (vm, + canonical_version (wildcard_version), + false /* revision */)); if (c.min_version && c.max_version && *c.min_version == *c.max_version) { const version& v (*c.min_version); - q = q && (compare_version_eq (vm, v, v.revision != 0) || qs); + q = q && + (compare_version_eq (vm, + canonical_version (v), + v.revision.has_value ()) || + qs); } else { @@ -875,21 +881,25 @@ resolve_dependencies (package& p, database& db) if (c.min_version) { const version& v (*c.min_version); + canonical_version cv (v); + bool rv (v.revision); if (c.min_open) - qr = compare_version_gt (vm, v, v.revision != 0); + qr = compare_version_gt (vm, cv, rv); else - qr = compare_version_ge (vm, v, v.revision != 0); + qr = compare_version_ge (vm, cv, rv); } if (c.max_version) { const version& v (*c.max_version); + canonical_version cv (v); + bool rv (v.revision); if (c.max_open) - qr = qr && compare_version_lt (vm, v, v.revision != 0); + qr = qr && compare_version_lt (vm, cv, rv); else - qr = qr && compare_version_le (vm, v, v.revision != 0); + qr = qr && compare_version_le (vm, cv, rv); } q = q && (qr || qs); diff --git a/migrate/migrate.cxx b/migrate/migrate.cxx index f012da1..3e2c53d 100644 --- a/migrate/migrate.cxx +++ b/migrate/migrate.cxx @@ -234,6 +234,27 @@ package_migrate_v15 ([] (database& db) "version_canonical_release = '~')"); }); +static const package_migration_entry<16> +package_migrate_v16 ([] (database& db) +{ + // Set the zero version revision to NULL. + // + auto migrate_rev = [&db] (const char* table, const char* column) + { + db.execute (string ("UPDATE ") + table + " SET " + column + " = NULL " + + "WHERE " + column + " = 0"); + }; + + // The depends package manifest value endpoint versions. + // + // Note that previously the zero and absent revisions had the same + // semantics. Now the semantics differs and the zero revision is preserved + // (see libbpkg/manifest.hxx for details). + // + migrate_rev ("package_dependency_alternatives", "dep_min_version_revision"); + migrate_rev ("package_dependency_alternatives", "dep_max_version_revision"); +}); + // main() function // int diff --git a/mod/mod-build-task.cxx b/mod/mod-build-task.cxx index ec94e89..030c41d 100644 --- a/mod/mod-build-task.cxx +++ b/mod/mod-build-task.cxx @@ -404,7 +404,7 @@ handle (request& rq, response& rs) bld_query::id.toolchain_name == tqm.toolchain_name && compare_version_eq (bld_query::id.toolchain_version, - toolchain_version, + canonical_version (toolchain_version), true /* revision */) && (bld_query::state == "built" || diff --git a/mod/mod-builds.cxx b/mod/mod-builds.cxx index dedabfe..71e7f7b 100644 --- a/mod/mod-builds.cxx +++ b/mod/mod-builds.cxx @@ -130,9 +130,15 @@ build_query (const brep::cstrings* configs, // Package version. // if (!params.version ().empty () && params.version () != "*") + { + // May throw invalid_argument. + // + version v (params.version (), false /* fold_zero_revision */); + q = q && compare_version_eq (pid.version, - version (params.version ()), // May throw. - true); + canonical_version (v), + v.revision.has_value ()); + } // Build toolchain name/version. // @@ -144,12 +150,18 @@ build_query (const brep::cstrings* configs, if (p == string::npos) // Invalid format. throw invalid_argument (""); + // Note that the toolchain version is selected from the list and denotes + // the exact version revision, so an absent and zero revisions have the + // same semantics and the zero revision is folded. + // string tn (tc, 0, p); version tv (string (tc, p + 1)); // May throw invalid_argument. q = q && qb::id.toolchain_name == tn && - compare_version_eq (qb::id.toolchain_version, tv, true); + compare_version_eq (qb::id.toolchain_version, + canonical_version (tv), + true /* revision */); } // Build configuration name. @@ -244,9 +256,15 @@ package_query (const brep::params::builds& params, // Package version. // if (!params.version ().empty () && params.version () != "*") + { + // May throw invalid_argument. + // + version v (params.version (), false /* fold_zero_revision */); + q = q && compare_version_eq (qp::id.version, - version (params.version ()), // May throw. - true); + canonical_version (v), + v.revision.has_value ()); + } } catch (const invalid_argument&) { @@ -683,6 +701,9 @@ handle (request& rq, response& rs) // May throw invalid_argument. // + // Note that an absent and zero revisions have the same semantics, + // so the zero revision is folded (see above for details). + // tc_version = version (string (tc, p + 1)); } catch (const invalid_argument&) diff --git a/tests/load/1/math/libfoo-1.2.4+1.tar.gz b/tests/load/1/math/libfoo-1.2.4+1.tar.gz index 94deae1..5d19fa6 100644 Binary files a/tests/load/1/math/libfoo-1.2.4+1.tar.gz and b/tests/load/1/math/libfoo-1.2.4+1.tar.gz differ diff --git a/tests/load/1/math/packages.manifest b/tests/load/1/math/packages.manifest index 74984e9..429e61f 100644 --- a/tests/load/1/math/packages.manifest +++ b/tests/load/1/math/packages.manifest @@ -72,7 +72,7 @@ src-url: http://scm.example.com/?p=odb/libodb.git\;a=tree; Source tree url. package-url: http://www.example.com/foo/pack; Package details. email: foo-users@example.com; Public mailing list. Read FAQ before posting. package-email: pack@example.com; Current packager. -depends: libmisc < 1.1 | libmisc > 2.3.0; Crashes with 1.1.0-2.3.0. +depends: libmisc < 1.1 | libmisc > 2.3.0+0; Crashes with 1.1.0-2.3.0. depends: libexp >= 1.0 depends: ? libstudxml | libexpat; The newer the better. requires: linux | windows | macosx; Symbian support is coming. @@ -80,7 +80,7 @@ requires: c++11 requires: ? ; libc++ standard library if using Clang on Mac OS X. requires: ? vc++ >= 12.0; Only if using VC++ on Windows. location: libfoo-1.2.4+1.tar.gz -sha256sum: 0a206d2b5e575549914ed43b87470b33512e975fffa4fc8f3eb92b3dea66979e +sha256sum: 533108c89724a80ba739168ec92540dff0b7d3660fa0771de780d8595ccff425 : name: libpq version: 0 diff --git a/tests/load/driver.cxx b/tests/load/driver.cxx index 1afa411..6877f4b 100644 --- a/tests/load/driver.cxx +++ b/tests/load/driver.cxx @@ -208,6 +208,12 @@ dep (const char* n, optional c) return dependency {package_name (n), move (c), nullptr}; } +static inline version +dep_ver (const char* v) +{ + return version (v, false /* fold_zero_revision */); +} + static void test_git_repos (const cstrings& loader_args, const dir_path& loadtab_dir, @@ -265,7 +271,7 @@ test_git_repos (const cstrings& loader_args, assert (p->dependencies[0][0] == dep ("libmisc", dependency_constraint ( - version ("1.0"), false, version ("1.0"), false))); + dep_ver ("1.0"), false, dep_ver ("1.0"), false))); assert (p->buildable); @@ -461,12 +467,12 @@ test_pkg_repos (const cstrings& loader_args, assert (fpv2->dependencies[0][0] == dep ("libbar", dependency_constraint ( - nullopt, true, version ("2.4.0"), false))); + nullopt, true, dep_ver ("2.4.0"), false))); assert (fpv2->dependencies[1][0] == dep ("libexp", dependency_constraint ( - version ("+2-1.2"), false, version ("+2-1.2"), false))); + dep_ver ("+2-1.2"), false, dep_ver ("+2-1.2"), false))); assert (check_location (fpv2)); @@ -508,27 +514,27 @@ test_pkg_repos (const cstrings& loader_args, assert (fpv2a->dependencies[0][0] == dep ("libmisc", dependency_constraint ( - version ("0.1"), false, version ("2.0.0-"), true))); + dep_ver ("0.1"), false, dep_ver ("2.0.0-"), true))); assert (fpv2a->dependencies[0][1] == dep ("libmisc", dependency_constraint ( - version ("2.0"), false, version ("5.0"), false))); + dep_ver ("2.0"), false, dep_ver ("5.0"), false))); assert (fpv2a->dependencies[1][0] == dep ("libgenx", dependency_constraint ( - version ("0.2"), true, version ("3.0"), true))); + dep_ver ("0.2"), true, dep_ver ("3.0"), true))); assert (fpv2a->dependencies[2][0] == dep ("libexpat", dependency_constraint ( - nullopt, true, version ("5.2"), true))); + nullopt, true, dep_ver ("5.2"), true))); assert (fpv2a->dependencies[2][1] == dep ("libexpat", dependency_constraint ( - version ("1"), true, version ("5.1"), false))); + dep_ver ("1"), true, dep_ver ("5.1"), false))); assert (fpv2a->requirements.empty ()); @@ -568,7 +574,7 @@ test_pkg_repos (const cstrings& loader_args, assert (fpv3->dependencies[0][0] == dep ("libmisc", dependency_constraint ( - version ("2.0.0"), false, nullopt, true))); + dep_ver ("2.0.0"), false, nullopt, true))); assert (check_location (fpv3)); @@ -607,7 +613,7 @@ test_pkg_repos (const cstrings& loader_args, assert (fpv4->dependencies[0][0] == dep ("libmisc", dependency_constraint ( - version ("2.0.0"), false, nullopt, true))); + dep_ver ("2.0.0"), false, nullopt, true))); assert (check_location (fpv4)); @@ -691,7 +697,7 @@ test_pkg_repos (const cstrings& loader_args, assert (xpv->dependencies[0][0] == dep ("libexpat", dependency_constraint ( - version ("2.0.0"), false, nullopt, true))); + dep_ver ("2.0.0"), false, nullopt, true))); assert (xpv->dependencies[1].size () == 1); assert (xpv->dependencies[1][0] == dep ("libgenx", nullopt)); @@ -776,12 +782,12 @@ test_pkg_repos (const cstrings& loader_args, assert (fpv5->dependencies[0][0] == dep ("libmisc", dependency_constraint ( - nullopt, true, version ("1.1"), true))); + nullopt, true, dep_ver ("1.1"), true))); assert (fpv5->dependencies[0][1] == dep ("libmisc", dependency_constraint ( - version ("2.3.0"), true, nullopt, true))); + dep_ver ("2.3.0+0"), true, nullopt, true))); assert (fpv5->dependencies[1].size () == 1); assert (fpv5->dependencies[1].comment.empty ()); @@ -789,7 +795,7 @@ test_pkg_repos (const cstrings& loader_args, assert (fpv5->dependencies[1][0] == dep ("libexp", dependency_constraint ( - version ("1.0"), false, nullopt, true))); + dep_ver ("1.0"), false, nullopt, true))); assert (fpv5->dependencies[2].size () == 2); assert (fpv5->dependencies[2].comment == "The newer the better."); @@ -820,7 +826,7 @@ test_pkg_repos (const cstrings& loader_args, assert (check_location (fpv5)); assert (fpv5->sha256sum && *fpv5->sha256sum == - "0a206d2b5e575549914ed43b87470b33512e975fffa4fc8f3eb92b3dea66979e"); + "533108c89724a80ba739168ec92540dff0b7d3660fa0771de780d8595ccff425"); assert (fpv5->buildable); @@ -861,7 +867,7 @@ test_pkg_repos (const cstrings& loader_args, assert (epv->dependencies[1][0] == dep ("libpq", dependency_constraint ( - version ("9.0.0"), false, nullopt, true))); + dep_ver ("9.0.0"), false, nullopt, true))); assert (epv->requirements.empty ()); -- cgit v1.1