fixes failure when a commit has already been cherry-picked

cherrypick_cl.py fails whenever a commit is already present, but the
revert checker should simply note this and move on.

BUG=chromium:1181376
TEST=nightly_revert_checker_test.py

Change-Id: I04500fef98a95607df3cb23bf0a901d3ba5062a7
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/toolchain-utils/+/2716971
Auto-Submit: Christopher Di Bella <cjdb@google.com>
Commit-Queue: Manoj Gupta <manojgupta@chromium.org>
Reviewed-by: Manoj Gupta <manojgupta@chromium.org>
Tested-by: Manoj Gupta <manojgupta@chromium.org>

3 files changed, 37 insertions, 8 deletions

diff --git a/llvm_tools/cherrypick_cl.py b/llvm_tools/cherrypick_cl.py
index ff385f97..ebf7c0f3 100755
--- a/llvm_tools/cherrypick_cl.py
+++ b/llvm_tools/cherrypick_cl.py
@@ -27,6 +27,10 @@ import git_llvm_rev
 import update_chromeos_llvm_hash
 
 
+class CherrypickError(ValueError):
+  """A ValueError that highlights the cherry-pick has been seen before"""
+
+
 def add_cherrypick(patches_json_path: str, patches_dir: str,
                    relative_patches_dir: str, start_version: git_llvm_rev.Rev,
                    llvm_dir: str, rev: git_llvm_rev.Rev, sha: str,
@@ -40,7 +44,8 @@ def add_cherrypick(patches_json_path: str, patches_dir: str,
   for p in patches_json:
     rel_path = p['rel_patch_path']
     if rel_path == rel_patch_path:
-      raise ValueError('Patch at %r already exists in PATCHES.json' % rel_path)
+      raise CherrypickError(
+          f'Patch at {rel_path} already exists in PATCHES.json')
     if sha in rel_path:
       logging.warning(
           'Similarly-named patch already exists in PATCHES.json: %r', rel_path)
diff --git a/llvm_tools/nightly_revert_checker.py b/llvm_tools/nightly_revert_checker.py
index 6b4ac53c..21dcf944 100755
--- a/llvm_tools/nightly_revert_checker.py
+++ b/llvm_tools/nightly_revert_checker.py
@@ -231,13 +231,16 @@ def do_cherrypick(chroot_path: str, llvm_dir: str,
       continue
     seen.add(friendly_name)
     for sha, reverted_sha in reverts:
-      cherrypick_cl.do_cherrypick(
-          chroot_path=chroot_path,
-          create_cl=True,
-          start_sha=reverted_sha,
-          shas=[sha],
-          reviewers=reviewers,
-          cc=cc)
+      try:
+        cherrypick_cl.do_cherrypick(
+            chroot_path=chroot_path,
+            create_cl=True,
+            start_sha=reverted_sha,
+            shas=[sha],
+            reviewers=reviewers,
+            cc=cc)
+      except cherrypick_cl.CherrypickError as e:
+        logging.info('%s, skipping...', str(e))
   return new_state
 
 
diff --git a/llvm_tools/nightly_revert_checker_test.py b/llvm_tools/nightly_revert_checker_test.py
index 72c51f81..5efe2aeb 100755
--- a/llvm_tools/nightly_revert_checker_test.py
+++ b/llvm_tools/nightly_revert_checker_test.py
@@ -12,6 +12,7 @@ import io
 import unittest
 from unittest.mock import patch
 
+import cherrypick_cl
 import cros_utils.tiny_render as tiny_render
 import nightly_revert_checker
 import revert_checker
@@ -171,6 +172,26 @@ class Test(unittest.TestCase):
     do_cherrypick.assert_called_once()
     find_reverts.assert_called_once()
 
+  @patch('revert_checker.find_reverts')
+  @patch('cherrypick_cl.do_cherrypick')
+  def test_do_cherrypick_handles_cherrypick_error(self, do_cherrypick,
+                                                  find_reverts):
+    find_reverts.return_value = [
+        revert_checker.Revert('12345abcdef', 'fedcba54321')
+    ]
+    do_cherrypick.side_effect = cherrypick_cl.CherrypickError(
+        'Patch at 12345abcdef already exists in PATCHES.json')
+    nightly_revert_checker.do_cherrypick(
+        chroot_path='/path/to/chroot',
+        llvm_dir='/path/to/llvm',
+        interesting_shas=[('12345abcdef', 'fedcba54321')],
+        state={},
+        reviewers=['meow@chromium.org'],
+        cc=['purr@chromium.org'])
+
+    do_cherrypick.assert_called_once()
+    find_reverts.assert_called_once()
+
 
 if __name__ == '__main__':
   unittest.main()