[llvm] [GitHub] Add workflows to manage merging of PRs from authors without commit access (PR #124910)
David Spickett via llvm-commits
llvm-commits at lists.llvm.org
Thu Feb 13 07:56:11 PST 2025
https://github.com/DavidSpickett updated https://github.com/llvm/llvm-project/pull/124910
>From 6f2f61a85f4d0f1158313a38ced72386cb5d677b Mon Sep 17 00:00:00 2001
From: David Spickett <david.spickett at linaro.org>
Date: Mon, 20 Jan 2025 11:08:25 +0000
Subject: [PATCH] [GitHub] Add workflows to manage merging of PRs from authors
without commit access
These changes aim to make contributors and reviewers aware when a PR
should be merged on the contributor's behalf.
It happens in two parts:
* Newly opened PRs will be labelled with "no-commit-access" if the
author does not have commit access.
* A new workflow will periodically check all open PRs with this
label to see if they have approvals, and all checks have finished
(not passed, just finished, some failures can be explained).
* If they do, it will remove the label and add a comment:
* Instructing the author to make it merge ready, if needed.
* Instructing approvers to merge it themselves, or to find
someone who can.
**Note:** This process could be simplified if we were able to write to the
repository in response to the event generated when an approval is given.
Due to security restrictions in GitHub, we cannot do this, see:
https://github.com/orgs/community/discussions/26651
https://github.com/orgs/community/discussions/55940
Instead, we run the new workflow periodically.
Checking Check status is done using PyGitHub's version of:
https://docs.github.com/en/rest/checks/runs?apiVersion=2022-11-28#list-check-runs-for-a-git-reference
There are some potential corner cases here, but the idea is that
this is enough to make both author and reviewers aware that
merge on behalf is an option. From there, I hope that they can
communicate directly on the PR.
If this does not happen in practice, we can revisit this and
add more automation where it makes sense.
---
.github/workflows/check-prs-need-merge.yml | 37 ++++++
.github/workflows/new-prs.yml | 28 +++++
llvm/utils/git/github-automation.py | 135 +++++++++++++++++++++
3 files changed, 200 insertions(+)
create mode 100644 .github/workflows/check-prs-need-merge.yml
diff --git a/.github/workflows/check-prs-need-merge.yml b/.github/workflows/check-prs-need-merge.yml
new file mode 100644
index 0000000000000..1b313976a5c10
--- /dev/null
+++ b/.github/workflows/check-prs-need-merge.yml
@@ -0,0 +1,37 @@
+name: "Find PRs That Need Merging on the Author's Behalf"
+
+permissions:
+ contents: read
+
+on:
+ schedule:
+ # * is a special character in YAML so you have to quote this string
+ # Run once an hour
+ - cron: '5 * * * *'
+
+jobs:
+ check_needs_merge:
+ runs-on: ubuntu-latest
+ permissions:
+ # May change labels and add a comment.
+ pull-requests: write
+ if: >-
+ (github.repository == 'llvm/llvm-project')
+ steps:
+ - name: Checkout Automation Script
+ uses: actions/checkout at 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 Open PRs
+ working-directory: ./llvm/utils/git/
+ run: |
+ python3 ./github-automation.py \
+ --token '${{ secrets.GITHUB_TOKEN }}' \
+ check-prs-need-merge
diff --git a/.github/workflows/new-prs.yml b/.github/workflows/new-prs.yml
index 88175d6f8d64d..bd8fe91c1e8ec 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 at 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 }}"
diff --git a/llvm/utils/git/github-automation.py b/llvm/utils/git/github-automation.py
index 6978da51ac645..4fc5aa31f0537 100755
--- a/llvm/utils/git/github-automation.py
+++ b/llvm/utils/git/github-automation.py
@@ -353,6 +353,127 @@ def run(self) -> bool:
return True
+"""
+CheckCommitAccess and CheckPRsNeedMerge work together to implement a flow that
+notifies approvers of PRs from authors without commit access, when an approved
+PR is ready to merge. The steps are as follows:
+
+* Newly opened PRs from users without commit access are labelled to indicate that
+ fact.
+* Periodically, PRs with this label are checked.
+* If they have approvals and all checks have finished, the author is prompted
+ to check that the PR is merge ready and the approvers are prompted to merge
+ it on their behalf if it is in a mergeable state.
+* The label is removed from the PR.
+
+If we were able to write to the repo in response to a pull_request_review event,
+we could run the second part in response to the review submitted event.
+We cannot, so the second part runs on a timer instead. See:
+https://github.com/orgs/community/discussions/26651
+https://github.com/orgs/community/discussions/55940
+
+"""
+
+
+def user_can_merge(user: str, repo):
+ try:
+ return 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
+
+
+NO_COMMIT_ACCESS_LABEL = "no-commit-access"
+
+
+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 run(self) -> bool:
+ if not user_can_merge(self.author, self.repo):
+ self.pr.as_issue().add_to_labels(NO_COMMIT_ACCESS_LABEL)
+
+ return True
+
+
+class CheckPRsNeedMerge:
+ PR_READY_COMMENT = "! This PR is ready to merge !"
+
+ def __init__(self, token: str, repo: str):
+ self.repo = github.Github(token).get_repo(repo)
+
+ @staticmethod
+ def at_users(users):
+ return ", ".join([f"@{user.login}" for user in users])
+
+ def run(self) -> bool:
+ # "Either open, closed, or all to filter by state." - no "approved"
+ # unfortunately.
+ for pull in self.repo.get_pulls(state="open"):
+ for label in pull.as_issue().get_labels():
+ if label.name == NO_COMMIT_ACCESS_LABEL:
+ break
+ else:
+ # PR is from someone with commit access.
+ continue
+
+ approvers = [r.user for r in pull.get_reviews() if r.state == "APPROVED"]
+ if not approvers:
+ # Can't do anything until there is at least one approval.
+ continue
+
+ # Wait for checks to finish before commenting.
+ found_check_in_progress = False
+ for commit in pull.get_commits():
+ for run in commit.get_check_runs():
+ if run.status in ["queued", "in_progress"]:
+ found_check_in_progress = True
+ break
+
+ if found_check_in_progress:
+ continue
+
+ # Even approvers may not have commit access.
+ can_merge = [a for a in approvers if user_can_merge(a, self.repo)]
+
+ if not can_merge:
+ to_approvers = f"please find someone who can merge this PR on behalf of {self.at_users([pull.user])}"
+ elif len(can_merge) == 1:
+ to_approvers = (
+ f"please merge this PR on behalf of {self.at_users([pull.user])}"
+ )
+ else:
+ to_approvers = f"one of you should merge this PR on behalf of {self.at_users([pull.user])}"
+
+ mergers = can_merge if can_merge else approvers
+
+ pull.as_issue().create_comment(
+ f"""\
+{self.at_users([pull.user])} please ensure that this PR is ready to be merged. Make sure that:
+* The PR title and description describe the final changes. These will be used as the title and message of the final squashed commit. The titles and messages of commits in the PR will **not** be used.
+* You have set a valid [email address](https://llvm.org/docs/DeveloperPolicy.html#github-email-address) in your GitHub account. This will be associated with this contribution.
+
+{self.at_users(mergers)}, check that:
+* The above items have been completed.
+* All checks have passed, or their failure has been adequately explained.
+
+If so, {to_approvers}."""
+ )
+
+ pull.as_issue().remove_from_labels(NO_COMMIT_ACCESS_LABEL)
+
+ return True
+
+
def setup_llvmbot_git(git_dir="."):
"""
Configure the git repo in `git_dir` with the llvmbot account so
@@ -746,6 +867,12 @@ 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)
+
+check_pr_needs_merge_parser = subparsers.add_parser("check-prs-need-merge")
+
release_workflow_parser = subparsers.add_parser("release-workflow")
release_workflow_parser.add_argument(
"--llvm-project-dir",
@@ -820,6 +947,14 @@ 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 == "check-prs-need-merge":
+ check_needs_merge = CheckPRsNeedMerge(args.token, args.repo)
+ check_needs_merge.run()
elif args.command == "release-workflow":
release_workflow = ReleaseWorkflow(
args.token,
More information about the llvm-commits
mailing list