From c51ccc69ed4039fac8ebfbd7c2fcaf0abb8341d0 Mon Sep 17 00:00:00 2001 From: Karen Arutyunov Date: Fri, 19 May 2017 19:39:21 +0300 Subject: Allow to enforce rebuild for building state --- libbrep/build.cxx | 26 +++++++++++++++++- libbrep/build.hxx | 28 ++++++++++++++++--- libbrep/build.xml | 2 +- mod/mod-build-force.cxx | 11 +++++--- mod/mod-build-result.cxx | 7 ++--- mod/mod-build-task.cxx | 70 +++++++++++++++++++++++++++--------------------- mod/mod-builds.cxx | 22 ++++++++------- 7 files changed, 113 insertions(+), 53 deletions(-) diff --git a/libbrep/build.cxx b/libbrep/build.cxx index ece2fd7..2391165 100644 --- a/libbrep/build.cxx +++ b/libbrep/build.cxx @@ -30,6 +30,30 @@ namespace brep else throw invalid_argument ("invalid build state '" + s + "'"); } + // force_state + // + string + to_string (force_state s) + { + switch (s) + { + case force_state::unforced: return "unforced"; + case force_state::forcing: return "forcing"; + case force_state::forced: return "forced"; + } + + return string (); // Should never reach. + } + + force_state + to_force_state (const string& s) + { + if (s == "unforced") return force_state::unforced; + else if (s == "forcing") return force_state::forcing; + else if (s == "forced") return force_state::forced; + else throw invalid_argument ("invalid force state '" + s + "'"); + } + // build // build:: @@ -46,7 +70,7 @@ namespace brep toolchain_version (move (tvr)), state (build_state::building), timestamp (timestamp_type::clock::now ()), - forced (false), + force (force_state::unforced), machine (move (mnm)), machine_summary (move (msm)), target (move (trg)) diff --git a/libbrep/build.hxx b/libbrep/build.hxx index 9258bb3..afa96ed 100644 --- a/libbrep/build.hxx +++ b/libbrep/build.hxx @@ -82,6 +82,28 @@ namespace brep to(to_string (?)) \ from(brep::to_build_state (?)) + // force_state + // + enum class force_state: std::uint8_t + { + unforced, + forcing, // Rebuild is forced while being in the building state. + forced // Rebuild is forced while being in the built state. + }; + + string + to_string (force_state); + + force_state + to_force_state (const string&); // May throw invalid_argument. + + inline ostream& + operator<< (ostream& os, force_state s) {return os << to_string (s);} + + #pragma db map type(force_state) as(string) \ + to(to_string (?)) \ + from(brep::to_force_state (?)) + // result_status // using bbot::result_status; @@ -118,7 +140,7 @@ namespace brep using timestamp_type = brep::timestamp; // Create the build object with the building state, non-existent status, - // the timestamp set to now and the forced flag set to false. + // the timestamp set to now and the force state set to unforced. // build (string package_name, version package_version, string configuration, @@ -140,9 +162,7 @@ namespace brep // timestamp_type timestamp; - // True if the package rebuild has been forced. - // - bool forced; + force_state force; // Must present for the built state, may present for the building state. // diff --git a/libbrep/build.xml b/libbrep/build.xml index 72cbd5f..7466c97 100644 --- a/libbrep/build.xml +++ b/libbrep/build.xml @@ -18,7 +18,7 @@ - + diff --git a/mod/mod-build-force.cxx b/mod/mod-build-force.cxx index 9389566..c0c3d2d 100644 --- a/mod/mod-build-force.cxx +++ b/mod/mod-build-force.cxx @@ -169,12 +169,17 @@ handle (request& rq, response& rs) // Respond with 409 (conflict) if the package configuration is in // inappropriate state for being rebuilt. // - else if (b->state != build_state::built) + else if (b->state != build_state::built && + b->state != build_state::building) throw invalid_request (409, "state is " + to_string (b->state)); - if (!b->forced) + force_state force (b->state == build_state::built + ? force_state::forced + : force_state::forcing); + + if (b->force != force) { - b->forced = true; + b->force = force; build_db_->update (b); l1 ([&]{trace << "force rebuild for " diff --git a/mod/mod-build-result.cxx b/mod/mod-build-result.cxx index d78c45f..f2b4ffd 100644 --- a/mod/mod-build-result.cxx +++ b/mod/mod-build-result.cxx @@ -248,13 +248,14 @@ handle (request& rq, response&) // build was forced. // notify = !(rqm.result.status == result_status::success && - b->status && *b->status == rqm.result.status && !b->forced); + b->status && *b->status == rqm.result.status && + b->force == force_state::unforced); prev_status = move (b->status); - b->state = build_state::built; + b->state = build_state::built; b->status = rqm.result.status; - b->forced = false; + b->force = force_state::unforced; // Mark the section as loaded, so results are updated. // diff --git a/mod/mod-build-task.cxx b/mod/mod-build-task.cxx index 911fb9b..e47ae60 100644 --- a/mod/mod-build-task.cxx +++ b/mod/mod-build-task.cxx @@ -149,7 +149,7 @@ handle (request& rq, response& rs) // 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. The rebuild preference is given in the following order: the - // greater force flag, the greater overall status, the lower timestamp. + // greater force state, the greater overall status, the lower timestamp. // if (!cfg_machines.empty ()) { @@ -212,15 +212,18 @@ handle (request& rq, response& rs) expiration (timeout).time_since_epoch ()).count (); }; - uint64_t build_expiration_ns ( + uint64_t normal_result_expiration_ns ( expiration_ns (options_->build_result_timeout ())); - timestamp forced_rebuild_expiration ( - expiration (options_->build_forced_rebuild_timeout ())); + uint64_t forced_result_expiration_ns ( + expiration_ns (options_->build_forced_rebuild_timeout ())); timestamp normal_rebuild_expiration ( expiration (options_->build_normal_rebuild_timeout ())); + timestamp forced_rebuild_expiration ( + expiration (options_->build_forced_rebuild_timeout ())); + // Convert butl::standard_version type to brep::version. // brep::version toolchain_version (tqm.toolchain_version.string ()); @@ -309,7 +312,10 @@ handle (request& rq, response& rs) (bld_query::state == "built" || (bld_query::state == "building" && - bld_query::timestamp > build_expiration_ns))); + ((bld_query::force == "forcing" && + bld_query::timestamp > forced_result_expiration_ns) || + (bld_query::force != "forcing" && // Unforced or forced. + bld_query::timestamp > normal_result_expiration_ns))))); connection_ptr bld_conn (build_db_->connection ()); @@ -370,11 +376,15 @@ handle (request& rq, response& rs) assert (j != configs.end ()); configs.erase (j); - if (i->state == build_state::built && - i->timestamp <= (i->forced - ? forced_rebuild_expiration - : normal_rebuild_expiration)) - rebuilds.emplace_back (i.load ()); + if (i->state == build_state::built) + { + 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 (!configs.empty ()) @@ -404,31 +414,29 @@ handle (request& rq, response& rs) else { // The package configuration can be in the building or unbuilt - // state, so the forced flag is false and the status is absent, - // unless in the building state (in which case they may or may - // not be set/exist), and there are no results. + // state, and there are no results. // - // Note that in the building state the status can be present if - // the rebuild task have been issued. 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. We - // just assert that if the force flag is set, then the status - // exists. + // 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::built && + assert (b->state != build_state::built && b->results.empty ()); - ((!b->forced && !b->status) || - (b->state == build_state::building && - (!b->forced || b->status))) && + b->state = build_state::building; - b->results.empty ()); + // 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->state = build_state::building; b->toolchain_name = move (tqm.toolchain_name); b->machine = mh.name; b->machine_summary = move (mh.summary); @@ -476,15 +484,15 @@ handle (request& rq, response& rs) // Sort the package configuration rebuild list with the following sort // priority: // - // 1: forced flag + // 1: force state // 2: overall status // 3: timestamp (less is preferred) // auto cmp = [] (const shared_ptr& x, const shared_ptr& y) -> bool { - if (x->forced != y->forced) - return x->forced > y->forced; // Forced goes first. + if (x->force != y->force) + return x->force > y->force; // Forced goes first. assert (x->status && y->status); // Both built. @@ -502,7 +510,7 @@ handle (request& rq, response& rs) // anymore (as we have committed the database transactions that were // used to collect this data) so we recheck. If we find one that matches // then put it into the building state, refresh the timestamp and - // update. Note that we don't amend the status and the force flag to + // update. Note that we don't amend the status and the force state to // have them available in the result request handling (see above). // for (auto& b: rebuilds) @@ -514,7 +522,7 @@ handle (request& rq, response& rs) b = build_db_->find (b->id); if (b != nullptr && b->state == build_state::built && - b->timestamp <= (b->forced + b->timestamp <= (b->force == force_state::forced ? forced_rebuild_expiration : normal_rebuild_expiration)) { diff --git a/mod/mod-builds.cxx b/mod/mod-builds.cxx index c831359..e62e1e5 100644 --- a/mod/mod-builds.cxx +++ b/mod/mod-builds.cxx @@ -162,7 +162,7 @@ build_query (const C& configs, const brep::params::builds& params) if (rs != "*") { if (rs == "pending") - q = q && query::forced; + q = q && query::force != "unforced"; else if (rs == "building") q = q && query::state == "building"; else @@ -382,7 +382,7 @@ handle (request& rq, response& rs) << SPAN(CLASS="value"); if (b.state == build_state::building) - s << "building"; + s << "building | "; else { assert (b.state == build_state::built); @@ -423,16 +423,18 @@ handle (request& rq, response& rs) << ~A << " | "; } - - if (b.forced) - s << "pending"; - else - s << A - << HREF << force_rebuild_url (host, root, b) << ~HREF - << "rebuild" - << ~A; } + if (b.force == (b.state == build_state::building + ? force_state::forcing + : force_state::forced)) + s << "pending"; + else + s << A + << HREF << force_rebuild_url (host, root, b) << ~HREF + << "rebuild" + << ~A; + s << ~SPAN << ~TD << ~TR -- cgit v1.1