[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.

Author: DavidSpickett

.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@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

.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 }}"

llvm/utils/git/github-automation.py

@@ -353,6 +353,141 @@ 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
+
+ # Each reviewer may leave multiple reviews and could change their mind.
+ # Find the most recent review for each user.
+ reviews_by_user = {}
+ for review in pull.get_reviews():
+ if review.user in reviews_by_user:
+ if review.sumitted_at > reviews_by_user[review.user].submitted_at:
+ reviews_by_user[review.user] = review.state
+ else:
+ reviews_by_user[review.user] = review.state
+
+ approvers = []
+ for user, state in reviews_by_user.items():
+ if state == "APPROVED":
+ approvers.append(user)
+
+ 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 +881,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 +961,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,