uros-b commented on code in PR #57764:
URL: https://github.com/apache/spark/pull/57764#discussion_r3714347663
##########
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:
You are right, and my earlier reply was wrong on this point -- I claimed a
PR body "cannot fake the footer structure", but the body is copied verbatim, so
it can quote a complete footer including the authors paragraph. Reproduced
against the previous commit:
```
has_merge_footer(msg, 57713) = True <- #57713 only quoted in the body;
real footer closes #58000
```
Fixed in 0853820 by anchoring on **position** rather than structure.
`merge_pr` appends the footer last, so the generated one is the final `Closes`
paragraph in the message; a quoted footer inside the body is necessarily
followed by the real one. `merge_footer_pr` reads that last paragraph and
compares its number, so identification no longer depends on scanning for a
given PR number anywhere:
```
merge_footer_pr(msg) = 58000
has_merge_footer(msg, 57713) = False
has_merge_footer(msg, 58000) = True
```
Regression cases added as doctests on both `merge_footer_pr` and
`has_merge_footer`, including the complete-quoted-footer case you asked for.
Two things I checked so the stricter rule does not lose recall:
- `cherry-pick -x` provenance (`(cherry picked from commit ...)` plus a
trailing `Signed-off-by:`) follows the footer on every backport, and those
messages still resolve correctly -- covered by a doctest.
- All **592** commits carrying a `Closes` footer in the last 600 on master
are still identified as merges.
--
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]
