uros-b commented on code in PR #57764:
URL: https://github.com/apache/spark/pull/57764#discussion_r3714350854
##########
dev/merge_spark_pr.py:
##########
@@ -615,6 +705,72 @@ def _do_cherry_pick(pr_num, merge_hash, pick_ref):
return pick_ref, pick_hash
+def branches_with_merge_footer(pr_num, branch_names):
+ """Release branches from `branch_names` that already carry `pr_num`'s
merge footer.
+
+ A cherry-pick is a new commit, so `git branch --contains <merge_hash>`
finds only the
+ branch the change was merged into; what identifies a backport is the
footer, which
+ `cherry-pick -x` copies verbatim (the same signal `dev/pr_merge_status.py`
reads).
+ Best-effort: this only sees branches already fetched into
PUSH_REMOTE_NAME's tracking
+ refs, so a backport pushed from elsewhere and not yet fetched is simply
not reported --
+ the committer is still prompted and can type any branch.
+ """
+ trailer = "Closes #%s from " % pr_num
+ try:
+ out = run_cmd(
+ [
+ "git",
+ "log",
+ "--remotes=%s" % PUSH_REMOTE_NAME,
+ "--fixed-strings",
+ "--grep",
+ trailer,
+ "--format=%H",
+ ]
+ )
+ except Exception as e:
+ print_error("Could not scan for existing backports of #%s (%s)." %
(pr_num, e))
+ return []
+
+ landed = set()
+ prefix = "%s/" % PUSH_REMOTE_NAME
+ for commit_hash in out.split():
+ refs = run_cmd(
+ [
+ "git",
+ "for-each-ref",
+ "--contains",
+ commit_hash,
+ "--format=%(refname:short)",
+ "refs/remotes/%s/" % PUSH_REMOTE_NAME,
+ ]
+ )
+ for ref in refs.splitlines():
+ if ref.startswith(prefix):
+ landed.add(ref[len(prefix) :])
+ # Keep branch_names' newest-first order, and drop anything not a known
release branch.
+ return [b for b in branch_names if b in landed]
+
+
+def default_pick_branch(branch_names, already_picked):
+ """Highest-ranked release branch that has not already received the change.
+
+ `branch_names` is ordered newest-first (see `semver_branch_rank`) and
`already_picked`
+ holds the branches the change is known to be on, so the prompt never
defaults to a
+ branch where the cherry-pick would come up empty. Falls back to the newest
branch when
+ every known branch is accounted for, leaving the committer to type a
target.
+
+ >>> default_pick_branch(["branch-4.x", "branch-4.3", "branch-4.2"],
("branch-4.x",))
+ 'branch-4.3'
+ >>> default_pick_branch(["branch-4.x", "branch-4.3"], ())
+ 'branch-4.x'
+ >>> default_pick_branch(["branch-4.x"], ("branch-4.x",))
+ 'branch-4.x'
+ """
+ remaining = [b for b in branch_names if b not in already_picked]
+ return remaining[0] if remaining else branch_names[0]
Review Comment:
Agreed -- and the consequence is worse in backport mode than in the merge
path, because backport mode performs the first `cherry_pick` before asking
whether another is wanted, so a rerun after every branch has the change offered
an empty cherry-pick immediately with no prompt to decline first.
Fixed in 0853820: `default_pick_branch` now returns `None` when no branch
remains, and both call sites (backport mode and the merge path's pick loop)
report it and stop rather than defaulting to an already-picked branch:
```
Every known release branch already contains #57713; nothing to pick.
```
```
>>> default_pick_branch(["branch-4.x"], ("branch-4.x",)) is None
True
```
I also re-checked that returning `None` did not disturb the merge path: its
prompt-default sequence is unchanged across five multi-pick scenarios
(including the Upstream-First two-branch path and a mid-list `target_ref`), and
`_upstream_first_sibling` is identical for every (target, pick) pair.
--
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]
