From 281b9ef7a740f89175a4feb29447153307f4802e Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Wed, 30 Dec 2015 20:19:32 +0200 Subject: Support package dependency version range --- bpkg/manifest | 41 +++-- bpkg/manifest.cxx | 346 ++++++++++++++++++++++++++++----------- tests/manifest/packages | 1 + tests/package-version/driver.cxx | 76 ++++++--- 4 files changed, 328 insertions(+), 136 deletions(-) diff --git a/bpkg/manifest b/bpkg/manifest index e53bc69..7df052b 100644 --- a/bpkg/manifest +++ b/bpkg/manifest @@ -32,7 +32,7 @@ namespace bpkg // const std::uint16_t epoch; const std::string upstream; - const std::string release; + const butl::optional release; const std::uint16_t revision; // Upstream part canonical representation. @@ -43,9 +43,10 @@ namespace bpkg // const std::string canonical_release; - // Create a special empty version. + // Create a special empty version. It is less than any other valid + // version (and is conceptually equivalent to 0-). // - version (): epoch (0), revision (0) {} + version (): epoch (0), release (""), revision (0) {} // Throw std::invalid_argument if the passed string is not a valid // version representation. @@ -59,9 +60,12 @@ namespace bpkg // Create the version object from separate epoch, upstream, release, and // revision parts. // + // Note that it is possible (and legal) to create the special empty + // version via this interface as version(0, string(), string(), 0). + // version (std::uint16_t epoch, std::string upstream, - std::string release, + butl::optional release, std::uint16_t revision); version (version&&) = default; @@ -112,7 +116,8 @@ namespace bpkg empty () const noexcept { bool e (upstream.empty ()); - assert (!e || (epoch == 0 && release.empty () && revision == 0)); + assert (!e || + (epoch == 0 && release && release->empty () && revision == 0)); return e; } @@ -125,7 +130,7 @@ namespace bpkg std::uint16_t epoch; std::string upstream; - std::string release; + butl::optional release; std::uint16_t revision; std::string canonical_upstream; std::string canonical_release; @@ -240,21 +245,23 @@ namespace bpkg // depends // - enum class comparison {eq, lt, gt, le, ge}; + struct dependency_constraint + { + butl::optional min_version; + butl::optional max_version; + bool min_open; + bool max_open; - std::string - to_string (comparison); + dependency_constraint (butl::optional min_version, bool min_open, + butl::optional max_version, bool max_open); - comparison - to_comparison (const std::string&); // May throw invalid_argument. + dependency_constraint (const version& v) + : dependency_constraint (v, false, v, false) {} - inline std::ostream& - operator<< (std::ostream& os, comparison c) {return os << to_string (c);} + dependency_constraint () = default; - struct dependency_constraint - { - comparison operation; - bpkg::version version; + bool + empty () const noexcept {return !min_version && !max_version;} }; std::ostream& diff --git a/bpkg/manifest.cxx b/bpkg/manifest.cxx index cca4e9d..9ded0b1 100644 --- a/bpkg/manifest.cxx +++ b/bpkg/manifest.cxx @@ -27,8 +27,6 @@ using namespace butl; namespace bpkg { - using std::to_string; // Add to bpkg::to_string(). - using parser = manifest_parser; using parsing = manifest_parsing; using serializer = manifest_serializer; @@ -178,7 +176,7 @@ namespace bpkg // version // version:: - version (uint16_t e, std::string u, std::string l, uint16_t r) + version (uint16_t e, std::string u, optional l, uint16_t r) : epoch (e), upstream (move (u)), release (move (l)), @@ -187,10 +185,24 @@ namespace bpkg data_type (upstream.c_str (), data_type::parse::upstream). canonical_upstream), canonical_release ( - data_type (release.c_str (), data_type::parse::release). + data_type (release ? release->c_str () : nullptr, + data_type::parse::release). canonical_release) { - if (release.empty () && r != 0) + // Check members constrains. + // + if (upstream.empty ()) // Constructing empty version. + { + if (epoch != 0) + throw invalid_argument ("epoch for empty version"); + + if (!release || !release->empty ()) + throw invalid_argument ("not-empty release for empty version"); + + if (revision != 0) + throw invalid_argument ("revision for empty version"); + } + else if (release && release->empty () && revision != 0) // Empty release signifies the earliest possible release. Revision is // meaningless in such a context. // @@ -216,7 +228,7 @@ namespace bpkg if (end - begin > 8) throw invalid_argument ("8 digits maximum allowed in a component"); - append (8 - (end - begin), '0'); // Add padding spaces. + append (8 - (end - begin), '0'); // Add padding zeros. string c (begin, end); append (c); @@ -233,28 +245,26 @@ namespace bpkg } private: - // Length without trailing digit-only zero components. - // - size_t len_{0}; + size_t len_ = 0; // Length without the trailing digit-only zero components. }; version::data_type:: - data_type (const char* v, parse pr): epoch (0), revision (0) + data_type (const char* v, parse pr): epoch (0), revision (0) { // Otherwise compiler gets confused with string() member. // using std::string; - assert (v != nullptr); - - if (pr == parse::release && strcmp (v, "~") == 0) + if (pr == parse::release && v == nullptr) { - // Special case: composing final version release part. + // Special case: final version release part. // - canonical_release = v; + canonical_release = "~"; return; } + assert (v != nullptr); + auto bad_arg ([](const string& d) {throw invalid_argument (d);}); auto uint16 ( @@ -307,6 +317,7 @@ namespace bpkg bad_arg ("epoch should be 2-byte unsigned integer"); epoch = uint16 (string (cb, p), "epoch"); + m = mode::upstream; cb = p + 1; ub = cb; @@ -374,9 +385,10 @@ namespace bpkg assert (p >= cb); // 'p' denotes the end of the last component. - // An empty component is valid for the release part only. + // An empty component is valid for the release part, and for the upstream + // part when constructing empty or max limit version. // - if (p == cb && m != mode::release) + if (p == cb && m != mode::release && pr != parse::upstream) bad_arg ("unexpected end"); // Parse the last component. @@ -398,7 +410,7 @@ namespace bpkg re = p; } - // Upstream and release pointer ranges are valid at the and of the day. + // Upstream and release pointer ranges are valid at the end of the day. // assert (ub <= ue && rb <= re); @@ -406,11 +418,14 @@ namespace bpkg { // Fill upstream original and canonical parts. // - assert (ub != ue); // Can't happen if through all previous checks. - canonical_upstream = canon_upstream.final (); + if (!canon_upstream.empty ()) + { + assert (ub != ue); // Can't happen if through all previous checks. + canonical_upstream = canon_upstream.final (); - if (pr == parse::full) - upstream.assign (ub, ue); + if (pr == parse::full) + upstream.assign (ub, ue); + } } if (pr != parse::upstream) @@ -423,29 +438,35 @@ namespace bpkg canonical_release = canon_release.final (); if (pr == parse::full) - release.assign (rb, re); + release = string (rb, re); } else { if (m == mode::release) { // Empty release part signifies the earliest possible version - // release. Do nothing, keep original and canonical representations - // empty. + // release. Make original, and keep canonical representations empty. // + if (pr == parse::full) + release = ""; } else { // Absent release part signifies the final (max) version release. - // Assign the special value to canonical and original representations. + // Assign the special value to the canonical representation, keep + // the original one nullopt. // canonical_release = "~"; - - if (pr == parse::full) - release = "~"; } } } + + if (pr == parse::full && epoch == 0 && canonical_upstream.empty () && + canonical_release.empty ()) + { + assert (revision == 0); // Can't happen if through all previous checks. + bad_arg ("empty version"); + } } version& version:: @@ -470,14 +491,15 @@ namespace bpkg string version:: string (bool ignore_revision) const { + if (empty ()) + throw logic_error ("empty version"); + std::string v (epoch != 0 ? to_string (epoch) + "~" + upstream : upstream); - // The empty version represented as an empty string. - // - if (!empty () && release != "~") + if (release) { v += '-'; - v += release; + v += *release; } if (!ignore_revision && revision != 0) @@ -491,29 +513,55 @@ namespace bpkg // depends // - static const char* comparison_str[] = {"==", "<", ">", "<=", ">="}; - string - to_string (comparison c) + dependency_constraint:: + dependency_constraint (optional mnv, bool mno, + optional mxv, bool mxo) + : min_version (move (mnv)), + max_version (move (mxv)), + min_open (mno), + max_open (mxo) { - return comparison_str[static_cast (c)]; - } + assert ( + // Min and max versions can't both be absent. + // + (min_version || max_version) && - comparison - to_comparison (const string& s) - { - if (s == "==") return comparison::eq; - else if (s == ">" ) return comparison::gt; - else if (s == "<" ) return comparison::lt; - else if (s == ">=") return comparison::ge; - else if (s == "<=") return comparison::le; - else throw invalid_argument ("invalid comparion operator '" + s + "'"); + // Version should be non-empty. + // + (!min_version || !min_version->empty ()) && + (!max_version || !max_version->empty ()) && + + // Absent version endpoint (infinity) should be open. + // + (min_version || min_open) && (max_version || max_open)); + + if (min_version && max_version) + { + if (*min_version > *max_version) + throw invalid_argument ("min version is greater than max version"); + + if (*min_version == *max_version && (min_open || max_open)) + throw invalid_argument ("equal version endpoints not closed"); + } } - inline ostream& + ostream& operator<< (ostream& o, const dependency_constraint& c) { - return o << c.operation << ' ' << c.version; + assert (!c.empty ()); + + if (!c.min_version) + return o << (c.max_open ? "< " : "<= ") << *c.max_version; + + if (!c.max_version) + return o << (c.min_open ? "> " : ">= ") << *c.min_version; + + if (*c.min_version == *c.max_version) + return o << "== " << *c.min_version; + + return o << (c.min_open ? '(' : '[') << *c.min_version << " " + << *c.max_version << (c.max_open ? ')' : ']'); } ostream& @@ -606,7 +654,7 @@ namespace bpkg // Versions like 1.2.3- are forbidden in manifest as intended to be // used for version constrains rather than actual releases. // - if (version.release.empty ()) + if (version.release && version.release->empty ()) bad_name ("invalid package version release"); } else if (n == "summary") @@ -811,7 +859,8 @@ namespace bpkg // Find end of name (ne). // - for (char c; i != e && (c = *i) != '=' && c != '<' && c != '>'; ++i) + static const string cb ("=<>(["); + for (char c; i != e && cb.find (c = *i) == string::npos; ++i) { if (!space (c)) ne = i + 1; @@ -826,61 +875,162 @@ namespace bpkg if (nm.empty ()) bad_value ("prerequisite package name not specified"); - // Got to version comparison. + // Got to version range. // + dependency_constraint dc; const char* op (&*i); - comparison operation (comparison::eq); // Uninitialized warning. - - // While we have to_comparison(), using it in this situation - // won't save us anything. - // - if (strncmp (op, "==", 2) == 0) + char mnv (*op); + if (mnv == '(' || mnv == '[') { - operation = comparison::eq; - i += 2; - } - else if (strncmp (op, ">=", 2) == 0) - { - operation = comparison::ge; - i += 2; - } - else if (strncmp (op, "<=", 2) == 0) - { - operation = comparison::le; - i += 2; - } - else if (*op == '>') - { - operation = comparison::gt; - ++i; - } - else if (*op == '<') - { - operation = comparison::lt; - ++i; + bool min_open (mnv == '('); + + string::size_type pos (lv.find_first_not_of (spaces, ++i - b)); + if (pos == string::npos) + bad_value ("no prerequisite package min version specified"); + + i = b + pos; + pos = lv.find_first_of (spaces, pos); + + static const char* no_max_version ( + "no prerequisite package max version specified"); + + if (pos == string::npos) + bad_value (no_max_version); + + version_type min_version; + + try + { + min_version = version_type (string (i, b + pos)); + } + catch (const invalid_argument& e) + { + bad_value ( + string ("invalid prerequisite package min version: ") + + e.what ()); + } + + pos = lv.find_first_not_of (spaces, pos); + if (pos == string::npos) + bad_value (no_max_version); + + i = b + pos; + static const string mve (spaces + "])"); + pos = lv.find_first_of (mve, pos); + + static const char* invalid_range ( + "invalid prerequisite package version range"); + + if (pos == string::npos) + bad_value (invalid_range); + + version_type max_version; + + try + { + max_version = version_type (string (i, b + pos)); + } + catch (const invalid_argument& e) + { + bad_value ( + string ("invalid prerequisite package max version: ") + + e.what ()); + } + + pos = lv.find_first_of ("])", pos); // Might be a space. + if (pos == string::npos) + bad_value (invalid_range); + + try + { + dc = dependency_constraint (move (min_version), + min_open, + move (max_version), + lv[pos] == ')'); + } + catch (const invalid_argument& e) + { + bad_value ( + string ("invalid dependency constraint: ") + e.what ()); + } + + if (lv[pos + 1] != '\0') + bad_value ( + "unexpected text after prerequisite package version range"); } else - bad_value ("invalid prerequisite package version comparison"); - - string::size_type pos = lv.find_first_not_of (spaces, i - b); - - if (pos == string::npos) - bad_value ("no prerequisite package version specified"); - - version_type v; - - try - { - v = version_type (lv.c_str () + pos); - } - catch (const invalid_argument& e) { - bad_value ( - string ("invalid prerequisite package version: ") + e.what ()); + // Version comparison notation. + // + enum comparison {eq, lt, gt, le, ge}; + comparison operation (eq); // Uninitialized warning. + + if (strncmp (op, "==", 2) == 0) + { + operation = eq; + i += 2; + } + else if (strncmp (op, ">=", 2) == 0) + { + operation = ge; + i += 2; + } + else if (strncmp (op, "<=", 2) == 0) + { + operation = le; + i += 2; + } + else if (*op == '>') + { + operation = gt; + ++i; + } + else if (*op == '<') + { + operation = lt; + ++i; + } + else + bad_value ("invalid prerequisite package version comparison"); + + string::size_type pos (lv.find_first_not_of (spaces, i - b)); + + if (pos == string::npos) + bad_value ("no prerequisite package version specified"); + + version_type v; + + try + { + v = version_type (lv.c_str () + pos); + } + catch (const invalid_argument& e) + { + bad_value (string ("invalid prerequisite package version: ") + + e.what ()); + } + + switch (operation) + { + case comparison::eq: + dc = dependency_constraint (v); + break; + case comparison::lt: + dc = dependency_constraint (nullopt, true, move (v), true); + break; + case comparison::le: + dc = dependency_constraint (nullopt, true, move (v), false); + break; + case comparison::gt: + dc = dependency_constraint (move (v), true, nullopt, true); + break; + case comparison::ge: + dc = dependency_constraint (move (v), false, nullopt, true); + break; + } } - dependency d {move (nm), - dependency_constraint {operation, move (v)}}; + dependency d {move (nm), move (dc)}; da.push_back (move (d)); } } diff --git a/tests/manifest/packages b/tests/manifest/packages index d2058e3..d204c32 100644 --- a/tests/manifest/packages +++ b/tests/manifest/packages @@ -36,6 +36,7 @@ tags: c++, xml, modern description-file: README; Comprehensive description url: http://www.example.org/projects/libbar/ email: libbar-users@example.org +depends: libbaz (1- 2-) | libbaz [3 4-) | libbaz (5 6] | libbaz [7 8] location: bar/libbar-3.4A.5+6.tbz : name: libbaz diff --git a/tests/package-version/driver.cxx b/tests/package-version/driver.cxx index d28fabf..a48d4cf 100644 --- a/tests/package-version/driver.cxx +++ b/tests/package-version/driver.cxx @@ -12,6 +12,7 @@ #include using namespace std; +using namespace butl; using namespace bpkg; static bool @@ -29,7 +30,7 @@ bad_version (const string& v) } static bool -bad_version (uint16_t e, const string& u, const string& l, uint16_t r) +bad_version (uint16_t e, const string& u, const optional& l, uint16_t r) { try { @@ -43,6 +44,12 @@ bad_version (uint16_t e, const string& u, const string& l, uint16_t r) } static bool +bad_version (uint16_t e, const string& u, const char* l, uint16_t r) +{ + return bad_version (e, u, string (l), r); +} + +static bool test_constructor (const version& v) { return v == version (v.epoch, v.upstream, v.release, v.revision); @@ -90,27 +97,44 @@ main (int argc, char* argv[]) assert (bad_version ("a.")); // Not completed upstream. assert (bad_version ("a..b")); // Empty upstream component. assert (bad_version ("a.b-+1")); // Revision for empty release. + assert (bad_version ("0.0-+3")); // Same. + assert (bad_version ("1.2.3-~")); // Invalid release. + assert (bad_version ("0-")); // Illegal version. + assert (bad_version ("0.0-")); // Same. - assert (bad_version (0, "", "", 0)); // Empty upstream. assert (bad_version (0, "1", "", 1)); // Revision for empty release. - assert (bad_version (1, "1~1.1", "~", 2)); // Epoch in upstream. - assert (bad_version (1, "1.1-1", "~", 2)); // Release in upstream. - assert (bad_version (1, "1.1+1", "~", 2)); // Revision in upstream. + assert (bad_version (1, "1~1.1", "", 2)); // Epoch in upstream. + assert (bad_version (1, "1.1-1", "", 2)); // Release in upstream. + assert (bad_version (1, "1.1+1", "", 2)); // Revision in upstream. assert (bad_version (1, "1", "1~1.1", 2)); // Epoch in release. assert (bad_version (1, "1", "1.1-1", 2)); // Release in release. assert (bad_version (1, "1", "1.1+1", 2)); // Revision in release. + assert (bad_version (1, "", "", 0)); // Unexpected epoch. + assert (bad_version (0, "", "1", 0)); // Unexpected release. + assert (bad_version (0, "", "", 1)); // Unexpected revision. + { version v1; assert (v1.empty ()); - assert (v1.string ().empty ()); + assert (v1.canonical_upstream.empty ()); + assert (v1.canonical_release.empty ()); version v2 ("0.0.0"); assert (!v2.empty ()); + assert (v1.canonical_upstream.empty ()); + assert (v2.canonical_release == "~"); - // @@ It doesn't look nice. - // - assert (v1.canonical_upstream == v2.canonical_upstream); + assert (v1 != v2); + } + + { + version v ("1~0.0-"); + assert (!v.empty ()); + assert (v.string () == "1~0.0-"); + assert (v.canonical_upstream.empty ()); + assert (v.canonical_release.empty ()); + assert (test_constructor (v)); } { @@ -121,8 +145,8 @@ main (int argc, char* argv[]) } { - version v ("65535~ab+65535"); - assert (v.string () == "65535~ab+65535"); + version v ("65534~ab+65535"); + assert (v.string () == "65534~ab+65535"); assert (v.canonical_upstream == "ab"); assert (test_constructor (v)); } @@ -207,7 +231,7 @@ main (int argc, char* argv[]) { version v ("1.2.3"); assert (v.string () == "1.2.3"); - assert (v.release == "~"); + assert (!v.release); assert (v.canonical_release == "~"); assert (test_constructor (v)); } @@ -215,7 +239,7 @@ main (int argc, char* argv[]) { version v ("1.2.3+1"); assert (v.string () == "1.2.3+1"); - assert (v.release == "~"); + assert (!v.release); assert (v.canonical_release == "~"); assert (test_constructor (v)); } @@ -223,7 +247,7 @@ main (int argc, char* argv[]) { version v ("1.2.3-"); assert (v.string () == "1.2.3-"); - assert (v.release.empty ()); + assert (v.release && v.release->empty ()); assert (v.canonical_release.empty ()); assert (test_constructor (v)); } @@ -231,23 +255,31 @@ main (int argc, char* argv[]) { version v ("1~A-1.2.3B.00+0"); assert (v.string () == "1~A-1.2.3B.00"); - assert (v.release == "1.2.3B.00"); + assert (v.release && *v.release == "1.2.3B.00"); assert (v.canonical_release == "00000001.00000002.3b"); assert (test_constructor (v)); } { - version v (1, "1", "~", 2); + version v ("65535~q.3+65535"); + assert (v.string () == "65535~q.3+65535"); + assert (!v.release); + assert (v.canonical_release == "~"); + assert (test_constructor (v)); + } + + { + version v (1, "1", nullopt, 2); assert (v.string () == "1~1+2"); - assert (v.release == "~"); + assert (!v.release); assert (v.canonical_release == "~"); assert (test_constructor (v)); } { - version v (1, "1", "", 0); + version v (1, "1", string (), 0); assert (v.string () == "1~1-"); - assert (v.release.empty ()); + assert (v.release && v.release->empty ()); assert (v.canonical_release.empty ()); assert (test_constructor (v)); } @@ -291,8 +323,10 @@ main (int argc, char* argv[]) assert (version ("1.0-alpha+1") < version ("1.1")); assert (version ("1.0-alpha") > version ("1.0-1")); assert (version ("1.0-alpha") == version ("1.0-alpha.0")); - assert (version (1, "2.0", "~", 3) == version ("1~2+3")); - assert (version (1, "2.0", "", 0) == version ("1~2-")); + + assert (version (1, "2.0", nullopt, 3) == version ("1~2+3")); + assert (version (1, "2.0", string (), 0) == version ("1~2-")); + assert (version (0, "", string (), 0) == version ()); } catch (const exception& e) { -- cgit v1.1