viirya commented on code in PR #56058:
URL: https://github.com/apache/spark/pull/56058#discussion_r3289106183
##########
dev/merge_spark_pr.py:
##########
@@ -506,6 +503,69 @@ def cherry_pick(pr_num, merge_hash, default_branch):
return pick_ref
+def cherry_pick(pr_num, merge_hash, default_branch, branch_names,
already_picked=()):
+ """Prompt for a target branch and cherry-pick `merge_hash` onto it.
+
+ Enforces the Upstream-First policy (see header comment): when the
committer types a
+ branch-M.N target while branch-M.x is also a known release branch AND has
not already
+ received this commit, prompt to confirm whether to pick into BOTH (the
policy-compliant
+ default) or branch-M.N only (treated as a maintenance-only bugfix).
Returns the list of
+ refs actually picked into, so the main loop can advance its
remaining-branches list
+ correctly.
+ """
+ pick_ref = bold_input("Enter a branch name [%s]: " % default_branch)
+ if pick_ref == "":
+ pick_ref = default_branch
+
+ # Upstream-First check: if the committer typed branch-M.N and branch-M.x
exists as a
+ # sibling release branch that has not yet received this commit, surface
the policy and
+ # offer to pick both in one step. Skipping when sibling_x is the merge
target or already
+ # in already_picked avoids a redundant prompt / failing re-cherry-pick.
+ sibling_x = None
+ m = re.match(r"^branch-(\d+)\.(\d+)$", pick_ref)
Review Comment:
Agreed — the previous gating was incidental (the `already_picked` set
happened to cover the branch-M.x merge target only because we seeded it with
`target_ref`). Switched to explicit `target_ref == "master"` gating as you
suggested. Plumbed `target_ref` into `cherry_pick` and into the new
`_upstream_first_sibling` helper so the check is self-contained.
Addressed in the follow-up commit.
##########
dev/merge_spark_pr.py:
##########
@@ -506,6 +503,69 @@ def cherry_pick(pr_num, merge_hash, default_branch):
return pick_ref
+def cherry_pick(pr_num, merge_hash, default_branch, branch_names,
already_picked=()):
+ """Prompt for a target branch and cherry-pick `merge_hash` onto it.
+
+ Enforces the Upstream-First policy (see header comment): when the
committer types a
+ branch-M.N target while branch-M.x is also a known release branch AND has
not already
+ received this commit, prompt to confirm whether to pick into BOTH (the
policy-compliant
+ default) or branch-M.N only (treated as a maintenance-only bugfix).
Returns the list of
+ refs actually picked into, so the main loop can advance its
remaining-branches list
+ correctly.
+ """
+ pick_ref = bold_input("Enter a branch name [%s]: " % default_branch)
+ if pick_ref == "":
+ pick_ref = default_branch
+
+ # Upstream-First check: if the committer typed branch-M.N and branch-M.x
exists as a
+ # sibling release branch that has not yet received this commit, surface
the policy and
+ # offer to pick both in one step. Skipping when sibling_x is the merge
target or already
+ # in already_picked avoids a redundant prompt / failing re-cherry-pick.
+ sibling_x = None
+ m = re.match(r"^branch-(\d+)\.(\d+)$", pick_ref)
+ if m:
+ candidate = "branch-%s.x" % m.group(1)
+ if (
+ candidate in branch_names
+ and candidate != pick_ref
+ and candidate not in already_picked
+ ):
Review Comment:
Good catch — removed. The regex already pins `pick_ref` to `branch-M.N` with
digit minor, so `candidate = branch-M.x` can never equal `pick_ref`. Dead check.
--
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]