diff options
| -rw-r--r-- | mod/mod-build-result.cxx | 30 | ||||
| -rw-r--r-- | mod/mod-build-task.cxx | 27 | ||||
| -rw-r--r-- | mod/mod-ci-github-gq.cxx | 60 | ||||
| -rw-r--r-- | mod/mod-ci-github-gq.hxx | 29 | ||||
| -rw-r--r-- | mod/mod-ci-github-service-data.cxx | 3 | ||||
| -rw-r--r-- | mod/mod-ci-github.cxx | 521 | ||||
| -rw-r--r-- | mod/mod-ci-github.hxx | 8 | ||||
| -rw-r--r-- | mod/mod-ci.cxx | 7 | ||||
| -rw-r--r-- | mod/mod-ci.hxx | 4 | ||||
| -rw-r--r-- | mod/tenant-service.cxx | 18 | ||||
| -rw-r--r-- | mod/tenant-service.hxx | 12 | ||||
| -rw-r--r-- | tests/ci/ci-dir.testscript | 2 | ||||
| -rw-r--r-- | tests/ci/ci-load.testscript | 2 | ||||
| -rw-r--r-- | tests/submit/submit-git.testscript | 9 |
14 files changed, 468 insertions, 264 deletions
diff --git a/mod/mod-build-result.cxx b/mod/mod-build-result.cxx index cc058b5..b303386 100644 --- a/mod/mod-build-result.cxx +++ b/mod/mod-build-result.cxx @@ -565,7 +565,7 @@ handle (request& rq, response&) { assert (tss); // Wouldn't be here otherwise. - const tenant_service& ss (tss->first); + tenant_service& ss (tss->first); const build& b (*tss->second); // Release the database connection since build_built() notification can @@ -576,7 +576,33 @@ handle (request& rq, response&) if (auto f = tsb->build_built (b.tenant, ss, b, log_writer_)) { conn = build_db_->connection (); - update_tenant_service_state (conn, ss.type, ss.id, f); + + bool build_completed (false); + + if (optional<string> data = + update_tenant_service_state ( + conn, ss.type, ss.id, + [&f, &build_completed] (const string& tid, + const tenant_service& ts) + { + auto r (f (tid, ts)); + build_completed = r.second; + return move (r.first); + })) + { + ss.data = move (data); + } + + if (build_completed) + { + // Release the database connection since the build_completed() + // notification can potentially be time-consuming (e.g., it may + // perform an HTTP request). + // + conn.reset (); + + tsb->build_completed (b.tenant, ss, log_writer_); + } } } diff --git a/mod/mod-build-task.cxx b/mod/mod-build-task.cxx index c8b1bb2..88e5618 100644 --- a/mod/mod-build-task.cxx +++ b/mod/mod-build-task.cxx @@ -2407,7 +2407,7 @@ handle (request& rq, response& rs) } // If a third-party service needs to be notified about the package - // build, then call the tenant_service_build_built::build_building() + // build, then call the tenant_service_build_building::build_building() // callback function and, if requested, update the tenant-associated // service state. // @@ -2556,9 +2556,32 @@ handle (request& rq, response& rs) { conn = build_db_->connection (); + bool build_completed (false); + if (optional<string> data = - update_tenant_service_state (conn, ss.type, ss.id, f)) + update_tenant_service_state ( + conn, ss.type, ss.id, + [&f, &build_completed] (const string& tid, + const tenant_service& ts) + { + auto r (f (tid, ts)); + build_completed = r.second; + return move (r.first); + })) + { ss.data = move (data); + } + + if (build_completed) + { + // Release the database connection since the build_completed() + // notification can potentially be time-consuming (e.g., it may + // perform an HTTP request). + // + conn.reset (); + + tsb->build_completed (b.tenant, ss, log_writer_); + } } } } diff --git a/mod/mod-ci-github-gq.cxx b/mod/mod-ci-github-gq.cxx index 37d944c..7abd709 100644 --- a/mod/mod-ci-github-gq.cxx +++ b/mod/mod-ci-github-gq.cxx @@ -511,9 +511,13 @@ namespace brep const string& ni, // Node ID. const string& st, // Check run status. optional<timestamp> sa, // Started at. - optional<gq_built_result> br) + const string& ti, // Output title. + const string& su, // Output summary. + optional<string> co = nullopt) // Conclusion. { - assert (st != "COMPLETED" || br); + // Ensure we have conclusion if the status is completed. + // + assert (st != "COMPLETED" || co); ostringstream os; @@ -538,15 +542,13 @@ namespace brep ": " + e.what ()); } } - if (br) - { - os << '\n'; - os << " conclusion: " << gq_enum (br->conclusion) << '\n' - << " output: {" << '\n' - << " title: " << gq_str (br->title) << '\n' - << " summary: " << gq_str (br->summary) << '\n' - << " }"; - } + os << '\n'; + if (co) + os << " conclusion: " << gq_enum (*co) << '\n'; + os << " output: {" << '\n' + << " title: " << gq_str (ti) << '\n' + << " summary: " << gq_str (su) << '\n' + << " }"; os << "})" << '\n' // Specify the selection set (fields to be returned). Note that we // rename `id` to `node_id` (using a field alias) for consistency with @@ -654,11 +656,11 @@ namespace brep const string& rid, const string& nid, build_state st, - optional<gq_built_result> br) + string ti, string su) { - // Must have a result if state is built. + // State cannot be built without a conclusion. // - assert (st != build_state::built || br); + assert (st != build_state::built && !ti.empty () && !su.empty ()); // Set `startedAt` to current time if updating to building. // @@ -673,7 +675,8 @@ namespace brep nid, gh_to_status (st), sa, - move (br)))); + move (ti), move (su), + nullopt /* conclusion */))); vector<check_run> crs {move (cr)}; crs[0].state = st; @@ -685,6 +688,33 @@ namespace brep return r; } + bool + gq_update_check_run (const basic_mark& error, + check_run& cr, + const string& iat, + const string& rid, + const string& nid, + gq_built_result br) + { + string rq ( + gq_serialize_request ( + gq_mutation_update_check_run (rid, + nid, + gh_to_status (build_state::built), + nullopt /* startedAt */, + move (br.title), move (br.summary), + move (br.conclusion)))); + + vector<check_run> crs {move (cr)}; + crs[0].state = build_state::built; + + bool r (gq_mutate_check_runs (error, crs, iat, move (rq))); + + cr = move (crs[0]); + + return r; + } + // Serialize a GraphQL query that fetches a pull request from GitHub. // // Throw invalid_argument if the node id is not a valid GraphQL string. diff --git a/mod/mod-ci-github-gq.hxx b/mod/mod-ci-github-gq.hxx index 0fc3817..19c4924 100644 --- a/mod/mod-ci-github-gq.hxx +++ b/mod/mod-ci-github-gq.hxx @@ -82,28 +82,41 @@ namespace brep const optional<string>& details_url, gq_built_result); - // Update a check run on GitHub. Update `cr` with the new data (state and - // state_synced). Return false and issue diagnostics if the request failed. + // Update a check run on GitHub to the queued or building state. Note that + // the state cannot be built because in that case a conclusion is required. + // + // Update `cr` with the new data (state and state_synced). Return false and + // issue diagnostics if the request failed. // // Throw invalid_argument if the passed data is invalid, missing, or // inconsistent. // + // Title and summary are required and cannot be empty. + // + bool + 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, + string title, + string summary); + + // As above but update a check run to the built state (which requires a + // conclusion). + // // Note that GitHub allows any state transitions except from built (but // built to built is allowed). The latter case is signalled by setting the // check_run state_synced member to false and the state member to built. // - // The gq_built_result is required if the build_state is built because - // GitHub does not allow a check run status of `completed` without at least - // a conclusion. - // bool 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<gq_built_result> = nullopt); + gq_built_result); // Fetch pre-check information for a pull request from GitHub. This // information is used to decide whether or not to CI the PR and is diff --git a/mod/mod-ci-github-service-data.cxx b/mod/mod-ci-github-service-data.cxx index 4598302..c51f791 100644 --- a/mod/mod-ci-github-service-data.cxx +++ b/mod/mod-ci-github-service-data.cxx @@ -120,7 +120,8 @@ namespace brep s, ss, rs, - nullopt /* details_url */}); + nullopt, /* details_url */ + nullopt /* description */}); p.next_expect (event::end_object); } diff --git a/mod/mod-ci-github.cxx b/mod/mod-ci-github.cxx index 96c5889..6cf3b80 100644 --- a/mod/mod-ci-github.cxx +++ b/mod/mod-ci-github.cxx @@ -564,6 +564,31 @@ namespace brep // return handle_branch_push (move (ps), warning_success); } + // Ignore marketplace_purchase events (sent by the GitHub Marketplace) by + // sending a 200 response with empty body. We offer a free plan only and + // do not support user accounts so there is nothing to be done. + // + else if (event == "marketplace_purchase") + { + return true; + } + // Ignore GitHub App installation events by sending a 200 response with + // empty body. These are triggered when a user installs a GitHub App in a + // repository or organization. + // + else if (event == "installation") + { + return true; + } + // Ignore ping events by sending a 200 response with empty body. This + // event occurs when you create a new webhook. The ping event is a + // confirmation from GitHub that you configured the webhook correctly. One + // of its triggers is listing an App on the GitHub Marketplace. + // + else if (event == "ping") + { + return true; + } else { // Log to investigate. @@ -577,11 +602,51 @@ namespace brep // Let's capitalize the synthetic conclusion check run name to make it // easier to distinguish from the regular ones. // - static string conclusion_check_run_name ("CONCLUSION"); + static const string conclusion_check_run_name ("CONCLUSION"); + + // Yellow circle. + // + static const string conclusion_building_title ("\U0001F7E1 IN PROGRESS"); + static const string conclusion_building_summary ( + "Waiting for all the builds to complete."); + + // "Medium white" circle. + // + static const string check_run_queued_title ("\U000026AA QUEUED"); + static const string check_run_queued_summary ( + "Waiting for the build to start."); + + // Yellow circle. + // + static const string check_run_building_title ("\U0001F7E1 BUILDING"); + static const string check_run_building_summary ( + "Waiting for the build to complete."); - static check_run::description_type conclusion_check_run_building_description { - "\U000026AA IN PROGRESS", // "Medium white" circle. - "Waiting for all builds to complete"}; + // Return the colored circle corresponding to a result_status. + // + // Note: the rest of the title is produced by to_string(result_status). + // + static string + circle (result_status rs) + { + switch (rs) + { + case result_status::success: return "\U0001F7E2"; // Green circle. + case result_status::warning: return "\U0001F7E0"; // Orange circle. + case result_status::error: + case result_status::abort: + case result_status::abnormal: return "\U0001F534"; // Red circle. + + // Valid values we should never encounter. + // + case result_status::skip: + case result_status::interrupt: + throw invalid_argument ("unexpected result_status value: " + + to_string (rs)); + } + + return ""; // Should never reach. + } bool ci_github:: handle_branch_push (gh_push_event ps, bool warning_success) @@ -1115,30 +1180,6 @@ namespace brep return true; } - // Return the colored circle corresponding to a result_status. - // - static string - circle (result_status rs) - { - switch (rs) - { - case result_status::success: return "\U0001F7E2"; // Green circle. - case result_status::warning: return "\U0001F7E0"; // Orange circle. - case result_status::error: - case result_status::abort: - case result_status::abnormal: return "\U0001F534"; // Red circle. - - // Valid values we should never encounter. - // - case result_status::skip: - case result_status::interrupt: - throw invalid_argument ("unexpected result_status value: " + - to_string (rs)); - } - - return ""; // Should never reach. - } - // Make a check run summary from a CI start_result. // static string @@ -1329,7 +1370,7 @@ namespace brep if (gq_update_check_run (error, bcr, iat->token, repo_node_id, cr.check_run.node_id, - build_state::built, br)) + br)) { l3 ([&]{trace << "updated check_run { " << bcr << " }";}); } @@ -1342,7 +1383,7 @@ namespace brep if (gq_update_check_run (error, ccr, iat->token, repo_node_id, *sd.conclusion_node_id, - build_state::built, move (br))) + move (br))) { l3 ([&]{trace << "updated conclusion check_run { " << ccr << " }";}); } @@ -1390,7 +1431,7 @@ namespace brep // if (gq_update_check_run (error, ccr, iat->token, repo_node_id, *sd.conclusion_node_id, - build_state::built, move (br))) + move (br))) { l3 ([&]{trace << "updated conclusion check_run { " << ccr << " }";}); } @@ -1467,11 +1508,13 @@ namespace brep bcr.state = build_state::queued; bcr.state_synced = false; bcr.details_url = cr.check_run.details_url; + bcr.description = {check_run_queued_title, check_run_queued_summary}; ccr.state = build_state::building; ccr.state_synced = false; ccr.details_url = details_url (tenant_id); - ccr.description = conclusion_check_run_building_description; + ccr.description = {conclusion_building_title, + conclusion_building_summary}; if (gq_create_check_runs (error, check_runs, iat->token, repo_node_id, head_sha)) @@ -1622,7 +1665,7 @@ namespace brep // if (gq_update_check_run (error, bcr, iat->token, repo_node_id, *bcr.node_id, - build_state::built, br)) + br)) { l3 ([&]{trace << "updated check_run { " << bcr << " }";}); } @@ -1638,7 +1681,7 @@ namespace brep // if (gq_update_check_run (error, ccr, iat->token, repo_node_id, *ccr.node_id, - build_state::built, move (br))) + move (br))) { l3 ([&]{trace << "updated conclusion check_run { " << ccr << " }";}); } @@ -1941,7 +1984,8 @@ namespace brep &sd, &error, this] (string name, - const check_run::description_type& output) + const string& title, + const string& summary) -> optional<check_run> { check_run cr; @@ -1956,7 +2000,7 @@ namespace brep sd.report_sha, details_url (tenant_id), build_state::building, - output.title, output.summary)) + title, summary)) { return cr; } @@ -1991,7 +2035,6 @@ namespace brep iat->token, sd.repository_node_id, node_id, - build_state::built, move (br))) { assert (cr.state == build_state::built); @@ -2013,9 +2056,9 @@ namespace brep if (!sd.conclusion_node_id) { - if (auto cr = - create_synthetic_cr (conclusion_check_run_name, - conclusion_check_run_building_description)) + if (auto cr = create_synthetic_cr (conclusion_check_run_name, + conclusion_building_title, + conclusion_building_summary)) { l3 ([&]{trace << "created check_run { " << *cr << " }";}); @@ -2316,7 +2359,9 @@ namespace brep build_state::queued, false /* state_synced */, nullopt /* status */, - details_url (b)}); + details_url (b), + check_run::description_type {check_run_queued_title, + check_run_queued_summary}}); } } @@ -2520,7 +2565,9 @@ namespace brep iat->token, sd.repository_node_id, *cr->node_id, - build_state::building)) + build_state::building, + check_run_building_title, + check_run_building_summary)) { // Do nothing further if the state was already built on GitHub (note // that this is based on the above-mentioned special GitHub semantics @@ -2595,7 +2642,8 @@ namespace brep return nullptr; } - function<optional<string> (const string&, const tenant_service&)> ci_github:: + function<pair<optional<string>, bool> (const string&, + const tenant_service&)> ci_github:: build_built (const string& tenant_id, const tenant_service& ts, const build& b, @@ -2626,89 +2674,17 @@ namespace brep if (sd.completed) return nullptr; - // Here we need to update the state of this check run and, if there are no - // more unbuilt ones, update the synthetic conclusion check run and mark - // the check suite as completed. - // - // Absent means we still have unbuilt check runs. - // - optional<result_status> conclusion (*b.status); - - // Conclusion check run summary. Will include the success/warning/failure - // count breakdown. - // - string summary; + // Here we only update the state of this check run. If there are no more + // unbuilt ones, then the synthetic conclusion check run will be updated + // in build_completed(). Note that determining whether we have no more + // unbuilt would be racy here so instead we do it in the service data + // update function that we return. check_run cr; // Updated check run. { - // The success/warning/failure counts. - // - // Note that the warning count will be included in the success or - // failure count (depending on the value of sd.warning_success). - // - size_t succ_count (0), warn_count (0), fail_count (0); - - // Count a result_status under the appropriate category. - // - auto count = [&succ_count, - &warn_count, - &fail_count, - ws = sd.warning_success] (result_status rs) - { - switch (rs) - { - case result_status::success: ++succ_count; break; - - case result_status::error: - case result_status::abort: - case result_status::abnormal: ++fail_count; break; - - case result_status::warning: - { - ++warn_count; - - if (ws) - ++succ_count; - else - ++fail_count; - - break; - } - } - }; - - count (*b.status); - string bid (gh_check_run_name (b)); // Full build id. - optional<check_run> scr; - for (check_run& cr: sd.check_runs) - { - if (cr.build_id == bid) - { - assert (!scr); - scr = move (cr); - } - else - { - if (cr.state == build_state::built) - { - assert (cr.status); - - if (conclusion) - *conclusion |= *cr.status; - - count (*cr.status); - } - else - conclusion = nullopt; - } - - if (scr && !conclusion.has_value ()) - break; - } - - if (scr) + if (check_run* scr = sd.find_check_run (bid)) { if (scr->state != build_state::building) { @@ -2736,29 +2712,6 @@ namespace brep } cr.state_synced = false; - - // Construct the conclusion check run summary if all check runs are - // built. - // - if (conclusion) - { - ostringstream os; - - // Note: the warning count has already been included in the success or - // failure count. - // - os << fail_count << " failed"; - if (!sd.warning_success && warn_count != 0) - os << " (" << warn_count << " due to warnings)"; - - os << ", " << succ_count << " succeeded"; - if (sd.warning_success && warn_count != 0) - os << " (" << warn_count << " with warnings)"; - - os << ", " << (succ_count + fail_count) << " total"; - - summary = os.str (); - } } // Get a new installation access token if the current one has expired. @@ -2780,8 +2733,6 @@ namespace brep else iat = &sd.installation_access; - bool completed (false); - // Note: we treat the failure to obtain the installation access token the // same as the failure to notify GitHub (state is updated but not marked // synced). @@ -2903,7 +2854,6 @@ namespace brep iat->token, sd.repository_node_id, *cr.node_id, - build_state::built, move (br))) { assert (cr.state == build_state::built); @@ -2935,69 +2885,27 @@ namespace brep if (cr.state_synced) { - // Check run was created/updated successfully to built (with - // status we specified). + // Check run was created/updated successfully to built (with status we + // specified). // cr.status = b.status; - - // Update the conclusion check run if all check runs are now built. - // - if (conclusion) - { - assert (sd.conclusion_node_id); - - result_status rs (*conclusion); - - gq_built_result br ( - make_built_result (rs, sd.warning_success, move (summary))); - - check_run cr; - - // Set some fields for display purposes. - // - cr.node_id = *sd.conclusion_node_id; - cr.name = conclusion_check_run_name; - - // Let unlikely invalid_argument propagate. - // - if (gq_update_check_run (error, - cr, - iat->token, - sd.repository_node_id, - *sd.conclusion_node_id, - build_state::built, - move (br))) - { - assert (cr.state == build_state::built); - l3 ([&]{trace << "updated conclusion check_run { " << cr << " }";}); - } - else - { - // Nothing we can do here except log the error. - // - error << "tenant_service id " << ts.id - << ": unable to update conclusion check run " - << *sd.conclusion_node_id; - } - - completed = true; - } } } return [tenant_id, iat = move (new_iat), cr = move (cr), - completed = completed, error = move (error), warn = move (warn)] (const string& ti, - const tenant_service& ts) -> optional<string> + const tenant_service& ts) { // NOTE: this lambda may be called repeatedly (e.g., due to transaction // being aborted) and so should not move out of its captures. + // Do nothing if the tenant has been replaced. + // if (tenant_id != ti) - return nullopt; // Do nothing if the tenant has been replaced. + return make_pair (optional<string> (), false); service_data sd; try @@ -3007,7 +2915,19 @@ namespace brep catch (const invalid_argument& e) { error << "failed to parse service data: " << e; - return nullopt; + return make_pair (optional<string> (), false); + } + + // Feel like this could potentially happen in case of an out of order + // notification (see above). + // + if (sd.completed) + { + // @@ Perhaps this should be a warning but let's try error for now (we + // essentially missed a build, which could have failed). + // + error << "built notification for completed check suite"; + return make_pair (optional<string> (), false); } if (iat) @@ -3037,46 +2957,203 @@ namespace brep else sd.check_runs.push_back (cr); - if (bool c = completed) + // Determine of this check suite is completed. + // + sd.completed = find_if (sd.check_runs.begin (), sd.check_runs.end (), + [] (const check_run& scr) + { + return scr.state != build_state::built; + }) == sd.check_runs.end (); + } + + return make_pair (optional<string> (sd.json ()), sd.completed); + }; + } + catch (const std::exception& e) + { + NOTIFICATION_DIAG (log_writer); + + string bid (gh_check_run_name (b)); // Full build id. + + error << "check run " << bid << ": unhandled exception: " << e.what (); + + return nullptr; + } + + void ci_github:: + build_completed (const string& /* tenant_id */, + const tenant_service& ts, + const diag_epilogue& log_writer) const noexcept + try + { + // NOTE: this function is noexcept and should not throw. + + NOTIFICATION_DIAG (log_writer); + + service_data sd; + try + { + sd = service_data (*ts.data); + } + catch (const invalid_argument& e) + { + error << "failed to parse service data: " << e; + return; + } + + // This could have been reset by handle_check_run_rerequest(). + // + if (!sd.completed) + return; + + assert (!sd.check_runs.empty ()); + + // Here we need to update the state of the synthetic conclusion check run. + // + result_status result (result_status::success); + + // Conclusion check run summary. Will include the success/warning/failure + // count breakdown. + // + string summary; + { + // The success/warning/failure counts. + // + // Note that the warning count will be included in the success or + // failure count (depending on the value of sd.warning_success). + // + size_t succ_count (0), warn_count (0), fail_count (0); + + // Count a result_status under the appropriate category. + // + auto count = [&succ_count, + &warn_count, + &fail_count, + ws = sd.warning_success] (result_status rs) + { + switch (rs) { - // Note that this can be racy: while we calculated the completed - // value based on the snapshot of the service data, it could have - // been changed (e.g., by handle_check_run_rerequest()). So we - // re-calculate it based on the check run states and only update if - // it matches. Otherwise, we log an error. - // - for (const check_run& scr: sd.check_runs) + case result_status::success: ++succ_count; break; + + case result_status::error: + case result_status::abort: + case result_status::abnormal: ++fail_count; break; + + case result_status::warning: { - if (scr.state != build_state::built) - { - string sid (sd.repository_node_id + ':' + sd.report_sha); + ++warn_count; - error << "tenant_service id " << sid - << ": out of order built notification service data update; " - << "check suite is no longer complete"; + if (ws) + ++succ_count; + else + ++fail_count; - c = false; - break; - } + break; } - if (c) - sd.completed = true; + case result_status::skip: + case result_status::interrupt: + { + assert (false); + } } + }; + + for (const check_run& cr: sd.check_runs) + { + assert (cr.state == build_state::built && cr.status); + + result |= *cr.status; + count (*cr.status); } - return sd.json (); - }; + // Construct the conclusion check run summary. + // + ostringstream os; + + // Note: the warning count has already been included in the success or + // failure count. + // + os << fail_count << " failed"; + if (!sd.warning_success && warn_count != 0) + os << " (" << warn_count << " due to warnings)"; + + os << ", " << succ_count << " succeeded"; + if (sd.warning_success && warn_count != 0) + os << " (" << warn_count << " with warnings)"; + + os << ", " << (succ_count + fail_count) << " total"; + + summary = os.str (); + } + + // Get a new installation access token if the current one has expired + // (unlikely since we just returned from build_built()). Note also that we + // are not saving the new token in the service data. + // + const gh_installation_access_token* iat (nullptr); + optional<gh_installation_access_token> new_iat; + + if (system_clock::now () > sd.installation_access.expires_at) + { + if (optional<string> jwt = generate_jwt (sd.app_id, trace, error)) + { + new_iat = obtain_installation_access_token (sd.installation_id, + move (*jwt), + error); + if (new_iat) + iat = &*new_iat; + } + } + else + iat = &sd.installation_access; + + // Note: we treat the failure to obtain the installation access token the + // same as the failure to notify GitHub. + // + if (iat != nullptr) + { + // Update the conclusion check run if all check runs are now built. + // + assert (sd.conclusion_node_id); + + gq_built_result br ( + make_built_result (result, sd.warning_success, move (summary))); + + check_run cr; + + // Set some fields for display purposes. + // + cr.node_id = *sd.conclusion_node_id; + cr.name = conclusion_check_run_name; + + // Let unlikely invalid_argument propagate. + // + if (gq_update_check_run (error, + cr, + iat->token, + sd.repository_node_id, + *sd.conclusion_node_id, + move (br))) + { + assert (cr.state == build_state::built); + l3 ([&]{trace << "updated conclusion check_run { " << cr << " }";}); + } + else + { + // Nothing we can do here except log the error. + // + error << "tenant_service id " << ts.id + << ": unable to update conclusion check run " + << *sd.conclusion_node_id; + } + } } catch (const std::exception& e) { NOTIFICATION_DIAG (log_writer); - string bid (gh_check_run_name (b)); // Full build id. - - error << "check run " << bid << ": unhandled exception: " << e.what(); - - return nullptr; + error << "unhandled exception: " << e.what (); } string ci_github:: diff --git a/mod/mod-ci-github.hxx b/mod/mod-ci-github.hxx index c21d3db..4cedc94 100644 --- a/mod/mod-ci-github.hxx +++ b/mod/mod-ci-github.hxx @@ -72,12 +72,18 @@ namespace brep const build&, const diag_epilogue& log_writer) const noexcept override; - virtual function<optional<string> (const string&, const tenant_service&)> + virtual function<pair<optional<string>, bool> (const string&, + const tenant_service&)> build_built (const string& tenant_id, const tenant_service&, const build&, const diag_epilogue& log_writer) const noexcept override; + virtual void + build_completed (const string& tenant_id, + const tenant_service& ts, + const diag_epilogue& log_writer) const noexcept override; + private: virtual void init (cli::scanner&); diff --git a/mod/mod-ci.cxx b/mod/mod-ci.cxx index 16ec5a7..85c00c6 100644 --- a/mod/mod-ci.cxx +++ b/mod/mod-ci.cxx @@ -496,8 +496,8 @@ build_building (const string& /*tenant_id*/, }; } -function<optional<string> (const string& tenant_id, - const brep::tenant_service&)> brep::ci:: +function<pair<optional<string>, bool> (const string& tenant_id, + const brep::tenant_service&)> brep::ci:: build_built (const string& /*tenant_id*/, const tenant_service&, const build& b, @@ -515,7 +515,8 @@ build_built (const string& /*tenant_id*/, b.toolchain_name + '/' + b.toolchain_version.string ()); - return ts.data ? *ts.data + ", " + s : s; + return make_pair ( + optional<string> (ts.data ? *ts.data + ", " + s : s), false); }; } diff --git a/mod/mod-ci.hxx b/mod/mod-ci.hxx index 132b5b0..54532e6 100644 --- a/mod/mod-ci.hxx +++ b/mod/mod-ci.hxx @@ -87,8 +87,8 @@ namespace brep const build&, const diag_epilogue& log_writer) const noexcept override; - virtual function<optional<string> (const string& tenant_id, - const tenant_service&)> + virtual function<pair<optional<string>, bool> (const string& tenant_id, + const tenant_service&)> build_built (const string& tenant_id, const tenant_service&, const build&, diff --git a/mod/tenant-service.cxx b/mod/tenant-service.cxx new file mode 100644 index 0000000..2c1f3bc --- /dev/null +++ b/mod/tenant-service.cxx @@ -0,0 +1,18 @@ +// file : mod/tenant-service.cxx -*- C++ -*- +// license : MIT; see accompanying LICENSE file + +#include <mod/tenant-service.hxx> + +namespace brep +{ + void tenant_service_build_built:: + build_completed (const string& /* tenant_id */, + const tenant_service&, + const diag_epilogue& /* log_writer */) const noexcept + { + // If this notification is requested, then this function needs to be + // overridden by the tenant service implementation. + // + assert (false); + } +} diff --git a/mod/tenant-service.hxx b/mod/tenant-service.hxx index 5564a56..d909eaa 100644 --- a/mod/tenant-service.hxx +++ b/mod/tenant-service.hxx @@ -127,12 +127,20 @@ namespace brep class tenant_service_build_built: public virtual tenant_service_base { public: - virtual function<optional<string> (const string& tenant_id, - const tenant_service&)> + // The second half of the pair signals whether to call the + // build_completed() notification. + // + virtual function<pair<optional<string>, bool> (const string& tenant_id, + const tenant_service&)> build_built (const string& tenant_id, const tenant_service&, const build&, const diag_epilogue& log_writer) const noexcept = 0; + + virtual void + build_completed (const string& tenant_id, + const tenant_service&, + const diag_epilogue& log_writer) const noexcept; }; // This notification is only made on unloaded CI requests created with the diff --git a/tests/ci/ci-dir.testscript b/tests/ci/ci-dir.testscript index be5a9b9..f3fbe18 100644 --- a/tests/ci/ci-dir.testscript +++ b/tests/ci/ci-dir.testscript @@ -87,7 +87,7 @@ { $clone_root_data_clean; - sed -i -e "s%^\(repository:\) .+\$%\\1 http://example.com/repo.git%" \ + sed -i -e "s%^\(repository:\) .+\$%\\1 https://git.build2.org/no-such-repo.git%" \ $data_dir/request.manifest; $* >>~"%EOO%" diff --git a/tests/ci/ci-load.testscript b/tests/ci/ci-load.testscript index ff75493..9935071 100644 --- a/tests/ci/ci-load.testscript +++ b/tests/ci/ci-load.testscript @@ -275,7 +275,7 @@ { $clone_root_data_clean; - sed -i -e "s%^\(repository:\) .+\$%\\1 http://example.com/repo.git%" \ + sed -i -e "s%^\(repository:\) .+\$%\\1 https://git.build2.org/no-such-repo.git%" \ $data_dir/request.manifest; $* >>~"%EOO%" diff --git a/tests/submit/submit-git.testscript b/tests/submit/submit-git.testscript index 7093142..f71f164 100644 --- a/tests/submit/submit-git.testscript +++ b/tests/submit/submit-git.testscript @@ -1056,7 +1056,7 @@ pkg_ctl="$prj_ctl/hello.git" : { $clone_root_data_clean; - sed -i -e 's%^(control:) .+$%\1 http://example.com/path/rep.git%' \ + sed -i -e 's%^(control:) .+$%\1 https://git.build2.org/no-such-repo.git%' \ $data_dir/request.manifest; $clone_root_tgt; @@ -1066,8 +1066,9 @@ pkg_ctl="$prj_ctl/hello.git" status: 422 message: \\ - failed to git-clone build2-control branch of http://example.com/path/rep.git - info: repository 'http://example.com/path/rep.git/' not found + failed to git-clone build2-control branch of https://git.build2.org/no-such-r\\ + epo.git + info: repository 'https://git.build2.org/no-such-repo.git/' not found \\ reference: $checksum EOO @@ -1093,7 +1094,7 @@ pkg_ctl="$prj_ctl/hello.git" { $clone_root_data_clean; - $* 'http://example.com/path/rep.git' $data_dir 2>>~%EOE% != 0 + $* 'https://git.build2.org/no-such-repo.git' $data_dir 2>>~%EOE% != 0 %fatal: .+% EOE } |