From 9d09f33f5db1c93b45994b31901b5b548b1ce15f Mon Sep 17 00:00:00 2001
From: George Burgess IV <george.burgess.iv@gmail.com>
Date: Wed, 2 Apr 2025 10:18:16 -0600
Subject: [PATCH] [llvm][utils] skip revert-checking reverts across branches

e2ba1b6ffde4ec607342b1b746d1b57f0f04390a references that it reverts a
commit that's not a parent of e2ba1b6ffde4ec607342b1b746d1b57f0f04390a.

Functionally, this can (and demonstrably does) work(*), but from the
standpoint of the revert checker, it's nonsense. Print a `logging.error`
when it's detected.

Tested by running the revert checker against a commit range that
includes the aforementioned commit; the logging.error was fired
appropriately.

(*) - the specifics here are:
- the _SHA_ that was referenced was on a non-main branch, but
- the commit from the non-main branch was merged into the non-main
  branch from main
- ...so the _functional_ commit being reverted was originally landed on
  main, but the _SHA_ referenced from main was from a branch that was
  cut before the reverted-commit was landed on main
---
 llvm/utils/revert_checker.py | 38 +++++++++++++++++++++++++++---------
 1 file changed, 29 insertions(+), 9 deletions(-)

diff --git a/llvm/utils/revert_checker.py b/llvm/utils/revert_checker.py
index b1c6e228e4d41..3c3ff3e6b846f 100755
--- a/llvm/utils/revert_checker.py
+++ b/llvm/utils/revert_checker.py
@@ -211,7 +211,12 @@ def _rev_parse(git_dir: str, ref: str) -> str:
 
 
 def _find_common_parent_commit(git_dir: str, ref_a: str, ref_b: str) -> str:
-    """Finds the closest common parent commit between `ref_a` and `ref_b`."""
+    """Finds the closest common parent commit between `ref_a` and `ref_b`.
+
+    Returns:
+        A SHA. Note that `ref_a` will be returned if `ref_a` is a parent of
+        `ref_b`, and vice-versa.
+    """
     return subprocess.check_output(
         ["git", "-C", git_dir, "merge-base", ref_a, ref_b],
         encoding="utf-8",
@@ -341,16 +346,31 @@ def find_reverts(
                 )
                 continue
 
-            if object_type == "commit":
-                all_reverts.append(Revert(sha, reverted_sha))
+            if object_type != "commit":
+                logging.error(
+                    "%s claims to revert the %s %s, which isn't a commit",
+                    sha,
+                    object_type,
+                    reverted_sha,
+                )
+                continue
+
+            # Rarely, reverts will cite SHAs on other branches (e.g., revert
+            # commit says it reverts a commit with SHA ${X}, but ${X} is not a
+            # parent of the revert). This can happen if e.g., the revert has
+            # been mirrored to another branch. Treat them the same as
+            # reverts of non-commits.
+            if _find_common_parent_commit(git_dir, sha, reverted_sha) != reverted_sha:
+                logging.error(
+                    "%s claims to revert %s, which is a commit that is not "
+                    "a parent of the revert",
+                    sha,
+                    reverted_sha,
+                )
                 continue
 
-            logging.error(
-                "%s claims to revert %s -- which isn't a commit -- %s",
-                sha,
-                object_type,
-                reverted_sha,
-            )
+            all_reverts.append(Revert(sha, reverted_sha))
+
 
     # Since `all_reverts` contains reverts in log order (e.g., newer comes before
     # older), we need to reverse this to keep with our guarantee of older =