Reland "gclient: delete unversioned directory before adding cipd dep for the same path"

This is a reland of 67ef3f67e816839ce8b3984ecba9406961583eff

We no longer call cipd unconditionally.

Original change's description:
> gclient: delete unversioned directory before adding cipd dep for the same path
>
> Bug: 882611
> Change-Id: I46e41cc9693b90874b5d6569a12ec638eaac1050
> Reviewed-on: https://chromium-review.googlesource.com/1228655
> Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
> Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>

Bug: 882611
Change-Id: I683bfc62bd1eebfec0853583f96f3981c2c6bdf2
Reviewed-on: https://chromium-review.googlesource.com/1232891
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>

gclient.py

@@ -1614,9 +1614,6 @@ it or fix the checkout.
               patch_repo + '@' + patch_ref
               for patch_repo, patch_ref in patch_refs.iteritems())))
 
-    if self._cipd_root:
-      self._cipd_root.run(command)
-
     # Once all the dependencies have been processed, it's now safe to write
     # out the gn_args_file and run the hooks.
     if command == 'update':
@@ -1724,6 +1721,13 @@ it or fix the checkout.
             gclient_utils.rmtree(e_dir)
       # record the current list of entries for next time
       self._SaveEntries()
+
+    # Sync CIPD dependencies once removed deps are deleted. In case a git
+    # dependency was moved to CIPD, we want to remove the old git directory
+    # first and then sync the CIPD dep.
+    if self._cipd_root:
+      self._cipd_root.run(command)
+
     return 0
 
   def PrintRevInfo(self):

testing_support/fake_cipd.py

@@ -5,10 +5,27 @@
 
 import argparse
 import os
+import re
 import shutil
 import sys
 
 
+CIPD_SUBDIR_RE = '@Subdir (.*)'
+
+
+def parse_cipd(root, contents):
+  tree = {}
+  current_subdir = None
+  for line in contents:
+    line = line.strip()
+    match = re.match(CIPD_SUBDIR_RE, line)
+    if match:
+      current_subdir = os.path.join(root, *match.group(1).split('/'))
+    elif line and current_subdir:
+      tree.setdefault(current_subdir, []).append(line)
+  return tree
+
+
 def main():
   assert sys.argv[1] == 'ensure'
   parser = argparse.ArgumentParser()
@@ -16,12 +33,18 @@ def main():
   parser.add_argument('-root')
   args, _ = parser.parse_known_args()
 
-  cipd_root = os.path.join(args.root, '.cipd')
-  if not os.path.isdir(cipd_root):
-    os.makedirs(cipd_root)
+  with open(args.ensure_file) as f:
+    new_content = parse_cipd(args.root, f.readlines())
+
+  # Install new packages
+  for path, packages in new_content.iteritems():
+    if not os.path.exists(path):
+      os.makedirs(path)
+    with open(os.path.join(path, '_cipd'), 'w') as f:
+      f.write('\n'.join(packages))
 
-  if args.ensure_file:
-    shutil.copy(args.ensure_file, os.path.join(cipd_root, 'ensure'))
+  # Save the ensure file that we got
+  shutil.copy(args.ensure_file, os.path.join(args.root, '_cipd'))
 
   return 0
 

testing_support/fake_repos.py

@@ -743,6 +743,23 @@ deps = {
       'origin': 'git/repo_13@2\n',
     })
 
+    # src/repo12 is now a CIPD dependency.
+    self._commit_git('repo_13', {
+      'DEPS': """
+deps = {
+  'src/repo12': {
+    'packages': [
+      {
+        'package': 'foo',
+        'version': '1.3',
+      },
+    ],
+    'dep_type': 'cipd',
+  },
+}""",
+      'origin': 'git/repo_13@3\n'
+    })
+
     self._commit_git('repo_14', {
       'DEPS': textwrap.dedent("""\
         vars = {}
@@ -932,9 +949,9 @@ class FakeReposTestBase(trial_dir.TestCase):
     actual = read_tree(tree_root)
     diff = dict_diff(tree, actual)
     if diff:
-      logging.debug('Actual %s\n%s' % (tree_root, pprint.pformat(actual)))
-      logging.debug('Expected\n%s' % pprint.pformat(tree))
-      logging.debug('Diff\n%s' % pprint.pformat(diff))
+      logging.error('Actual %s\n%s' % (tree_root, pprint.pformat(actual)))
+      logging.error('Expected\n%s' % pprint.pformat(tree))
+      logging.error('Diff\n%s' % pprint.pformat(diff))
     self.assertEquals(diff, {})
 
   def mangle_git_tree(self, *args):

tests/gclient_smoketest.py

@@ -514,7 +514,8 @@ class GClientSmokeGIT(GClientSmokeBase):
     if not self.enabled:
       return
     self.gclient(['config', self.git_base + 'repo_13', '--name', 'src'])
-    _out, _err, rc = self.gclient(['sync', '-v', '-v', '-v'])
+    _out, _err, rc = self.gclient(
+        ['sync', '-v', '-v', '-v', '--revision', self.githash('repo_13', 2)])
     self.assertEquals(0, rc)
 
   def testSyncFetchUpdate(self):
@@ -529,7 +530,8 @@ class GClientSmokeGIT(GClientSmokeBase):
     self.assertEquals(0, rc)
 
     # Make sure update that pulls a non-standard ref works.
-    _out, _err, rc = self.gclient(['sync', '-v', '-v', '-v'])
+    _out, _err, rc = self.gclient(
+        ['sync', '-v', '-v', '-v', '--revision', self.githash('repo_13', 2)])
     self.assertEquals(0, rc)
 
   def testSyncDirect(self):
@@ -694,7 +696,7 @@ class GClientSmokeGIT(GClientSmokeBase):
                              '--revision', 'src@' + self.githash('repo_5', 2)],
                             expectation)
     self.assertEquals('Cloning into ', out[0][1][:13])
-    self.assertEquals(2, len(out[1]))
+    self.assertEquals(2, len(out[1]), out[1])
     self.assertEquals('pre-deps hook', out[1][1])
     tree = self.mangle_git_tree(('repo_5@2', 'src'),
                                 ('repo_1@2', 'src/repo1'),
@@ -1893,25 +1895,109 @@ class GClientSmokeCipd(GClientSmokeBase):
 
   def testSyncCipd(self):
     self.gclient(['config', self.git_base + 'repo_14', '--name', 'src'])
-    self.gclient(['sync'])
+    out, err, rc = self.gclient(['sync'])
+    self.assertEquals(0, rc, out + err)
+
+    tree = self.mangle_git_tree(('repo_14@1', 'src'))
+    tree.update({
+        '_cipd': '\n'.join([
+            '$ParanoidMode CheckPresence',
+            '',
+            '@Subdir src/another_cipd_dep',
+            'package1 1.1-cr0',
+            'package2 1.13',
+            '',
+            '@Subdir src/cipd_dep',
+            'package0 0.1',
+            '',
+            '@Subdir src/cipd_dep_with_cipd_variable',
+            'package3/${platform} 1.2',
+            '',
+            '',
+        ]),
+        'src/another_cipd_dep/_cipd': '\n'.join([
+            'package1 1.1-cr0',
+            'package2 1.13',
+        ]),
+        'src/cipd_dep/_cipd': 'package0 0.1',
+        'src/cipd_dep_with_cipd_variable/_cipd': 'package3/${platform} 1.2',
+    })
+    self.assertTree(tree)
 
-    with open(os.path.join(self.root_dir, '.cipd', 'ensure')) as f:
-      contents = f.read()
+  def testConvertGitToCipd(self):
+    self.gclient(['config', self.git_base + 'repo_13', '--name', 'src'])
 
-    self.assertEqual([
-        '$ParanoidMode CheckPresence',
-        '',
-        '@Subdir src/another_cipd_dep',
-        'package1 1.1-cr0',
-        'package2 1.13',
-        '',
-        '@Subdir src/cipd_dep',
-        'package0 0.1',
-        '',
-        '@Subdir src/cipd_dep_with_cipd_variable',
-        'package3/${platform} 1.2',
-        '',
-    ], contents.splitlines())
+    # repo_13@1 has src/repo12 as a git dependency.
+    out, err, rc = self.gclient(
+        ['sync', '-v', '-v', '-v', '--revision', self.githash('repo_13', 1)])
+    self.assertEquals(0, rc, out + err)
+
+    tree = self.mangle_git_tree(('repo_13@1', 'src'),
+                                ('repo_12@1', 'src/repo12'))
+    self.assertTree(tree)
+
+    # repo_13@3 has src/repo12 as a cipd dependency.
+    out, err, rc = self.gclient(
+        ['sync', '-v', '-v', '-v', '--revision', self.githash('repo_13', 3),
+         '--delete_unversioned_trees'])
+    self.assertEquals(0, rc, out + err)
+
+    tree = self.mangle_git_tree(('repo_13@3', 'src'))
+    tree.update({
+        '_cipd': '\n'.join([
+            '$ParanoidMode CheckPresence',
+            '',
+            '@Subdir src/repo12',
+            'foo 1.3',
+            '',
+            '',
+        ]),
+        'src/repo12/_cipd': 'foo 1.3',
+    })
+    self.assertTree(tree)
+
+  def testConvertCipdToGit(self):
+    self.gclient(['config', self.git_base + 'repo_13', '--name', 'src'])
+
+    # repo_13@3 has src/repo12 as a cipd dependency.
+    out, err, rc = self.gclient(
+        ['sync', '-v', '-v', '-v', '--revision', self.githash('repo_13', 3),
+         '--delete_unversioned_trees'])
+    self.assertEquals(0, rc, out + err)
+
+    tree = self.mangle_git_tree(('repo_13@3', 'src'))
+    tree.update({
+        '_cipd': '\n'.join([
+            '$ParanoidMode CheckPresence',
+            '',
+            '@Subdir src/repo12',
+            'foo 1.3',
+            '',
+            '',
+        ]),
+        'src/repo12/_cipd': 'foo 1.3',
+    })
+    self.assertTree(tree)
+
+    # repo_13@1 has src/repo12 as a git dependency.
+    out, err, rc = self.gclient(
+        ['sync', '-v', '-v', '-v', '--revision', self.githash('repo_13', 1)])
+    self.assertEquals(0, rc, out + err)
+
+    tree = self.mangle_git_tree(('repo_13@1', 'src'),
+                                ('repo_12@1', 'src/repo12'))
+    tree.update({
+        '_cipd': '\n'.join([
+            '$ParanoidMode CheckPresence',
+            '',
+            '@Subdir src/repo12',
+            'foo 1.3',
+            '',
+            '',
+        ]),
+        'src/repo12/_cipd': 'foo 1.3',
+    })
+    self.assertTree(tree)
 
 
 if __name__ == '__main__':