commit 2e0f7c655333c4336937f72a6ceca9d9fe1f4ea9
author Christopher Di Bella <cjdb@google.com> Wed Feb 24 02:25:18 2021 +0000
committer Commit Bot <commit-bot@chromium.org> Wed Feb 24 07:46:28 2021 +0000
tree fe02824ce6f27f3009ebcc9a2efd5892751915b3
parent 3645b9850bc5a6a6c2d95a47636a463c6d7af8fe

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>

llvm_tools/cherrypick_cl.py

diff --git a/llvm_tools/cherrypick_cl.py b/llvm_tools/cherrypick_cl.py
index ff385f9..ebf7c0f 100755
--- a/llvm_tools/cherrypick_cl.py
+++ b/llvm_tools/cherrypick_cl.py
@@ -27,6 +27,10 @@
 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 @@
   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)

llvm_tools/nightly_revert_checker.py

diff --git a/llvm_tools/nightly_revert_checker.py b/llvm_tools/nightly_revert_checker.py
index 6b4ac53..21dcf94 100755
--- a/llvm_tools/nightly_revert_checker.py
+++ b/llvm_tools/nightly_revert_checker.py
@@ -231,13 +231,16 @@
       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
 
 

llvm_tools/nightly_revert_checker_test.py

diff --git a/llvm_tools/nightly_revert_checker_test.py b/llvm_tools/nightly_revert_checker_test.py
index 72c51f8..5efe2ae 100755
--- a/llvm_tools/nightly_revert_checker_test.py
+++ b/llvm_tools/nightly_revert_checker_test.py
@@ -12,6 +12,7 @@
 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 @@
     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()