cloud-fan commented on code in PR #57764:
URL: https://github.com/apache/spark/pull/57764#discussion_r3717495017
##########
dev/merge_spark_pr.py:
##########
@@ -415,6 +415,59 @@ def get_json(url):
sys.exit(-1)
+def merge_commit_candidates(pr_events):
+ """Split `pr_events` into (closed_commits, referenced_commits), each
oldest-first.
+
+ Ordered by time so that a PR reopened and merged again yields its latest
merge last.
+
+ >>> merge_commit_candidates([{"event": "closed", "commit_id": "a",
"created_at": "t2"},
+ ... {"event": "referenced", "commit_id": "b",
"created_at": "t1"}])
+ (['a'], ['b'])
+ >>> merge_commit_candidates([{"event": "closed", "commit_id": None,
"created_at": "t1"}])
+ ([], [])
+ >>> merge_commit_candidates([{"event": "referenced", "commit_id": "c",
"created_at": "t2"},
+ ... {"event": "referenced", "commit_id": "b",
"created_at": "t1"}])
+ ([], ['b', 'c'])
+ """
+
+ def commits_of(event_name):
+ matched = [e for e in pr_events if e["event"] == event_name and
e["commit_id"] is not None]
+ return [e["commit_id"] for e in sorted(matched, key=lambda x:
x["created_at"])]
+
+ return commits_of("closed"), commits_of("referenced")
+
+
+def find_merge_commit(pr_num, pr_events):
+ """Return (hash, message) of the commit that merged `pr_num`, or (None,
None).
+
+ GitHub attributes the merge commit to the `closed` event only when that
commit lands
+ on the default branch (master), 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, and that `closed` event carries no commit, so
the merge
+ survives only as a `referenced` event. Prefer the `closed` commit, which
GitHub itself
+ linked; otherwise fall back to `referenced` events, confirming each
against the
+ "Closes #N from " line that `merge_pr` writes so that an unrelated commit
merely
+ mentioning the PR is not mistaken for its merge.
+ """
+
+ def message_of(commit_hash):
+ return get_json("%s/commits/%s" % (GITHUB_API_BASE,
commit_hash))["commit"]["message"]
+
+ closed_commits, referenced_commits = merge_commit_candidates(pr_events)
+ if closed_commits:
+ return closed_commits[-1], message_of(closed_commits[-1])
+
+ # Anchored to line start: a PR body quoting "Closes #N from ..." is copied
into the merge
+ # commit message too, and only the script's own trailer sits at the start
of a line.
+ marker = re.compile(r"^Closes #%s from " % pr_num, re.MULTILINE)
Review Comment:
Confirmed: selecting the final merge-footer paragraph addresses the
quoted-footer false positive, including cherry-pick provenance after it.
Resolved.
##########
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:
Confirmed: the validated footer scan now reconstructs previously picked
release branches, and the multi-pick loop carries that state forward. Resolved.
##########
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,
Review Comment:
Confirmed: `--grep` is now only a prefilter and each candidate message is
validated before branch mapping. Resolved.
##########
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:
Confirmed: exhausted branch selection now returns `None`, and both call
sites stop without attempting an empty cherry-pick. Resolved.
--
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]
