From 0d0dc982a8858ed87be71cff61cc7394b3d08c0f Mon Sep 17 00:00:00 2001 From: StevenGood154 Date: Sun, 9 May 2021 20:14:05 -0400 Subject: [PATCH] CI: Fix a bug in the cherry pick checker The check_cherry_pick() function in .github/workflows/git-commit-checks.py would incorrectly allow some cherry-pick messages reffering to commits that do not exist in the base repository, specifially when the commit exists only on a pull request that has not been merged. This has been resolved; the workflow will exit with a failure unless the referenced commit exists in the base repository. Note that this approach only works because all commits are introduced to the open-mpi/ompi through pull requests from forks. Signed-off-by: Steven Good Signed-off-by: Brett Wiseman Signed-off-by: Christine Van Kirk (cherry picked from commit a641680bc7f0f9365d98acfda6c81c61ef6cf5e8) --- .github/workflows/git-commit-checks.py | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/.github/workflows/git-commit-checks.py b/.github/workflows/git-commit-checks.py index 9f212328f21..d6f4544d8c8 100755 --- a/.github/workflows/git-commit-checks.py +++ b/.github/workflows/git-commit-checks.py @@ -186,6 +186,7 @@ def _is_entirely_submodule_updates(repo, commit): return GOOD, "skipped (submodules updates)" non_existent = dict() + unmerged = dict() found_cherry_pick_line = False for match in prog_cp.findall(commit.message): found_cherry_pick_line = True @@ -195,9 +196,11 @@ def _is_entirely_submodule_updates(repo, commit): # These errors mean that the git library recognized the # hash as a valid commit, but the GitHub Action didn't # fetch the entire repo, so we don't have all the meta - # data about this commit. Bottom line: it's a good hash. - # So -- no error here. - pass + # data about this commit. This occurs because the commit + # only exists in an as-yet unmerged pull request on github. Therefore, we + # want to fail this commit until the corresponding pull request + # is merged. + unmerged[match] = True except git.BadName as e: # Use a dictionary to track the non-existent hashes, just # on the off chance that the same non-existent hash exists @@ -208,15 +211,27 @@ def _is_entirely_submodule_updates(repo, commit): # Process the results for this commit if found_cherry_pick_line: - if len(non_existent) == 0: + if len(non_existent) == 0 and len(unmerged) == 0: return GOOD, None - else: + elif len(non_existent) > 0 and len(unmerged) == 0: str = f"contains a cherry pick message that refers to non-existent commit" if len(non_existent) > 1: str += "s" str += ": " str += ", ".join(non_existent) return BAD, str + elif len(non_existent) == 0 and len(unmerged) > 0: + str = f"contains a cherry pick message that refers to a commit that exists, but is in an as-yet unmerged pull request" + if len(non_existent) > 1: + str += "s" + str += ": " + str += ", ".join(unmerged) + return BAD, str + else: + str = f"contains a cherry pick message that refers to both non-existent commits and commits that exist but are in as-yet unmerged pull requests" + str += ": " + str += ", ".join(non_existent + unmerged) + return BAD, str else: if config['cherry pick required']: