autoland: refactor mozreview transplant into subclass (bug 1368516) r?smacleod draft
authorbyron jones <glob@mozilla.com>
Wed, 26 Jul 2017 15:50:57 +0800
changeset 11687 5ef83a3f1a74b9a89233ae3278bd3ec081d677a9
parent 11686 c56e3c87507a40565d06c4078a2788fed57ce88d
child 11688 39c03e85555f860d26d6a204f74eef14226c9aad
push id1790
push userbjones@mozilla.com
push dateTue, 19 Sep 2017 04:17:41 +0000
reviewerssmacleod
bugs1368516
autoland: refactor mozreview transplant into subclass (bug 1368516) r?smacleod In preparation for landing patch based requests, refactor the transplant class into a base Transplant class, and one that implements repository based landings. MozReview-Commit-ID: FzFqXXtNg3d
autoland/autoland/autoland.py
autoland/autoland/transplant.py
--- a/autoland/autoland/autoland.py
+++ b/autoland/autoland/autoland.py
@@ -12,17 +12,17 @@ import time
 import traceback
 
 sys.path.insert(0, os.path.normpath(os.path.join(os.path.normpath(
                 os.path.abspath(os.path.dirname(__file__))), '..',
                                                              '..',
                                                              'pylib',
                                                              'mozautomation')))
 
-from transplant import Transplant
+from transplant import RepoTransplant
 import treestatus
 
 
 # max attempts to transplant before bailing
 MAX_TRANSPLANT_ATTEMPTS = 50
 
 # max updates to post to reviewboard / iteration
 MOZREVIEW_COMMENT_LIMIT = 10
@@ -123,31 +123,26 @@ def handle_pending_transplants(dbconn):
                         'to destination: %s, attempt %s' % (
                             tree, rev, destination, attempts + 1))
 
             if patch_urls:
                 result = 'patch based landings not implemented'
                 landed = False
                 break
 
-            # TODO: We should break the transplant call into two steps, one
-            #       to pull down the commits to transplant, and another
-            #       one to rebase it and attempt to push so we don't
-            #       duplicate work unnecessarily if we have to rebase more
-            #       than once.
             os.environ['AUTOLAND_REQUEST_USER'] = requester
             try:
-                with Transplant(tree, destination, rev) as tp:
+                with RepoTransplant(tree, destination, rev,
+                                    commit_descriptions) as tp:
                     if trysyntax:
                         result = tp.push_try(str(trysyntax))
                     elif push_bookmark:
-                        result = tp.push_bookmark(commit_descriptions,
-                                                  push_bookmark)
+                        result = tp.push_bookmark(push_bookmark)
                     else:
-                        result = tp.push(commit_descriptions)
+                        result = tp.push()
                 landed = True
             except Exception as e:
                 result = str(e)
                 landed = False
             finally:
                 del os.environ['AUTOLAND_REQUEST_USER']
 
             logger.info('transplant from tree: %s rev: %s attempt: %s: %s' % (
--- a/autoland/autoland/transplant.py
+++ b/autoland/autoland/transplant.py
@@ -1,13 +1,12 @@
 import config
 import hglib
 import json
 import logging
-import os
 import re
 import tempfile
 
 REPO_CONFIG = {}
 
 logger = logging.getLogger('autoland')
 
 
@@ -43,51 +42,53 @@ class Transplant(object):
     def __exit__(self, exc_type, exc_val, exc_tb):
         self.strip_drafts()
         self.hg_repo.close()
 
     def push_try(self, trysyntax):
         # Don't let unicode leak into command arguments.
         assert isinstance(trysyntax, str), "trysyntax arg is not str"
 
-        self.update_repo()
+        remote_tip = self.update_repo()
+
+        self.apply_changes(remote_tip)
 
         if not trysyntax.startswith("try: "):
             trysyntax = "try: %s" % trysyntax
         rev = self.run_hg_cmds([
             [
                 '--encoding=utf-8',
                 '--config', 'ui.allowemptycommit=true',
                 'commit',
                 '-m', trysyntax
             ],
             ['push', '-r', '.', '-f', 'try'],
             ['log', '-r', 'tip', '-T', '{node|short}'],
         ])
 
         return rev
 
-    def push_bookmark(self, commit_descriptions, bookmark):
+    def push_bookmark(self, bookmark):
         # Don't let unicode leak into command arguments.
         assert isinstance(bookmark, str), "bookmark arg is not str"
 
         remote_tip = self.update_repo()
 
-        rev = self.apply_changes(remote_tip, commit_descriptions)
+        rev = self.apply_changes(remote_tip)
         self.run_hg_cmds([
             ['bookmark', bookmark],
             ['push', '-B', bookmark, self.destination],
         ])
 
         return rev
 
-    def push(self, commit_descriptions):
+    def push(self):
         remote_tip = self.update_repo()
 
-        rev = self.apply_changes(remote_tip, commit_descriptions)
+        rev = self.apply_changes(remote_tip)
         self.run_hg_cmds([
             ['push', '-r', 'tip', self.destination]
         ])
 
         return rev
 
     def update_repo(self):
         # Obtain remote tip. We assume there is only a single head.
@@ -96,25 +97,18 @@ class Transplant(object):
         # Strip any lingering draft changesets.
         self.strip_drafts()
 
         # Pull from "upstream".
         self.update_from_upstream(remote_tip)
 
         return remote_tip
 
-    def apply_changes(self, remote_tip, commit_descriptions):
-        base_revision = self.rewrite_commit_descriptions(commit_descriptions)
-        logger.info('base revision: %s' % base_revision)
-
-        base_revision = self.rebase(base_revision, remote_tip)
-        logger.info('rebased (tip) revision: %s' % base_revision)
-
-        self.validate_descriptions(commit_descriptions)
-        return base_revision
+    def apply_changes(self, remote_tip):
+        raise NotImplemented('abstract method call: apply_changes')
 
     def run_hg(self, args):
         logger.info('rev: %s: executing: %s' % (self.source_rev, args))
         out = hglib.util.BytesIO()
         out_channels = {b'o': out.write, b'e': out.write}
         ret = self.hg_repo.runcommand(args, {}, out_channels)
         out = out.getvalue()
         if ret:
@@ -146,65 +140,32 @@ class Transplant(object):
         remote_tip = self.run_hg_cmds([
             ['identify', 'upstream', '-r', 'tip']
         ])
         remote_tip = remote_tip.split()[0]
         assert len(remote_tip) == 12, remote_tip
         return remote_tip
 
     def update_from_upstream(self, remote_rev):
-        # Pull "upstream" and update to remote tip. Pull revisions to land and
-        # update to them.
+        # Pull "upstream" and update to remote tip.
         cmds = [['pull', 'upstream'],
                 ['rebase', '--abort', '-r', remote_rev],
-                ['update', '--clean', '-r', remote_rev],
-                ['pull', self.tree, '-r', self.source_rev],
-                ['update', self.source_rev]]
+                ['update', '--clean', '-r', remote_rev]]
 
         for cmd in cmds:
             try:
                 self.run_hg(cmd)
             except hglib.error.CommandError as e:
                 output = e.out
-                if 'no changes found' in output:
-                    # we've already pulled this revision
-                    continue
-                elif 'abort: no rebase in progress' in output:
+                if 'abort: no rebase in progress' in output:
                     # there was no rebase in progress, nothing to see here
                     continue
                 else:
                     raise HgCommandError(cmd, e.out)
 
-    def rewrite_commit_descriptions(self, commit_descriptions):
-        # Rewrite commit descriptions as per the mapping provided.  Returns the
-        # revision of the base commit.
-        assert commit_descriptions
-
-        with tempfile.NamedTemporaryFile() as f:
-            json.dump(commit_descriptions, f)
-            f.flush()
-
-            cmd_output = self.run_hg_cmds([
-                ['rewritecommitdescriptions', '--descriptions=%s' % f.name,
-                 self.source_rev]
-            ])
-
-            base_revision = None
-            for line in cmd_output.splitlines():
-                m = re.search(r'^rev: [0-9a-z]+ -> ([0-9a-z]+)', line)
-                if m and m.groups():
-                    base_revision = m.groups()[0]
-                    break
-
-            if not base_revision:
-                raise Exception('Could not determine base revision for '
-                                'rebase: %s' % cmd_output)
-
-            return base_revision
-
     def rebase(self, base_revision, remote_tip):
         # Perform rebase if necessary. Returns tip revision.
         cmd = ['rebase', '-s', base_revision, '-d', remote_tip]
 
         assert len(remote_tip) == 12
 
         # If rebasing onto the null revision, force the merge policy to take
         # our content, as there is no content in the destination to conflict
@@ -217,22 +178,84 @@ class Transplant(object):
         except hglib.error.CommandError as e:
             if 'nothing to rebase' not in e.out:
                 raise HgCommandError(cmd, e.out)
 
         return self.run_hg_cmds([
             ['log', '-r', 'tip', '-T', '{node|short}']
         ])
 
-    def validate_descriptions(self, commit_descriptions):
+
+class RepoTransplant(Transplant):
+    def __init__(self, tree, destination, rev, commit_descriptions):
+        self.commit_descriptions = commit_descriptions
+
+        super(RepoTransplant, self).__init__(tree, destination, rev)
+
+    def apply_changes(self, remote_tip):
+        # Pull in changes from the source repo.
+        cmds = [['pull', self.tree, '-r', self.source_rev],
+                ['update', self.source_rev]]
+        for cmd in cmds:
+            try:
+                self.run_hg(cmd)
+            except hglib.error.CommandError as e:
+                output = e.out
+                if 'no changes found' in output:
+                    # we've already pulled this revision
+                    continue
+                else:
+                    raise HgCommandError(cmd, e.out)
+
+        # try runs don't have commit descriptions.
+        if not self.commit_descriptions:
+            return
+
+        base_revision = self.rewrite_commit_descriptions()
+        logger.info('base revision: %s' % base_revision)
+
+        base_revision = self.rebase(base_revision, remote_tip)
+        logger.info('rebased (tip) revision: %s' % base_revision)
+
+        self.validate_descriptions()
+        return base_revision
+
+    def rewrite_commit_descriptions(self):
+        # Rewrite commit descriptions as per the mapping provided.  Returns the
+        # revision of the base commit.
+
+        with tempfile.NamedTemporaryFile() as f:
+            json.dump(self.commit_descriptions, f)
+            f.flush()
+
+            cmd_output = self.run_hg_cmds([
+                ['rewritecommitdescriptions', '--descriptions=%s' % f.name,
+                 self.source_rev]
+            ])
+
+            base_revision = None
+            for line in cmd_output.splitlines():
+                m = re.search(r'^rev: [0-9a-z]+ -> ([0-9a-z]+)', line)
+                if m and m.groups():
+                    base_revision = m.groups()[0]
+                    break
+
+            if not base_revision:
+                raise Exception('Could not determine base revision for '
+                                'rebase: %s' % cmd_output)
+
+            return base_revision
+
+    def validate_descriptions(self):
         # Match outgoing commit descriptions against incoming commit
         # descriptions. If these don't match exactly, prevent the landing
         # from occurring.
-        incoming_descriptions = set([c.encode(self.hg_repo.encoding)
-                                     for c in commit_descriptions.values()])
+        incoming_descriptions = set(
+            [c.encode(self.hg_repo.encoding)
+             for c in self.commit_descriptions.values()])
         outgoing = self.hg_repo.outgoing('tip', self.destination)
         outgoing_descriptions = set([commit[5] for commit in outgoing])
 
         if incoming_descriptions ^ outgoing_descriptions:
             logger.error('unexpected outgoing commits:')
             for commit in outgoing:
                 logger.error('outgoing: %s: %s' % (commit[1], commit[5]))