github: Automatically assign reviewers for backport requests

[tstellar]

Summary

When there is a backport request, the GitHub Action that handles the
backport will now automatically assign the issue to the user(s) who
approved the commit in Phabricator and create an issue comment asking
them to review the request.

[kwk]

Can you please add links to the phabricator HTTP API documentation that you're using?

llvm/utils/git/github-automation.py

62

245

When I use capital L I get: NameError: name 'List' is not defined. Did you mean: 'list'?

267

What if the list of reviewers is empty? Don't you want to skip that call to add_to_assignees?

285

Do you still want to continue when the exception was caught?

392

[thieta]

llvm/utils/git/github-automation.py

245

List is a typing hint. You need to import it from the typing module.

[tstellar]

llvm/utils/git/github-automation.py

245

When I use list[str], I get:

TypeError: 'type' object is not subscriptable

[tstellar]

Re-wrote the patch so that the pull request is assigned to the reviewers rather
than the issue. This seems to make more sense since the reviewers will be reviewing
the code.

[tstellar]

llvm/utils/git/github-automation.py

285

Yes, we want to continue, because assigning reviewers is optional.

[thieta]

LGTM.

[kwk]

llvm/utils/git/github-automation.py

245

I think this is deprecated since Python 3.9 (https://docs.python.org/3/library/typing.html#typing.List). And this whole discussion rings a bell. @tstellar weren't the Github runners running an older version than what was required for list[str] to work?

[tstellar]

llvm/utils/git/github-automation.py

245

@kwk Yes, I believe we ran into the same issue on another patch. I see we use List one other place in the patch

[kwk]

LGTM.

llvm/utils/git/github-automation.py

78

I tried to find limit in the documentation (see section "Paging and Limits") and it is always used like this:

{
   "cursor": {
       "limit": 100
   }
}

I wonder if this only works because of luck that there only is 1 result.

94

Are fields safe to access or is there any guarantee that a field exists in a result? Should fields be tested for existence before accessing them?

[tstellar]

llvm/utils/git/github-automation.py

78

I think that's just the output json? If you put in an example query with the limit parameter, it shows the limit value being passed to curl simply as limit=1.

94

It looks like the fields are always there based on my testing.

[tstellar]

Rebased.

.github/workflows/issue-release-workflow.yml

steps:
       - name: Backport Commits
         run: |
           printf "$COMMENT_BODY" |
           ./llvm/utils/git/github-automation.py \
           --repo $GITHUB_REPOSITORY \
           --token ${{ secrets.RELEASE_WORKFLOW_PUSH_SECRET }} \
           release-workflow \
           --issue-number ${{ github.event.issue.number }} \
+          --phab-token ${{ secrets.RELEASE_WORKFLOW_PHAB_TOKEN }} \
           auto
 
   create-pull-request:
     name: Create Pull Request
     runs-on: ubuntu-20.04
     if: >-
       (github.repository == 'llvm/llvm-project') &&
       !startswith(github.event.comment.body, '<!--IGNORE-->') &&

llvm/utils/git/github-automation.py

 #!/usr/bin/env python3
 #
 # ======- github-automation - LLVM GitHub Automation Routines--*- python -*--==#
 #
 # Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 # See https://llvm.org/LICENSE.txt for license information.
 # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 #
 # ==-------------------------------------------------------------------------==#
 
 import argparse
 from git import Repo # type: ignore
 import github
 import os
 import re
+import requests
 import sys
 from typing import *
 
 class IssueSubscriber:
 
     @property
     def team_name(self) -> str:
         return self._team_name
def setup_llvmbot_git(git_dir = '.'):
     Configure the git repo in `git_dir` with the llvmbot account so
     commits are attributed to llvmbot.
     """
     repo = Repo(git_dir)
     with repo.config_writer() as config:
         config.set_value('user', 'name', 'llvmbot')
         config.set_value('user', 'email', '[e-mail]')
 
+def phab_api_call(phab_token:str, url:str, args:dict) -> dict:
+    """
+    Make an API call to the Phabricator web service and return a dictionary
+    containing the json response.
+    """
+    data = { "api.token" : phab_token }
+    data.update(args)
+    response = requests.post(url, data = data)
+    return response.json()
+
+
+def phab_login_to_github_login(phab_token:str, repo:github.Repository.Repository, phab_login:str) -> str:
+    """
+    Tries to translate a Phabricator login to a github login by
+    finding a commit made in Phabricator's Differential.
+    The commit's SHA1 is then looked up in the github repo and
+    the committer's login associated with that commit is returned.
+
+    :param str phab_token: The Conduit API token to use for communication with Pabricator
+    :param github.Repository.Repository repo: The github repo to use when looking for the SHA1 found in Differential
+    :param str phab_login: The Phabricator login to be translated.
+    """
+
+    args = {
+        "constraints[authors][0]" : phab_login,
+        # PHID for "LLVM Github Monorepo" repository
+        "constraints[repositories][0]" : "PHID-REPO-f4scjekhnkmh7qilxlcy",
+        "limit" : 1
+    }
+    # API documentation: https://reviews.llvm.org/conduit/method/diffusion.commit.search/
+    r = phab_api_call(phab_token, "https://reviews.llvm.org/api/diffusion.commit.search", args)
+    data = r['result']['data']
+    if len(data) == 0:
+        # Can't find any commits associated with this user
+        return None
+
+    commit_sha = data[0]['fields']['identifier']
+    return repo.get_commit(commit_sha).committer.login
+
+def phab_get_commit_approvers(phab_token:str, repo:github.Repository.Repository, commit:github.Commit.Commit) -> list:
+    args = { "corpus" : commit.commit.message }
+    # API documentation: https://reviews.llvm.org/conduit/method/differential.parsecommitmessage/
+    r = phab_api_call(phab_token, "https://reviews.llvm.org/api/differential.parsecommitmessage", args)
+    review_id = r['result']['revisionIDFieldInfo']['value']
+
+    args = {
+        'constraints[ids][0]' : review_id,
+        'attachments[reviewers]' : True
+    }
+    # API documentation: https://reviews.llvm.org/conduit/method/differential.revision.search/
+    r = phab_api_call(phab_token, "https://reviews.llvm.org/api/differential.revision.search", args)
+    reviewers = r['result']['data'][0]['attachments']['reviewers']['reviewers']
+    accepted = []
+    for reviewer in reviewers:
+        if reviewer['status'] != 'accepted':
+            continue
+        phid = reviewer['reviewerPHID']
+        args = { 'constraints[phids][0]' : phid }
+        # API documentation: https://reviews.llvm.org/conduit/method/user.search/
+        r = phab_api_call(phab_token, "https://reviews.llvm.org/api/user.search", args)
+        accepted.append(r['result']['data'][0]['fields']['username'])
+    return accepted
+
 class ReleaseWorkflow:
 
     CHERRY_PICK_FAILED_LABEL = 'release:cherry-pick-failed'
 
     """
     This class implements the sub-commands for the release-workflow command.
     The current sub-commands are:
         * create-branch
         * create-pull-request
 
     The execute_command method will automatically choose the correct sub-command
     based on the text in stdin.
     """
 
     def __init__(self, token:str, repo:str, issue_number:int,
                        branch_repo_name:str, branch_repo_token:str,
-                       llvm_project_dir:str) -> None:
+                       llvm_project_dir:str, phab_token:str) -> None:
         self._token = token
         self._repo_name = repo
         self._issue_number = issue_number
         self._branch_repo_name = branch_repo_name
         if branch_repo_token:
             self._branch_repo_token = branch_repo_token
         else:
             self._branch_repo_token = self.token
         self._llvm_project_dir = llvm_project_dir
+        self._phab_token = phab_token
 
     @property
     def token(self) -> str:
         return self._token
 
     @property
     def repo_name(self) -> str:
         return self._repo_name
class ReleaseWorkflow:
     def branch_repo_token(self) -> str:
         return self._branch_repo_token
 
     @property
     def llvm_project_dir(self) -> str:
         return self._llvm_project_dir
 
     @property
-    def __repo(self) -> github.Repository.Repository:
+    def phab_token(self) -> str:
+        return self._phab_token
+
+    @property
+    def repo(self) -> github.Repository.Repository:
         return github.Github(self.token).get_repo(self.repo_name)
 
     @property
     def issue(self) -> github.Issue.Issue:
-        return self.__repo.get_issue(self.issue_number)
+        return self.repo.get_issue(self.issue_number)
 
     @property
     def push_url(self) -> str:
         return 'https://{}@github.com/{}'.format(self.branch_repo_token, self.branch_repo_name)
 
     @property
     def branch_name(self) -> str:
         return 'issue{}'.format(self.issue_number)
def issue_notify_pull_request_failure(self, branch:str) -> github.IssueComment.IssueComment:
         message = "Failed to create pull request for {} ".format(branch)
         message += self.action_url
         return self.issue.create_comment(message)
 
     def issue_remove_cherry_pick_failed_label(self):
         if self.CHERRY_PICK_FAILED_LABEL in [l.name for l in self.issue.labels]:
             self.issue.remove_from_labels(self.CHERRY_PICK_FAILED_LABEL)
 
+    def pr_request_review(self, pr:github.PullRequest.PullRequest):
+        """
+        This function will try to find the best reviewers for `commits` and
+        then add a comment requesting review of the backport and assign the
+        pull request to the selected reviewers.
+
+        The reviewers selected are those users who approved the patch in
+        Phabricator.
+        """
+        reviewers = []
+        for commit in pr.get_commits():
+            approvers = phab_get_commit_approvers(self.phab_token, self.repo, commit)
+            for a in approvers:
+                login = phab_login_to_github_login(self.phab_token, self.repo, a)
+                if not login:
+                    continue
+                reviewers.append(login)
+        if len(reviewers):
+            message = "{} What do you think about merging this PR to the release branch?".format(
+                    " ".join(["@" + r for r in reviewers]))
+            pr.create_issue_comment(message)
+            pr.add_to_assignees(*reviewers)
+
     def create_branch(self, commits:List[str]) -> bool:
         """
         This function attempts to backport `commits` into the branch associated
         with `self.issue_number`.
 
         If this is successful, then the branch is pushed to `self.branch_repo_name`, if not,
         a comment is added to the issue saying that the cherry-pick failed.
 
         :param list commits: List of commits to cherry-pick.
 
         """
         print('cherry-picking', commits)
         branch_name = self.branch_name
         local_repo = Repo(self.llvm_project_dir)
         local_repo.git.checkout(self.release_branch_for_issue)
 
         for c in commits:
             try:
                 local_repo.git.cherry_pick('-x', c)
             except Exception as e:
                 self.issue_notify_cherry_pick_failure(c)
                 raise e
 
         push_url = self.push_url
         print('Pushing to {} {}'.format(push_url, branch_name))
         local_repo.git.push(push_url, 'HEAD:{}'.format(branch_name), force=True)
def create_pull_request(self, owner:str, branch:str) -> bool:
             print("PR already exists...")
             return True
         try:
             pull = repo.create_pull(title=f"PR for {issue_ref}",
                                     body='resolves {}'.format(issue_ref),
                                     base=release_branch_for_issue,
                                     head=head,
                                     maintainer_can_modify=False)
+
+            try:
+                if self.phab_token:
+                    self.pr_request_review(pull)
+            except Exception as e:
+                print("error: Failed while searching for reviewers", e)
+
         except Exception as e:
             self.issue_notify_pull_request_failure(branch)
             raise e
 
         if pull is None:
             return False
 
         self.issue_notify_pull_request(pull)

 
 issue_subscriber_parser = subparsers.add_parser('issue-subscriber')
 issue_subscriber_parser.add_argument('--label-name', type=str, required=True)
 issue_subscriber_parser.add_argument('--issue-number', type=int, required=True)
 
 release_workflow_parser = subparsers.add_parser('release-workflow')
 release_workflow_parser.add_argument('--llvm-project-dir', type=str, default='.', help='directory containing the llvm-project checout')
 release_workflow_parser.add_argument('--issue-number', type=int, required=True, help='The issue number to update')
+release_workflow_parser.add_argument('--phab-token', type=str, help='Phabricator conduit API token. See https://reviews.llvm.org/settings/user/<USER>/page/apitokens/')
 release_workflow_parser.add_argument('--branch-repo-token', type=str,
                                      help='GitHub authentication token to use for the repository where new branches will be pushed. Defaults to TOKEN.')
 release_workflow_parser.add_argument('--branch-repo', type=str, default='llvm/llvm-project-release-prs',
                                      help='The name of the repo where new branches will be pushed (e.g. llvm/llvm-project)')
 release_workflow_parser.add_argument('sub_command', type=str, choices=['print-release-branch', 'auto'],
                                      help='Print to stdout the name of the release branch ISSUE_NUMBER should be backported to')
 
 llvmbot_git_config_parser = subparsers.add_parser('setup-llvmbot-git', help='Set the default user and email for the git repo in LLVM_PROJECT_DIR to llvmbot')
 
 args = parser.parse_args()
 
 if args.command == 'issue-subscriber':
     issue_subscriber = IssueSubscriber(args.token, args.repo, args.issue_number, args.label_name)
     issue_subscriber.run()
 elif args.command == 'release-workflow':
     release_workflow = ReleaseWorkflow(args.token, args.repo, args.issue_number,
                                        args.branch_repo, args.branch_repo_token,
-                                       args.llvm_project_dir)
+                                       args.llvm_project_dir, args.phab_token)
     if not release_workflow.release_branch_for_issue:
         release_workflow.issue_notify_no_milestone(sys.stdin.readlines())
         sys.exit(1)
     if args.sub_command == 'print-release-branch':
         release_workflow.print_release_branch()
     else:
         if not release_workflow.execute_command():
             sys.exit(1)
 elif args.command == 'setup-llvmbot-git':
     setup_llvmbot_git()