Changes to support One-Click Revert in Commit Queue

pending_manager.py

 # coding=utf8
 # Copyright (c) 2012 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 """Commit queue manager class.
 
 Security implications:
 
 The following hypothesis are made:
 - Commit queue:
@@ skipping 28 matching lines @@
   description = unicode
   files = list
   # Only a cache, these values can be regenerated.
   owner = unicode
   reviewers = list
   base_url = unicode
   messages = list
   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)
     for message in self.messages:
       # Save storage, no verifier really need 'text', just 'approval'.
       if 'text' in message:
         del message['text']
 
   def pending_name(self):
     """The name that should be used for try jobs.
@@ skipping 16 matching lines @@
       patches = context_obj.rietveld.get_patch(self.issue, self.patchset)
       if not patches:
         raise base.DiscardPending(
             self, 'No diff was found for this patchset.')
       if self.relpath:
         patches.set_relpath(self.relpath)
       self.files = [p.filename for p in patches]
       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:
       out = 'Failed to apply the patch.'
       if e.stdout:
         out += '\n%s' % e.stdout
       raise base.DiscardPending(self, out)
     except urllib2.HTTPError as e:
       raise base.DiscardPending(
           self,
@@ skipping 42 matching lines @@
   DESCRIPTION_UPDATED = (
       'Commit queue rejected this change because the description was changed\n'
       'between the time the change entered the commit queue and the time it\n'
       'was ready to commit. You can safely check the commit box again.')
   TRYING_PATCH = 'CQ is trying da patch. Follow status at\n'
   # Maximum number of commits done in a burst.
   MAX_COMMIT_BURST = 4
   # 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):
     """Looks for new reviews on self.context.rietveld with c+ set.
 
     Calls _new_pending_commit() on all new review found.
     """
     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.
       for pending in self.queue.iterate():
         # Note that pending.issue is a int but self.queue.pending_commits keys
         # are str due to json support.
         if pending.issue not in new_issues:
           logging.info('Flushing issue %d' % pending.issue)
           self.context.status.send(
               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)
 
       # Find new issues.
       for issue_id in new_issues:
         if str(issue_id) not in self.queue.pending_commits:
           issue_data = self.context.rietveld.get_issue_properties(
               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,
                     owner=issue_data['owner_email'],
                     reviewers=issue_data['reviewers'],
                     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
       # stack trace though.
       errors.send_stack(e)
 
   def _fetch_pending_issues(self):
     """Returns the list of issue number for reviews on Rietveld with their last
     patchset with commit+ flag set.
     """
     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)
     for pending in self.queue.iterate():
       try:
         # Take in account the case where a verifier was removed.
         done = set(pending.verifications.keys())
         missing = expected - done
         if (not missing or pending.get_state() != base.PROCESSING):
           continue
@@ skipping 66 matching lines @@
         # When state is IGNORED, we need to keep this issue so it's not fetched
         # another time but we can't discard it since we don't want to remove the
         # commit bit for another project hosted on the same code review
         # instance.
         assert state in (base.PROCESSING, base.IGNORED)
 
   def _verify_pending(self, pending):
     """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
     # patch.  Note that this is done *after* syncing but *before* applying the
     # patch.
     self.context.status.send(
         pending,
         { 'verification': 'initial',
           'payload': {'revision': pending.revision}})
     self.context.rietveld.add_comment(
         pending.issue,
         self.TRYING_PATCH + '%s/%s/%d/%d\n' % (
           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)
 
     # Send the initial 'why not' message.
     if pending.why_not():
       self.context.status.send(
           pending,
           {'verification': 'why not',
            'payload': {'message': pending.why_not()}})
 
@@ skipping 21 matching lines @@
       if pending.get_state() == base.FAILED:
         # Throw if it didn't pass, so the error message is not lost.
         raise base.DiscardPending(
             pending, pending.error_message() or cls.FAILED_NO_MESSAGE)
     return True
 
   def _last_minute_checks(self, pending):
     """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])
     expected = set(pending.reviewers) - commit_user
     actual  = set(pending_data['reviewers']) - commit_user
     # Try to be nice, if there was a drive-by review and the new reviewer left
     # a lgtm, don't abort.
     def is_approver(r):
       return any(
           m.get('approval') for m in pending_data['messages']
           if m['sender'] == r)
     drivers_by = [r for r in (actual - expected) if not is_approver(r)]
     if drivers_by:
       # That annoying driver-by.
       raise base.DiscardPending(
           pending,
           'List of reviewers changed. %s did a drive-by without LGTM\'ing!' %
           ','.join(drivers_by))
     if pending.patchset != pending_data['patchsets'][-1]:
       raise base.DiscardPending(pending,
           'Commit queue failed due to new patchset.')
 
   def _discard_pending(self, pending, message):
     """Discards a pending commit. Attach an optional message to the review."""
     logging.debug('_discard_pending(%s, %s)', pending.issue, message)
     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' % (
               pending.pending_name(), message))
         traceback.print_stack()
         errors.send_stack(e)
       if message:
         try:
           self.context.rietveld.add_comment(pending.issue, message)
         except urllib2.HTTPError as e:
@@ skipping 23 matching lines @@
         pending.apply_patch(self.context, True)
         # Commit it.
         commit_desc = git_cl.ChangeDescription(pending.description)
         if (self.context.server_hooks_missing and
             self.context.rietveld.email != pending.owner):
           commit_desc.update_reviewers(pending.reviewers)
           commit_desc.append_footer('Author: ' + pending.owner)
         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:
           raise base.DiscardPending(pending, 'Failed to commit patch.')
 
         # Note that the commit succeeded for commit throttling.
         self.recent_commit_timestamps.append(time.time())
         self.recent_commit_timestamps = (
             self.recent_commit_timestamps[-(self.MAX_COMMIT_BURST + 1):])
 
         viewvc_url = self.context.checkout.get_settings('VIEW_VC')
         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.
         self.context.status.send(
             pending,
             { 'verification': 'commit',
               'payload': {
                 'revision': pending.revision,
                 'output': msg,
                 'url': viewvc_url}})
 
         # 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')
         # And finally remove the issue. If the close_issue() call above failed,
         # it is possible the dashboard will be confused but it is harmless.
         try:
           self.queue.get(pending.issue)
         except KeyError:
           logging.error('Internal inconsistency for %d', pending.issue)
         self.queue.remove(pending.issue)
@@ skipping 45 matching lines @@
     """Loads the commit queue state from a JSON file."""
     self.queue = model.load_from_json_file(filename)
 
   def save(self, filename):
     """Save the commit queue state in a simple JSON file."""
     model.save_to_json_file(filename, self.queue)
 
   def close(self):
     """Close all the active pending manager items."""
     self.context.status.close()