diff options
Diffstat (limited to 'mod')
| -rw-r--r-- | mod/mod-build-result.cxx | 238 | ||||
| -rw-r--r-- | mod/mod-build-task.cxx | 27 |
2 files changed, 152 insertions, 113 deletions
diff --git a/mod/mod-build-result.cxx b/mod/mod-build-result.cxx index 6f40a0b..99bdc8d 100644 --- a/mod/mod-build-result.cxx +++ b/mod/mod-build-result.cxx @@ -435,126 +435,164 @@ handle (request& rq, response&) if (auth) { - // Verify the result status/checksums. + // If the build is interrupted, then revert it to the original built + // state if this is a rebuild and delete it from the database + // otherwise. // - // Specifically, if the result status is skip, then it can only be in - // response to the soft rebuild task (all checksums are present in the - // build object) and the result checksums must match the build object - // checksums. On verification failure respond with the bad request - // HTTP code (400). - // - if (rqm.result.status == result_status::skip) + if (rqm.result.status == result_status::interrupt) { - if (!b->agent_checksum || - !b->worker_checksum || - !b->dependency_checksum) - throw invalid_request (400, "unexpected skip result status"); - - // Can only be absent for initial build, in which case the checksums - // are also absent and we would end up with the above 400 response. - // - assert (b->status); - - // Verify that the result checksum matches the build checksum and - // throw invalid_request(400) if that's not the case. - // - auto verify = [] (const string& build_checksum, - const optional<string>& result_checksum, - const char* what) + if (b->status) // Is this a rebuild? { - if (!result_checksum) - throw invalid_request ( - 400, - string (what) + - " checksum is expected for skip result status"); - - if (*result_checksum != build_checksum) - throw invalid_request ( - 400, - string (what) + " checksum '" + build_checksum + - "' is expected instead of '" + *result_checksum + - "' for skip result status"); - }; - - verify (*b->agent_checksum, rqm.agent_checksum, "agent"); - - verify (*b->worker_checksum, - rqm.result.worker_checksum, - "worker"); - - verify (*b->dependency_checksum, - rqm.result.dependency_checksum, - "dependency"); - } + b->state = build_state::built; - unforced = b->force == force_state::unforced; + // Keep the force rebuild indication. Note that the forcing state + // is only valid for the building state. + // + if (b->force == force_state::forcing) + b->force = force_state::forced; - // Don't send email to the build-email address for the - // success-to-success status change, unless the build was forced. - // - build_notify = !(rqm.result.status == result_status::success && - b->status && - *b->status == rqm.result.status && - unforced); + // Cleanup the interactive build login information. + // + b->interactive = nullopt; - b->state = build_state::built; - b->force = force_state::unforced; + // Cleanup the authentication data. + // + b->agent_fingerprint = nullopt; + b->agent_challenge = nullopt; - // Cleanup the interactive build login information. - // - b->interactive = nullopt; + // Note that we are unable to restore the pre-rebuild timestamp + // since it has been overwritten when the build task was issued. + // That, however, feels ok and we just keep it unchanged. - // Cleanup the authentication data. - // - b->agent_fingerprint = nullopt; - b->agent_challenge = nullopt; + build_db_->update (b); + } + else + build_db_->erase (b); + } + else + { + // Verify the result status/checksums. + // + // Specifically, if the result status is skip, then it can only be + // in response to the soft rebuild task (all checksums are present + // in the build object) and the result checksums must match the + // build object checksums. On verification failure respond with the + // bad request HTTP code (400). + // + if (rqm.result.status == result_status::skip) + { + if (!b->agent_checksum || + !b->worker_checksum || + !b->dependency_checksum) + throw invalid_request (400, "unexpected skip result status"); + + // Can only be absent for initial build, in which case the + // checksums are also absent and we would end up with the above + // 400 response. + // + assert (b->status); + + // Verify that the result checksum matches the build checksum and + // throw invalid_request(400) if that's not the case. + // + auto verify = [] (const string& build_checksum, + const optional<string>& result_checksum, + const char* what) + { + if (!result_checksum) + throw invalid_request ( + 400, + string (what) + + " checksum is expected for skip result status"); + + if (*result_checksum != build_checksum) + throw invalid_request ( + 400, + string (what) + " checksum '" + build_checksum + + "' is expected instead of '" + *result_checksum + + "' for skip result status"); + }; + + verify (*b->agent_checksum, rqm.agent_checksum, "agent"); + + verify (*b->worker_checksum, + rqm.result.worker_checksum, + "worker"); + + verify (*b->dependency_checksum, + rqm.result.dependency_checksum, + "dependency"); + } - b->timestamp = system_clock::now (); - b->soft_timestamp = b->timestamp; + unforced = b->force == force_state::unforced; - // If the result status is other than skip, then save the status, - // results, and checksums and update the hard timestamp. - // - if (rqm.result.status != result_status::skip) - { - b->status = rqm.result.status; - b->hard_timestamp = b->soft_timestamp; + // Don't send email to the build-email address for the + // success-to-success status change, unless the build was forced. + // + build_notify = !(rqm.result.status == result_status::success && + b->status && + *b->status == rqm.result.status && + unforced); + + b->state = build_state::built; + b->force = force_state::unforced; - // Mark the section as loaded, so results are updated. + // Cleanup the interactive build login information. // - b->results_section.load (); - b->results = move (rqm.result.results); + b->interactive = nullopt; - // Save the checksums. + // Cleanup the authentication data. // - b->agent_checksum = move (rqm.agent_checksum); - b->worker_checksum = move (rqm.result.worker_checksum); - b->dependency_checksum = move (rqm.result.dependency_checksum); - } + b->agent_fingerprint = nullopt; + b->agent_challenge = nullopt; - build_db_->update (b); + b->timestamp = system_clock::now (); + b->soft_timestamp = b->timestamp; - // Don't send the build notification email if the task result is - // `skip`, the configuration is hidden, or is now excluded by the - // package. - // - if (rqm.result.status != result_status::skip && belongs (*tc, "all")) - { - shared_ptr<build_package> p ( - build_db_->load<build_package> (b->id.package)); + // If the result status is other than skip, then save the status, + // results, and checksums and update the hard timestamp. + // + if (rqm.result.status != result_status::skip) + { + b->status = rqm.result.status; + b->hard_timestamp = b->soft_timestamp; + + // Mark the section as loaded, so results are updated. + // + b->results_section.load (); + b->results = move (rqm.result.results); + + // Save the checksums. + // + b->agent_checksum = move (rqm.agent_checksum); + b->worker_checksum = move (rqm.result.worker_checksum); + b->dependency_checksum = move (rqm.result.dependency_checksum); + } - // The package configuration should be present (see mod-builds.cxx - // for details) but if it is not, let's log the warning. + build_db_->update (b); + + // Don't send the build notification email if the task result is + // `skip`, the configuration is hidden, or is now excluded by the + // package. // - if (const build_package_config* pc = find (b->package_config_name, - p->configs)) + if (rqm.result.status != result_status::skip && belongs (*tc, "all")) { - if (!exclude (*pc, p->builds, p->constraints, *tc)) - bld = move (b); + shared_ptr<build_package> p ( + build_db_->load<build_package> (b->id.package)); + + // The package configuration should be present (see mod-builds.cxx + // for details) but if it is not, let's log the warning. + // + if (const build_package_config* pc = find (b->package_config_name, + p->configs)) + { + if (!exclude (*pc, p->builds, p->constraints, *tc)) + bld = move (b); + } + else + warn << "cannot find configuration '" << b->package_config_name + << "' for package " << p->id.name << '/' << p->version; } - else - warn << "cannot find configuration '" << b->package_config_name - << "' for package " << p->id.name << '/' << p->version; } } } diff --git a/mod/mod-build-task.cxx b/mod/mod-build-task.cxx index bd5034b..9ce2520 100644 --- a/mod/mod-build-task.cxx +++ b/mod/mod-build-task.cxx @@ -720,25 +720,19 @@ handle (request& rq, response& rs) b.hard_timestamp <= hard_rebuild_expiration; }; - // Convert a build to the hard rebuild, resetting the agent checksum and - // dropping the previous build task result. + // Convert a build to the hard rebuild, resetting the agent checksum. // // Note that since the checksums are hierarchical, the agent checksum // reset will trigger resets of the "subordinate" checksums up to the // dependency checksum and so the package will be rebuilt. // - // Also note that there is no sense to keep the build task result since we - // don't accept the skip result for the hard rebuild task. We, however, - // keep the status intact (see below for the reasoning). + // Also note that we keep the previous build task result and status + // intact since we may still need to revert the build into the built + // state if the task execution is interrupted. // auto convert_to_hard = [] (const shared_ptr<build>& b) { b->agent_checksum = nullopt; - - // Mark the section as loaded, so results are updated. - // - b->results_section.load (); - b->results.clear (); }; // Return SHA256 checksum of the controller logic and the configuration @@ -1051,8 +1045,9 @@ handle (request& rq, response& rs) // 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 - // email. The same is true for the forced flag (in the sense - // that we don't set the force state to unforced). + // notification email or to revert it to the built state if + // interrupted. The same is true for the forced flag (in + // the sense that we don't set the force state to unforced). // assert (b->state == build_state::building); @@ -1139,7 +1134,13 @@ handle (request& rq, response& rs) if (x->status != y->status) return x->status > y->status; // Larger status goes first. - return x->timestamp < y->timestamp; // Older goes first. + // Older build completion goes first. + // + // Note that a completed build can have the state change timestamp + // (timestamp member) newer than the completion timestamp + // (soft_timestamp member) if the build was interrupted. + // + return x->soft_timestamp < y->soft_timestamp; }; sort (rebuilds.begin (), rebuilds.end (), cmp); |