From a8bbc544cac714378e18f85b5f18d2988a752c5f Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Wed, 30 Dec 2015 20:25:12 +0200 Subject: Support package dependency version range --- INSTALL-DEV | 1 + brep/mod-repository-details.cxx | 2 +- brep/package | 71 ++++++++++++++++---------------- brep/package-extra.sql | 12 +++--- loader/loader.cxx | 38 +++++++++--------- tests/loader/1/stable/packages | 3 ++ tests/loader/driver.cxx | 89 +++++++++++++++++++++++++++++++++++------ 7 files changed, 142 insertions(+), 74 deletions(-) diff --git a/INSTALL-DEV b/INSTALL-DEV index 3ba9ff5..b81d7fb 100644 --- a/INSTALL-DEV +++ b/INSTALL-DEV @@ -118,3 +118,4 @@ data, and reload the Apache2 plugin), execute the following from brep/: psql --quiet -d brep -f brep/package.sql loader/brep-loader tests/loader/r.conf sudo /etc/init.d/apache2 restart +sudo systemctl restart apache2 diff --git a/brep/mod-repository-details.cxx b/brep/mod-repository-details.cxx index 18da188..3147369 100644 --- a/brep/mod-repository-details.cxx +++ b/brep/mod-repository-details.cxx @@ -46,7 +46,7 @@ init (scanner& s) db_ = shared_database (*options_); - tzset(); // To use butl::to_stream() later on. + tzset (); // To use butl::to_stream() later on. } bool brep::repository_details:: diff --git a/brep/package b/brep/package index 9e4dde6..af59462 100644 --- a/brep/package +++ b/brep/package @@ -19,6 +19,11 @@ #include #include +// The uint16_t value range is not fully covered by SMALLINT PostgreSQL type +// to which uint16_t is mapped by default. +// +#pragma db value(uint16_t) type("INTEGER") + namespace brep { // Use an image type to map bpkg::version to the database since there @@ -32,12 +37,18 @@ namespace brep string canonical_release; uint16_t revision; string upstream; - string release; + optional release; }; } #include +namespace brep +{ + using optional_version = optional; + using _optional_version = optional<_version>; +} + // Prevent assert() macro expansion in get/set expressions. This should // appear after all #include directives since the assert() macro is // redefined in each inclusion. @@ -50,8 +61,7 @@ void assert (int); // We have to keep these mappings at the global scope instead of inside // the brep namespace because they need to be also effective in the -// bpkg namespace from which we "borrow" types (and some of them use -// version and comparison). +// bpkg namespace from which we "borrow" types (and some of them use version). // #pragma db map type(bpkg::version) as(brep::_version) \ to(brep::_version{(?).epoch, \ @@ -65,14 +75,24 @@ void assert (int); std::move ((?).release), \ (?).revision)) -#pragma db map type(bpkg::comparison) as(brep::string) \ - to(bpkg::to_string (?)) \ - from(bpkg::to_comparison (?)) +#pragma db map type(brep::optional_version) as(brep::_optional_version) \ + to((?) \ + ? brep::_version{(?)->epoch, \ + (?)->canonical_upstream, \ + (?)->canonical_release, \ + (?)->revision, \ + (?)->upstream, \ + (?)->release} \ + : brep::_optional_version ()) \ + from((?) \ + ? bpkg::version ((?)->epoch, \ + std::move ((?)->upstream), \ + std::move ((?)->release), \ + (?)->revision) \ + : brep::optional_version ()) namespace brep { - // @@ If namespace, then should probably call it 'repo'. - // // @@ Might make sense to put some heavy members (e.g., description, // containers) into a separate section. // @@ -130,24 +150,6 @@ namespace brep string canonical_release; uint16_t revision; - // @@ The following function will fail the assertion for versions like - // 0.0.0 (canonical_upstream is empty, canonical_release set to "~"). - // The problem here is that it's possible to specify such a version - // which canonical representation is indistinguishable from that of - // the empty version. Example: 0.0.0- - // This effectivelly make it impossible to use canonical_version as a - // primary key. Need to think on fixing that problem on the version - // specification level. Probably canonical upstream (and release) for - // non-empty version should always start with '.'. - // Examples: ".00000001.a" for "1.A", ".", for "0.0.0" - // - // @@ Also version spec do not mention that trailing zero-only components - // get removed. - // - // @@ If by some reason decide to leave canonical upstream representation - // as is, then need to drop the function below, make the loader not - // using it. - // bool empty () const noexcept { @@ -172,13 +174,14 @@ 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 (?), "~", 0)) + #pragma db member(upstream_) virtual(string) \ + get(this.upstream) \ + set(this = brep::version (0, std::move (?), std::string (), 0)) - #pragma db member(release_) virtual(string) \ - get(this.release) \ - set(this = brep::version (0, this.upstream, std::move (?), 0)) + #pragma db member(release_) virtual(optional_string) \ + get(this.release) \ + set(this = brep::version ( \ + 0, std::move (this.upstream), std::move (?), 0)) upstream_version () = default; upstream_version (version v): version (move (v)) {} @@ -224,12 +227,9 @@ namespace brep // dependencies // - using bpkg::comparison; using bpkg::dependency_constraint; #pragma db value(dependency_constraint) definition - #pragma db member(dependency_constraint::operation) column("") - #pragma db member(dependency_constraint::version) column("") #pragma db value struct package_id @@ -303,6 +303,7 @@ namespace brep // Database mapping. // #pragma db member(package) column("") not_null + #pragma db member(constraint) column("") }; std::ostream& diff --git a/brep/package-extra.sql b/brep/package-extra.sql index acf1489..dc37d1f 100644 --- a/brep/package-extra.sql +++ b/brep/package-extra.sql @@ -33,10 +33,10 @@ $$ LANGUAGE SQL STABLE; -- CREATE FUNCTION latest_package(INOUT name TEXT, - OUT version_epoch SMALLINT, + OUT version_epoch INTEGER, OUT version_canonical_upstream TEXT, OUT version_canonical_release TEXT, - OUT version_revision SMALLINT) + OUT version_revision INTEGER) RETURNS SETOF record AS $$ SELECT name, version_epoch, version_canonical_upstream, version_canonical_release, version_revision @@ -52,10 +52,10 @@ $$ LANGUAGE SQL STABLE; CREATE FUNCTION search_latest_packages(IN query tsquery, OUT name TEXT, - OUT version_epoch SMALLINT, + OUT version_epoch INTEGER, OUT version_canonical_upstream TEXT, OUT version_canonical_release TEXT, - OUT version_revision SMALLINT, + OUT version_revision INTEGER, OUT rank real) RETURNS SETOF record AS $$ SELECT name, version_epoch, version_canonical_upstream, @@ -76,10 +76,10 @@ $$ LANGUAGE SQL STABLE; CREATE FUNCTION search_packages(IN query tsquery, INOUT name TEXT, - OUT version_epoch SMALLINT, + OUT version_epoch INTEGER, OUT version_canonical_upstream TEXT, OUT version_canonical_release TEXT, - OUT version_revision SMALLINT, + OUT version_revision INTEGER, OUT rank real) RETURNS SETOF record AS $$ SELECT name, version_epoch, version_canonical_upstream, diff --git a/loader/loader.cxx b/loader/loader.cxx index 2c74e34..6457adf 100644 --- a/loader/loader.cxx +++ b/loader/loader.cxx @@ -272,18 +272,7 @@ load_packages (const shared_ptr& rp, database& db) for (auto& pm: pkm) { - shared_ptr p ( - db.find ( - package_id - { - pm.name, - { - pm.version.epoch, - pm.version.canonical_upstream, - pm.version.canonical_release, - pm.version.revision - } - })); + shared_ptr p (db.find (package_id (pm.name, pm.version))); if (p == nullptr) { @@ -321,6 +310,7 @@ load_packages (const shared_ptr& rp, database& db) } dependencies ds; + for (auto& pda: pm.dependencies) { ds.emplace_back (pda.conditional, move (pda.comment)); @@ -595,22 +585,30 @@ resolve_dependencies (package& p, database& db) using query = query; query q (query::id.name == d.name ()); + const auto& vm (query::id.version); if (d.constraint) { auto c (*d.constraint); - switch (c.operation) + + if (c.min_version) { - case comparison::eq: q = q && query::id.version == c.version; break; - case comparison::lt: q = q && query::id.version < c.version; break; - case comparison::gt: q = q && query::id.version > c.version; break; - case comparison::le: q = q && query::id.version <= c.version; break; - case comparison::ge: q = q && query::id.version >= c.version; break; + 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; } } - for (const auto& pp: - db.query (q + order_by_version_desc (query::id.version))) + for (const auto& pp: db.query (q + order_by_version_desc (vm))) { if (find (p.internal_repository, pp)) { diff --git a/tests/loader/1/stable/packages b/tests/loader/1/stable/packages index 1a385c3..afa168a 100644 --- a/tests/loader/1/stable/packages +++ b/tests/loader/1/stable/packages @@ -27,6 +27,9 @@ license: MIT tags: c++, foo url: http://www.example.com/foo/ email: foo-users@example.com +depends: libmisc [0.1 2.0-) | libmisc [2.0 5.0] +depends: libgenx (0.2 3.0) +depends: libexpat < 5.2 | libexpat (1 5.1] location: libfoo-1.2.2-alpha.1.tar.gz : name: libfoo diff --git a/tests/loader/driver.cxx b/tests/loader/driver.cxx index 51f2ed8..eea3d0c 100644 --- a/tests/loader/driver.cxx +++ b/tests/loader/driver.cxx @@ -25,12 +25,30 @@ using namespace odb::core; using namespace butl; using namespace brep; +// @@ Rather add this to optional in libbutl. See: +// +// http://en.cppreference.com/w/cpp/experimental/optional/operator_cmp + +template +static inline auto +operator== (const optional& a, const optional& b) -> decltype (*a == *b) +{ + return !a == !b && (!a || *a == *b); +} + +// @@ Add it to libbrep rather? +// +static inline bool +operator== (const dependency_constraint& a, const dependency_constraint& b) +{ + return a.min_version == b.min_version && a.max_version == b.max_version && + a.min_open == b.min_open && a.max_open == b.max_open; +} + static inline bool operator== (const dependency& a, const dependency& b) { - return a.name () == b.name () && !a.constraint == !b.constraint && - (!a.constraint || (a.constraint->operation == b.constraint->operation && - a.constraint->version == b.constraint->version)); + return a.name () == b.name () && a.constraint == b.constraint; } static bool @@ -216,13 +234,15 @@ main (int argc, char* argv[]) dep ( "libbar", optional ( - dependency_constraint{comparison::le, version ("2.4.0")}))); + dependency_constraint ( + nullopt, true, version ("2.4.0"), false)))); assert (fpv2->dependencies[1][0] == dep ( "libexp", optional ( - dependency_constraint{comparison::eq, version ("1~1.2")}))); + dependency_constraint ( + version ("1~1.2"), false, version ("1~1.2"), false)))); // libfoo-1.2.2-alpha.1 // @@ -243,7 +263,46 @@ main (int argc, char* argv[]) assert (fpv2a->license_alternatives[0].size () == 1); assert (fpv2a->license_alternatives[0][0] == "MIT"); - assert (fpv2a->dependencies.empty ()); + assert (fpv2a->dependencies.size () == 3); + assert (fpv2a->dependencies[0].size () == 2); + assert (fpv2a->dependencies[1].size () == 1); + assert (fpv2a->dependencies[2].size () == 2); + + assert (fpv2a->dependencies[0][0] == + dep ( + "libmisc", + optional ( + dependency_constraint ( + version ("0.1"), false, version ("2.0.0-"), true)))); + + assert (fpv2a->dependencies[0][1] == + dep ( + "libmisc", + optional ( + dependency_constraint ( + version ("2.0"), false, version ("5.0"), false)))); + + assert (fpv2a->dependencies[1][0] == + dep ( + "libgenx", + optional ( + dependency_constraint ( + version ("0.2"), true, version ("3.0"), true)))); + + assert (fpv2a->dependencies[2][0] == + dep ( + "libexpat", + optional ( + dependency_constraint ( + nullopt, true, version ("5.2"), true)))); + + assert (fpv2a->dependencies[2][1] == + dep ( + "libexpat", + optional ( + dependency_constraint ( + version ("1"), true, version ("5.1"), false)))); + assert (fpv2a->requirements.empty ()); // libfoo-1.2.3-4 @@ -272,7 +331,8 @@ main (int argc, char* argv[]) dep ( "libmisc", optional ( - dependency_constraint{comparison::ge, version ("2.0.0")}))); + dependency_constraint ( + version ("2.0.0"), false, nullopt, true)))); // libfoo-1.2.4 // @@ -301,7 +361,8 @@ main (int argc, char* argv[]) dep ( "libmisc", optional ( - dependency_constraint{comparison::ge, version ("2.0.0")}))); + dependency_constraint ( + version ("2.0.0"), false, nullopt, true)))); // Verify 'math' repository. // @@ -373,7 +434,8 @@ main (int argc, char* argv[]) dep ( "libexpat", optional ( - dependency_constraint{comparison::ge, version ("2.0.0")}))); + dependency_constraint ( + version ("2.0.0"), false, nullopt, true)))); assert (xpv->dependencies[1].size () == 1); assert (xpv->dependencies[1][0] == dep ("libgenx", nullopt)); @@ -436,13 +498,15 @@ main (int argc, char* argv[]) dep ( "libmisc", optional ( - dependency_constraint{comparison::lt, version ("1.1")}))); + dependency_constraint ( + nullopt, true, version ("1.1"), true)))); assert (fpv5->dependencies[0][1] == dep ( "libmisc", optional ( - dependency_constraint{comparison::gt, version ("2.3.0")}))); + dependency_constraint ( + version ("2.3.0"), true, nullopt, true)))); assert (fpv5->dependencies[1].size () == 1); assert (fpv5->dependencies[1].comment.empty ()); @@ -450,7 +514,8 @@ main (int argc, char* argv[]) assert (fpv5->dependencies[1][0] == dep ("libexp", optional ( - dependency_constraint{comparison::ge, version ("1.0")}))); + dependency_constraint ( + version ("1.0"), false, nullopt, true)))); assert (fpv5->dependencies[2].size () == 2); assert (fpv5->dependencies[2].comment == "The newer the better."); -- cgit v1.1