From 5bc506147c08ac257d52614b867c8049a78f5e25 Mon Sep 17 00:00:00 2001 From: David Spickett Date: Mon, 20 Jan 2025 11:08:25 +0000 Subject: [PATCH 1/3] [GitHub] Add workflow to check author's commit access on new PRs This workflow will run on every opened PR and add a label if the author does not have the required permissions to merge their own PR. The permission check is based on code from https://github.com/llvm/llvm-project/pull/81142, which tried to do this when a review was approved. That had to be reverted in https://github.com/llvm/llvm-project/pull/81722 because the event that it was triggered by did not have permissions to write to the PR. So we have a slight disadvantage that a label could be wrong by the time the review is approved but ok, the author can click the button themselves then anyway. Plus, you could search by the label to find anything waiting for someone to merge on behalf. --- .github/workflows/new-prs.yml | 28 ++++++++++++++++++++++ llvm/utils/git/github-automation.py | 36 +++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+) diff --git a/.github/workflows/new-prs.yml b/.github/workflows/new-prs.yml index 88175d6f8d64d..344976b6be92c 100644 --- a/.github/workflows/new-prs.yml +++ b/.github/workflows/new-prs.yml @@ -73,3 +73,31 @@ jobs: # workaround for https://github.com/actions/labeler/issues/112 sync-labels: '' repo-token: ${{ secrets.ISSUE_SUBSCRIBER_TOKEN }} + + check-commit-access: + runs-on: ubuntu-latest + permissions: + pull-requests: write + if: >- + (github.repository == 'llvm/llvm-project') && + (github.event.action == 'opened') + steps: + - name: Checkout Automation Script + uses: actions/checkout@v4 + with: + sparse-checkout: llvm/utils/git/ + ref: main + + - name: Setup Automation Script + working-directory: ./llvm/utils/git/ + run: | + pip install --require-hashes -r requirements.txt + + - name: Check Commit Access + working-directory: ./llvm/utils/git/ + run: | + python3 ./github-automation.py \ + --token '${{ secrets.GITHUB_TOKEN }}' \ + check-commit-access \ + --issue-number "${{ github.event.pull_request.number }}" \ + --author "${{ github.event.pull_request.user.login }}" \ No newline at end of file diff --git a/llvm/utils/git/github-automation.py b/llvm/utils/git/github-automation.py index da467f46b4dd3..43d81e644e202 100755 --- a/llvm/utils/git/github-automation.py +++ b/llvm/utils/git/github-automation.py @@ -290,6 +290,33 @@ def run(self) -> bool: return True +class CheckCommitAccess: + def __init__( + self, token: str, repo: str, pr_number: int, author: str): + self.repo = github.Github(token).get_repo(repo) + self.pr = self.repo.get_issue(pr_number).as_pull_request() + self.author = author + + def can_merge(self, user: str) -> bool: + try: + return self.repo.get_collaborator_permission(user) in ["admin", "write"] + # There is a UnknownObjectException for this scenario, but this method + # does not use it. + except github.GithubException as e: + # 404 means the author was not found in the collaborator list, so we + # know they don't have push permissions. Anything else is a real API + # issue, raise it so it is visible. + if e.status != 404: + raise e + return False + + def run(self) -> bool: + if not self.can_merge(self.author): + self.pr.as_issue().add_to_labels("no-commit-access") + + return True + + def setup_llvmbot_git(git_dir="."): """ Configure the git repo in `git_dir` with the llvmbot account so @@ -680,6 +707,10 @@ def request_release_note(token: str, repo_name: str, pr_number: int): pr_buildbot_information_parser.add_argument("--issue-number", type=int, required=True) pr_buildbot_information_parser.add_argument("--author", type=str, required=True) +check_commit_access_parser = subparsers.add_parser("check-commit-access") +check_commit_access_parser.add_argument("--issue-number", type=int, required=True) +check_commit_access_parser.add_argument("--author", type=str, required=True) + release_workflow_parser = subparsers.add_parser("release-workflow") release_workflow_parser.add_argument( "--llvm-project-dir", @@ -751,6 +782,11 @@ def request_release_note(token: str, repo_name: str, pr_number: int): args.token, args.repo, args.issue_number, args.author ) pr_buildbot_information.run() +elif args.command == "check-commit-access": + check_commit_access = CheckCommitAccess( + args.token, args.repo, args.issue_number, args.author + ) + check_commit_access.run() elif args.command == "release-workflow": release_workflow = ReleaseWorkflow( args.token, From 61c0baac58fe128db7ee3b95dad190b977bee0bd Mon Sep 17 00:00:00 2001 From: David Spickett Date: Mon, 20 Jan 2025 11:45:50 +0000 Subject: [PATCH 2/3] newline --- .github/workflows/new-prs.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/new-prs.yml b/.github/workflows/new-prs.yml index 344976b6be92c..bd8fe91c1e8ec 100644 --- a/.github/workflows/new-prs.yml +++ b/.github/workflows/new-prs.yml @@ -100,4 +100,4 @@ jobs: --token '${{ secrets.GITHUB_TOKEN }}' \ check-commit-access \ --issue-number "${{ github.event.pull_request.number }}" \ - --author "${{ github.event.pull_request.user.login }}" \ No newline at end of file + --author "${{ github.event.pull_request.user.login }}" From 7bd01033c77d52053678ad37e4189d98856c4a6d Mon Sep 17 00:00:00 2001 From: David Spickett Date: Mon, 20 Jan 2025 11:55:34 +0000 Subject: [PATCH 3/3] format --- llvm/utils/git/github-automation.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/llvm/utils/git/github-automation.py b/llvm/utils/git/github-automation.py index 43d81e644e202..5489b19a2eac3 100755 --- a/llvm/utils/git/github-automation.py +++ b/llvm/utils/git/github-automation.py @@ -291,8 +291,7 @@ def run(self) -> bool: class CheckCommitAccess: - def __init__( - self, token: str, repo: str, pr_number: int, author: str): + def __init__(self, token: str, repo: str, pr_number: int, author: str): self.repo = github.Github(token).get_repo(repo) self.pr = self.repo.get_issue(pr_number).as_pull_request() self.author = author