From d290d2c3f6107cf297119c22340f70a1904c1ca5 Mon Sep 17 00:00:00 2001 From: Jeff Squyres Date: Mon, 15 Aug 2022 14:43:02 -0400 Subject: [PATCH] Revert "git-commit-checks: comment on PR with error(s)" This reverts commit 4b0ce7e5e1a1b7429e22ea861d6d84ecb478fb90. --- .github/workflows/git-commit-checks.py | 102 ++++++------------------ .github/workflows/git-commit-checks.yml | 7 +- 2 files changed, 27 insertions(+), 82 deletions(-) diff --git a/.github/workflows/git-commit-checks.py b/.github/workflows/git-commit-checks.py index 8acbd4f4c94..4a9f8e50bfc 100755 --- a/.github/workflows/git-commit-checks.py +++ b/.github/workflows/git-commit-checks.py @@ -8,15 +8,13 @@ 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_HEAD_REF and +This script tests each git commit between (and not including) GITHUB_SHA and GITHUB_BASE_REF multiple ways: 1. Ensure that the committer and author do not match any bad patterns (e.g., @@ -55,25 +53,19 @@ 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 or - GITHUB_RUN_ID is None or - GITHUB_SERVER_URL is None or - PR_NUM is None): + GITHUB_REF is None): print("Error: this script is designed to run as a Github Action") exit(1) @@ -93,50 +85,6 @@ 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: @@ -294,15 +242,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 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}". + # 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. git_cli = git.cmd.Git(GITHUB_WORKSPACE) - hashes = git_cli.log(f"--pretty=format:%h", - f"{GITHUB_BASE_REF}..origin/{GITHUB_HEAD_REF}").splitlines() + 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] #------------------------------------------------------------------------ @@ -344,7 +292,15 @@ 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, pr): +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) + if pr.body and NACP in pr.body: config['cherry pick required'] = False @@ -378,17 +334,11 @@ def load_config(): def main(): config = load_config() + check_github_pr_description(config) - 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) + repo = git.Repo(GITHUB_WORKSPACE) + results, hashes = check_all_commits(config, repo) + print_results(results, repo, hashes) 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 6f1078f7ca6..4cf1036878e 100644 --- a/.github/workflows/git-commit-checks.yml +++ b/.github/workflows/git-commit-checks.yml @@ -1,11 +1,7 @@ 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_target: + pull_request: # 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: @@ -36,4 +32,3 @@ jobs: run: $GITHUB_WORKSPACE/.github/workflows/git-commit-checks.py env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - PR_NUM: ${{ github.event.number }}