cloud-fan commented on code in PR #57764:
URL: https://github.com/apache/spark/pull/57764#discussion_r3713657299
##########
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:
Backport mode exits after one pick, so backporting the same PR to another
maintenance branch requires rerunning the script. On that next run, this resets
`already_picked` to only `target_ref`, and the default is the branch just
picked, producing the same empty cherry-pick this change aims to avoid. Please
derive the already-picked set from release branches containing `merge_hash` (as
`dev/pr_merge_status.py` does) before selecting the default.
##########
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:
The fallback must distinguish the generated merge trailer from identical
text in the copied PR body. `merge_pr` preserves body line starts in its `-m`
paragraph, so an unrelated referenced commit whose body contains `Closes #<n>
from ...` satisfies this regex and can be backported as the merge. Please
validate the full generated footer structure and cover this case with a
regression test.
--
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]
