mirror of
https://github.com/NixOS/nixpkgs.git
synced 2025-11-09 16:18:34 +01:00
ci/github-script/merge: switch order of merge operations
We previously used auto-merge first and then enqueued explicitly on the assumption that auto-merge would fail if the PR was actually in mergeable state already. This turned out to be false. Instead, we currently face the problem of auto-merge sometimes getting stuck. This seems to happen when, at the time of enabling auto-merge, the required status checks already passed and the PR would be ready to go - but sometimes GitHub doesn't do it. This *can* be unblocked by approving the PR again, which seems to run the internal "let's check whether we can merge this" procedures on the GitHub side again. However, we can probably also solve this by just explicitly trying to enqueue the PR first - and only if that fails, fall back to auto-merge. I previously argued against that, based on a potential race condition, in which a PR could become ready to merge between these two requests - at which point the auto-merge operation would fail, if the original assumption was true. But since we don't observe this, we might as well switch.
This commit is contained in:
parent
6a54ddb71f
commit
747d9e2d34
|
|
@ -184,32 +184,9 @@ async function handleMerge({
|
||||||
return 'Merge completed (dry)'
|
return 'Merge completed (dry)'
|
||||||
}
|
}
|
||||||
|
|
||||||
// Using GraphQL's enablePullRequestAutoMerge mutation instead of the REST
|
// Using GraphQL mutations instead of the REST /merge endpoint, because the latter
|
||||||
// /merge endpoint, because the latter doesn't work with Merge Queues.
|
// doesn't work with Merge Queues. We now have merge queues enabled on all development
|
||||||
// This mutation works both with and without Merge Queues.
|
// branches, so we don't need a fallback for regular merges.
|
||||||
// It doesn't work when there are no required status checks for the target branch.
|
|
||||||
// All development branches have these enabled, so this is a non-issue.
|
|
||||||
try {
|
|
||||||
await github.graphql(
|
|
||||||
`mutation($node_id: ID!, $sha: GitObjectID) {
|
|
||||||
enablePullRequestAutoMerge(input: {
|
|
||||||
expectedHeadOid: $sha,
|
|
||||||
pullRequestId: $node_id
|
|
||||||
})
|
|
||||||
{ clientMutationId }
|
|
||||||
}`,
|
|
||||||
{ node_id: pull_request.node_id, sha: pull_request.head.sha },
|
|
||||||
)
|
|
||||||
return 'Enabled Auto Merge'
|
|
||||||
} catch (e) {
|
|
||||||
log('Auto Merge failed', e.response.errors[0].message)
|
|
||||||
}
|
|
||||||
|
|
||||||
// TODO: Observe whether the below is true and whether manual enqueue is actually needed.
|
|
||||||
// Auto-merge doesn't work if the target branch has already run all CI, in which
|
|
||||||
// case the PR must be enqueued explicitly.
|
|
||||||
// We now have merge queues enabled on all development branches, thus don't need a
|
|
||||||
// fallback after this.
|
|
||||||
try {
|
try {
|
||||||
const resp = await github.graphql(
|
const resp = await github.graphql(
|
||||||
`mutation($node_id: ID!, $sha: GitObjectID) {
|
`mutation($node_id: ID!, $sha: GitObjectID) {
|
||||||
|
|
@ -227,6 +204,25 @@ async function handleMerge({
|
||||||
return `[Queued](${resp.enqueuePullRequest.mergeQueueEntry.mergeQueue.url}) for merge`
|
return `[Queued](${resp.enqueuePullRequest.mergeQueueEntry.mergeQueue.url}) for merge`
|
||||||
} catch (e) {
|
} catch (e) {
|
||||||
log('Enqueing failed', e.response.errors[0].message)
|
log('Enqueing failed', e.response.errors[0].message)
|
||||||
|
}
|
||||||
|
|
||||||
|
// If required status checks are not satisfied, yet, the above will fail. In this case
|
||||||
|
// we can enable auto-merge. We could also only use auto-merge, but this often gets
|
||||||
|
// stuck for no apparent reason.
|
||||||
|
try {
|
||||||
|
await github.graphql(
|
||||||
|
`mutation($node_id: ID!, $sha: GitObjectID) {
|
||||||
|
enablePullRequestAutoMerge(input: {
|
||||||
|
expectedHeadOid: $sha,
|
||||||
|
pullRequestId: $node_id
|
||||||
|
})
|
||||||
|
{ clientMutationId }
|
||||||
|
}`,
|
||||||
|
{ node_id: pull_request.node_id, sha: pull_request.head.sha },
|
||||||
|
)
|
||||||
|
return 'Enabled Auto Merge'
|
||||||
|
} catch (e) {
|
||||||
|
log('Auto Merge failed', e.response.errors[0].message)
|
||||||
throw new Error(e.response.errors[0].message)
|
throw new Error(e.response.errors[0].message)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue