From 515e9aa1a67e46bbc17553bc161db2dcb0a1feb8 Mon Sep 17 00:00:00 2001 From: Joe Downs Date: Mon, 1 Aug 2022 17:28:16 -0400 Subject: [PATCH 1/2] git-commit-checks: comment on PR with error(s) This adds comments onto the pull request containing what caused the commit checks to fail, if any, and suggests fixes to the user. If no errors are raised, no comment is made. GitHub says there's a limit of 65536 characters on comments. If the bot's comment is over that limit, it will truncate the comment to fit, and add a message explaining where the remaining errors can be found. Unfortunately, the GitHub API doesn't seem to provide a job's unique ID for linking to a job run (this is different than an action run: ".../runs/..." vs ".../actions/runs/...", respectively), so we can't directly link to the error messages printed to the console. Additionally, to create this link, two new environment variables are used: GITHUB_RUN_ID and GITHUB_SERVER_URL. Because we need the PR object twice, check_github_pr_description() was also changed to have the PR object passed into it; the PR object is gotten with a new function, get_github_pr(). The GitHub action configuration was changed to run on pull_request_target, instead of pull_request. This allows the action to be run in the context of the base of the PR, rather than in the context of the merge commit. Therefore, the action is run even if the PR has merge conflicts. Signed-off-by: Joe Downs --- .github/workflows/git-commit-checks.py | 67 +++++++++++++++++++++++-- .github/workflows/git-commit-checks.yml | 6 ++- 2 files changed, 67 insertions(+), 6 deletions(-) diff --git a/.github/workflows/git-commit-checks.py b/.github/workflows/git-commit-checks.py index 4a9f8e50bfc..9dca0242811 100755 --- a/.github/workflows/git-commit-checks.py +++ b/.github/workflows/git-commit-checks.py @@ -13,6 +13,8 @@ * 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 GITHUB_BASE_REF multiple ways: @@ -58,6 +60,8 @@ 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') # Sanity check if (GITHUB_WORKSPACE is None or @@ -65,7 +69,9 @@ GITHUB_BASE_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): print("Error: this script is designed to run as a Github Action") exit(1) @@ -85,6 +91,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: @@ -292,7 +342,13 @@ 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): +def check_github_pr_description(config, pr): + if pr.body and NACP in pr.body: + config['cherry pick required'] = False + +#---------------------------------------------------------------------------- + +def get_github_pr(): g = Github(GITHUB_TOKEN) repo = g.get_repo(GITHUB_REPOSITORY) @@ -301,8 +357,7 @@ def check_github_pr_description(config): 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 + return pr #---------------------------------------------------------------------------- @@ -334,11 +389,13 @@ def load_config(): def main(): config = load_config() - check_github_pr_description(config) + pr = get_github_pr() + check_github_pr_description(config, pr) repo = git.Repo(GITHUB_WORKSPACE) results, hashes = check_all_commits(config, repo) print_results(results, repo, hashes) + comment_on_pr(pr, results, 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..288d43bf160 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: From 8f6cddd0281c476dd7ab932f57fe27cb0d13ea82 Mon Sep 17 00:00:00 2001 From: Joe Downs Date: Tue, 9 Aug 2022 10:06:42 -0400 Subject: [PATCH 2/2] Revert "git-commit-checks: comment on PR with error(s)" --- .github/workflows/git-commit-checks.py | 67 ++----------------------- .github/workflows/git-commit-checks.yml | 6 +-- 2 files changed, 6 insertions(+), 67 deletions(-) diff --git a/.github/workflows/git-commit-checks.py b/.github/workflows/git-commit-checks.py index 9dca0242811..4a9f8e50bfc 100755 --- a/.github/workflows/git-commit-checks.py +++ b/.github/workflows/git-commit-checks.py @@ -13,8 +13,6 @@ * 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 GITHUB_BASE_REF multiple ways: @@ -60,8 +58,6 @@ 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') # Sanity check if (GITHUB_WORKSPACE is None or @@ -69,9 +65,7 @@ GITHUB_BASE_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): + GITHUB_REF is None): print("Error: this script is designed to run as a Github Action") exit(1) @@ -91,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: @@ -342,13 +292,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, pr): - if pr.body and NACP in pr.body: - config['cherry pick required'] = False - -#---------------------------------------------------------------------------- - -def get_github_pr(): +def check_github_pr_description(config): g = Github(GITHUB_TOKEN) repo = g.get_repo(GITHUB_REPOSITORY) @@ -357,7 +301,8 @@ def get_github_pr(): pr_num = int(match.group(1)) pr = repo.get_pull(pr_num) - return pr + if pr.body and NACP in pr.body: + config['cherry pick required'] = False #---------------------------------------------------------------------------- @@ -389,13 +334,11 @@ def load_config(): def main(): config = load_config() - pr = get_github_pr() - check_github_pr_description(config, pr) + check_github_pr_description(config) repo = git.Repo(GITHUB_WORKSPACE) results, hashes = check_all_commits(config, repo) print_results(results, repo, hashes) - comment_on_pr(pr, results, 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 288d43bf160..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: