Skip to content

Commit 1892b37

Browse files
committed
Reland "[GitHub][workflows] Ask reviewers to merge PRs when author cannot (llvm#81142)"
This reverts commit 124cd11. The job originally failed because any workflow run on a PR runs in a context that cannot write to the PR itself (otherwise people could damage the repo using a workflow in a PR). llvm#80495 recently added a job that is purely for commenting on issues, to solve a similair problem. This limits the damage the PR can do to adding a spam comment.
1 parent 163301d commit 1892b37

File tree

3 files changed

+121
-0
lines changed

3 files changed

+121
-0
lines changed

.github/workflows/approved-prs.yml

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
name: "Prompt reviewers to merge PRs on behalf of authors"
2+
3+
permissions:
4+
contents: read
5+
6+
on:
7+
pull_request_review:
8+
types:
9+
- submitted
10+
11+
jobs:
12+
merge-on-behalf-information-comment:
13+
runs-on: ubuntu-latest
14+
permissions:
15+
pull-requests: write
16+
if: >-
17+
(github.repository == 'llvm/llvm-project') &&
18+
(github.event.review.state == 'APPROVED')
19+
steps:
20+
- name: Checkout Automation Script
21+
uses: actions/checkout@v4
22+
with:
23+
sparse-checkout: llvm/utils/git/
24+
ref: main
25+
26+
- name: Setup Automation Script
27+
working-directory: ./llvm/utils/git/
28+
run: |
29+
pip install -r requirements.txt
30+
31+
- name: Generate Merge On Behalf Comment
32+
working-directory: ./llvm/utils/git/
33+
run: |
34+
# Writes to a file "comments"
35+
python3 ./github-automation.py \
36+
--token '${{ secrets.GITHUB_TOKEN }}' \
37+
pr-merge-on-behalf-information \
38+
--issue-number "${{ github.event.pull_request.number }}" \
39+
--author "${{ github.event.pull_request.user.login }}" \
40+
--reviewer "${{ github.event.review.user.login }}"
41+
42+
- uses: actions/upload-artifact@26f96dfa697d77e81fd5907df203aa23a56210a8 #v4.3.0
43+
if: always()
44+
working-directory: ./llvm/utils/git/
45+
with:
46+
name: Upload Comments File
47+
path: |
48+
comments
49+

.github/workflows/issue-write.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ on:
55
workflows:
66
- "Check code formatting"
77
- "Check for private emails used in PRs"
8+
- "Prompt reviewers to merge PRs on behalf of authors"
89
types:
910
- completed
1011

llvm/utils/git/github-automation.py

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
from git import Repo # type: ignore
1313
import html
1414
import github
15+
import json
1516
import os
1617
import re
1718
import requests
@@ -299,6 +300,60 @@ def run(self) -> bool:
299300
return True
300301

301302

303+
class PRMergeOnBehalfInformation:
304+
COMMENT_TAG = "<!--LLVM MERGE ON BEHALF INFORMATION COMMENT-->\n"
305+
306+
def __init__(
307+
self, token: str, repo: str, pr_number: int, author: str, reviewer: str
308+
):
309+
self.repo = github.Github(token).get_repo(repo)
310+
self.pr = self.repo.get_issue(pr_number).as_pull_request()
311+
self.author = author
312+
self.reviewer = reviewer
313+
314+
def can_merge(self, user: str) -> bool:
315+
try:
316+
return self.repo.get_collaborator_permission(user) in ["admin", "write"]
317+
# There is a UnknownObjectException for this scenario, but this method
318+
# does not use it.
319+
except github.GithubException as e:
320+
# 404 means the author was not found in the collaborator list, so we
321+
# know they don't have push permissions. Anything else is a real API
322+
# issue, raise it so it is visible.
323+
if e.status != 404:
324+
raise e
325+
return False
326+
327+
def run(self) -> bool:
328+
# Check this first because it only costs 1 API point.
329+
if self.can_merge(self.author):
330+
return
331+
332+
# A review can be approved more than once, only comment the first time.
333+
for comment in self.pr.as_issue().get_comments():
334+
if self.COMMENT_TAG in comment.body:
335+
return
336+
337+
# This text is using Markdown formatting.
338+
if self.can_merge(self.reviewer):
339+
comment = f"""\
340+
{self.COMMENT_TAG}
341+
@{self.reviewer} the PR author does not have permission to merge their own PRs yet. Please merge on their behalf."""
342+
else:
343+
comment = f"""\
344+
{self.COMMENT_TAG}
345+
@{self.reviewer} the author of this PR does not have permission to merge and neither do you.
346+
Please find someone who has merge permissions who can merge it on the author's behalf. This could be one of the other reviewers or you can ask on [Discord](https://discord.com/invite/xS7Z362)."""
347+
348+
# This command will run on PRs which means the initial workflow can't
349+
# write to the PR. Instead write the comment to a file that will be
350+
# picked up by issue-write.
351+
with open("comments", "w") as f:
352+
json.dump({"body": comment})
353+
354+
return True
355+
356+
302357
def setup_llvmbot_git(git_dir="."):
303358
"""
304359
Configure the git repo in `git_dir` with the llvmbot account so
@@ -664,6 +719,17 @@ def execute_command(self) -> bool:
664719
pr_buildbot_information_parser.add_argument("--issue-number", type=int, required=True)
665720
pr_buildbot_information_parser.add_argument("--author", type=str, required=True)
666721

722+
pr_merge_on_behalf_information_parser = subparsers.add_parser(
723+
"pr-merge-on-behalf-information"
724+
)
725+
pr_merge_on_behalf_information_parser.add_argument(
726+
"--issue-number", type=int, required=True
727+
)
728+
pr_merge_on_behalf_information_parser.add_argument("--author", type=str, required=True)
729+
pr_merge_on_behalf_information_parser.add_argument(
730+
"--reviewer", type=str, required=True
731+
)
732+
667733
release_workflow_parser = subparsers.add_parser("release-workflow")
668734
release_workflow_parser.add_argument(
669735
"--llvm-project-dir",
@@ -723,6 +789,11 @@ def execute_command(self) -> bool:
723789
args.token, args.repo, args.issue_number, args.author
724790
)
725791
pr_buildbot_information.run()
792+
elif args.command == "pr-merge-on-behalf-information":
793+
pr_merge_on_behalf_information = PRMergeOnBehalfInformation(
794+
args.token, args.repo, args.issue_number, args.author, args.reviewer
795+
)
796+
pr_merge_on_behalf_information.run()
726797
elif args.command == "release-workflow":
727798
release_workflow = ReleaseWorkflow(
728799
args.token,

0 commit comments

Comments
 (0)