diff --git a/gclient.py b/gclient.py index 9fe1b8756..4d963c60d 100755 --- a/gclient.py +++ b/gclient.py @@ -358,7 +358,7 @@ class Dependency(gclient_utils.WorkItem, DependencySettings): def __init__(self, parent, name, url, managed, custom_deps, custom_vars, custom_hooks, deps_file, should_process, - relative, condition, print_outbuf=False): + should_recurse, relative, condition, print_outbuf=False): gclient_utils.WorkItem.__init__(self, name) DependencySettings.__init__( self, parent, url, managed, custom_deps, custom_vars, @@ -402,17 +402,14 @@ class Dependency(gclient_utils.WorkItem, DependencySettings): # unavailable self._got_revision = None - # This is a mutable value that overrides the normal recursion limit for this - # dependency. It is read from the actual DEPS file so cannot be set on - # class instantiation. - self.recursion_override = None # recursedeps is a mutable value that selectively overrides the default - # 'no recursion' setting on a dep-by-dep basis. It will replace - # recursion_override. + # 'no recursion' setting on a dep-by-dep basis. # - # It will be a dictionary of {deps_name: {"deps_file": depfile_name}} or - # None. - self.recursedeps = None + # It will be a dictionary of {deps_name: depfile_namee} + self.recursedeps = {} + + # Whether we should process this dependency's DEPS file. + self._should_recurse = should_recurse self._OverrideUrl() # This is inherited from WorkItem. We want the URL to be a resource. @@ -497,9 +494,6 @@ class Dependency(gclient_utils.WorkItem, DependencySettings): # This becomes messy for >2 depth as the DEPS file format is a dictionary, # thus unsorted, while the .gclient format is a list thus sorted. # - # * _recursion_limit is hard coded 2 and there is no hope to change this - # value. - # # Interestingly enough, the following condition only works in the case we # want: self is a 2nd level node. 3nd level node wouldn't need this since # they already have their parent as a requirement. @@ -517,24 +511,8 @@ class Dependency(gclient_utils.WorkItem, DependencySettings): return requirements @property - def try_recursedeps(self): - """Returns False if recursion_override is ever specified.""" - if self.recursion_override is not None: - return False - return self.parent.try_recursedeps - - @property - def recursion_limit(self): - """Returns > 0 if this dependency is not too recursed to be processed.""" - # We continue to support the absence of recursedeps until tools and DEPS - # using recursion_override are updated. - if self.try_recursedeps and self.parent.recursedeps != None: - if self.name in self.parent.recursedeps: - return 1 - - if self.recursion_override is not None: - return self.recursion_override - return max(self.parent.recursion_limit - 1, 0) + def should_recurse(self): + return self._should_recurse def verify_validity(self): """Verifies that this Dependency is fine to add as a child of another one. @@ -609,12 +587,7 @@ class Dependency(gclient_utils.WorkItem, DependencySettings): """Convert a deps dict to a dict of Dependency objects.""" deps_to_add = [] for name, dep_value in deps.iteritems(): - should_process = self.recursion_limit and self.should_process - deps_file = self.deps_file - if self.recursedeps is not None: - ent = self.recursedeps.get(name) - if ent is not None: - deps_file = ent['deps_file'] + should_process = self.should_process if dep_value is None: continue @@ -653,8 +626,9 @@ class Dependency(gclient_utils.WorkItem, DependencySettings): custom_deps=None, custom_vars=self.custom_vars, custom_hooks=None, - deps_file=deps_file, + deps_file=self.recursedeps.get(name, self.deps_file), should_process=should_process, + should_recurse=name in self.recursedeps, relative=use_relative_paths, condition=condition)) @@ -743,17 +717,15 @@ class Dependency(gclient_utils.WorkItem, DependencySettings): rel_prefix = os.path.dirname(self.name) if 'recursion' in local_scope: - self.recursion_override = local_scope.get('recursion') logging.warning( - 'Setting %s recursion to %d.', self.name, self.recursion_limit) - self.recursedeps = None + '%s: Ignoring recursion = %d.', self.name, local_scope['recursion']) + if 'recursedeps' in local_scope: - self.recursedeps = {} for ent in local_scope['recursedeps']: if isinstance(ent, basestring): - self.recursedeps[ent] = {"deps_file": self.deps_file} + self.recursedeps[ent] = self.deps_file else: # (depname, depsfilename) - self.recursedeps[ent[0]] = {"deps_file": ent[1]} + self.recursedeps[ent[0]] = ent[1] logging.warning('Found recursedeps %r.', repr(self.recursedeps)) if rel_prefix: @@ -788,7 +760,7 @@ class Dependency(gclient_utils.WorkItem, DependencySettings): if 'action' in hook: hooks_to_run.append(hook) - if self.recursion_limit: + if self.should_recurse: self._pre_deps_hooks = [ Hook.from_dict(hook, variables=self.get_vars(), verbose=True, conditions=self.condition) @@ -914,12 +886,12 @@ class Dependency(gclient_utils.WorkItem, DependencySettings): while file_list[i].startswith(('\\', '/')): file_list[i] = file_list[i][1:] - if self.recursion_limit: + if self.should_recurse: self.ParseDepsFile() self._run_is_done(file_list or []) - if self.recursion_limit: + if self.should_recurse: if command in ('update', 'revert') and not options.noprehooks: self.RunPreDepsHooks() # Parse the dependencies of this dependency. @@ -1015,7 +987,7 @@ class Dependency(gclient_utils.WorkItem, DependencySettings): RunOnDeps() must have been called before to load the DEPS. """ result = [] - if not self.should_process or not self.recursion_limit: + if not self.should_process or not self.should_recurse: # Don't run the hook when it is above recursion_limit. return result # If "--force" was specified, run all hooks regardless of what files have @@ -1304,6 +1276,7 @@ solutions = %(solution_list)s custom_hooks=None, deps_file='unused', should_process=True, + should_recurse=True, relative=None, condition=None, print_outbuf=True) @@ -1407,6 +1380,7 @@ it or fix the checkout. custom_hooks=s.get('custom_hooks', []), deps_file=s.get('deps_file', 'DEPS'), should_process=True, + should_recurse=True, relative=None, condition=None, print_outbuf=True)) @@ -1798,16 +1772,6 @@ it or fix the checkout. """What deps_os entries that are to be parsed.""" return self._enforced_os - @property - def recursion_limit(self): - """How recursive can each dependencies in DEPS file can load DEPS file.""" - return self._recursion_limit - - @property - def try_recursedeps(self): - """Whether to attempt using recursedeps-style recursion processing.""" - return True - @property def target_os(self): return self._enforced_os @@ -1837,6 +1801,7 @@ class CipdDependency(Dependency): custom_hooks=None, deps_file=None, should_process=should_process, + should_recurse=False, relative=relative, condition=condition) if relative: @@ -2047,8 +2012,7 @@ class Flattener(object): def add_deps_file(dep): # Only include DEPS files referenced by recursedeps. - if not (dep.parent is None or - (dep.name in (dep.parent.recursedeps or {}))): + if not dep.should_recurse: return deps_file = dep.deps_file deps_path = os.path.join(self._client.root_dir, dep.name, deps_file) @@ -2129,9 +2093,9 @@ class Flattener(object): for sub_dep in dep.dependencies: self._add_dep(sub_dep) - deps_by_name = {d.name: d for d in dep.dependencies} - for recurse_dep_name in (dep.recursedeps or []): - self._flatten_dep(deps_by_name[recurse_dep_name]) + for d in dep.dependencies: + if d.should_recurse: + self._flatten_dep(d) def CMDflatten(parser, args): diff --git a/gclient_scm.py b/gclient_scm.py index 1f4c07776..c7a035e7f 100644 --- a/gclient_scm.py +++ b/gclient_scm.py @@ -1321,7 +1321,7 @@ class CipdRoot(object): suffix='.ensure', delete=False) as ensure_file: for subdir, packages in sorted(self._packages_by_subdir.iteritems()): ensure_file.write('@Subdir %s\n' % subdir) - for package in packages: + for package in sorted(packages, key=lambda p: p.name): ensure_file.write('%s %s\n' % (package.name, package.version)) ensure_file.write('\n') yield ensure_file.name diff --git a/tests/gclient_smoketest.py b/tests/gclient_smoketest.py index ef421e486..45c68b95e 100755 --- a/tests/gclient_smoketest.py +++ b/tests/gclient_smoketest.py @@ -1063,7 +1063,8 @@ class GClientSmokeGIT(GClientSmokeBase): '', '# ' + self.git_base + 'repo_2@%s, DEPS' % ( self.githash('repo_2', 1)[:7]), - '# ' + self.git_base + 'repo_8, DEPS' + '# ' + self.git_base + 'repo_6, DEPS', + '# ' + self.git_base + 'repo_8, DEPS', ], deps_contents.splitlines()) def testFlattenPinAllDeps(self): @@ -1232,6 +1233,8 @@ class GClientSmokeGIT(GClientSmokeBase): '', '# ' + self.git_base + 'repo_2@%s, DEPS' % ( self.githash('repo_2', 1)), + '# ' + self.git_base + 'repo_6@%s, DEPS' % ( + self.githash('repo_6', 1)), '# ' + self.git_base + 'repo_8@%s, DEPS' % ( self.githash('repo_8', 1)), ], deps_contents.splitlines()) @@ -1317,6 +1320,7 @@ class GClientSmokeGIT(GClientSmokeBase): '', '}', '', + '# ' + self.git_base + 'repo_10, DEPS', '# ' + self.git_base + 'repo_11, DEPS', '# ' + self.git_base + 'repo_8, DEPS', '# ' + self.git_base + 'repo_9, DEPS', @@ -1326,6 +1330,8 @@ class GClientSmokeGIT(GClientSmokeBase): deps_files_contents = json.load(f) self.assertEqual([ + {'url': self.git_base + 'repo_10', 'deps_file': 'DEPS', + 'hierarchy': [['src', self.git_base + 'repo_10']]}, {'url': self.git_base + 'repo_11', 'deps_file': 'DEPS', 'hierarchy': [['src', self.git_base + 'repo_10'], ['src/repo11', self.git_base + 'repo_11']]}, @@ -1387,7 +1393,8 @@ class GClientSmokeGIT(GClientSmokeBase): ' },', '', '}', - '' + '', + '# ' + self.git_base + 'repo_14, DEPS', ], deps_contents.splitlines()) diff --git a/tests/gclient_test.py b/tests/gclient_test.py index f5473e9e0..98b83a33f 100755 --- a/tests/gclient_test.py +++ b/tests/gclient_test.py @@ -201,8 +201,19 @@ class GclientTest(trial_dir.TestCase): # auto-fixed. url = 'proto://host/path/@revision' d = gclient.Dependency( - None, 'name', url, None, None, None, - None, '', True, False, None, True) + parent=None, + name='name', + url=url, + managed=None, + custom_deps=None, + custom_vars=None, + custom_hooks=None, + deps_file='', + should_process=True, + should_recurse=False, + relative=False, + condition=None, + print_outbuf=True) self.assertEquals('proto://host/path@revision', d.url) def testStr(self): @@ -212,19 +223,51 @@ class GclientTest(trial_dir.TestCase): obj.add_dependencies_and_close( [ gclient.Dependency( - obj, 'foo', 'svn://example.com/foo', None, - None, None, None, 'DEPS', True, False, None, True), + parent=obj, + name='foo', + url='svn://example.com/foo', + managed=None, + custom_deps=None, + custom_vars=None, + custom_hooks=None, + deps_file='DEPS', + should_process=True, + should_recurse=True, + relative=False, + condition=None, + print_outbuf=True), gclient.Dependency( - obj, 'bar', 'svn://example.com/bar', None, - None, None, None, 'DEPS', True, False, None, True), + parent=obj, + name='bar', + url='svn://example.com/bar', + managed=None, + custom_deps=None, + custom_vars=None, + custom_hooks=None, + deps_file='DEPS', + should_process=True, + should_recurse=False, + relative=False, + condition=None, + print_outbuf=True), ], []) obj.dependencies[0].add_dependencies_and_close( [ gclient.Dependency( - obj.dependencies[0], 'foo/dir1', 'svn://example.com/foo/dir1', - None, None, None, None, 'DEPS', True, - False, None, True), + parent=obj.dependencies[0], + name='foo/dir1', + url='svn://example.com/foo/dir1', + managed=None, + custom_deps=None, + custom_vars=None, + custom_hooks=None, + deps_file='DEPS', + should_process=True, + should_recurse=False, + relative=False, + condition=None, + print_outbuf=True), ], []) # TODO(ehmaldonado): Improve this test. @@ -593,56 +636,6 @@ class GclientTest(trial_dir.TestCase): ], sorted(self._get_processed())) - def testRecursionOverride(self): - """Verifies gclient respects the |recursion| var syntax. - - We check several things here: - - |recursion| = 3 sets recursion on the foo dep to exactly 3 - (we pull /fizz, but not /fuzz) - - pulling foo/bar at recursion level 1 (in .gclient) is overriden by - a later pull of foo/bar at recursion level 2 (in the dep tree) - """ - write( - '.gclient', - 'solutions = [\n' - ' { "name": "foo", "url": "svn://example.com/foo" },\n' - ' { "name": "foo/bar", "url": "svn://example.com/bar" },\n' - ']') - write( - os.path.join('foo', 'DEPS'), - 'deps = {\n' - ' "bar": "/bar",\n' - '}\n' - 'recursion = 3') - write( - os.path.join('bar', 'DEPS'), - 'deps = {\n' - ' "baz": "/baz",\n' - '}') - write( - os.path.join('baz', 'DEPS'), - 'deps = {\n' - ' "fizz": "/fizz",\n' - '}') - write( - os.path.join('fizz', 'DEPS'), - 'deps = {\n' - ' "fuzz": "/fuzz",\n' - '}') - - options, _ = gclient.OptionParser().parse_args([]) - obj = gclient.GClient.LoadCurrentConfig(options) - obj.RunOnDeps('None', []) - self.assertEquals( - [ - ('foo', 'svn://example.com/foo'), - ('foo/bar', 'svn://example.com/bar'), - ('bar', 'svn://example.com/bar'), - ('baz', 'svn://example.com/baz'), - ('fizz', 'svn://example.com/fizz'), - ], - self._get_processed()) - def testRecursedepsOverride(self): """Verifies gclient respects the |recursedeps| var syntax. @@ -777,95 +770,6 @@ class GclientTest(trial_dir.TestCase): ], self._get_processed()) - def testRecursionOverridesRecursedeps(self): - """Verifies gclient respects |recursion| over |recursedeps|. - - |recursion| is set in a top-level DEPS file. That value is meant - to affect how many subdeps are parsed via recursion. - - |recursedeps| is set in each DEPS file to control whether or not - to recurse into the immediate next subdep. - - This test verifies that if both syntaxes are mixed in a DEPS file, - we disable |recursedeps| support and only obey |recursion|. - - Since this setting is evaluated per DEPS file, recursed DEPS - files will each be re-evaluated according to the per DEPS rules. - So a DEPS that only contains |recursedeps| could then override any - previous |recursion| setting. There is extra processing to ensure - this does not happen. - - For this test to work correctly, we need to use a DEPS chain that - only contains recursion controls in the top DEPS file. - - In foo, |recursion| and |recursedeps| are specified. When we see - |recursion|, we stop trying to use |recursedeps|. - - There are 2 constructions of DEPS here that are key to this test: - - (1) In foo, if we used |recursedeps| instead of |recursion|, we - would also pull in bar. Since bar's DEPS doesn't contain any - recursion statements, we would stop processing at bar. - - (2) In fizz, if we used |recursedeps| at all, we should pull in - fuzz. - - We expect to keep going past bar (satisfying 1) and we don't - expect to pull in fuzz (satisfying 2). - """ - write( - '.gclient', - 'solutions = [\n' - ' { "name": "foo", "url": "svn://example.com/foo" },\n' - ' { "name": "foo/bar", "url": "svn://example.com/bar" },\n' - ']') - write( - os.path.join('foo', 'DEPS'), - 'deps = {\n' - ' "bar": "/bar",\n' - '}\n' - 'recursion = 3\n' - 'recursedeps = ["bar"]') - write( - os.path.join('bar', 'DEPS'), - 'deps = {\n' - ' "baz": "/baz",\n' - '}') - write( - os.path.join('baz', 'DEPS'), - 'deps = {\n' - ' "fizz": "/fizz",\n' - '}') - write( - os.path.join('fizz', 'DEPS'), - 'deps = {\n' - ' "fuzz": "/fuzz",\n' - '}\n' - 'recursedeps = ["fuzz"]') - write( - os.path.join('fuzz', 'DEPS'), - 'deps = {\n' - ' "tar": "/tar",\n' - '}') - - options, _ = gclient.OptionParser().parse_args([]) - obj = gclient.GClient.LoadCurrentConfig(options) - obj.RunOnDeps('None', []) - self.assertEquals( - [ - ('foo', 'svn://example.com/foo'), - ('foo/bar', 'svn://example.com/bar'), - ('bar', 'svn://example.com/bar'), - # Deps after this would have been skipped if we were obeying - # |recursedeps|. - ('baz', 'svn://example.com/baz'), - ('fizz', 'svn://example.com/fizz'), - # And this dep would have been picked up if we were obeying - # |recursedeps|. - # 'svn://example.com/foo/bar/baz/fuzz', - ], - self._get_processed()) - def testRecursedepsAltfile(self): """Verifies gclient respects the |recursedeps| var syntax with overridden target DEPS file. @@ -1130,23 +1034,43 @@ class GclientTest(trial_dir.TestCase): obj.add_dependencies_and_close( [ gclient.Dependency( - obj, 'foo', 'svn://example.com/foo', None, - None, None, None, 'DEPS', True, - False, None, True), + parent=obj, + name='foo', + url='svn://example.com/foo', + managed=None, + custom_deps=None, + custom_vars=None, + custom_hooks=None, + deps_file='DEPS', + should_process=True, + should_recurse=True, + relative=False, + condition=None, + print_outbuf=True), ], []) obj.dependencies[0].add_dependencies_and_close( [ - gclient.CipdDependency(obj.dependencies[0], 'foo', - {'package': 'foo_package', - 'version': 'foo_version'}, - cipd_root, None, True, False, - 'fake_condition'), - gclient.CipdDependency(obj.dependencies[0], 'foo', - {'package': 'bar_package', - 'version': 'bar_version'}, - cipd_root, None, True, False, - 'fake_condition'), + gclient.CipdDependency( + parent=obj.dependencies[0], + name='foo', + dep_value={'package': 'foo_package', + 'version': 'foo_version'}, + cipd_root=cipd_root, + custom_vars=None, + should_process=True, + relative=False, + condition='fake_condition'), + gclient.CipdDependency( + parent=obj.dependencies[0], + name='bar', + dep_value={'package': 'bar_package', + 'version': 'bar_version'}, + cipd_root=cipd_root, + custom_vars=None, + should_process=True, + relative=False, + condition='fake_condition'), ], []) dep0 = obj.dependencies[0].dependencies[0]