uros-b opened a new pull request, #57764:
URL: https://github.com/apache/spark/pull/57764
### What changes were proposed in this pull request?
Make `dev/merge_spark_pr.py` able to backport a PR that was merged into a
non-default branch, and default its cherry-pick prompt to a branch that does
not already have the change.
Backport mode is entered when the script finds the commit that merged an
already-closed PR. That lookup previously read the merge commit off the PR's
`closed` event:
```python
merge_commits = [e for e in pr_events if e["event"] == "closed" and
e["commit_id"] is not None]
```
GitHub only attributes a commit to the `closed` event when that commit lands
on the **default branch**, because the `Closes #N` keyword in the commit
message is what closes the PR and the keyword is honored only there. A PR
merged into any other branch -- e.g. one opened against a rolling `branch-M.x`
-- is instead closed by this script through the API (`close_pr`), and that
`closed` event carries no commit. The merge then survives only as a
`referenced` event, so `merge_commits` came up empty, backport mode never
engaged, and the script fell through to the normal merge path -- offering to
merge the PR a second time.
This adds two helpers:
- `find_merge_commit(pr_num, pr_events)` prefers the `closed` event's commit
(unchanged behavior, and GitHub's own authoritative link), and only when that
is absent falls back to `referenced` events. Since a `referenced` event is
raised by any commit merely mentioning the PR, each fallback candidate is
confirmed against the `Closes #N from ` line that `merge_pr` writes into every
merge commit.
- `default_pick_branch(branch_names, already_picked)` returns the
highest-ranked release branch that has not already received the change.
Backport mode previously defaulted the prompt to `branch_names[0]`, which for a
PR merged into `branch-M.x` is that very branch: accepting the default asked
git to cherry-pick a commit onto the branch that already had it, which fails
with `The previous cherry-pick is now empty` and lands the committer in the
"Would you like to manually fix-up this merge?" prompt. The normal merge path
already excluded `target_ref` from its defaults, so this consolidates both call
sites onto the shared helper.
### Why are the changes needed?
A PR opened against `branch-4.x` and merged there cannot currently be
backported to `branch-4.3` with the merge script at all -- the script's own
API-close erases the trail that its backport mode depends on. Concretely, for
#57713 (merged into `branch-4.x` as `881e5a94a15`), re-running the script
prints:
```
Start to merge pull request #57713
Pull request 57713 is not mergeable in its current form.
Continue? (experts only!) (y/N):
```
Answering `y` there would create a duplicate squash commit on `branch-4.x`;
the cherry-pick prompt is never reached. The only recourse was a manual `git
cherry-pick -sx` outside the tool, which also skips the merge comment the
script would post. With this change the same invocation reaches:
```
Pull request 57713 has already been merged, assuming you want to backport
Found commit 881e5a94a15...
Enter a branch name [branch-4.3]:
```
### Does this PR introduce _any_ user-facing change?
No. `dev/merge_spark_pr.py` is a committer tool and is not part of any
released artifact.
### How was this patch tested?
- Added doctests for both new helpers; the script runs `doctest.testmod()`
on startup. The suite goes from 68 to 76 passing tests, 0 failures.
- Replayed the real GitHub event payloads for three PRs with `get_json`
stubbed to read genuine commit messages from a local clone (no network),
asserting the resolved merge commit:
| PR | Base | closed / referenced events | Resolved | |
|---|---|---|---|---|
| #57713 | `branch-4.x` | 0 / 1 | `881e5a94a15` | fixed by this PR |
| #57696 | `master` | 1 / 2 | `f0e2b19b82d` | unchanged |
| #57682 | `master` | 1 / 2 | `bf57ee78058` | unchanged |
- Verified the fallback rejects false positives: #57696's `referenced` list
also contains `881e5a94a15` (#57713's commit, which mentions it). With #57696's
`closed` commit stripped to force the fallback, the marker check still resolves
to `f0e2b19b82d`.
- Verified a PR closed without merging yields `(None, None)`, so it does not
wrongly enter backport mode.
- Verified the merge-path refactor is behavior-preserving by transcribing
the previous `remaining_branches` logic and diffing its prompt-default sequence
against `default_pick_branch` across five multi-pick scenarios, including the
Upstream-First two-branch path and a mid-list `target_ref`; all sequences match.
- Verified passing `already_picked=(target_ref,)` in backport mode leaves
the Upstream-First policy prompt unchanged by comparing
`_upstream_first_sibling` under the old and new arguments for every (target,
pick) pair.
Note: `ruff` could not be run in the authoring environment (no PyPI access);
formatting follows the surrounding conventions in the file and is left to CI's
lint job to confirm.
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Opus 5)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
