From cbc25fec328fb6a457422e4edfbdb81c3ca6b6ab Mon Sep 17 00:00:00 2001 From: Eli Schwartz Date: Wed, 23 Aug 2023 00:39:59 -0400 Subject: [PATCH 1/2] update patch generation to directly use the merged commit shas Make use of cherry-pick + format-patch to generate the patches, and in particular, use the -x flag to produce a paper trail of how these commits were selected. This makes it easy to look at a backported commit and see where it came from and what PR it was associated with. --- milestone-patches.py | 33 +++++++++++++++++++++++++++------ 1 file changed, 27 insertions(+), 6 deletions(-) diff --git a/milestone-patches.py b/milestone-patches.py index c49fcd2..ccfbd59 100755 --- a/milestone-patches.py +++ b/milestone-patches.py @@ -11,11 +11,13 @@ # are not handled correctly. Maybe we should fetch SHAs from the repository # instead and use `git cherry-pick` instead. It's easier to resolve conflicts # this way. +from __future__ import annotations import os import sys import argparse import requests +import typing as T from pathlib import Path from configparser import ConfigParser @@ -171,6 +173,27 @@ def verify_issue_fixes_are_milestoned(repo, issues, pulls): if len(issues) > 0 and not options.no_verify: verify_issue_fixes_are_milestoned(repo, issues, pulls) +def pr_to_patch(shas: T.List[str]) -> T.Optional[str]: + resp = [] + for sha in reversed(shas): + url = repo.get_commit(sha).html_url + '.patch' + r = requests.get(url) + if r.status_code != 200: + # Print url for manual checking + print(" failed: {} ({})".format(r.status_code, url)) + return + cherrypick = f'--trailer=(cherry picked from commit {sha})=deleteme' + gitfilter = Popen(['git', 'interpret-trailers', cherrypick], stdin=PIPE, stdout=PIPE) + patch = gitfilter.communicate(r.stdout)[0] + patch = patch.decode('utf-8').replace('=deleteme:', '') + resp.append(patch) + resp.append('') + + print(' ok') + return '\n'.join(resp) + '\n' + + + # Fetch and write patches from all PRs for d, issuepr in pulls.items(): # Name the patch @@ -183,12 +206,10 @@ def verify_issue_fixes_are_milestoned(repo, issues, pulls): continue # Fetch the patch if required print("Fetching patch for PR #{} ...".format(issuepr.number), end="", flush=True) - r = requests.get(url) - if r.status_code != 200: - # Print url for manual checking - print(" failed: {} ({})".format(r.status_code, url)) + shas = pr_get_repo_shas(issuepr, repo) + patch = pr_to_patch(shas) + if not patch: continue - print(" ok") print("Writing to {}".format(foname)) with open(foname, 'w') as f: - f.write(r.text) + f.write(patch) From ee7573757ccbe795692b12fcbbdc6c0bc8369a4b Mon Sep 17 00:00:00 2001 From: Eli Schwartz Date: Thu, 24 Jul 2025 13:26:02 -0400 Subject: [PATCH 2/2] port closed-via-PR detection to graphql graphql feels extremely unpleasant to work with. But this info is available there, and never has been for the REST api (why???) as evidenced by the reliance on a crude hack, namely scanning events for a commit reference "in the right vicinity". And github stopped generating those reference events at all, so... --- milestone-patches.py | 45 ++++++++++++++++++++++++-------------------- 1 file changed, 25 insertions(+), 20 deletions(-) diff --git a/milestone-patches.py b/milestone-patches.py index ccfbd59..3cd73cb 100755 --- a/milestone-patches.py +++ b/milestone-patches.py @@ -53,26 +53,31 @@ def issue_get_closing_sha(issue): Then, if it's the one made immediately after the issue was closed and also points to our repository, it's probably the right one. ''' - events = iter(issue.get_events()) - for e in events: - # Find the event in which the issue was closed - if e.event != 'closed': - continue - # If the event closing the issue was not a commit, it was closed via PR - # and the immediate next event is most likely the commit that closed it - if not e.commit_id: - try: - e = next(events) - except StopIteration: - print('Issue {} was closed, but could not find associated PR'.format(issue.number)) - return None - if not e.commit_id or not e.commit_url: - print('Issue {} was closed but no commit was associated!?'.format(issue.number)) - return None - if not e.commit_url.startswith('https://api.github.com/repos/mesonbuild/meson/commits/'): - raise AssertionError('Closing event for issue {} has a url to a different repo: {!r}'.format(issue.number, e.commit_url)) - return e.commit_id - raise AssertionError('Issue {} is not closed, double-check'.format(issue.number)) + q = ''' + query($issue: Int!) + { + repository(owner: "mesonbuild", name: "meson") { + issue(number: $issue) { + closedByPullRequestsReferences(includeClosedPrs:true, first:10) { + nodes { + permalink + mergeCommit { oid } + } + } + } + } + }''' + resp = issue.requester.graphql_query(q, {'issue': issue.number}) + for i in resp[1]['data']['repository']['issue']['closedByPullRequestsReferences']['nodes']: + if not i['permalink'].startswith('https://github.com/mesonbuild/meson/pull/'): + raise AssertionError('Closing PR for issue {} has a url to a different repo: {!r}'.format(issue.number, i['permalink'])) + return i['mergeCommit']['oid'] + + if issue.state != 'closed': + raise AssertionError('Issue {} is not closed, double-check'.format(issue.number)) + + print('Issue {} was closed, but could not find associated PR'.format(issue.number)) + return None def pr_get_merging_sha(issuepr): '''