diff options
| -rw-r--r-- | mod/mod-ci-github-gh.cxx | 17 | ||||
| -rw-r--r-- | mod/mod-ci-github-gh.hxx | 5 | ||||
| -rw-r--r-- | mod/mod-ci-github-gq.cxx | 2 | ||||
| -rw-r--r-- | mod/mod-ci-github-gq.hxx | 20 | ||||
| -rw-r--r-- | mod/mod-ci-github-service-data.hxx | 5 | ||||
| -rw-r--r-- | mod/mod-ci-github.cxx | 117 |
6 files changed, 87 insertions, 79 deletions
diff --git a/mod/mod-ci-github-gh.cxx b/mod/mod-ci-github-gh.cxx index 5c4809c..c97f938 100644 --- a/mod/mod-ci-github-gh.cxx +++ b/mod/mod-ci-github-gh.cxx @@ -20,9 +20,6 @@ namespace brep case build_state::queued: return "QUEUED"; case build_state::building: return "IN_PROGRESS"; case build_state::built: return "COMPLETED"; - default: - throw invalid_argument ("invalid build_state value: " + - to_string (static_cast<int> (st))); } } @@ -36,19 +33,21 @@ namespace brep else if (s == "IN_PROGRESS") return build_state::building; else if (s == "COMPLETED") return build_state::built; else - throw invalid_argument ("invalid GitHub check run status: '" + s + + throw invalid_argument ("unexpected GitHub check run status: '" + s + '\''); } string - gh_to_conclusion (result_status rs) + gh_to_conclusion (result_status rs, bool warning_success) { switch (rs) { case result_status::success: - case result_status::warning: return "SUCCESS"; + case result_status::warning: + return warning_success ? "SUCCESS" : "FAILURE"; + case result_status::error: case result_status::abort: case result_status::abnormal: @@ -60,12 +59,6 @@ namespace brep case result_status::interrupt: throw invalid_argument ("unexpected result_status value: " + to_string (rs)); - - // Invalid value. - // - default: - throw invalid_argument ("invalid result_status value: " + - to_string (static_cast<int> (rs))); } } diff --git a/mod/mod-ci-github-gh.hxx b/mod/mod-ci-github-gh.hxx index 45b5359..7e46cbb 100644 --- a/mod/mod-ci-github-gh.hxx +++ b/mod/mod-ci-github-gh.hxx @@ -81,8 +81,11 @@ namespace brep build_state gh_from_status (const string&); + // If warning_success is true, then map result_status::warning to SUCCESS + // and to FAILURE otherwise. + // string - gh_to_conclusion (result_status); + gh_to_conclusion (result_status, bool warning_success); // Create a check_run name from a build. If the second argument is not // NULL, return an abbreviated id if possible. diff --git a/mod/mod-ci-github-gq.cxx b/mod/mod-ci-github-gq.cxx index 4e13199..bedf301 100644 --- a/mod/mod-ci-github-gq.cxx +++ b/mod/mod-ci-github-gq.cxx @@ -324,7 +324,7 @@ namespace brep gq_mutation_create_check_runs ( const string& ri, // Repository ID const string& hs, // Head SHA - const vector<reference_wrapper<const build>>& bs, + const vector<reference_wrapper<const build>>& bs, // Pass build ids. build_state st, optional<result_status> rs, const build_queued_hints* bh) { diff --git a/mod/mod-ci-github-gq.hxx b/mod/mod-ci-github-gq.hxx index 9331b2b..16117ea 100644 --- a/mod/mod-ci-github-gq.hxx +++ b/mod/mod-ci-github-gq.hxx @@ -24,14 +24,14 @@ namespace brep // request failed. // bool - gq_create_check_runs (vector<check_run>& check_runs, + gq_create_check_runs (const basic_mark& error, + vector<check_run>& check_runs, const string& installation_access_token, const string& repository_id, const string& head_sha, const vector<reference_wrapper<const build>>&, build_state, - const build_queued_hints&, - const basic_mark& error); + const build_queued_hints&); // Create a new check run on GitHub for a build. Update `cr` with the new // state and the node ID. Return false and issue diagnostics if the request @@ -43,15 +43,15 @@ namespace brep // @@ TODO Support output (title, summary, text). // bool - gq_create_check_run (check_run& cr, + gq_create_check_run (const basic_mark& error, + check_run& cr, const string& installation_access_token, const string& repository_id, const string& head_sha, const build&, build_state, - optional<result_status>, - const build_queued_hints&, - const basic_mark& error); + optional<result_status> = nullopt, + const build_queued_hints&); // Update a check run on GitHub. // @@ -65,13 +65,13 @@ namespace brep // @@ TODO Support output (title, summary, text). // bool - gq_update_check_run (check_run& cr, + gq_update_check_run (const basic_mark& error, + check_run& cr, const string& installation_access_token, const string& repository_id, const string& node_id, build_state, - optional<result_status>, - const basic_mark& error); + optional<result_status> = nullopt); } #endif // MOD_MOD_CI_GITHUB_GQ_HXX diff --git a/mod/mod-ci-github-service-data.hxx b/mod/mod-ci-github-service-data.hxx index d4a47d5..99c50ae 100644 --- a/mod/mod-ci-github-service-data.hxx +++ b/mod/mod-ci-github-service-data.hxx @@ -25,6 +25,7 @@ namespace brep struct check_run { string build_id; // Full build id. + string name; // Potentially shortened build id. optional<string> node_id; // GitHub id. build_state state; @@ -46,6 +47,10 @@ namespace brep // uint64_t version = 1; + // Check suite settings. + // + bool warning_success; // See gh_to_conclusion(). + // Check suite-global data. // gh_installation_access_token installation_access; diff --git a/mod/mod-ci-github.cxx b/mod/mod-ci-github.cxx index 9449f97..3e6803e 100644 --- a/mod/mod-ci-github.cxx +++ b/mod/mod-ci-github.cxx @@ -411,8 +411,8 @@ namespace brep // building Skip if there is no check run in service data or it's // not in the queued state, otherwise update. // - // built Update if there is check run in service data and its - // state is not built, otherwise create new. + // built Update if there is check run in service data unless its + // state is built, otherwise create new. // // The rationale for this semantics is as follows: the building // notification is a "nice to have" and can be skipped if things are not @@ -603,7 +603,8 @@ namespace brep } function<optional<string> (const tenant_service&)> ci_github:: - build_building (const tenant_service& ts, const build& b, + build_building (const tenant_service& ts, + const build& b, const diag_epilogue& log_writer) const noexcept { NOTIFICATION_DIAG (log_writer); @@ -743,7 +744,8 @@ namespace brep } function<optional<string> (const tenant_service&)> ci_github:: - build_built (const tenant_service& ts, const build& b, + build_built (const tenant_service& ts, + const build& b, const diag_epilogue& log_writer) const noexcept { NOTIFICATION_DIAG (log_writer); @@ -760,32 +762,35 @@ namespace brep } check_run cr; // Updated check run. - string bid (gh_check_run_name (b)); // Full Build ID. - - if (check_run* scr = sd.find_check_run (bid)) { - if (scr->state != build_state::building) + string bid (gh_check_run_name (b)); // Full Build ID. + + if (check_run* scr = sd.find_check_run (bid)) { - warn << "check run " << bid << ": out of order built " - << "notification; existing state: " << scr->state_string (); - } + if (scr->state != build_state::building) + { + warn << "check run " << bid << ": out of order built notification; " + << "existing state: " << scr->state_string (); + } - // Do nothing if already built. - // - if (scr->state == build_state::built) - return nullptr; + // Do nothing if already built (e.g., rebuild). + // + if (scr->state == build_state::built) + return nullptr; - cr = move (*scr); - } - else - { - warn << "check run " << bid << ": out of order built " - << "notification; no check run state in service data"; + cr = move (*scr); + } + else + { + warn << "check run " << bid << ": out of order built notification; " + << "no check run state in service data"; - cr.build_id = move (bid); - } + cr.build_id = move (bid); + cr.name = cr.build_id; + } - cr.state_synced = false; + cr.state_synced = false; + } // Get a new installation access token if the current one has expired. // @@ -838,9 +843,6 @@ namespace brep // check run to the service data it will create another check run with // the shortened name which will never get to the built state. // - // @@ TMP If build_queued() runs but fails to create we could store - // the build hints and use them now. - // if (gq_create_check_run (cr, iat->token, sd.repository_id, @@ -861,40 +863,45 @@ namespace brep cr = move (cr), error = move (error), warn = move (warn)] (const tenant_service& ts) -> optional<string> - { - // NOTE: this lambda may be called repeatedly (e.g., due to transaction - // being aborted) and so should not move out of its captures. + { + // NOTE: this lambda may be called repeatedly (e.g., due to transaction + // being aborted) and so should not move out of its captures. - service_data sd; - try - { - sd = service_data (*ts.data); - } - catch (const invalid_argument& e) - { - error << "failed to parse service data: " << e; - return nullopt; - } + service_data sd; + try + { + sd = service_data (*ts.data); + } + catch (const invalid_argument& e) + { + error << "failed to parse service data: " << e; + return nullopt; + } - if (iat) - sd.installation_access = *iat; + if (iat) + sd.installation_access = *iat; - if (check_run* scr = sd.find_check_run (cr.build_id)) + if (check_run* scr = sd.find_check_run (cr.build_id)) + { + // This will most commonly generate a duplicate warning (see above). + // We could save the old state and only warn if it differs but let's + // not complicate things for now. + // +#if 0 + if (scr->state != build_state::building) { - if (scr->state != build_state::building) - { - warn << "check run " << cr.build_id << ": out of order built " - << "notification service data update; existing state: " - << scr->state_string (); - } - - *scr = cr; + warn << "check run " << cr.build_id << ": out of order built " + << "notification service data update; existing state: " + << scr->state_string (); } - else - sd.check_runs.push_back (cr); +#endif + *scr = cr; + } + else + sd.check_runs.push_back (cr); - return sd.json (); - }; + return sd.json (); + }; } optional<string> ci_github:: |