From 6e90b57a442424876b1325b9209f79c8a885a479 Mon Sep 17 00:00:00 2001 From: Karen Arutyunov Date: Tue, 4 Jul 2017 11:27:47 +0300 Subject: Make use of foreign package objects in build-related functionality --- clean/clean.cli | 45 +----- clean/clean.cxx | 72 +++------ libbrep/build-extra.sql | 15 +- libbrep/build-package.hxx | 69 ++++++-- libbrep/build.hxx | 59 ++++--- libbrep/common.hxx | 29 ++++ libbrep/package.hxx | 44 +----- load/load.cxx | 2 +- mod/mod-build-force.cxx | 24 +-- mod/mod-build-log.cxx | 27 +--- mod/mod-build-result.cxx | 15 +- mod/mod-build-task.cxx | 302 ++++++++++++++++-------------------- mod/mod-builds.cxx | 28 ++-- mod/mod-package-version-details.cxx | 3 +- mod/options.cli | 2 +- 15 files changed, 344 insertions(+), 392 deletions(-) diff --git a/clean/clean.cli b/clean/clean.cli index 3f4e2ea..c80c1b1 100644 --- a/clean/clean.cli +++ b/clean/clean.cli @@ -47,28 +47,28 @@ class options explicitly. If unspecified, the default timeout is zero (never expire)." } - std::string --build-db-user + std::string --db-user { "", "Build database user name. If not specified, then operating system (login) name is used." } - std::string --build-db-password + std::string --db-password { "", "Build database password. If not specified, then login without password is expected to work." } - std::string --build-db-name = "brep_build" + std::string --db-name = "brep_build" { "", "Build database name. If not specified, then \cb{brep_build} is used by default." } - std::string --build-db-host + std::string --db-host { "", "Build database host name, address, or socket. If not specified, then @@ -76,47 +76,12 @@ class options (Unix-domain socket, etc)." } - std::uint16_t --build-db-port = 0 + std::uint16_t --db-port = 0 { "", "Build database port number. If not specified, the default port is used." } - std::string --package-db-user - { - "", - "Package database user name. If not specified, then operating system - (login) name is used." - } - - std::string --package-db-password - { - "", - "Package database password. If not specified, then login without password - is expected to work." - } - - std::string --package-db-name = "brep_package" - { - "", - "Package database name. If not specified, then \cb{brep_package} is used by - default." - } - - std::string --package-db-host - { - "", - "Package database host name, address, or socket. If not specified, then - connect to \cb{localhost} using the operating system-default mechanism - (Unix-domain socket, etc)." - } - - std::uint16_t --package-db-port = 0 - { - "", - "Package database port number. If not specified, the default port is used." - } - std::string --pager // String to allow empty value. { "", diff --git a/clean/clean.cxx b/clean/clean.cxx index bedc886..f6d7eff 100644 --- a/clean/clean.cxx +++ b/clean/clean.cxx @@ -17,8 +17,8 @@ #include #include -#include -#include +#include +#include #include #include @@ -101,19 +101,11 @@ try } odb::pgsql::database build_db ( - ops.build_db_user (), - ops.build_db_password (), - ops.build_db_name (), - ops.build_db_host (), - ops.build_db_port (), - "options='-c default_transaction_isolation=serializable'"); - - odb::pgsql::database package_db ( - ops.package_db_user (), - ops.package_db_password (), - ops.package_db_name (), - ops.package_db_host (), - ops.package_db_port (), + ops.db_user (), + ops.db_password (), + ops.db_name (), + ops.db_host (), + ops.db_port (), "options='-c default_transaction_isolation=serializable'"); // Prevent several brep-clean/migrate instances from updating build database @@ -121,7 +113,7 @@ try // database_lock l (build_db); - // Check that the build and package database schemas match the current ones. + // Check that the build database schema matches the current one. // const string bs ("build"); if (schema_catalog::current_version (build_db, bs) != @@ -132,15 +124,6 @@ try return 1; } - const string ps ("package"); - if (schema_catalog::current_version (package_db, ps) != - package_db.schema_version (ps)) - { - cerr << "error: package database schema differs from the current one" - << endl << " info: use brep-migrate to migrate the database" << endl; - return 1; - } - // Prepare the build prepared query. // // Query package builds in chunks in order not to hold locks for too long. @@ -155,35 +138,32 @@ try order_by_version_desc (bld_query::id.package.version, false) + "OFFSET" + bld_query::_ref (offset) + "LIMIT 100"); - connection_ptr bld_conn (build_db.connection ()); + connection_ptr conn (build_db.connection ()); prep_bld_query bld_prep_query ( - bld_conn->prepare_query ("build-query", bq)); + conn->prepare_query ("build-query", bq)); // Prepare the package version query. // - // Query package versions every time the new package name is encountered + // Query buildable packages every time the new package name is encountered // during iterating over the package builds. Such a query will be made once // per package name due to the builds query sorting criteria (see above). // - using pkg_query = query; - using prep_pkg_query = prepared_query; + using pkg_query = query; + using prep_pkg_query = prepared_query; string package_name; set package_versions; - pkg_query pq (pkg_query::package::id.name == pkg_query::_ref (package_name)); - - connection_ptr pkg_conn (package_db.connection ()); + pkg_query pq ( + pkg_query::build_package::id.name == pkg_query::_ref (package_name)); prep_pkg_query pkg_prep_query ( - pkg_conn->prepare_query ("package-version-query", pq)); + conn->prepare_query ("package-query", pq)); for (bool ne (true); ne; ) { - // Start the build database transaction. - // - transaction bt (bld_conn->begin ()); + transaction t (conn->begin ()); // Query builds. // @@ -191,10 +171,6 @@ try if ((ne = !builds.empty ())) { - // Start the package database transaction. - // - transaction pt (pkg_conn->begin (), false); - for (const auto& b: builds) { auto i (timeouts.find (b.toolchain_name)); @@ -221,19 +197,11 @@ try { if (package_name != b.package_name) { - // Switch to the package database transaction. - // - transaction::current (pt); - package_name = b.package_name; package_versions.clear (); for (auto& v: pkg_prep_query.execute ()) package_versions.emplace (move (v.version)); - - // Switch back to the build database transaction. - // - transaction::current (bt); } cleanup = package_versions.find (b.package_version) == @@ -245,13 +213,9 @@ try else ++offset; } - - // Commit the package database transaction. - // - pt.commit (); } - bt.commit (); + t.commit (); } return 0; diff --git a/libbrep/build-extra.sql b/libbrep/build-extra.sql index 31736c8..6a222a7 100644 --- a/libbrep/build-extra.sql +++ b/libbrep/build-extra.sql @@ -3,11 +3,22 @@ -- file for details. -- --- The foreign table for build_package object. +DROP FOREIGN TABLE IF EXISTS build_package; + +DROP FOREIGN TABLE IF EXISTS build_repository; + +-- The foreign table for build_repository object. -- -- -DROP FOREIGN TABLE IF EXISTS build_package; +CREATE FOREIGN TABLE build_repository ( + name TEXT NOT NULL, + location TEXT NOT NULL, + certificate_fingerprint TEXT NULL) +SERVER package_server OPTIONS (table_name 'repository'); +-- The foreign table for build_package object. +-- +-- CREATE FOREIGN TABLE build_package ( name TEXT NOT NULL, version_epoch INTEGER NOT NULL, diff --git a/libbrep/build-package.hxx b/libbrep/build-package.hxx index b0688c2..8bc703a 100644 --- a/libbrep/build-package.hxx +++ b/libbrep/build-package.hxx @@ -14,13 +14,39 @@ namespace brep { - // This is a "foreign object" that is mapped to the subset of package object - // using PostgreSQL foreign table mechanism. Note that since we maintain the - // two in sync by hand, we should only a have a minimal subset of "core" - // members (ideally just the primary key) that are unlikly to disappear or - // change. + // These are "foreign objects" that are mapped to subsets of the package + // database objects using the PostgreSQL foreign table mechanism. Note that + // since we maintain the pair in sync by hand, we should only have a minimal + // subset of "core" members (ideally just the primary key) that are unlikly + // to disappear or change. // - // The mapping is established in build-extra.sql. + // The mapping is established in build-extra.sql. We also explicitly mark + // non-primary key foreign-mapped members in the source object. + // + // Foreign object that is mapped to the subset of repository object. + // + #pragma db object table("build_repository") pointer(shared_ptr) readonly + class build_repository + { + public: + string name; // Object id (canonical name). + repository_location location; + optional certificate_fingerprint; + + // Database mapping. + // + #pragma db member(name) id + + #pragma db member(location) \ + set(this.location = std::move (?); \ + assert (this.name == this.location.canonical_name ())) + + private: + friend class odb::access; + build_repository () = default; + }; + + // Foreign object that is mapped to the subset of package object. // #pragma db object table("build_package") pointer(shared_ptr) readonly class build_package @@ -28,7 +54,7 @@ namespace brep public: package_id id; upstream_version version; - optional internal_repository; + lazy_shared_ptr internal_repository; // Database mapping. // @@ -40,8 +66,33 @@ namespace brep build_package () = default; }; - #pragma db view object(build_package) - struct build_package_count + // Packages that can potentially be built (internal non-stub). + // + #pragma db view \ + object(build_package) \ + object(build_repository inner: \ + build_package::internal_repository == build_repository::name && \ + brep::compare_version_ne (build_package::id.version, \ + brep::wildcard_version, \ + false)) + struct buildable_package + { + package_id id; + upstream_version version; + + // Database mapping. + // + #pragma db member(version) set(this.version.init (this.id.version, (?))) + }; + + #pragma db view \ + object(build_package) \ + object(build_repository inner: \ + build_package::internal_repository == build_repository::name && \ + brep::compare_version_ne (build_package::id.version, \ + brep::wildcard_version, \ + false)) + struct buildable_package_count { size_t result; diff --git a/libbrep/build.hxx b/libbrep/build.hxx index 7a231bd..7f0aba6 100644 --- a/libbrep/build.hxx +++ b/libbrep/build.hxx @@ -57,10 +57,36 @@ namespace brep inline bool operator< (const build_id& x, const build_id& y) { - return - x.package < y.package ? true : - y.package < x.package ? false : - x.configuration < y.configuration; + if (x.package != y.package) + return x.package < y.package; + + if (int r = x.configuration.compare (y.configuration)) + return r < 0; + + return compare_version_lt (x.toolchain_version, y.toolchain_version, true); + } + + // These allow comparing objects that have package, configuration and + // toolchain_version data members to build_id values. The idea is that this + // works for both query members of build id types as well as for values of + // the build_id type. + // + template + inline auto + operator== (const T& x, const build_id& y) + -> decltype (x.package == y.package) + { + return x.package == y.package && x.configuration == y.configuration && + compare_version_eq (x.toolchain_version, y.toolchain_version, true); + } + + template + inline auto + operator!= (const T& x, const build_id& y) + -> decltype (x.package == y.package) + { + return x.package != y.package || x.configuration != y.configuration || + compare_version_ne (x.toolchain_version, y.toolchain_version, true); } // build_state @@ -267,26 +293,23 @@ namespace brep // Build of an existing internal package. // - #pragma db view \ - object(build) \ - object(build_package inner: \ - build::id.package.name == build_package::id.name && \ - brep::compare_version_eq (build::id.package.version, \ - build_package::id.version, \ - true) && \ + // Note that ADL can't find the equal operator, so we use the function call + // notation. + // + #pragma db view \ + object(build) \ + object(build_package inner: \ + brep::operator== (build::id.package, build_package::id) && \ build_package::internal_repository.is_not_null ()) struct package_build { shared_ptr build; }; - #pragma db view \ - object(build) \ - object(build_package inner: \ - build::id.package.name == build_package::id.name && \ - brep::compare_version_eq (build::id.package.version, \ - build_package::id.version, \ - true) && \ + #pragma db view \ + object(build) \ + object(build_package inner: \ + brep::operator== (build::id.package, build_package::id) && \ build_package::internal_repository.is_not_null ()) struct package_build_count { diff --git a/libbrep/common.hxx b/libbrep/common.hxx index 6bc5aca..56d6768 100644 --- a/libbrep/common.hxx +++ b/libbrep/common.hxx @@ -201,6 +201,13 @@ namespace brep } }; + // repository_location + // + using bpkg::repository_location; + + #pragma db map type(repository_location) as(string) \ + to((?).string ()) from(brep::repository_location (?)) + // Version comparison operators. // // They allow comparing objects that have epoch, canonical_upstream, @@ -330,6 +337,8 @@ namespace brep + x.revision + "DESC"; } + // Package id comparison operators. + // inline bool operator< (const package_id& x, const package_id& y) { @@ -338,6 +347,26 @@ namespace brep return compare_version_lt (x.version, y.version, true); } + + // They allow comparing objects that have name and version data members. The + // idea is that this works for both query members of package id types (in + // particular in join conditions) as well as for values of package_id type. + // + template + inline auto + operator== (const T1& x, const T2& y) + -> decltype (x.name == y.name && x.version.epoch == y.version.epoch) + { + return x.name == y.name && compare_version_eq (x.version, y.version, true); + } + + template + inline auto + operator!= (const T1& x, const T2& y) + -> decltype (x.name == y.name && x.version.epoch == y.version.epoch) + { + return x.name != y.name || compare_version_ne (x.version, y.version, true); + } } #endif // LIBBREP_COMMON_HXX diff --git a/libbrep/package.hxx b/libbrep/package.hxx index 4693a34..5a159ae 100644 --- a/libbrep/package.hxx +++ b/libbrep/package.hxx @@ -164,18 +164,11 @@ namespace brep #pragma db value(requirement_alternatives) definition - // repository_location - // - using bpkg::repository_location; - - #pragma db map type(repository_location) as(string) \ - to((?).string ()) from(brep::repository_location (?)) - #pragma db value class certificate { public: - string fingerprint; // SHA256 fingerprint. + string fingerprint; // SHA256 fingerprint. Note: foreign-mapped in build. string name; // CN component of Subject. string organization; // O component of Subject. string email; // email: in Subject Alternative Name. @@ -202,8 +195,8 @@ namespace brep explicit repository (repository_location); - string name; // Object id (canonical name). - repository_location location; + string name; // Object id (canonical name). + repository_location location; // Note: foreign-mapped in build. string display_name; // The order in the internal repositories configuration file, starting from @@ -224,7 +217,8 @@ namespace brep // repository_location cache_location; - // Present only for internal signed repositories. + // Present only for internal signed repositories. Note that it is + // foreign-mapped in build. // optional certificate; @@ -342,6 +336,9 @@ namespace brep optional build_email; dependencies_type dependencies; requirements_type requirements; + + // Note that it is foreign-mapped in build. + // lazy_shared_ptr internal_repository; // Path to the package file. Present only for internal packages. @@ -483,31 +480,6 @@ namespace brep { package_id id; }; - - #pragma db view object(package) \ - object(repository: package::internal_repository) - struct package_version - { - package_id id; - upstream_version version; - - // Database mapping. - // - #pragma db member(version) set(this.version.init (this.id.version, (?))) - }; - - #pragma db view object(package) \ - object(repository: package::internal_repository) - struct package_version_count - { - size_t result; - - operator size_t () const {return result;} - - // Database mapping. - // - #pragma db member(result) column("count(" + package::id.name + ")") - }; } #endif // LIBBREP_PACKAGE_HXX diff --git a/load/load.cxx b/load/load.cxx index 5a33631..bab35ec 100644 --- a/load/load.cxx +++ b/load/load.cxx @@ -1010,7 +1010,7 @@ try transaction t (db.begin ()); - // Check that the database 'package' schema matches the current one. + // Check that the package database schema matches the current one. // const string ds ("package"); if (schema_catalog::current_version (db, ds) != db.schema_version (ds)) diff --git a/mod/mod-build-force.cxx b/mod/mod-build-force.cxx index 325a1c7..a1d424c 100644 --- a/mod/mod-build-force.cxx +++ b/mod/mod-build-force.cxx @@ -13,8 +13,6 @@ #include #include -#include -#include #include @@ -42,9 +40,6 @@ init (scanner& s) options_ = make_shared ( s, unknown_mode::fail, unknown_mode::fail); - database_module::init (static_cast (*options_), - options_->package_db_retry ()); - if (options_->build_config_specified ()) database_module::init (static_cast (*options_), static_cast (*options_), @@ -145,27 +140,18 @@ handle (request& rq, response& rs) build_conf_map_->end ()) config_expired ("no configuration"); - // Make sure the package still exists. - // - { - transaction t (package_db_->begin ()); - shared_ptr p (package_db_->find (id.package)); - t.commit (); - - if (p == nullptr) - config_expired ("no package"); - } - // Load the package build configuration (if present), set the force flag and // update the object's persistent state. // { transaction t (build_db_->begin ()); - shared_ptr b (build_db_->find (id)); - if (b == nullptr) - config_expired ("no package configuration"); + package_build pb; + if (!build_db_->query_one ( + query::build::id == id, pb)) + config_expired ("no package build"); + shared_ptr b (pb.build); force_state force (b->state == build_state::built ? force_state::forced : force_state::forcing); diff --git a/mod/mod-build-log.cxx b/mod/mod-build-log.cxx index 281eec6..f9fb0e5 100644 --- a/mod/mod-build-log.cxx +++ b/mod/mod-build-log.cxx @@ -15,8 +15,6 @@ #include #include -#include -#include #include @@ -44,9 +42,6 @@ init (scanner& s) options_ = make_shared ( s, unknown_mode::fail, unknown_mode::fail); - database_module::init (static_cast (*options_), - options_->package_db_retry ()); - if (options_->build_config_specified ()) database_module::init (static_cast (*options_), static_cast (*options_), @@ -174,27 +169,19 @@ handle (request& rq, response& rs) build_conf_map_->end ()) config_expired ("no configuration"); - // Make sure the package still exists. - // - { - transaction t (package_db_->begin ()); - shared_ptr p (package_db_->find (id.package)); - t.commit (); - - if (p == nullptr) - config_expired ("no package"); - } - // Load the package build configuration (if present). // shared_ptr b; { transaction t (build_db_->begin ()); - b = build_db_->find (id); - if (b == nullptr) - config_expired ("no package configuration"); - else if (b->state != build_state::built) + package_build pb; + if (!build_db_->query_one ( + query::build::id == id, pb)) + config_expired ("no package build"); + + b = pb.build; + if (b->state != build_state::built) config_expired ("state is " + to_string (b->state)); else build_db_->load (*b, b->results_section); diff --git a/mod/mod-build-result.cxx b/mod/mod-build-result.cxx index 042114d..1f5eb69 100644 --- a/mod/mod-build-result.cxx +++ b/mod/mod-build-result.cxx @@ -215,6 +215,12 @@ handle (request& rq, response&) // Load the built package (if present). // + // The only way not to deal with 2 databases simultaneously is to pull + // another bunch of the package fields into the build_package foreign + // object, which is a pain (see build_package.hxx for details). Doesn't seem + // worth it here: email members are really secondary and we don't need to + // switch transactions back and forth. + // shared_ptr p; { transaction t (package_db_->begin ()); @@ -242,11 +248,12 @@ handle (request& rq, response&) { transaction t (build_db_->begin ()); - b = build_db_->find (id); - if (b == nullptr) - warn_expired ("no package configuration"); - else if (b->state != build_state::building) + package_build pb; + if (!build_db_->query_one ( + query::build::id == id, pb)) + warn_expired ("no package build"); + else if ((b = pb.build)->state != build_state::building) warn_expired ("package configuration state is " + to_string (b->state)); else if (b->timestamp != session_timestamp) warn_expired ("non-matching timestamp"); diff --git a/mod/mod-build-task.cxx b/mod/mod-build-task.cxx index 86acbf6..1a7b272 100644 --- a/mod/mod-build-task.cxx +++ b/mod/mod-build-task.cxx @@ -27,8 +27,8 @@ #include #include -#include -#include +#include +#include #include @@ -57,9 +57,6 @@ init (scanner& s) options_ = make_shared ( s, unknown_mode::fail, unknown_mode::fail); - database_module::init (static_cast (*options_), - options_->package_db_retry ()); - if (options_->build_config_specified ()) { database_module::init (static_cast (*options_), @@ -165,7 +162,7 @@ handle (request& rq, response& rs) // // While trying to find a non-built package configuration we will also // collect the list of the built package configurations which it's time to - // rebuilt. So if no unbuilt package is found, we will pickup one to + // rebuild. So if no unbuilt package is found, we will pickup one to // rebuild. The rebuild preference is given in the following order: the // greater force state, the greater overall status, the lower timestamp. // @@ -177,7 +174,7 @@ handle (request& rq, response& rs) // repository loaded. // auto task = [this] (shared_ptr&& b, - shared_ptr&& p, + shared_ptr&& p, const config_machine& cm) -> task_response_manifest { uint64_t ts ( @@ -192,11 +189,11 @@ handle (request& rq, response& rs) string result_url (options_->host () + options_->root ().string () + "?build-result"); - lazy_shared_ptr r (p->internal_repository); + lazy_shared_ptr r (p->internal_repository); strings fp; - if (r->certificate) - fp.emplace_back (move (r->certificate->fingerprint)); + if (r->certificate_fingerprint) + fp.emplace_back (move (*r->certificate_fingerprint)); task_manifest task (move (b->package_name), move (b->package_version), @@ -302,7 +299,7 @@ handle (request& rq, response& rs) // brep::version toolchain_version (tqm.toolchain_version.string ()); - // Prepare the package version prepared query. + // Prepare the buildable package prepared query. // // Note that the number of packages can be large and so, in order not to // hold locks for too long, we will restrict the number of packages being @@ -314,41 +311,36 @@ handle (request& rq, response& rs) // harmful in that: updates are infrequent and missed packages will be // picked up on the next request. // - using pkg_query = query; - using prep_pkg_query = prepared_query; - - size_t offset (0); // See the package version query. + using pkg_query = query; + using prep_pkg_query = prepared_query; - // Skip external and stub packages. - // - pkg_query pq (pkg_query::package::internal_repository.is_not_null () && - compare_version_ne (pkg_query::package::id.version, - wildcard_version, - false)); + pkg_query pq (true); - // Filter by repositories display names (if requested). + // Filter by repositories canonical names (if requested). // const vector& rp (params.repository ()); if (!rp.empty ()) pq = pq && - pkg_query::repository::display_name.in_range (rp.begin (), rp.end ()); + pkg_query::build_repository::name.in_range (rp.begin (), rp.end ()); // Specify the portion. // + size_t offset (0); + pq += "ORDER BY" + - pkg_query::package::id.name + "," + - pkg_query::package::id.version.epoch + "," + - pkg_query::package::id.version.canonical_upstream + "," + - pkg_query::package::id.version.canonical_release + "," + - pkg_query::package::id.version.revision + + pkg_query::build_package::id.name + "," + + pkg_query::build_package::id.version.epoch + "," + + pkg_query::build_package::id.version.canonical_upstream + "," + + pkg_query::build_package::id.version.canonical_release + "," + + pkg_query::build_package::id.version.revision + "OFFSET" + pkg_query::_ref (offset) + "LIMIT 50"; - connection_ptr pkg_conn (package_db_->connection ()); + connection_ptr conn (build_db_->connection ()); prep_pkg_query pkg_prep_query ( - pkg_conn->prepare_query ( - "mod-build-task-package-version-query", pq)); + conn->prepare_query ( + "mod-build-task-package-query", pq)); // Prepare the build prepared query. // @@ -364,8 +356,7 @@ handle (request& rq, response& rs) using bld_query = query; using prep_bld_query = prepared_query; - package_id id; // See the build query. - + package_id id; const auto& qv (bld_query::id.package.version); bld_query bq ( @@ -390,169 +381,147 @@ handle (request& rq, response& rs) (bld_query::force != "forcing" && // Unforced or forced. bld_query::timestamp > normal_result_expiration_ns)))); - connection_ptr bld_conn (build_db_->connection ()); - prep_bld_query bld_prep_query ( - bld_conn->prepare_query ( - "mod-build-task-package-build-query", bq)); + conn->prepare_query ("mod-build-task-build-query", bq)); while (tsm.session.empty ()) { - // Start the package database transaction. - // - transaction pt (pkg_conn->begin ()); + transaction t (conn->begin ()); - // Query package versions. + // Query (and cache) buildable packages. // - auto package_versions (pkg_prep_query.execute ()); + auto packages (pkg_prep_query.execute ()); // Bail out if there is nothing left. // - if (package_versions.empty ()) + if (packages.empty ()) { - pt.commit (); + t.commit (); break; } - offset += package_versions.size (); + offset += packages.size (); - // Start the build database transaction. + // Iterate over packages until we find one that needs building. // + for (auto& bp: packages) { - transaction bt (bld_conn->begin (), false); - transaction::current (bt); + id = move (bp.id); - // Iterate over packages until we find one that needs building. + // Iterate through the package configurations and erase those that + // don't need building from the build configuration map. All those + // configurations that remained can be built. We will take the first + // one, if present. // - for (auto& pv: package_versions) + // Also save the built package configurations for which it's time to be + // rebuilt. + // + config_machines configs (cfg_machines); // Make a copy for this pkg. + auto pkg_builds (bld_prep_query.execute ()); + + for (auto i (pkg_builds.begin ()); i != pkg_builds.end (); ++i) { - id = move (pv.id); + auto j (configs.find (i->id.configuration.c_str ())); - // Iterate through the package configurations and erase those that - // don't need building from the build configuration map. All those - // configurations that remained can be built. We will take the first - // one, if present. + // Outdated configurations are already excluded with the database + // query. // - // Also save the built package configurations for which it's time - // to be rebuilt. - // - config_machines configs (cfg_machines); // Make a copy for this pkg. - auto pkg_builds (bld_prep_query.execute ()); + assert (j != configs.end ()); + configs.erase (j); - for (auto i (pkg_builds.begin ()); i != pkg_builds.end (); ++i) + if (i->state == build_state::built) { - auto j (configs.find (i->id.configuration.c_str ())); - - // Outdated configurations are already excluded with the database - // query. - // - assert (j != configs.end ()); - configs.erase (j); - - if (i->state == build_state::built) - { - assert (i->force != force_state::forcing); + assert (i->force != force_state::forcing); - if (i->timestamp <= (i->force == force_state::forced - ? forced_rebuild_expiration - : normal_rebuild_expiration)) - rebuilds.emplace_back (i.load ()); - } + if (i->timestamp <= (i->force == force_state::forced + ? forced_rebuild_expiration + : normal_rebuild_expiration)) + rebuilds.emplace_back (i.load ()); } + } - if (!configs.empty ()) + if (!configs.empty ()) + { + config_machine& cm (configs.begin ()->second); + machine_header_manifest& mh (*cm.machine); + build_id bid (move (id), cm.config->name, toolchain_version); + shared_ptr b (build_db_->find (bid)); + optional cl (challenge ()); + + // If build configuration doesn't exist then create the new one and + // persist. Otherwise put it into the building state, refresh the + // timestamp and update. + // + if (b == nullptr) { - config_machine& cm (configs.begin ()->second); - machine_header_manifest& mh (*cm.machine); - build_id bid (move (id), cm.config->name, toolchain_version); - shared_ptr b (build_db_->find (bid)); - optional cl (challenge ()); - - // If build configuration doesn't exist then create the new one - // and persist. Otherwise put it into the building state, refresh - // the timestamp and update. + b = make_shared (move (bid.package.name), + move (bp.version), + move (bid.configuration), + move (tqm.toolchain_name), + move (toolchain_version), + move (agent_fp), + move (cl), + mh.name, + move (mh.summary), + cm.config->target); + + build_db_->persist (b); + } + else + { + // The package configuration is in the building state, and there + // are no results. // - if (b == nullptr) - { - b = make_shared (move (bid.package.name), - move (pv.version), - move (bid.configuration), - move (tqm.toolchain_name), - move (toolchain_version), - move (agent_fp), - move (cl), - mh.name, - move (mh.summary), - cm.config->target); - - build_db_->persist (b); - } - else - { - // The package configuration is in the building state, and there - // are no results. - // - // Note that in both cases we keep the status intact to be able - // to compare it with the final one in the result request - // handling in order to decide if to send the notification email. - // The same is true for the forced flag (in the sense that we - // don't set the force state to unforced). - // - // Load the section to assert the above statement. - // - build_db_->load (*b, b->results_section); - - assert (b->state == build_state::building && - b->results.empty ()); - - b->state = build_state::building; - - // Switch the force state not to reissue the task after the - // forced rebuild timeout. Note that the result handler will - // still recognize that the rebuild was forced. - // - if (b->force == force_state::forcing) - b->force = force_state::forced; - - b->toolchain_name = move (tqm.toolchain_name); - b->agent_fingerprint = move (agent_fp); - b->agent_challenge = move (cl); - b->machine = mh.name; - b->machine_summary = move (mh.summary); - b->target = cm.config->target; - b->timestamp = timestamp::clock::now (); - - build_db_->update (b); - } - - // Finally, prepare the task response manifest. + // Note that in both cases we keep the status intact to be able to + // compare it with the final one in the result request handling in + // order to decide if to send the notification email. The same is + // true for the forced flag (in the sense that we don't set the + // force state to unforced). // - // Switch to the package database transaction to load the package. + // Load the section to assert the above statement. // - transaction::current (pt); + build_db_->load (*b, b->results_section); - shared_ptr p (package_db_->load (b->id.package)); - p->internal_repository.load (); + assert (b->state == build_state::building && b->results.empty ()); - // Switch back to the build database transaction. - // - transaction::current (bt); + b->state = build_state::building; - tsm = task (move (b), move (p), cm); + // Switch the force state not to reissue the task after the forced + // rebuild timeout. Note that the result handler will still + // recognize that the rebuild was forced. + // + if (b->force == force_state::forcing) + b->force = force_state::forced; + + b->toolchain_name = move (tqm.toolchain_name); + b->agent_fingerprint = move (agent_fp); + b->agent_challenge = move (cl); + b->machine = mh.name; + b->machine_summary = move (mh.summary); + b->target = cm.config->target; + b->timestamp = timestamp::clock::now (); + + build_db_->update (b); } - // If the task response manifest is prepared, then bail out from the - // package loop, commit transactions and respond. + // Finally, prepare the task response manifest. // - if (!tsm.session.empty ()) - break; + shared_ptr p ( + build_db_->load (b->id.package)); + + p->internal_repository.load (); + + tsm = task (move (b), move (p), cm); } - bt.commit (); // Commit the build database transaction. + // If the task response manifest is prepared, then bail out from the + // package loop, commit transactions and respond. + // + if (!tsm.session.empty ()) + break; } - transaction::current (pt); // Switch to the package database transaction. - pt.commit (); + t.commit (); } // If we don't have an unbuilt package, then let's see if we have a @@ -598,7 +567,7 @@ handle (request& rq, response& rs) { try { - transaction bt (build_db_->begin ()); + transaction t (build_db_->begin ()); b = build_db_->find (b->id); @@ -617,19 +586,8 @@ handle (request& rq, response& rs) // Load the package (if still present). // - transaction pt (package_db_->begin (), false); - transaction::current (pt); - - shared_ptr p (package_db_->find (b->id.package)); - - if (p != nullptr) - p->internal_repository.load (); - - // Commit the package database transaction and switch back to the - // build database transaction. - // - pt.commit (); - transaction::current (bt); + shared_ptr p ( + build_db_->find (b->id.package)); if (p != nullptr) { @@ -656,11 +614,13 @@ handle (request& rq, response& rs) build_db_->update (b); + p->internal_repository.load (); + tsm = task (move (b), move (p), cm); } } - bt.commit (); + t.commit (); } catch (const odb::deadlock&) {} // Just try with the next rebuild. diff --git a/mod/mod-builds.cxx b/mod/mod-builds.cxx index 979121f..439bd05 100644 --- a/mod/mod-builds.cxx +++ b/mod/mod-builds.cxx @@ -205,11 +205,9 @@ package_query (const brep::params::builds& params) { using namespace brep; using query = query; + using qp = typename query::build_package; - // Skip external and stub packages. - // - query q (query::internal_repository.is_not_null () && - compare_version_ne (query::id.version, wildcard_version, false)); + query q (true); // Note that there is no error reported if the filter parameters parsing // fails. Instead, it is considered that no packages match such a query. @@ -219,12 +217,12 @@ package_query (const brep::params::builds& params) // Package name. // if (!params.name ().empty ()) - q = q && query::id.name.like (transform (params.name ())); + q = q && qp::id.name.like (transform (params.name ())); // Package version. // if (!params.version ().empty () && params.version () != "*") - q = q && compare_version_eq (query::id.version, + q = q && compare_version_eq (qp::id.version, version (params.version ()), // May throw. true); } @@ -554,8 +552,8 @@ handle (request& rq, response& rs) // configurations and the number of existing package builds. // size_t nmax (config_toolchains.size () * - build_db_->query_value ( - package_query (params))); + build_db_->query_value ( + package_query (params))); size_t ncur = build_db_->query_value ( build_query (*build_conf_names_, bld_params)); @@ -592,10 +590,10 @@ handle (request& rq, response& rs) // the pager to just '' links, and pass the offset as a // URL query parameter. Alternatively, we can invent the page number cap. // - using pkg_query = query; - using prep_pkg_query = prepared_query; + using pkg_query = query; + using prep_pkg_query = prepared_query; - pkg_query pq (package_query (params)); + pkg_query pq (package_query (params)); // Specify the portion. Note that we will still be querying packages in // chunks, not to hold locks for too long. @@ -603,14 +601,14 @@ handle (request& rq, response& rs) size_t offset (0); pq += "ORDER BY" + - pkg_query::id.name + - order_by_version_desc (pkg_query::id.version, false) + + pkg_query::build_package::id.name + + order_by_version_desc (pkg_query::build_package::id.version, false) + "OFFSET" + pkg_query::_ref (offset) + "LIMIT 50"; connection_ptr conn (build_db_->connection ()); prep_pkg_query pkg_prep_query ( - conn->prepare_query ("mod-builds-package-query", pq)); + conn->prepare_query ("mod-builds-package-query", pq)); // Prepare the build prepared query. // @@ -655,7 +653,7 @@ handle (request& rq, response& rs) { transaction t (conn->begin ()); - // Query (and cache) build packages. + // Query (and cache) buildable packages. // auto packages (pkg_prep_query.execute ()); diff --git a/mod/mod-package-version-details.cxx b/mod/mod-package-version-details.cxx index d1b6dfe..fd8d9a1 100644 --- a/mod/mod-package-version-details.cxx +++ b/mod/mod-package-version-details.cxx @@ -338,8 +338,7 @@ handle (request& rq, response& rs) using query = query; for (auto& b: build_db_->query ( - (query::id.package.name == name && - compare_version_eq (query::id.package.version, ver, true) && + (query::id.package == pkg->id && query::id.configuration.in_range (build_conf_names_->begin (), build_conf_names_->end ())) + diff --git a/mod/options.cli b/mod/options.cli index 051081f..857c2f3 100644 --- a/mod/options.cli +++ b/mod/options.cli @@ -441,7 +441,7 @@ namespace brep class build_task { - // Package repository display name. + // Package repository canonical name. // vector repository | r; }; -- cgit v1.1