From 647e1e79eb7129ecdc021b7535d90e6d06475603 Mon Sep 17 00:00:00 2001 From: Edward Lemur Date: Wed, 19 Sep 2018 18:15:29 +0000 Subject: [PATCH] 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 > Reviewed-by: Andrii Shyshkalov Bug: 882611 Change-Id: I683bfc62bd1eebfec0853583f96f3981c2c6bdf2 Reviewed-on: https://chromium-review.googlesource.com/1232891 Reviewed-by: Andrii Shyshkalov Commit-Queue: Edward Lesmes --- gclient.py | 10 ++- testing_support/fake_cipd.py | 33 +++++++-- testing_support/fake_repos.py | 23 ++++++- tests/gclient_smoketest.py | 126 ++++++++++++++++++++++++++++------ 4 files changed, 161 insertions(+), 31 deletions(-) diff --git a/gclient.py b/gclient.py index 1637190a3..2b8ae3564 100755 --- a/gclient.py +++ b/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): diff --git a/testing_support/fake_cipd.py b/testing_support/fake_cipd.py index 782289383..511a8bff6 100644 --- a/testing_support/fake_cipd.py +++ b/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 diff --git a/testing_support/fake_repos.py b/testing_support/fake_repos.py index 076900661..e0e2147dc 100755 --- a/testing_support/fake_repos.py +++ b/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): diff --git a/tests/gclient_smoketest.py b/tests/gclient_smoketest.py index d7eae07cc..ecd31987b 100755 --- a/tests/gclient_smoketest.py +++ b/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__':