uros-b commented on code in PR #57764:
URL: https://github.com/apache/spark/pull/57764#discussion_r3713765574
##########
dev/merge_spark_pr.py:
##########
@@ -1683,9 +1753,11 @@ def main():
fail("Couldn't find any merge commit for #%s, you may need to
update HEAD." % pr_num)
print("Found commit %s:\n%s" % (merge_hash, message))
- default = branch_names[0]
+ # The change is already on target_ref, so default to the next branch
down and mark
+ # target_ref as picked: defaulting to it would cherry-pick an empty
commit.
+ default = default_pick_branch(branch_names, (target_ref,))
Review Comment:
Agreed on the bug -- backport mode exits after one pick, and the next
invocation rebuilt `already_picked` as only `(target_ref,)`, so the default was
the branch the previous run had just picked into. Fixed in 87c42a3.
One correction on the suggested mechanism, though: deriving the set from
*branches containing `merge_hash`* does not work, because a cherry-pick is a
new commit. On the current repo:
```
$ git for-each-ref --contains f0e2b19b82d --format='%(refname:short)'
refs/remotes/upstream/
upstream/master
```
That is #57696's master merge; its `branch-4.x` pick (`881e5a94a15`) is not
reported, so the set would come back as just `target_ref` and nothing would
change. `dev/pr_merge_status.py` does not use `--contains` on the merge hash
either -- it greps for the `Closes #<n> from ` **trailer**, which `cherry-pick
-x` copies verbatim into the picked commit, then maps those commits to
branches. Confirmed on a real pick, `9f027a981f2` on `branch-4.x`:
```
23: Closes #57745 from zhengruifeng/single-lookup-string-indexer.
27: (cherry picked from commit 1f2b47d4884...)
```
So `branches_with_merge_footer` follows that approach. Verified against real
repo state:
| PR | state | detected backports | prompt default |
|---|---|---|---|
| #57745 | merged master, picked to `branch-4.x` | `branch-4.x` |
`branch-4.3` |
| #57713 | merged `branch-4.x` only | `branch-4.x` | `branch-4.3` |
| #57696 | merged master only | none | `branch-4.x` |
And across successive runs on a PR merged into `branch-4.x`: run 1 defaults
to `branch-4.3`, run 2 to `branch-4.2`, run 3 to `branch-4.1` (previously every
run said `branch-4.3`).
Backport mode also gained the same "pick into another branch?" loop the
normal merge path has, so one invocation can reach several maintenance branches
rather than requiring a rerun per branch.
One limitation, documented in the docstring: this reads only the tracking
refs already fetched for `PUSH_REMOTE_NAME`, so a backport pushed elsewhere and
not yet fetched locally is not detected. It degrades safely -- the committer is
still prompted and can type any branch -- and I opted against adding a fetch
here to keep the script from doing unexpected network work mid-merge. Happy to
add one if you would prefer that.
--
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]
