From abfeebfbb1912934954335d3c36baea12624c130 Mon Sep 17 00:00:00 2001 From: Francois Kritzinger Date: Tue, 30 Apr 2024 14:38:08 +0200 Subject: Post-review changes --- mod/mod-ci-github-gq.cxx | 101 ++++++++++++++++++++++++++++++----------------- mod/mod-ci-github-gq.hxx | 16 +++++--- mod/mod-ci-github.cxx | 29 +++++++++++++- mod/mod-ci-github.hxx | 5 +++ mod/module.cli | 3 +- 5 files changed, 110 insertions(+), 44 deletions(-) diff --git a/mod/mod-ci-github-gq.cxx b/mod/mod-ci-github-gq.cxx index c37c889..d65d701 100644 --- a/mod/mod-ci-github-gq.cxx +++ b/mod/mod-ci-github-gq.cxx @@ -321,12 +321,15 @@ namespace brep // because GitHub does not allow a check run status of completed without a // conclusion. // + // `du` can be empty for queued but not for the other states. + // static string gq_mutation_create_check_runs (const string& ri, // Repository ID const string& hs, // Head SHA + const string& du, // Details URL. const vector& crs, const string& st, // Check run status. - const optional& co = nullopt) + optional br = nullopt) { ostringstream os; @@ -339,22 +342,31 @@ namespace brep string al ("cr" + to_string (i)); // Field alias. os << gq_name (al) << ":createCheckRun(input: {" << '\n' - << " name: " << gq_str (crs[i].name) << ',' << '\n' - << " repositoryId: " << gq_str (ri) << ',' << '\n' - << " headSha: " << gq_str (hs) << ',' << '\n' + << " name: " << gq_str (crs[i].name) << '\n' + << " repositoryId: " << gq_str (ri) << '\n' + << " headSha: " << gq_str (hs) << '\n' << " status: " << gq_enum (st); - if (co) + if (!du.empty ()) + { + os << '\n'; + os << " detailsUrl: " << gq_str (du); + } + if (br) { - os << ',' << '\n' - << " conclusion: " << gq_enum (*co); + 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' // Specify the selection set (fields to be returned). // << "{" << '\n' << " checkRun {" << '\n' - << " id," << '\n' - << " name," << '\n' + << " id" << '\n' + << " name" << '\n' << " status" << '\n' << " }" << '\n' << "}" << '\n'; @@ -374,28 +386,38 @@ namespace brep static string gq_mutation_update_check_run (const string& ri, // Repository ID. const string& ni, // Node ID. + const string& du, // Details URL. const string& st, // Check run status. - const optional& co) + optional br) { ostringstream os; os << "mutation {" << '\n' << "cr0:updateCheckRun(input: {" << '\n' - << " checkRunId: " << gq_str (ni) << ',' << '\n' - << " repositoryId: " << gq_str (ri) << ',' << '\n' + << " checkRunId: " << gq_str (ni) << '\n' + << " repositoryId: " << gq_str (ri) << '\n' << " status: " << gq_enum (st); - if (co) + if (!du.empty ()) { - os << ',' << '\n' - << " conclusion: " << gq_enum (*co); + os << '\n'; + os << " detailsUrl: " << gq_str (du); + } + 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' // Specify the selection set (fields to be returned). // << "{" << '\n' << " checkRun {" << '\n' - << " id," << '\n' - << " name," << '\n' + << " id" << '\n' + << " name" << '\n' << " status" << '\n' << " }" << '\n' << "}" << '\n' @@ -416,8 +438,10 @@ namespace brep // assert (st != build_state::built); + // Empty details URL because it's not available until building. + // string rq (gq_serialize_request ( - gq_mutation_create_check_runs (rid, hs, crs, gh_to_status (st)))); + gq_mutation_create_check_runs (rid, hs, "", crs, gh_to_status (st)))); return gq_mutate_check_runs (error, crs, iat, move (rq), st); } @@ -428,22 +452,27 @@ namespace brep const string& iat, const string& rid, const string& hs, + const string& du, build_state st, - optional rs, - bool ws) + optional br) { // Must have a result if state is built. // - assert (st != build_state::built || rs); + assert (st != build_state::built || br); - vector crs {move (cr)}; + // Must have a details URL because `st` should never be queued. + // + assert (!du.empty ()); - optional co; // Conclusion. - if (rs) - co = gh_to_conclusion (*rs, ws); + vector crs {move (cr)}; string rq (gq_serialize_request ( - gq_mutation_create_check_runs (rid, hs, crs, gh_to_status (st), co))); + gq_mutation_create_check_runs (rid, + hs, + du, + crs, + gh_to_status (st), + move (br)))); bool r (gq_mutate_check_runs (error, crs, iat, move (rq), st)); @@ -458,20 +487,20 @@ namespace brep const string& iat, const string& rid, const string& nid, + const string& du, build_state st, - optional rs, - bool ws) + optional br) { // Must have a result if state is built. // - assert (st != build_state::built || rs); - - optional co; // Conclusion. - if (rs) - co = gh_to_conclusion (*rs, ws); - - string rq (gq_serialize_request ( - gq_mutation_update_check_run (rid, nid, gh_to_status (st), co))); + assert (st != build_state::built || br); + + string rq ( + gq_serialize_request (gq_mutation_update_check_run (rid, + nid, + du, + gh_to_status (st), + move (br)))); vector crs {move (cr)}; diff --git a/mod/mod-ci-github-gq.hxx b/mod/mod-ci-github-gq.hxx index e8bb397..5ad77e1 100644 --- a/mod/mod-ci-github-gq.hxx +++ b/mod/mod-ci-github-gq.hxx @@ -26,6 +26,10 @@ namespace brep // Note: no details_url yet since there will be no entry in the build result // search page until the task starts building. // + // @@ TMP We only create multiple check runs in build_queued() so + // build_state is redundant. Maybe we should rename this + // gq_queue_check_runs()? + // bool gq_create_check_runs (const basic_mark& error, vector& check_runs, @@ -38,14 +42,15 @@ namespace brep // state and the node ID. Return false and issue diagnostics if the request // failed. // - // The result_status is required if the build_state is built because GitHub - // does not allow a check run status of `completed` without a conclusion. @@ + // 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. // struct gq_built_result { string conclusion; string title; - string summmary; + string summary; }; bool @@ -64,8 +69,9 @@ namespace brep // with the new state. Return false and issue diagnostics if the request // failed. // - // The result_status is required if the build_state is built because GitHub - // does not allow a check run status of `completed` without a conclusion. @@ + // 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, diff --git a/mod/mod-ci-github.cxx b/mod/mod-ci-github.cxx index 29097be..52431d2 100644 --- a/mod/mod-ci-github.cxx +++ b/mod/mod-ci-github.cxx @@ -5,6 +5,8 @@ #include +#include // mime_url_encode() + #include #include #include @@ -711,6 +713,7 @@ namespace brep iat->token, sd.repository_id, *cr->node_id, + details_url (b), build_state::building)) { // Do nothing further if the state was already built on GitHub (note @@ -851,6 +854,12 @@ namespace brep // instead of passing it to gq_*() functions? Let's see how we handle // the report. + // @@ TODO Summary + // + gq_built_result br (gh_to_conclusion (*b.status, sd.warning_success), + ucase (to_string (*b.status)), + "SUMMARY"); + if (cr.node_id) { // Update existing check run to built. @@ -860,8 +869,9 @@ namespace brep iat->token, sd.repository_id, *cr.node_id, + "", // Don't update details_url again. build_state::built, - *b.status, sd.warning_success)) + move (br))) { assert (cr.state == build_state::built); @@ -883,8 +893,9 @@ namespace brep iat->token, sd.repository_id, sd.head_sha, + details_url (b), build_state::built, - *b.status, sd.warning_success)) + move (br))) { assert (cr.state == build_state::built); @@ -938,6 +949,20 @@ namespace brep }; } + string ci_github:: + details_url (const build& b) const + { + return options_->host () + + "/@" + b.tenant + + "?builds=" + mime_url_encode (b.package_name.string ()) + + "&pv=" + b.package_version.string () + + "&tg=" + mime_url_encode (b.target.string ()) + + "&tc=" + mime_url_encode (b.target_config_name) + + "&pc=" + mime_url_encode (b.package_config_name) + + "&th=" + mime_url_encode (b.toolchain_version.string ()) + + "&rs=*"; + } + optional ci_github:: generate_jwt (const basic_mark& trace, const basic_mark& error) const diff --git a/mod/mod-ci-github.hxx b/mod/mod-ci-github.hxx index b16085e..6bb01e3 100644 --- a/mod/mod-ci-github.hxx +++ b/mod/mod-ci-github.hxx @@ -66,6 +66,11 @@ namespace brep bool handle_check_suite_request (gh_check_suite_event, bool warning_success); + // Build a check run details_url for a build. + // + string + details_url (const build&) const; + optional generate_jwt (const basic_mark& trace, const basic_mark& error) const; diff --git a/mod/module.cli b/mod/module.cli index bec4c2d..e8ebbb0 100644 --- a/mod/module.cli +++ b/mod/module.cli @@ -818,7 +818,8 @@ namespace brep // @@ TODO Is etc/brep-module.conf updated manually? Yes, will need to // replicate there eventually. // - class ci_github: ci_start, ci_cancel, + class ci_github: repository_url, + ci_start, ci_cancel, build, build_db, handler, openssl_options -- cgit v1.1