node-core-utils icon indicating copy to clipboard operation
node-core-utils copied to clipboard

Commit-queue should automatically handle PRs with merge commits from the target branch

Open pimterry opened this issue 5 months ago • 4 comments

Some contributors merge into their PR branch from main rather than rebasing. We might not recommend this, but it shouldn't be a major issue. Unfortunately it seems that the commit-queue label can't handle this, and fails with:

error: commit ... is a merge but no -m option was given.
fatal: cherry-pick failed

Here's a example I just ran into: https://github.com/nodejs/node/pull/59375#issuecomment-3178324429. More examples here.

AFAICT this then always requires manually landing the PR, but it feels like we should be able to handle these automatically without much trouble.

pimterry avatar Aug 12 '25 09:08 pimterry

it feels like we should be able to handle these automatically without much trouble.

It would require changing how to pick up commits quite a lot though, and would only work for --fixupAll/commit-queue-squash landing sessions. Currently we download the patches from https://github.com/nodejs/node/pulls/${PR_ID}.patch, but in order to support merge commits, we'd probably have to fetch to actual PR head branch

aduh95 avatar Aug 12 '25 10:08 aduh95

I thought merge commits weren't supported by the Jenkins CI workflows anyway but it looks like it wasn't an issue for that PR

targos avatar Aug 12 '25 10:08 targos

I thought merge commits weren't supported by the Jenkins CI workflows anyway but it looks like it wasn't an issue for that PR

Maybe a side effect of having COMMIT_SHA_CHECK is that the CI now supports merge commits?

aduh95 avatar Aug 12 '25 10:08 aduh95

I thought merge commits weren't supported by the Jenkins CI workflows anyway but it looks like it wasn't an issue for that PR

FWIW the Jenkins CI doesn't explicitly block merge commits but it does attempt to rebase PRs onto the base branch (e.g. main) and it's that rebase that more often than not fails for merge commits.

richardlau avatar Aug 12 '25 11:08 richardlau