diff options
Diffstat (limited to 'mod/mod-ci-github.cxx')
| -rw-r--r-- | mod/mod-ci-github.cxx | 63 |
1 files changed, 50 insertions, 13 deletions
diff --git a/mod/mod-ci-github.cxx b/mod/mod-ci-github.cxx index ba80ed6..119968d 100644 --- a/mod/mod-ci-github.cxx +++ b/mod/mod-ci-github.cxx @@ -406,8 +406,7 @@ namespace brep // synchronize // A pull request's head branch was updated from the base branch or // new commits were pushed to the head branch. (Note that there is - // no equivalent event for the base branch. That case gets handled - // in handle_check_suite_request() instead. @@ Not anymore.) + // no equivalent event for the base branch.) // // Note that both cases are handled the same: we start a new CI // request which will be reported on the new commit id. @@ -507,11 +506,13 @@ namespace brep // // While it would have been nice to cancel CIs of PRs with this branch as - // base not to waste resources, there are complications: Firsty, we can - // only do this for remote PRs (since local PRs may share the result with - // branch push). Secondly, we try to do our best even if the branch - // protection rule for head behind is not enabled. In this case, it would - // be good to complete the CI. So maybe/later. + // base not to waste resources, there are complications: Firstly, we can + // only do this for remote PRs (since local PRs will most likely share the + // result with branch push). Secondly, we try to do our best even if the + // branch protection rule for head behind is not enabled. In this case, it + // would be good to complete the CI. So maybe/later. See also the head + // case in handle_pull_request(), where we do cancel remote PRs that are + // not shared. // Service id that uniquely identifies the CI tenant. // @@ -1480,12 +1481,6 @@ namespace brep l3 ([&]{trace << "installation_access_token { " << *iat << " }";}); - // Note that similar to the branch push case above, while it would have - // been nice to cancel the previous CI job once the PR head moves (the - // "synchronize" event), due to the head sharing problem the previous CI - // job might actually still be relevant (in both local and remote PR - // cases). - // Distinguish between local and remote PRs by comparing the head and base // repositories' paths. // @@ -1494,6 +1489,48 @@ namespace brep ? service_data::local : service_data::remote); + // Note that similar to the branch push case above, while it would have + // been nice to cancel the previous CI job once the PR head moves (the + // "synchronize" event), due to the head sharing problem the previous CI + // job might actually still be relevant (in both local and remote PR + // cases). So we only do it for the remote PRs and only if the head is not + // shared (via tenant reference counting). + // + if (kind == service_data::remote && pr.action == "synchronize") + { + if (pr.before) + { + // Service id that will uniquely identify the CI tenant. + // + string sid (pr.repository.node_id + ':' + *pr.before); + + if (optional<tenant_service> ts = cancel (error, warn, + verb_ ? &trace : nullptr, + *build_db_, retry_, + "ci-github", sid, + true /* ref_count */)) + { + l3 ([&]{trace << "pull request " << pr.pull_request.node_id + << ": attempted to cancel CI of previous head commit" + << " (ref_count: " << ts->ref_count << ')';}); + } + else + { + // It's possible that there was no CI for the previous commit for + // various reasons (e.g., CI was not enabled). + // + l3 ([&]{trace << "pull request " << pr.pull_request.node_id + << ": failed to cancel CI of previous head commit " + << "with tenant_service id " << sid;}); + } + } + else + { + error << "pull request " << pr.pull_request.node_id + << ": before commit is missing in synchronize event"; + } + } + // Note: for remote PRs the check_sha will be set later, in // build_unloaded_pre_check(), to test merge commit id. // |