From 16f840d9ff5551b4b9577084f1587c524b05d2b0 Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Tue, 28 May 2024 11:32:51 +0200 Subject: Review --- mod/mod-ci-github-gq.hxx | 14 +++++- mod/mod-ci-github-service-data.hxx | 32 ++++++++++++-- mod/mod-ci-github.cxx | 91 +++++++++++++++++++++++++++++++++++--- 3 files changed, 126 insertions(+), 11 deletions(-) (limited to 'mod') diff --git a/mod/mod-ci-github-gq.hxx b/mod/mod-ci-github-gq.hxx index 7a15b5e..3d697a9 100644 --- a/mod/mod-ci-github-gq.hxx +++ b/mod/mod-ci-github-gq.hxx @@ -96,7 +96,19 @@ namespace brep string merge_commit_id; }; - pair, bool> + + // Fetch a pull request's mergeability from GitHub. Return absent value if + // the merge commit is still being generated. Return empty string if the + // pull request is not auto-mergeable. Otherwise return the test merge + // commit id. + // + // Issue diagnostics and return absent if the request failed (which means it + // will be treated by the caller as still being generated). + // + // Note that the first request causes GitHub to start preparing the test + // merge commit. + // + optional gq_pull_request_mergeable (const basic_mark& error, const string& installation_access_token, const string& node_id); diff --git a/mod/mod-ci-github-service-data.hxx b/mod/mod-ci-github-service-data.hxx index 252f992..4a433c5 100644 --- a/mod/mod-ci-github-service-data.hxx +++ b/mod/mod-ci-github-service-data.hxx @@ -61,17 +61,30 @@ namespace brep // The following two are only used for pull requests. // + // @@ TODO/LATER: maybe put them in a struct? + // optional repository_clone_url; optional pr_number; + // The GitHub ID of the synthetic PR merge check run or absent if it + // hasn't been created yet. + // + optional merge_node_id; + // The commit ID the check suite or pull request (and its check runs) are - // associated with. In the case of a pull request this will be - // `pull_request.head.sha`. + // associated with. Note that in the case of a pull request this will be + // the head commit (`pull_request.head.sha`) as opposed to the merge + // commit. // string head_sha; vector check_runs; + // The GitHub ID of the synthetic conclusion check run or absent if it + // hasn't been created yet. See also merge_node_id above. + // + optional conclusion_node_id; + // Return the check run with the specified build ID or nullptr if not // found. // @@ -85,14 +98,25 @@ namespace brep explicit service_data (const string& json); + // The check_suite constructor. + // + service_data (bool warning_success, + string iat_token, + timestamp iat_expires_at, + uint64_t installation_id, + string repository_node_id, + string head_sha); + + // The pull_request constructor. + // service_data (bool warning_success, string iat_token, timestamp iat_expires_at, uint64_t installation_id, string repository_node_id, string head_sha, - optional repository_clone_url = nullopt, - optional pr_number = nullopt); + string repository_clone_url, + uint32_t pr_number); service_data () = default; diff --git a/mod/mod-ci-github.cxx b/mod/mod-ci-github.cxx index f1d6be9..f9fd453 100644 --- a/mod/mod-ci-github.cxx +++ b/mod/mod-ci-github.cxx @@ -536,6 +536,12 @@ namespace brep l3 ([&]{trace << "pull_request event { " << pr << " }";}); + // @@ TODO: check action, ignore those we don't care about. + + // While we don't need the installation access token in this request, + // let's obtain it to flush out any permission issues early. Also, it is + // valid for an hour so we will most likely make use of it. + // optional jwt (generate_jwt (trace, error)); if (!jwt) throw server_error (); @@ -544,7 +550,6 @@ namespace brep obtain_installation_access_token (pr.installation.id, move (*jwt), error)); - if (!iat) throw server_error (); @@ -560,14 +565,21 @@ namespace brep pr.pull_request.number) .json ()); + // Create unloaded CI request. After this call we will start getting the + // build_unloaded() notifications until (1) we load the request, (2) we + // cancel it, or (3) it gets archived after some timeout. + // + // Note: use no delay since we need to (re)create the synthetic merge + // check run as soon as possible. + // optional tid ( create (error, warn, &trace, *build_db_, tenant_service (move (pr.pull_request.node_id), "ci-github", move (sd)), - chrono::seconds (30), // @@ TODO Proper values? - chrono::seconds (10))); + chrono::seconds (30) /* interval */, + chrono::seconds (0) /* delay */)); if (!tid) fail << "unable to create unloaded CI request"; @@ -577,7 +589,8 @@ namespace brep // Return the colored circle corresponding to a result_status. // - string circle (result_status rs) + static string + circle (result_status rs) { switch (rs) { @@ -637,9 +650,75 @@ namespace brep if (iat == nullptr) return nullptr; - // Check PR mergeability. + bool first (...); // @@ TODO + + // If this is the first call, (re)create the synthetic merge check run as + // soon as possible to make sure the previous check suite, if any, is no + // longer completed. + // + if (first) + { + // TODO: in-progress + } + + // Start/check PR mergeability. + // + optional mc ( + gq_pull_request_mergeable (error, iat->token, ts.id)); // Merge commit. + + if (!mc) // No merge commit yet. + { + // If this is a subseqent notification and there is no merge commit, + // then there is nothing to do. + // + if (!first) + return nullptr; + + // If this is a first notification, record the merge check run in the + // service data. + // + return [&error, + iat = move (new_iat), + cr = move (cr), + ccr = move (ccr)] (const tenant_service& ts) -> optional + { + // @@ TODO + } + } + else if (mc->empty ()) // Not meargable. + { + // If the commit is not mergable, cancel the CI request and fail the + // merge check run. + // + // Note that it feels like in this case we don't really need to create a + // failed synthetic conclusion check run since the PR cannot be merged + // anyway. + + // @@ TODO: cancel CI request + // @@ TODO: fail merge check run and update in service data. + + return [&error, + iat = move (new_iat), + cr = move (cr), + ccr = move (ccr)] (const tenant_service& ts) -> optional + { + // @@ TODO + } + } + + // If we are here, then it means we have a merge commit that we can load. // - optional ma; // Mergeability. + + // As a first step, (re)create the synthetic conclusion check run and then + // change the merge check run state to success. Do it in this order so + // that the check suite does not become completed. + + // @@ TODO: create synthetic conclusion check run (in progress) + // @@ TODO: update synthetic merge checj run (success) + // @@ TODO: will need to update conclusion check run node_id in service data + + // @@ TODO: load CI request. + { pair, bool> pr ( gq_pull_request_mergeable (error, iat->token, ts.id)); -- cgit v1.1