From 58357d79110a925dcb3cdc734d812f17b6465c17 Mon Sep 17 00:00:00 2001 From: Karen Arutyunov Date: Wed, 24 Jun 2020 22:12:01 +0300 Subject: Fix build order for test and main packages --- bpkg/checksum.cxx | 4 -- bpkg/fetch-git.cxx | 1 - bpkg/package.cxx | 88 ++++++++++++++++++++++++++++++++- bpkg/package.hxx | 132 ++++++++++++++++++++++++++++++++++++++++++++++--- bpkg/package.xml | 60 ++++++++++++++++++++++ bpkg/pkg-build.cxx | 113 ++++++++---------------------------------- bpkg/pkg-checkout.cxx | 5 +- bpkg/pkg-command.cxx | 2 - bpkg/pkg-configure.cxx | 10 +++- bpkg/pkg-unpack.cxx | 4 -- bpkg/rep-create.cxx | 1 - bpkg/rep-fetch.cxx | 69 ++++++++++++++++++++++++-- bpkg/rep-info.cxx | 3 +- bpkg/rep-remove.cxx | 1 - bpkg/utility.hxx | 13 ++--- 15 files changed, 376 insertions(+), 130 deletions(-) diff --git a/bpkg/checksum.cxx b/bpkg/checksum.cxx index efc86dd..9c52da0 100644 --- a/bpkg/checksum.cxx +++ b/bpkg/checksum.cxx @@ -3,10 +3,6 @@ #include -#ifdef _WIN32 -# include // replace() -#endif - #include using namespace std; diff --git a/bpkg/fetch-git.cxx b/bpkg/fetch-git.cxx index df59dde..2bdd2d5 100644 --- a/bpkg/fetch-git.cxx +++ b/bpkg/fetch-git.cxx @@ -4,7 +4,6 @@ #include #include -#include // find_if(), replace(), sort() #include #include // digit(), xdigit() diff --git a/bpkg/package.cxx b/bpkg/package.cxx index 8c3fd19..3532f3d 100644 --- a/bpkg/package.cxx +++ b/bpkg/package.cxx @@ -4,8 +4,6 @@ #include #include -#include // find_if() - #include #include #include @@ -28,6 +26,92 @@ namespace bpkg // available_package // + odb::result + query_available (database& db, + const package_name& name, + const optional& c, + bool order) + { + using query = query; + + query q (query::id.name == name); + const auto& vm (query::id.version); + + // If there is a constraint, then translate it to the query. Otherwise, + // get the latest version or stub versions if present. + // + if (c) + { + 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 */, + false /* iteration */)); + + 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, + canonical_version (v), + v.revision.has_value (), + false /* iteration */) || + qs); + } + else + { + query qr (true); + + 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, cv, rv, false /* iteration */); + else + qr = compare_version_ge (vm, cv, rv, false /* iteration */); + } + + 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, cv, rv, false /* iteration */); + else + qr = qr && compare_version_le (vm, cv, rv, false /* iteration */); + } + + q = q && (qr || qs); + } + } + + if (order) + q += order_by_version_desc (vm); + + return db.query (q); + } + // Check if the package is available from the specified repository fragment, // its prerequisite repositories, or one of their complements, recursively. // Return the first repository fragment that contains the package or NULL if diff --git a/bpkg/package.hxx b/bpkg/package.hxx index 8987069..f10e670 100644 --- a/bpkg/package.hxx +++ b/bpkg/package.hxx @@ -27,7 +27,7 @@ // #define DB_SCHEMA_VERSION_BASE 5 -#pragma db model version(DB_SCHEMA_VERSION_BASE, 6, closed) +#pragma db model version(DB_SCHEMA_VERSION_BASE, 7, closed) namespace bpkg { @@ -446,7 +446,61 @@ namespace bpkg #pragma db member(dependency::constraint) column("") #pragma db value(dependency_alternatives) definition - using dependencies = vector; + // Extend dependency_alternatives to also represent the special test + // dependencies of the test packages to the main packages, produced by + // inverting the main packages external test dependencies (specified with + // the tests, etc., manifest values). + // + #pragma db value + class dependency_alternatives_ex: public dependency_alternatives + { + public: + optional type; + + dependency_alternatives_ex () = default; + + // Create the regular dependency alternatives object. + // + dependency_alternatives_ex (dependency_alternatives da) + : dependency_alternatives (move (da)) {} + + // Create the special test dependencies object (built incrementally). + // + dependency_alternatives_ex (test_dependency_type t) + : dependency_alternatives (false /* conditional */, + false /* buildtime */, + "" /* comment */), + type (t) {} + }; + + using dependencies = vector; + + // Convert the regular dependency alternatives list (normally comes from a + // package manifest) to the extended version of it (see above). + // + inline dependencies + convert (vector&& das) + { + return dependencies (make_move_iterator (das.begin ()), + make_move_iterator (das.end ())); + } + + // tests + // + #pragma db value(test_dependency) definition + + using optional_test_dependency_type = optional; + + #pragma db map type(test_dependency_type) as(string) \ + to(to_string (?)) \ + from(bpkg::to_test_dependency_type (?)) + + #pragma db map type(optional_test_dependency_type) \ + as(bpkg::optional_string) \ + to((?) ? to_string (*(?)) : bpkg::optional_string ()) \ + from((?) \ + ? bpkg::to_test_dependency_type (*(?)) \ + : bpkg::optional_test_dependency_type ()) // Wildcard version. Satisfies any version constraint and is represented as // 0+0 (which is also the "stub version"; since a real version is always @@ -498,12 +552,17 @@ namespace bpkg // small_vector locations; - // Package manifest data. + // Package manifest data and, potentially, the special test dependencies. + // + // Note that there can be only one special test dependencies entry in the + // list and it's always the last one, if present. // using dependencies_type = bpkg::dependencies; dependencies_type dependencies; + small_vector tests; + // Present for non-transient objects only (and only for certain repository // types). // @@ -519,7 +578,8 @@ namespace bpkg available_package (package_manifest&& m) : id (move (m.name), m.version), version (move (m.version)), - dependencies (move (m.dependencies)), + dependencies (convert (move (m.dependencies))), + tests (move (m.tests)), sha256sum (move (m.sha256sum)) {} // Create available stub package. @@ -596,8 +656,8 @@ namespace bpkg // dependencies // - using _dependency_key = odb::nested_key; - using _dependency_alternatives_type = + using _dependency_key = odb::nested_key; + using _dependency_alternatives_ex_type = std::map<_dependency_key, dependency>; #pragma db value(_dependency_key) @@ -605,13 +665,18 @@ namespace bpkg #pragma db member(_dependency_key::inner) column("index") #pragma db member(dependencies) id_column("") value_column("") - #pragma db member(dependency_alternatives) \ - virtual(_dependency_alternatives_type) \ + #pragma db member(dependency_alternatives_ex) \ + table("available_package_dependency_alternatives") \ + virtual(_dependency_alternatives_ex_type) \ after(dependencies) \ get(odb::nested_get (this.dependencies)) \ set(odb::nested_set (this.dependencies, std::move (?))) \ id_column("") key_column("") value_column("dep_") + // tests + // + #pragma db member(tests) id_column("") value_column("test_") + private: friend class odb::access; available_package () = default; @@ -626,6 +691,57 @@ namespace bpkg operator size_t () const {return result;} }; + // Return the list of available test packages, that is, that are referred to + // as external tests by some main package(s). + // + // Note that there can be only one test dependency row per package, so the + // DISTINCT clause is not required. + // + #pragma db view object(available_package = package) \ + table("available_package_dependencies" = "pd" inner: \ + "pd.type IN ('tests', 'examples', 'benchmarks') AND " \ + "pd.name = " + package::id.name + "AND" + \ + "pd.version_epoch = " + package::id.version.epoch + "AND" + \ + "pd.version_canonical_upstream = " + \ + package::id.version.canonical_upstream + "AND" + \ + "pd.version_canonical_release = " + \ + package::id.version.canonical_release + "AND" + \ + "pd.version_revision = " + package::id.version.revision + "AND" + \ + "pd.version_iteration = " + package::id.version.iteration) + struct available_test + { + shared_ptr package; + }; + + // Return the list of available main packages, that is, that refer to some + // external test packages. + // + #pragma db view object(available_package = package) \ + table("available_package_tests" = "pt" inner: \ + "pt.name = " + package::id.name + "AND" + \ + "pt.version_epoch = " + package::id.version.epoch + "AND" + \ + "pt.version_canonical_upstream = " + \ + package::id.version.canonical_upstream + "AND" + \ + "pt.version_canonical_release = " + \ + package::id.version.canonical_release + "AND" + \ + "pt.version_revision = " + package::id.version.revision + "AND" + \ + "pt.version_iteration = " + package::id.version.iteration) \ + query(distinct) + struct available_main + { + shared_ptr package; + }; + + // Query the available packages that optionally satisfy the specified + // version constraint and return them in the version descending order, by + // default. Note that a stub satisfies any constraint. + // + odb::result + query_available (database&, + const package_name&, + const optional&, + bool order = true); + // Only return packages that are in the specified repository fragments, their // complements or prerequisites (if prereq is true), recursively. While you // could maybe come up with a (barely comprehensible) view/query to achieve diff --git a/bpkg/package.xml b/bpkg/package.xml index 2856be1..ed5dd64 100644 --- a/bpkg/package.xml +++ b/bpkg/package.xml @@ -1,4 +1,64 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/bpkg/pkg-build.cxx b/bpkg/pkg-build.cxx index 106666a..3ef513a 100644 --- a/bpkg/pkg-build.cxx +++ b/bpkg/pkg-build.cxx @@ -8,7 +8,6 @@ #include #include // strlen() #include // cout -#include // find_if() #include @@ -42,95 +41,6 @@ namespace bpkg // - Configuration vars (both passed and preserved) // - // Query the available packages that optionally satisfy the specified - // version constraint and return them in the version descending order. Note - // that a stub satisfies any constraint. Note also that this function does - // not return the extra stubs from the imaginary system repository (use one - // of the find_available*() overloads instead). - // - static odb::result - query_available (database& db, - const package_name& name, - const optional& c) - { - using query = query; - - query q (query::id.name == name); - const auto& vm (query::id.version); - - // If there is a constraint, then translate it to the query. Otherwise, - // get the latest version or stub versions if present. - // - if (c) - { - 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 */, - false /* iteration */)); - - 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, - canonical_version (v), - v.revision.has_value (), - false /* iteration */) || - qs); - } - else - { - query qr (true); - - 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, cv, rv, false /* iteration */); - else - qr = compare_version_ge (vm, cv, rv, false /* iteration */); - } - - 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, cv, rv, false /* iteration */); - else - qr = qr && compare_version_le (vm, cv, rv, false /* iteration */); - } - - q = q && (qr || qs); - } - } - - q += order_by_version_desc (vm); - return db.query (q); - } - // Try to find an available stub package in the imaginary system repository. // Such a repository contains stubs corresponding to the system packages // specified by the user on the command line with version information @@ -841,7 +751,7 @@ namespace bpkg const shared_ptr& af (pkg.repository_fragment); const package_name& name (ap->id.name); - for (const dependency_alternatives& da: ap->dependencies) + for (const dependency_alternatives_ex& da: ap->dependencies) { if (da.conditional) // @@ TODO fail << "conditional dependencies are not yet supported"; @@ -1018,6 +928,17 @@ namespace bpkg // the conservative choice and the user can always override it by // explicitly building libhello. // + // Note though, that if this is a test package, then its special + // test dependencies (main packages that refer to it) should be + // searched upstream through the complement repositories + // recursively, since the test packages may only belong to the main + // package's repository and its complements. + // + // @@ Currently we don't implement the reverse direction search for + // the test dependencies, effectively only supporting the common + // case where the main and test packages belong to the same + // repository. Will need to fix this eventually. + // // Note that this logic (naturally) does not apply if the package is // already selected by the user (see above). // @@ -1709,7 +1630,7 @@ namespace bpkg // be built in the order that is as close to the manifest as // possible. // - for (const dependency_alternatives& da: + for (const dependency_alternatives_ex& da: reverse_iterate (ap->dependencies)) { assert (!da.conditional && da.size () == 1); // @@ TODO @@ -4901,7 +4822,13 @@ namespace bpkg true /* ignore_unknown */, [&sp] (version& v) {v = sp->version;})); - pkg_configure (c, o, t, sp, m.dependencies, p.config_vars, simulate); + pkg_configure (c, + o, + t, + sp, + convert (move (m.dependencies)), + p.config_vars, + simulate); } assert (sp->state == package_state::configured); diff --git a/bpkg/pkg-checkout.cxx b/bpkg/pkg-checkout.cxx index 60a30ae..05875a4 100644 --- a/bpkg/pkg-checkout.cxx +++ b/bpkg/pkg-checkout.cxx @@ -226,7 +226,10 @@ namespace bpkg true /* ignore_unknown */, [&ap] (version& v) {v = ap->version;})); - pkg_configure_prerequisites (o, t, m.dependencies, m.name); + pkg_configure_prerequisites (o, + t, + convert (move (m.dependencies)), + m.name); // Form the buildspec. // diff --git a/bpkg/pkg-command.cxx b/bpkg/pkg-command.cxx index 6f330a6..0828118 100644 --- a/bpkg/pkg-command.cxx +++ b/bpkg/pkg-command.cxx @@ -3,8 +3,6 @@ #include -#include // find_if() - #include #include #include diff --git a/bpkg/pkg-configure.cxx b/bpkg/pkg-configure.cxx index 225291b..cd55575 100644 --- a/bpkg/pkg-configure.cxx +++ b/bpkg/pkg-configure.cxx @@ -28,7 +28,7 @@ namespace bpkg database& db (t.database ()); - for (const dependency_alternatives& da: deps) + for (const dependency_alternatives_ex& da: deps) { assert (!da.conditional); //@@ TODO @@ -319,7 +319,13 @@ namespace bpkg true /* ignore_unknown */, [&p] (version& v) {v = p->version;})); - pkg_configure (c, o, t, p, m.dependencies, vars, false /* simulate */); + pkg_configure (c, + o, + t, + p, + convert (move (m.dependencies)), + vars, + false /* simulate */); } if (verb && !o.no_result ()) diff --git a/bpkg/pkg-unpack.cxx b/bpkg/pkg-unpack.cxx index 10f8920..b8ac08e 100644 --- a/bpkg/pkg-unpack.cxx +++ b/bpkg/pkg-unpack.cxx @@ -3,10 +3,6 @@ #include -#ifdef _WIN32 -# include // replace() -#endif - #include #include diff --git a/bpkg/rep-create.cxx b/bpkg/rep-create.cxx index 0ca2e5f..f47bc09 100644 --- a/bpkg/rep-create.cxx +++ b/bpkg/rep-create.cxx @@ -4,7 +4,6 @@ #include #include -#include // count_if() #include // dir_iterator #include diff --git a/bpkg/rep-fetch.cxx b/bpkg/rep-fetch.cxx index 1b8e00a..b58ef2c 100644 --- a/bpkg/rep-fetch.cxx +++ b/bpkg/rep-fetch.cxx @@ -5,7 +5,6 @@ #include #include -#include // equal() #include @@ -1273,8 +1272,8 @@ namespace bpkg } } - // Finally, make sure that the external packages are available from a - // single directory-based repository. + // Make sure that the external packages are available from a single + // directory-based repository. // // Sort the packages by name and version. This way the external packages // with the same upstream version and revision will be adjacent. Note @@ -1324,6 +1323,70 @@ namespace bpkg ap = id; rf = f; } + + // Finally, invert the main packages external test dependencies into the + // the test packages special test dependencies. + // + // But first, remove the existing (and possibly outdated) special test + // dependencies from the test packages, unless all the available + // packages are (re)created from scratch. + // + if (!full_fetch) + { + for (const auto& at: db.query ()) + { + dependencies& ds (at.package->dependencies); + + // Note that the special test dependencies entry is always the last + // one, if present. + // + assert (!ds.empty () && ds.back ().type); + + ds.pop_back (); + + db.update (at.package); + } + } + + // Go through the available packages that have external tests and add + // them as the special test dependencies to these test packages. + // + for (const auto& am: db.query ()) + { + const shared_ptr& p (am.package); + + vector> rfs; + + for (const package_location& pl: p->locations) + rfs.push_back (pl.repository_fragment.load ()); + + for (const test_dependency& td: p->tests) + { + vector, + shared_ptr>> tps ( + filter (rfs, + query_available (db, + td.name, + td.constraint, + false /* order */), + false /* prereq */)); + + for (const auto& t: tps) + { + const shared_ptr& tp (t.first); + + dependencies& ds (tp->dependencies); + + if (ds.empty () || !ds.back ().type) + ds.push_back (dependency_alternatives_ex (td.type)); + + ds.back ().push_back ( + dependency {p->id.name, version_constraint (p->version)}); + + db.update (tp); + } + } + } } catch (const failed&) { diff --git a/bpkg/rep-info.cxx b/bpkg/rep-info.cxx index 6c0c7d5..1a23528 100644 --- a/bpkg/rep-info.cxx +++ b/bpkg/rep-info.cxx @@ -3,8 +3,7 @@ #include -#include // cout -#include // find_if() +#include // cout #include // sha256_to_fingerprint() #include diff --git a/bpkg/rep-remove.cxx b/bpkg/rep-remove.cxx index c0b67b6..c377fc5 100644 --- a/bpkg/rep-remove.cxx +++ b/bpkg/rep-remove.cxx @@ -4,7 +4,6 @@ #include #include -#include // find() #include // dir_iterator diff --git a/bpkg/utility.hxx b/bpkg/utility.hxx index e891f21..4360118 100644 --- a/bpkg/utility.hxx +++ b/bpkg/utility.hxx @@ -4,12 +4,13 @@ #ifndef BPKG_UTILITY_HXX #define BPKG_UTILITY_HXX -#include // make_shared() -#include // to_string() -#include // strcmp(), strchr() -#include // move(), forward(), declval(), make_pair() -#include // assert() -#include // make_move_iterator() +#include // make_shared() +#include // to_string() +#include // strcmp(), strchr() +#include // move(), forward(), declval(), make_pair() +#include // assert() +#include // make_move_iterator() +#include // * #include -- cgit v1.1