llvm_tools: use upstream revert_checker

Sourcing this properly from LLVM is awkward, since at the least, we'd
need to keep an LLVM git repo around. On top of that, we'd need to come
up with some way to pin this at a specific version (since the evolution
of LLVM ideally shouldn't impact the functionality of bits here).

Rather than dealing with all of that, let's just copy this from the
repo and declare victory. Changes should be super rare anyway. :)

BUG=chromium:194731505
TEST=./nightly_revert_checker_test.py

Change-Id: I6179cae57d01bee7cae7883c93f71cb9c2f1f3d1
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/toolchain-utils/+/3068861
Reviewed-by: Bob Haarman <inglorion@chromium.org>
Tested-by: George Burgess <gbiv@chromium.org>

3 files changed, 67 insertions, 144 deletions

diff --git a/llvm_tools/README.md b/llvm_tools/README.md
index f15040ec..43c80ad6 100644
--- a/llvm_tools/README.md
+++ b/llvm_tools/README.md
@@ -495,6 +495,10 @@ more information, please see the `--help`
 
 ### `revert_checker.py`
 
+**This script is copied from upstream LLVM. Please prefer to make upstream edits,
+rather than modifying this script. It's kept in a CrOS repo so we don't need an
+LLVM tree to `import` this from scripts here.**
+
 This script reports reverts which happen 'across' a certain LLVM commit.
 
 To clarify the meaning of 'across' with an example, if we had the following
diff --git a/llvm_tools/revert_checker.py b/llvm_tools/revert_checker.py
index bb9182b0..acc8b5fa 100755
--- a/llvm_tools/revert_checker.py
+++ b/llvm_tools/revert_checker.py
@@ -1,8 +1,17 @@
 #!/usr/bin/env python3
 # -*- coding: utf-8 -*-
-# Copyright 2020 The Chromium OS Authors. All rights reserved.
-# Use of this source code is governed by a BSD-style license that can be
-# found in the LICENSE file.
+#===----------------------------------------------------------------------===##
+#
+# Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+# See https://llvm.org/LICENSE.txt for license information.
+# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+#
+#===----------------------------------------------------------------------===##
+#
+# !!!!!!!!!!!! NOTE !!!!!!!!!!!!
+# This is copied directly from upstream LLVM. Please make any changes upstream,
+# rather than to this file directly. Once changes are made there, you're free
+# to integrate them here.
 
 """Checks for reverts of commits across a given git commit.
 
@@ -20,11 +29,21 @@ conflicts/etc always introduce _some_ amount of fuzziness. This script just
 uses a bundle of heuristics, and is bound to ignore / incorrectly flag some
 reverts. The hope is that it'll easily catch the vast majority (>90%) of them,
 though.
-"""
 
-# pylint: disable=cros-logging-import
+This is designed to be used in one of two ways: an import in Python, or run
+directly from a shell. If you want to import this, the `find_reverts`
+function is the thing to look at. If you'd rather use this from a shell, have a
+usage example:
+
+```
+./revert_checker.py c47f97169 origin/main origin/release/12.x
+```
 
-from __future__ import print_function
+This checks for all reverts from the tip of origin/main to c47f97169, which are
+across the latter. It then does the same for origin/release/12.x to c47f97169.
+Duplicate reverts discovered when walking both roots (origin/main and
+origin/release/12.x) are deduplicated in output.
+"""
 
 import argparse
 import collections
@@ -32,7 +51,9 @@ import logging
 import re
 import subprocess
 import sys
-import typing as t
+from typing import Generator, List, NamedTuple, Iterable
+
+assert sys.version_info >= (3, 6), 'Only Python 3.6+ is supported.'
 
 # People are creative with their reverts, and heuristics are a bit difficult.
 # Like 90% of of reverts have "This reverts commit ${full_sha}".
@@ -43,7 +64,7 @@ import typing as t
 # starts involving human intervention, which is probably not worth it for now.
 
 
-def _try_parse_reverts_from_commit_message(commit_message: str) -> t.List[str]:
+def _try_parse_reverts_from_commit_message(commit_message: str) -> List[str]:
   if not commit_message:
     return []
 
@@ -56,9 +77,10 @@ def _try_parse_reverts_from_commit_message(commit_message: str) -> t.List[str]:
   return results
 
 
-def _stream_stdout(command: t.List[str]) -> t.Generator[str, None, None]:
+def _stream_stdout(command: List[str]) -> Generator[str, None, None]:
   with subprocess.Popen(
       command, stdout=subprocess.PIPE, encoding='utf-8', errors='replace') as p:
+    assert p.stdout is not None  # for mypy's happiness.
     yield from p.stdout
 
 
@@ -73,14 +95,14 @@ def _resolve_sha(git_dir: str, sha: str) -> str:
   ).strip()
 
 
-_LogEntry = t.NamedTuple('_LogEntry', [
+_LogEntry = NamedTuple('_LogEntry', [
     ('sha', str),
-    ('commit_message', t.List[str]),
+    ('commit_message', str),
 ])
 
 
 def _log_stream(git_dir: str, root_sha: str,
-                end_at_sha: str) -> t.Iterable[_LogEntry]:
+                end_at_sha: str) -> Iterable[_LogEntry]:
   sep = 50 * '<>'
   log_command = [
       'git',
@@ -103,8 +125,6 @@ def _log_stream(git_dir: str, root_sha: str,
       break
 
   while found_commit_header:
-    # crbug.com/1041148
-    # pylint: disable=stop-iteration-return
     sha = next(stdout_stream, None)
     assert sha is not None, 'git died?'
     sha = sha.rstrip()
@@ -122,52 +142,54 @@ def _log_stream(git_dir: str, root_sha: str,
     yield _LogEntry(sha, '\n'.join(commit_message).rstrip())
 
 
-def _shas_between(git_dir: str, base_ref: str,
-                  head_ref: str) -> t.Iterable[str]:
+def _shas_between(git_dir: str, base_ref: str, head_ref: str) -> Iterable[str]:
   rev_list = [
       'git',
       '-C',
       git_dir,
       'rev-list',
       '--first-parent',
-      '%s..%s' % (base_ref, head_ref),
+      f'{base_ref}..{head_ref}',
   ]
   return (x.strip() for x in _stream_stdout(rev_list))
 
 
 def _rev_parse(git_dir: str, ref: str) -> str:
-  result = subprocess.check_output(
+  return subprocess.check_output(
       ['git', '-C', git_dir, 'rev-parse', ref],
       encoding='utf-8',
   ).strip()
-  return t.cast(str, result)
 
 
-Revert = t.NamedTuple('Revert', [
+Revert = NamedTuple('Revert', [
     ('sha', str),
     ('reverted_sha', str),
 ])
 
 
-def find_common_parent_commit(git_dir: str, ref_a: str, ref_b: 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`."""
   return subprocess.check_output(
       ['git', '-C', git_dir, 'merge-base', ref_a, ref_b],
       encoding='utf-8',
   ).strip()
 
 
-def find_reverts(git_dir: str, across_ref: str, root: str) -> t.List[Revert]:
-  """Finds reverts across `across_ref` in `git_dir`, starting from `root`."""
+def find_reverts(git_dir: str, across_ref: str, root: str) -> List[Revert]:
+  """Finds reverts across `across_ref` in `git_dir`, starting from `root`.
+
+  These reverts are returned in order of oldest reverts first.
+  """
   across_sha = _rev_parse(git_dir, across_ref)
   root_sha = _rev_parse(git_dir, root)
 
-  common_ancestor = find_common_parent_commit(git_dir, across_sha, root_sha)
+  common_ancestor = _find_common_parent_commit(git_dir, across_sha, root_sha)
   if common_ancestor != across_sha:
-    raise ValueError("%s isn't an ancestor of %s (common ancestor: %s)" %
-                     (across_sha, root_sha, common_ancestor))
+    raise ValueError(f"{across_sha} isn't an ancestor of {root_sha} "
+                     '(common ancestor: {common_ancestor})')
 
   intermediate_commits = set(_shas_between(git_dir, across_sha, root_sha))
-  assert across_ref not in intermediate_commits
+  assert across_sha not in intermediate_commits
 
   logging.debug('%d commits appear between %s and %s',
                 len(intermediate_commits), across_sha, root_sha)
@@ -204,10 +226,14 @@ def find_reverts(git_dir: str, across_ref: str, root: str) -> t.List[Revert]:
       logging.error("%s claims to revert %s -- which isn't a commit -- %s", sha,
                     object_type, 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 =
+  # earlier in the result.
+  all_reverts.reverse()
   return all_reverts
 
 
-def main(args: t.List[str]) -> int:
+def _main() -> None:
   parser = argparse.ArgumentParser(
       description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter)
   parser.add_argument(
@@ -217,7 +243,7 @@ def main(args: t.List[str]) -> int:
   parser.add_argument(
       'root', nargs='+', help='Root(s) to search for commits from.')
   parser.add_argument('--debug', action='store_true')
-  opts = parser.parse_args(args)
+  opts = parser.parse_args()
 
   logging.basicConfig(
       format='%(asctime)s: %(levelname)s: %(filename)s:%(lineno)d: %(message)s',
@@ -228,14 +254,17 @@ def main(args: t.List[str]) -> int:
   # out. The overwhelmingly common case is also to have one root, and it's way
   # easier to reason about output that comes in an order that's meaningful to
   # git.
-  all_reverts = collections.OrderedDict()
+  seen_reverts = set()
+  all_reverts = []
   for root in opts.root:
     for revert in find_reverts(opts.git_dir, opts.base_ref, root):
-      all_reverts[revert] = None
+      if revert not in seen_reverts:
+        seen_reverts.add(revert)
+        all_reverts.append(revert)
 
-  for revert in all_reverts.keys():
-    print('%s claims to revert %s' % (revert.sha, revert.reverted_sha))
+  for revert in all_reverts:
+    print(f'{revert.sha} claims to revert {revert.reverted_sha}')
 
 
 if __name__ == '__main__':
-  sys.exit(main(sys.argv[1:]))
+  _main()
diff --git a/llvm_tools/revert_checker_test.py b/llvm_tools/revert_checker_test.py
deleted file mode 100755
index 16b3c3f6..00000000
--- a/llvm_tools/revert_checker_test.py
+++ /dev/null
@@ -1,110 +0,0 @@
-#!/usr/bin/env python3
-# -*- coding: utf-8 -*-
-# Copyright 2020 The Chromium OS Authors. All rights reserved.
-# Use of this source code is governed by a BSD-style license that can be
-# found in the LICENSE file.
-
-"""Tests for revert_checker."""
-
-from __future__ import print_function
-
-# pylint: disable=cros-logging-import
-import logging
-import unittest
-
-import llvm_project
-import revert_checker
-
-# pylint: disable=protected-access
-
-
-class _SilencingFilter(object):
-  """Silences all log messages.
-
-  Also collects info about log messages that would've been emitted.
-  """
-
-  def __init__(self):
-    self.messages = []
-
-  def filter(self, record):
-    self.messages.append(record.getMessage())
-    return 0
-
-
-class Test(unittest.TestCase):
-  """Tests for revert_checker."""
-
-  def silence_logging(self):
-    root = logging.getLogger()
-    filt = _SilencingFilter()
-    root.addFilter(filt)
-    self.addCleanup(root.removeFilter, filt)
-    return filt
-
-  def test_known_log_stream(self):
-    start_sha = 'e241573d5972d34a323fa5c64774c4207340beb3'
-    end_sha = 'a7a37517751ffb0f5529011b4ba96e67fcb27510'
-    commits = [
-        revert_checker._LogEntry(
-            'e241573d5972d34a323fa5c64774c4207340beb3', '\n'.join((
-                '[mlir] NFC: remove IntegerValueSet / MutableIntegerSet',
-                '',
-                'Summary:',
-                '- these are unused and really not needed now given flat '
-                'affine',
-                '  constraints',
-                '',
-                'Differential Revision: https://reviews.llvm.org/D75792',
-            ))),
-        revert_checker._LogEntry(
-            '97572fa6e9daecd648873496fd11f7d1e25a55f0',
-            '[NFC] use hasAnyOperatorName and hasAnyOverloadedOperatorName '
-            'functions in clang-tidy matchers',
-        ),
-    ]
-
-    logs = list(
-        revert_checker._log_stream(
-            llvm_project.get_location(),
-            root_sha=start_sha,
-            end_at_sha=end_sha,
-        ))
-    self.assertEqual(commits, logs)
-
-  def test_reverted_noncommit_object_is_a_nop(self):
-    log_filter = self.silence_logging()
-    # c9944df916e41b1014dff5f6f75d52297b48ecdc mentions reverting a non-commit
-    # object. It sits between the given base_ref and root.
-    reverts = revert_checker.find_reverts(
-        git_dir=llvm_project.get_location(),
-        across_ref='c9944df916e41b1014dff5f6f75d52297b48ecdc~',
-        root='c9944df916e41b1014dff5f6f75d52297b48ecdc')
-    self.assertEqual(reverts, [])
-
-    complaint = ('Failed to resolve reverted object '
-                 'edd18355be574122aaa9abf58c15d8c50fb085a1')
-    self.assertTrue(
-        any(x.startswith(complaint) for x in log_filter.messages),
-        log_filter.messages)
-
-  def test_known_reverts_across_previous_llvm_next_rev(self):
-    # c9944df916e41b1014dff5f6f75d52297b48ecdc mentions reverting a non-commit
-    # object. It sits between the given base_ref and root.
-    reverts = revert_checker.find_reverts(
-        git_dir=llvm_project.get_location(),
-        across_ref='c47f971694be0159ffddfee8a75ae515eba91439',
-        root='9f981e9adf9c8d29bb80306daf08d2770263ade6')
-    self.assertEqual(reverts, [
-        revert_checker.Revert(
-            sha='9f981e9adf9c8d29bb80306daf08d2770263ade6',
-            reverted_sha='4060016fce3e6a0b926ee9fc59e440a612d3a2ec'),
-        revert_checker.Revert(
-            sha='4e0fe038f438ae1679eae9e156e1f248595b2373',
-            reverted_sha='65b21282c710afe9c275778820c6e3c1cf46734b'),
-    ])
-
-
-if __name__ == '__main__':
-  llvm_project.ensure_up_to_date()
-  unittest.main()