Changes to support One-Click Revert in Commit Queue

pending_manager.py

Index: pending_manager.py
===================================================================
--- pending_manager.py	(revision 220012)
+++ pending_manager.py	(working copy)
@@ -46,6 +46,8 @@
   relpath = unicode
   # Only used after a patch was committed. Keeping here for try job retries.
   revision = (None, int, unicode)
+  revert = bool
+  reverted_by = unicode
 
   def __init__(self, **kwargs):
     super(PendingCommit, self).__init__(**kwargs)
@@ -82,7 +84,7 @@
       if not self.files:
         raise base.DiscardPending(
             self, 'No file was found in this patchset.')
-      context_obj.checkout.apply_patch(patches)
+      context_obj.checkout.apply_patch(patches, revert=self.revert)
     except (checkout.PatchApplicationFailed, patch.UnsupportedPatchFormat) as e:
       raise base.DiscardPending(self, str(e))
     except subprocess2.CalledProcessError as e:
@@ -145,26 +147,43 @@
   # Delay (secs) between commit bursts.
   COMMIT_BURST_DELAY = 10*60
 
-  def __init__(self, context_obj, pre_patch_verifiers, verifiers):
+  def __init__(self, context_obj, pre_patch_verifiers, verifiers,
+               pre_patch_revert_verifiers=None, revert_verifiers=None):
     """
     Args:
       pre_patch_verifiers: Verifiers objects that are run before applying the
                            patch.
       verifiers: Verifiers object run after applying the patch.
+      pre_patch_revert_verifiers: Verifiers objects that are run before applying
+                                  the patch for reverts.
+      revert_verifiers: Verifiers object run after applying the patch for
+                        reverts.
     """
+    # Atleast one commit verifier must be provided. Revert verifiers are not
+    # compulsory.
     assert len(pre_patch_verifiers) or len(verifiers)
     self.context = context_obj
+
     self.pre_patch_verifiers = pre_patch_verifiers or []
     self.verifiers = verifiers or []
     self.all_verifiers = pre_patch_verifiers + verifiers
+
+    self.pre_patch_revert_verifiers = pre_patch_revert_verifiers or []
+    self.revert_verifiers = revert_verifiers or []
+    self.all_revert_verifiers = (
+        self.pre_patch_revert_verifiers + self.revert_verifiers)
+
     self.queue = PendingQueue()
     # Keep the timestamps of the last few commits so that we can control the
     # pace (burstiness) of commits.
     self.recent_commit_timestamps = []
     # Assert names are unique.
     names = [x.name for x in pre_patch_verifiers + verifiers]
+    revert_names = [x.name for x in self.pre_patch_revert_verifiers +
+                    self.revert_verifiers]
     assert len(names) == len(set(names))
-    for verifier in self.pre_patch_verifiers:
+    assert len(revert_names) == len(set(revert_names))
+    for verifier in self.pre_patch_verifiers + self.pre_patch_revert_verifiers:
       assert not isinstance(verifier, base.VerifierCheckout)
 
   def look_for_new_pending_commit(self):
@@ -174,6 +193,10 @@
     """
     try:
       new_issues = self._fetch_pending_issues()
+      if self.context.check_for_reverts:
+        # New issues consist of both regular commits and reverts if the check
+        # for reverts flag is enabled.
+        new_issues.extend(self._fetch_pending_reverts())
 
       # If there is an issue in processed_issues that is not in new_issues,
       # discard it.
@@ -186,7 +209,9 @@
               pending,
               { 'verification': 'abort',
                 'payload': {
-                  'output': 'CQ bit was unchecked on CL. Ignoring.' }})
+                  'output': 'CQ or Revert bit was unchecked on CL. Ignoring.' }
+              }
+          )
           pending.get_state = lambda: base.IGNORED
           self._discard_pending(pending, None)
 
@@ -197,8 +222,14 @@
               issue_id, True)
           # This assumption needs to hold.
           assert issue_id == issue_data['issue']
-          if issue_data['patchsets'] and issue_data['commit']:
+          if issue_data['patchsets'] and (
+              issue_data['commit'] or issue_data['revert']):
             logging.info('Found new issue %d' % issue_id)
+
+            reverted_by = issue_data.get('reverted_by')
+            if not reverted_by:
+              reverted_by = ''
+
             self.queue.add(
                 PendingCommit(
                     issue=issue_id,
@@ -207,7 +238,9 @@
                     patchset=issue_data['patchsets'][-1],
                     base_url=issue_data['base_url'],
                     description=issue_data['description'].replace('\r', ''),
-                    messages=issue_data['messages']))
+                    messages=issue_data['messages'],
+                    revert=issue_data.get('revert', False),
+                    reverted_by=reverted_by))
     except Exception as e:
       traceback.print_exc()
       # Swallow every exception in that code and move on. Make sure to send a
@@ -220,6 +253,11 @@
     """
     return self.context.rietveld.get_pending_issues()
 
+  def _fetch_pending_reverts(self):
+    """Returns the list of issue number for reviews on Rietveld with their last
+    patchset with revert+ flag set."""
+    return self.context.rietveld.get_revert_issues()
+
   def process_new_pending_commit(self):
     """Starts verification on newly found pending commits."""
     expected = set(i.name for i in self.all_verifiers)
@@ -306,10 +344,13 @@
     """Initiates all the verifiers on a pending change."""
     # Do not apply the patch if not necessary. It will be applied at commit
     # time anyway so if the patch doesn't apply, it'll be catch later.
-    if not self._pending_run_verifiers(pending, self.pre_patch_verifiers):
+    if not self._pending_run_verifiers(
+        pending,
+        self.pre_patch_revert_verifiers if pending.revert else
+        self.pre_patch_verifiers):
       return
 
-    if self.verifiers:
+    if self.verifiers or self.revert_verifiers:
       pending.prepare_for_patch(self.context)
 
     # This CL is real business, alert the user that we're going to try his
@@ -325,12 +366,14 @@
           self.context.status.url, pending.owner,
           pending.issue, pending.patchset))
 
-    if self.verifiers:
+    if self.verifiers or self.revert_verifiers:
       pending.apply_patch(self.context, False)
       previous_cwd = os.getcwd()
       try:
         os.chdir(self.context.checkout.project_path)
-        self._pending_run_verifiers(pending, self.verifiers)
+        self._pending_run_verifiers(
+            pending,
+            self.revert_verifiers if pending.revert else self.verifiers)
       finally:
         os.chdir(previous_cwd)
 
@@ -372,10 +415,20 @@
     """Does last minute checks on Rietvld before committing a pending patch."""
     pending_data = self.context.rietveld.get_issue_properties(
         pending.issue, True)
-    if pending_data['commit'] != True:
-      raise base.DiscardPending(pending, None)
-    if pending_data['closed'] != False:
-      raise base.DiscardPending(pending, None)
+    if pending.revert:
+      # For revert requests check that revert bit is checked and that the issue
+      # is closed.
+      if pending_data['revert'] != True:
+        raise base.DiscardPending(pending, None)
+      if pending_data['closed'] != True:
+        raise base.DiscardPending(pending, None)
+    else:
+      # For regular commit requests check that the commit bit is checked and
+      # that the issue is not closed.
+      if pending_data['commit'] != True:
+        raise base.DiscardPending(pending, None)
+      if pending_data['closed'] != False:
+        raise base.DiscardPending(pending, None)
     if pending.description != pending_data['description'].replace('\r', ''):
       raise base.DiscardPending(pending, self.DESCRIPTION_UPDATED)
     commit_user = set([self.context.rietveld.email])
@@ -404,8 +457,14 @@
     try:
       try:
         if pending.get_state() != base.IGNORED:
-          self.context.rietveld.set_flag(
-              pending.issue, pending.patchset, 'commit', 'False')
+          # Flip revert bit to False if it was a revert request else flip the
+          # commit bit to False.
+          if pending.revert:
+            self.context.rietveld.set_flag(
+                pending.issue, pending.patchset, 'revert', 'False')
+          else:
+            self.context.rietveld.set_flag(
+                pending.issue, pending.patchset, 'commit', 'False')
       except urllib2.HTTPError as e:
         logging.error(
             'Failed to set the flag to False for %s with message %s' % (
@@ -449,6 +508,8 @@
         commit_desc.append_footer('Review URL: %s/%s' % (
             self.context.rietveld.url,
             pending.issue))
+        if pending.revert:
+          commit_desc.update_as_reverted()
         pending.revision = self.context.checkout.commit(
             commit_desc.description, pending.owner)
         if not pending.revision:
@@ -463,8 +524,13 @@
         issue_desc = git_cl.ChangeDescription(pending.description)
         msg = 'Committed: %s' % pending.revision
         if viewvc_url:
-          viewvc_url = '%s%s' % (viewvc_url.rstrip('/'), pending.revision)
-          msg = 'Committed: %s' % viewvc_url
+          if 'googlesource' not in viewvc_url:
+            viewvc_url = viewvc_url.rstrip('/')
+          viewvc_url = '%s%s' % (viewvc_url, pending.revision)
+          if pending.revert:
+            msg = 'Reverted: %s' % viewvc_url
+          else:
+            msg = 'Committed: %s' % viewvc_url
           issue_desc.append_footer(msg)
 
         # Update the CQ dashboard.
@@ -479,11 +545,18 @@
         # Closes the issue on Rietveld.
         # TODO(csharp): Retry if exceptions are encountered.
         try:
-          self.context.rietveld.close_issue(pending.issue)
+          if pending.revert:
+            self.context.rietveld.open_issue(pending.issue)
+          else:
+            self.context.rietveld.close_issue(pending.issue)
           self.context.rietveld.update_description(
               pending.issue, issue_desc.description)
-          self.context.rietveld.add_comment(
-              pending.issue, 'Change committed as %s' % pending.revision)
+          if pending.revert:
+            self.context.rietveld.add_comment(
+                pending.issue, 'Change reverted as %s' % pending.revision)
+          else:
+            self.context.rietveld.add_comment(
+                pending.issue, 'Change committed as %s' % pending.revision)
         except (urllib2.HTTPError, urllib2.URLError) as e:
           # Ignore AppEngine flakiness.
           logging.warning('Unable to fully close the issue')