Make prepare() accept a revision argument.

R=dpranke@chromium.org
TEST=few
BUG=

Review URL: http://codereview.chromium.org/6995115

git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@88779 0039d316-1c4b-4281-b951-d872f2087c98

checkout.py

@@ -80,11 +80,14 @@ class CheckoutBase(object):
   def get_settings(self, key):
     return get_code_review_setting(self.project_path, key)
 
-  def prepare(self):
+  def prepare(self, revision):
     """Checks out a clean copy of the tree and removes any local modification.
 
     This function shouldn't throw unless the remote repository is inaccessible,
     there is no free disk space or hard issues like that.
+
+    Args:
+      revision: The revision it should sync to, SCM specific.
     """
     raise NotImplementedError()
 
@@ -109,7 +112,7 @@ class RawCheckout(CheckoutBase):
 
   To be used by the try server.
   """
-  def prepare(self):
+  def prepare(self, revision):
     """Stubbed out."""
     pass
 
@@ -244,17 +247,13 @@ class SvnCheckout(CheckoutBase, SvnMixIn):
     self.svn_url = svn_url
     assert bool(self.commit_user) >= bool(self.commit_pwd)
 
-  def prepare(self):
+  def prepare(self, revision):
     # Will checkout if the directory is not present.
     assert self.svn_url
     if not os.path.isdir(self.project_path):
       logging.info('Checking out %s in %s' %
           (self.project_name, self.project_path))
-    revision = self._revert()
-    if revision != self._last_seen_revision:
-      logging.info('Updated at revision %d' % revision)
-      self._last_seen_revision = revision
-    return revision
+    return self._revert(revision)
 
   def apply_patch(self, patches):
     for p in patches:
@@ -351,11 +350,13 @@ class SvnCheckout(CheckoutBase, SvnMixIn):
           'Couldn\'t make sense out of svn commit message:\n' + out)
     return int(match.group(1))
 
-  def _revert(self):
+  def _revert(self, revision):
     """Reverts local modifications or checks out if the directory is not
     present. Use depot_tools's functionality to do this.
     """
     flags = ['--ignore-externals']
+    if revision:
+      flags.extend(['--revision', str(revision)])
     if not os.path.isdir(self.project_path):
       logging.info(
           'Directory %s is not present, checking it out.' % self.project_path)
@@ -365,9 +366,15 @@ class SvnCheckout(CheckoutBase, SvnMixIn):
       scm.SVN.Revert(self.project_path)
       # Revive files that were deleted in scm.SVN.Revert().
       self._check_call_svn(['update', '--force'] + flags)
+    return self._get_revision()
 
+  def _get_revision(self):
     out = self._check_output_svn(['info', '.'])
-    return int(self._parse_svn_info(out, 'revision'))
+    revision = int(self._parse_svn_info(out, 'revision'))
+    if revision != self._last_seen_revision:
+      logging.info('Updated to revision %d' % revision)
+      self._last_seen_revision = revision
+    return revision
 
 
 class GitCheckoutBase(CheckoutBase):
@@ -381,7 +388,7 @@ class GitCheckoutBase(CheckoutBase):
     self.remote_branch = remote_branch
     self.working_branch = 'working_branch'
 
-  def prepare(self):
+  def prepare(self, revision):
     """Resets the git repository in a clean state.
 
     Checks it out if not present and deletes the working branch.
@@ -389,12 +396,21 @@ class GitCheckoutBase(CheckoutBase):
     assert self.remote_branch
     assert os.path.isdir(self.project_path)
     self._check_call_git(['reset', '--hard', '--quiet'])
-    branches, active = self._branches()
-    if active != 'master':
-      self._check_call_git(['checkout', 'master', '--force', '--quiet'])
-    self._check_call_git(['pull', self.remote, self.remote_branch, '--quiet'])
-    if self.working_branch in branches:
-      self._call_git(['branch', '-D', self.working_branch])
+    if revision:
+      try:
+        revision = self._check_output_git(['rev-parse', revision])
+      except subprocess.CalledProcessError:
+        self._check_call_git(
+            ['fetch', self.remote, self.remote_branch, '--quiet'])
+        revision = self._check_output_git(['rev-parse', revision])
+      self._check_call_git(['checkout', '--force', '--quiet', revision])
+    else:
+      branches, active = self._branches()
+      if active != 'master':
+        self._check_call_git(['checkout', '--force', '--quiet', 'master'])
+      self._check_call_git(['pull', self.remote, self.remote_branch, '--quiet'])
+      if self.working_branch in branches:
+        self._call_git(['branch', '-D', self.working_branch])
 
   def apply_patch(self, patches):
     """Applies a patch on 'working_branch' and switch to it.
@@ -510,25 +526,37 @@ class GitSvnCheckoutBase(GitCheckoutBase, SvnMixIn):
     assert self.trunk
     self._cache_svn_auth()
 
-  def prepare(self):
+  def prepare(self, revision):
     """Resets the git repository in a clean state."""
     self._check_call_git(['reset', '--hard', '--quiet'])
-    branches, active = self._branches()
-    if active != 'master':
-      if not 'master' in branches:
+    if revision:
+      try:
+        revision = self._check_output_git(
+            ['svn', 'find-rev', 'r%d' % revision])
+      except subprocess.CalledProcessError:
         self._check_call_git(
-            ['checkout', '--quiet', '-b', 'master',
-             '%s/%s' % (self.remote, self.remote_branch)])
-      else:
-        self._check_call_git(['checkout', 'master', '--force', '--quiet'])
-    # git svn rebase --quiet --quiet doesn't work, use two steps to silence it.
-    self._check_call_git_svn(['fetch', '--quiet', '--quiet'])
-    self._check_call_git(
-        ['rebase', '--quiet', '--quiet',
-          '%s/%s' % (self.remote, self.remote_branch)])
-    if self.working_branch in branches:
-      self._call_git(['branch', '-D', self.working_branch])
-    return int(self._git_svn_info('revision'))
+            ['fetch', self.remote, self.remote_branch, '--quiet'])
+        revision = self._check_output_git(
+            ['svn', 'find-rev', 'r%d' % revision])
+      super(GitSvnCheckoutBase, self).prepare(revision)
+    else:
+      branches, active = self._branches()
+      if active != 'master':
+        if not 'master' in branches:
+          self._check_call_git(
+              ['checkout', '--quiet', '-b', 'master',
+              '%s/%s' % (self.remote, self.remote_branch)])
+        else:
+          self._check_call_git(['checkout', 'master', '--force', '--quiet'])
+      # git svn rebase --quiet --quiet doesn't work, use two steps to silence
+      # it.
+      self._check_call_git_svn(['fetch', '--quiet', '--quiet'])
+      self._check_call_git(
+          ['rebase', '--quiet', '--quiet',
+            '%s/%s' % (self.remote, self.remote_branch)])
+      if self.working_branch in branches:
+        self._call_git(['branch', '-D', self.working_branch])
+    return self._get_revision()
 
   def _git_svn_info(self, key):
     """Calls git svn info. This doesn't support nor need --config-dir."""
@@ -573,7 +601,7 @@ class GitSvnCheckoutBase(GitCheckoutBase, SvnMixIn):
   def _get_revision(self):
     revision = int(self._git_svn_info('revision'))
     if revision != self._last_seen_revision:
-      logging.info('Updated at revision %d' % revision)
+      logging.info('Updated to revision %d' % revision)
       self._last_seen_revision = revision
     return revision
 
@@ -595,7 +623,7 @@ class GitSvnPremadeCheckout(GitSvnCheckoutBase):
     self.git_url = git_url
     assert self.git_url
 
-  def prepare(self):
+  def prepare(self, revision):
     """Creates the initial checkout for the repo."""
     if not os.path.isdir(self.project_path):
       logging.info('Checking out %s in %s' %
@@ -619,8 +647,7 @@ class GitSvnPremadeCheckout(GitSvnCheckoutBase):
            '-T', self.trunk,
            self.svn_url])
     self._check_call_git_svn(['fetch'])
-    super(GitSvnPremadeCheckout, self).prepare()
-    return self._get_revision()
+    return super(GitSvnPremadeCheckout, self).prepare(revision)
 
 
 class GitSvnCheckout(GitSvnCheckoutBase):
@@ -637,8 +664,9 @@ class GitSvnCheckout(GitSvnCheckoutBase):
         commit_user, commit_pwd,
         svn_url, trunk, post_processors)
 
-  def prepare(self):
+  def prepare(self, revision):
     """Creates the initial checkout for the repo."""
+    assert not revision, 'Implement revision if necessary'
     if not os.path.isdir(self.project_path):
       logging.info('Checking out %s in %s' %
           (self.project_name, self.project_path))
@@ -652,8 +680,7 @@ class GitSvnCheckout(GitSvnCheckoutBase):
            '--quiet'],
           cwd=self.root_dir,
           stderr=subprocess2.STDOUT)
-    super(GitSvnCheckout, self).prepare()
-    return self._get_revision()
+    return super(GitSvnCheckout, self).prepare(revision)
 
 
 class ReadOnlyCheckout(object):
@@ -661,8 +688,8 @@ class ReadOnlyCheckout(object):
   def __init__(self, checkout):
     self.checkout = checkout
 
-  def prepare(self):
-    return self.checkout.prepare()
+  def prepare(self, revision):
+    return self.checkout.prepare(revision)
 
   def get_settings(self, key):
     return self.checkout.get_settings(key)

tests/checkout_test.py

@@ -181,7 +181,7 @@ class BaseTest(fake_repos.FakeReposTestBase):
       self.FAKE_REPOS.svn_dirty = True
 
     self.assertEquals(root, co.project_path)
-    self.assertEquals(self.previous_log['revision'], co.prepare())
+    self.assertEquals(self.previous_log['revision'], co.prepare(None))
     self.assertEquals('pouet', co.get_settings('bar'))
     self.assertTree(self.get_trunk(False), root)
     patches = self.get_patches()
@@ -206,13 +206,13 @@ class BaseTest(fake_repos.FakeReposTestBase):
 
     if read_only:
       self.assertEquals('FAKE', revision)
-      self.assertEquals(self.previous_log['revision'], co.prepare())
+      self.assertEquals(self.previous_log['revision'], co.prepare(None))
       # Changes should be reverted now.
       self.assertTree(self.get_trunk(False), root)
       expected = self.previous_log
     else:
       self.assertEquals(self.previous_log['revision'] + 1, revision)
-      self.assertEquals(self.previous_log['revision'] + 1, co.prepare())
+      self.assertEquals(self.previous_log['revision'] + 1, co.prepare(None))
       self.assertTree(self.get_trunk(True), root)
       expected = expected.copy()
       expected['msg'] = 'msg'
@@ -223,7 +223,7 @@ class BaseTest(fake_repos.FakeReposTestBase):
     self.assertEquals(expected, actual)
 
   def _check_exception(self, co, err_msg):
-    co.prepare()
+    co.prepare(None)
     try:
       co.apply_patch([patch.FilePatchDiff('svn_utils_test.txt', BAD_PATCH, [])])
       self.fail()
@@ -237,7 +237,7 @@ class BaseTest(fake_repos.FakeReposTestBase):
   def _test_process(self, co):
     """Makes sure the process lambda is called correctly."""
     co.post_processors = [lambda *args: results.append(args)]
-    co.prepare()
+    co.prepare(None)
     ps = self.get_patches()
     results = []
     co.apply_patch(ps)
@@ -281,6 +281,9 @@ class SvnBaseTest(BaseTest):
         data['revprops'].append((prop.attrib['name'], prop.text))
     return data
 
+  def _test_prepare(self, co):
+    self.assertEquals(1, co.prepare(1))
+
 
 class SvnCheckout(SvnBaseTest):
   def _get_co(self, read_only):
@@ -317,7 +320,7 @@ class SvnCheckout(SvnBaseTest):
 
   def testSvnProps(self):
     co = self._get_co(False)
-    co.prepare()
+    co.prepare(None)
     try:
       # svn:ignore can only be applied to directories.
       svn_props = [('svn:ignore', 'foo')]
@@ -332,7 +335,7 @@ class SvnCheckout(SvnBaseTest):
           '--non-interactive;\n'
           'patching file svn_utils_test.txt\n'
           'svn: Cannot set \'svn:ignore\' on a file (\'svn_utils_test.txt\')\n')
-    co.prepare()
+    co.prepare(None)
     svn_props = [('svn:eol-style', 'LF'), ('foo', 'bar')]
     co.apply_patch(
         [patch.FilePatchDiff('svn_utils_test.txt', NAKED_PATCH, svn_props)])
@@ -379,7 +382,7 @@ class SvnCheckout(SvnBaseTest):
     co = self._get_co(False)
     co.svn_config = checkout.SvnConfig(
         os.path.join(ROOT_DIR, 'subversion_config'))
-    co.prepare()
+    co.prepare(None)
     patches = self.get_patches()
     co.apply_patch(patches)
     self.assertEquals(
@@ -398,6 +401,13 @@ class SvnCheckout(SvnBaseTest):
         self.svn_url)
     self._test_process(co)
 
+  def testPrepare(self):
+    co = checkout.SvnCheckout(
+        self.root_dir, self.name,
+        None, None,
+        self.svn_url)
+    self._test_prepare(co)
+
 
 class GitSvnCheckout(SvnBaseTest):
   name = 'foo.git'
@@ -430,7 +440,7 @@ class GitSvnCheckout(SvnBaseTest):
   def testGitSvnPremade(self):
     # Test premade git-svn clone. First make a git-svn clone.
     git_svn_co = self._get_co(True)
-    revision = git_svn_co.prepare()
+    revision = git_svn_co.prepare(None)
     self.assertEquals(self.previous_log['revision'], revision)
     # Then use GitSvnClone to clone it to lose the git-svn connection and verify
     # git svn init / git svn fetch works.
@@ -438,7 +448,8 @@ class GitSvnCheckout(SvnBaseTest):
         self.root_dir, self.name[:-4] + '2', 'trunk',
         self.usr, self.pwd,
         self.svn_base, self.svn_trunk, git_svn_co.project_path)
-    self.assertEquals(self.previous_log['revision'], git_svn_clone.prepare())
+    self.assertEquals(
+        self.previous_log['revision'], git_svn_clone.prepare(None))
 
   def testException(self):
     self._check_exception(
@@ -446,7 +457,7 @@ class GitSvnCheckout(SvnBaseTest):
 
   def testSvnProps(self):
     co = self._get_co(False)
-    co.prepare()
+    co.prepare(None)
     try:
       svn_props = [('foo', 'bar')]
       co.apply_patch(
@@ -457,7 +468,7 @@ class GitSvnCheckout(SvnBaseTest):
       self.assertEquals(
           e.status,
           'Cannot apply svn property foo to file svn_utils_test.txt.')
-    co.prepare()
+    co.prepare(None)
     # svn:eol-style is ignored.
     svn_props = [('svn:eol-style', 'LF')]
     co.apply_patch(
@@ -470,6 +481,13 @@ class GitSvnCheckout(SvnBaseTest):
         self.svn_url)
     self._test_process(co)
 
+  def testPrepare(self):
+    co = checkout.SvnCheckout(
+        self.root_dir, self.name,
+        None, None,
+        self.svn_url)
+    self._test_prepare(co)
+
 
 class RawCheckout(SvnBaseTest):
   def setUp(self):
@@ -477,7 +495,7 @@ class RawCheckout(SvnBaseTest):
     # Use a svn checkout as the base.
     self.base_co = checkout.SvnCheckout(
         self.root_dir, self.name, None, None, self.svn_url)
-    self.base_co.prepare()
+    self.base_co.prepare(None)
 
   def _get_co(self, read_only):
     co = checkout.RawCheckout(self.root_dir, self.name, None)
@@ -491,7 +509,7 @@ class RawCheckout(SvnBaseTest):
 
     # A copy of BaseTest._check_base()
     self.assertEquals(root, co.project_path)
-    self.assertEquals(None, co.prepare())
+    self.assertEquals(None, co.prepare(None))
     self.assertEquals('pouet', co.get_settings('bar'))
     self.assertTree(self.get_trunk(False), root)
     patches = self.get_patches()
@@ -513,7 +531,7 @@ class RawCheckout(SvnBaseTest):
         pass
     self.assertTree(self.get_trunk(True), root)
     # Verify that prepare() is a no-op.
-    self.assertEquals(None, co.prepare())
+    self.assertEquals(None, co.prepare(None))
     self.assertTree(self.get_trunk(True), root)
 
   def testAllRW(self):
@@ -537,6 +555,13 @@ class RawCheckout(SvnBaseTest):
         self.svn_url)
     self._test_process(co)
 
+  def testPrepare(self):
+    co = checkout.SvnCheckout(
+        self.root_dir, self.name,
+        None, None,
+        self.svn_url)
+    self._test_prepare(co)
+
 
 if __name__ == '__main__':
   if '-v' in sys.argv: