diff --git a/.github/workflows/git-commit-checks.py b/.github/workflows/git-commit-checks.py index 4a9f8e50bfc..8acbd4f4c94 100755 --- a/.github/workflows/git-commit-checks.py +++ b/.github/workflows/git-commit-checks.py @@ -8,13 +8,15 @@ variables that are available in the Github Action environment. Specifically: * GITHUB_WORKSPACE: directory where the git clone is located -* GITHUB_SHA: the git commit SHA of the artificial Github PR test merge commit * GITHUB_BASE_REF: the git ref for the base branch +* GITHUB_HEAD_REF: the git commit ref of the head branch * GITHUB_TOKEN: token authorizing Github API usage * GITHUB_REPOSITORY: "org/repo" name of the Github repository of this PR * GITHUB_REF: string that includes this Github PR number +* GITHUB_RUN_ID: unique ID for each workflow run +* GITHUB_SERVER_URL: the URL of the GitHub server -This script tests each git commit between (and not including) GITHUB_SHA and +This script tests each git commit between (and not including) GITHUB_HEAD_REF and GITHUB_BASE_REF multiple ways: 1. Ensure that the committer and author do not match any bad patterns (e.g., @@ -53,19 +55,25 @@ NACP = "bot:notacherrypick" GITHUB_WORKSPACE = os.environ.get('GITHUB_WORKSPACE') -GITHUB_SHA = os.environ.get('GITHUB_SHA') GITHUB_BASE_REF = os.environ.get('GITHUB_BASE_REF') +GITHUB_HEAD_REF = os.environ.get('GITHUB_HEAD_REF') GITHUB_TOKEN = os.environ.get('GITHUB_TOKEN') GITHUB_REPOSITORY = os.environ.get('GITHUB_REPOSITORY') GITHUB_REF = os.environ.get('GITHUB_REF') +GITHUB_RUN_ID = os.environ.get('GITHUB_RUN_ID') +GITHUB_SERVER_URL = os.environ.get('GITHUB_SERVER_URL') +PR_NUM = os.environ.get('PR_NUM') # Sanity check if (GITHUB_WORKSPACE is None or - GITHUB_SHA is None or GITHUB_BASE_REF is None or + GITHUB_HEAD_REF is None or GITHUB_TOKEN is None or GITHUB_REPOSITORY is None or - GITHUB_REF is None): + GITHUB_REF is None or + GITHUB_RUN_ID is None or + GITHUB_SERVER_URL is None or + PR_NUM is None): print("Error: this script is designed to run as a Github Action") exit(1) @@ -85,6 +93,50 @@ def make_commit_message(repo, hash): #---------------------------------------------------------------------------- +""" +Iterate through the BAD results, collect the error messages, and send a nicely +formatted comment to the PR. + +For the structure of the results dictionary, see comment for print_results() +below. + +""" +def comment_on_pr(pr, results, repo): + # If there are no BAD results, just return without posting a comment to the + # GitHub PR. + if len(results[BAD]) == 0: + return + + comment = "Hello! The Git Commit Checker CI bot found a few problems with this PR:" + for hash, entry in results[BAD].items(): + comment += f"\n\n**{hash[:8]}: {make_commit_message(repo, hash)}**" + for check_name, message in entry.items(): + if message is not None: + comment += f"\n * *{check_name}: {message}*" + comment_footer = "\n\nPlease fix these problems and, if necessary, force-push new commits back up to the PR branch. Thanks!" + + # GitHub says that 65536 characters is the limit of comment messages, so + # check if our comment is over that limit. If it is, truncate it to fit, and + # add a message explaining with a link to the full error list. + comment_char_limit = 65536 + if len(comment + comment_footer) >= comment_char_limit: + run_url = f"{GITHUB_SERVER_URL}/{GITHUB_REPOSITORY}/actions/runs/{GITHUB_RUN_ID}?check_suite_focus=true" + truncation_message = f"\n\n**Additional errors could not be shown...\n[Please click here for a full list of errors.]({run_url})**" + # Cut the comment down so we can get the comment itself, and the new + # message in. + comment = comment[:(comment_char_limit - len(comment_footer + truncation_message))] + # In case a newline gets split in half, remove the leftover '\' (if + # there is one). (This is purely an aesthetics choice). + comment = comment.rstrip("\\") + comment += truncation_message + + comment += comment_footer + pr.create_issue_comment(comment) + + return + +#---------------------------------------------------------------------------- + """ The results dictionary is in the following format: @@ -242,15 +294,15 @@ def _is_entirely_submodule_updates(repo, commit): #---------------------------------------------------------------------------- def check_all_commits(config, repo): - # Get a list of commits that we'll be examining. Use the progromatic form - # of "git log GITHUB_BASE_REF..GITHUB_SHA" (i.e., "git log ^GITHUB_BASE_REF - # GITHUB_SHA") to do the heavy lifting to find that set of commits. + # Get a list of commits that we'll be examining. Use the programmatic form + # of "git log GITHUB_BASE_REF..GITHUB_HEAD_REF" (i.e., "git log + # ^GITHUB_BASE_REF GITHUB_HEAD_REF") to do the heavy lifting to find that + # set of commits. Because we're using pull_request_target, GITHUB_BASE_REF + # is already checked out. GITHUB_HEAD_REF has never been checked out, so we + # specify "origin/{GITHUB_HEAD_REF}". git_cli = git.cmd.Git(GITHUB_WORKSPACE) - hashes = git_cli.log(f"--pretty=format:%h", f"origin/{GITHUB_BASE_REF}..{GITHUB_SHA}").splitlines() - - # The first entry in the list will be the artificial Github merge commit for - # this PR. We don't want to examine this commit. - del hashes[0] + hashes = git_cli.log(f"--pretty=format:%h", + f"{GITHUB_BASE_REF}..origin/{GITHUB_HEAD_REF}").splitlines() #------------------------------------------------------------------------ @@ -292,15 +344,7 @@ def check_all_commits(config, repo): If "bot:notacherrypick" is in the PR description, then disable the cherry-pick message requirement. """ -def check_github_pr_description(config): - g = Github(GITHUB_TOKEN) - repo = g.get_repo(GITHUB_REPOSITORY) - - # Extract the PR number from GITHUB_REF - match = re.search("/(\d+)/", GITHUB_REF) - pr_num = int(match.group(1)) - pr = repo.get_pull(pr_num) - +def check_github_pr_description(config, pr): if pr.body and NACP in pr.body: config['cherry pick required'] = False @@ -334,11 +378,17 @@ def load_config(): def main(): config = load_config() - check_github_pr_description(config) - repo = git.Repo(GITHUB_WORKSPACE) - results, hashes = check_all_commits(config, repo) - print_results(results, repo, hashes) + g = Github(GITHUB_TOKEN) + github_repo = g.get_repo(GITHUB_REPOSITORY) + pr_num = int(PR_NUM) + pr = github_repo.get_pull(pr_num) + check_github_pr_description(config, pr) + + local_repo = git.Repo(GITHUB_WORKSPACE) + results, hashes = check_all_commits(config, local_repo) + print_results(results, local_repo, hashes) + comment_on_pr(pr, results, local_repo) if len(results[BAD]) == 0: print("\nTest passed: everything was good!") diff --git a/.github/workflows/git-commit-checks.yml b/.github/workflows/git-commit-checks.yml index 4cf1036878e..6f1078f7ca6 100644 --- a/.github/workflows/git-commit-checks.yml +++ b/.github/workflows/git-commit-checks.yml @@ -1,7 +1,11 @@ name: GitHub Action CI +# We're using pull_request_target here instead of just pull_request so that the +# action runs in the context of the base of the pull request, rather than in the +# context of the merge commit. For more detail about the differences, see: +# https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request_target on: - pull_request: + pull_request_target: # We don't need this to be run on all types of PR behavior # See https://docs.github.com/en/actions/reference/events-that-trigger-workflows#pull_request types: @@ -32,3 +36,4 @@ jobs: run: $GITHUB_WORKSPACE/.github/workflows/git-commit-checks.py env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + PR_NUM: ${{ github.event.number }}