From e84ac911c595f04b914b65a38a6d95218602a257 Mon Sep 17 00:00:00 2001 From: "cmp@chromium.org" Date: Mon, 30 Jun 2014 23:14:35 +0000 Subject: [PATCH] Add recurselist DEPS var setting. Previously, recursion overrides were only available by setting a numeric 'depth' value in a DEPS file. This meant that it was not possible to control recursion per-dependency entry. This change adds a recurselist variable with a list structure. If a named dependency is present in the list, then gclient will recurse into that dependency's DEPS. As part of this change, I move the recursion controls off of DependencySetting and onto Dependency. The new setup of being based on Dependency allows access to the dependency's name. The controls are only called from Dependency instances. They have always needed access to self.parent (in the Dependency context), so this should be more correct than the previous setup. BUG=390246 Review URL: https://codereview.chromium.org/331373009 git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@280690 0039d316-1c4b-4281-b951-d872f2087c98 --- gclient.py | 48 ++++++++++--- tests/gclient_test.py | 153 +++++++++++++++++++++++++++++++++++++++++- 2 files changed, 188 insertions(+), 13 deletions(-) diff --git a/gclient.py b/gclient.py index 6a7b0e477..8d67b5a9e 100755 --- a/gclient.py +++ b/gclient.py @@ -178,10 +178,6 @@ class DependencySettings(GClientKeywords): # recursion limit and controls gclient's behavior so it does not misbehave. self._managed = managed self._should_process = should_process - # 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 # This is a mutable value which has the list of 'target_os' OSes listed in # the current deps file. self.local_target_os = None @@ -258,13 +254,6 @@ class DependencySettings(GClientKeywords): def url(self): return self._url - @property - def recursion_limit(self): - """Returns > 0 if this dependency is not too recursed to be processed.""" - if self.recursion_override is not None: - return self.recursion_override - return max(self.parent.recursion_limit - 1, 0) - @property def target_os(self): if self.local_target_os is not None: @@ -318,6 +307,15 @@ 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 + # recurselist is a mutable value that selectively overrides the default + # 'no recursion' setting on a dep-by-dep basis. It will replace + # recursion_override. + self.recurselist = None + if not self.name and self.parent: raise gclient_utils.Error('Dependency without name') @@ -357,6 +355,26 @@ class Dependency(gclient_utils.WorkItem, DependencySettings): logging.info('Dependency(%s).requirements = %s' % (self.name, requirements)) return requirements + @property + def try_recurselist(self): + """Returns False if recursion_override is ever specified.""" + if self.recursion_override is not None: + return False + return self.parent.try_recurselist + + @property + def recursion_limit(self): + """Returns > 0 if this dependency is not too recursed to be processed.""" + # We continue to support the absence of recurselist until tools and DEPS + # using recursion_override are updated. + if self.try_recurselist and self.parent.recurselist != None: + if self.name in self.parent.recurselist: + return 1 + + if self.recursion_override is not None: + return self.recursion_override + return max(self.parent.recursion_limit - 1, 0) + def verify_validity(self): """Verifies that this Dependency is fine to add as a child of another one. @@ -563,6 +581,9 @@ class Dependency(gclient_utils.WorkItem, DependencySettings): self.recursion_override = local_scope.get('recursion') logging.warning( 'Setting %s recursion to %d.', self.name, self.recursion_limit) + self.recurselist = local_scope.get('recurselist', None) + if 'recurselist' in local_scope: + logging.warning('Found recurselist %r.', repr(self.recurselist)) # If present, save 'target_os' in the local_target_os property. if 'target_os' in local_scope: self.local_target_os = local_scope['target_os'] @@ -1456,6 +1477,11 @@ want to set 'managed': False in .gclient. """How recursive can each dependencies in DEPS file can load DEPS file.""" return self._recursion_limit + @property + def try_recurselist(self): + """Whether to attempt using recurselist-style recursion processing.""" + return True + @property def target_os(self): return self._enforced_os diff --git a/tests/gclient_test.py b/tests/gclient_test.py index e3baa0f61..fb6577913 100755 --- a/tests/gclient_test.py +++ b/tests/gclient_test.py @@ -611,10 +611,10 @@ class GclientTest(trial_dir.TestCase): sorted(self._get_processed())) def testRecursionOverride(self): - """Verifies gclient respects the recursion var syntax. + """Verifies gclient respects the |recursion| var syntax. We check several things here: - - recursion = 3 sets recursion on the foo dep to exactly 3 + - |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) @@ -660,6 +660,155 @@ class GclientTest(trial_dir.TestCase): ], self._get_processed()) + def testRecurselistOverride(self): + """Verifies gclient respects the |recurselist| var syntax. + + This is what we mean to check here: + - |recurselist| = [...] on 2 levels means we pull exactly 3 deps + (up to /fizz, but not /fuzz) + - pulling foo/bar with no recursion (in .gclient) is overriden by + a later pull of foo/bar with recursion (in the dep tree) + - pulling foo/tar with no recursion (in .gclient) is no recursively + pulled (taz is left out) + """ + write( + '.gclient', + 'solutions = [\n' + ' { "name": "foo", "url": "svn://example.com/foo" },\n' + ' { "name": "foo/bar", "url": "svn://example.com/bar" },\n' + ' { "name": "foo/tar", "url": "svn://example.com/tar" },\n' + ']') + write( + os.path.join('foo', 'DEPS'), + 'deps = {\n' + ' "bar": "/bar",\n' + '}\n' + 'recurselist = ["bar"]') + write( + os.path.join('bar', 'DEPS'), + 'deps = {\n' + ' "baz": "/baz",\n' + '}\n' + 'recurselist = ["baz"]') + write( + os.path.join('baz', 'DEPS'), + 'deps = {\n' + ' "fizz": "/fizz",\n' + '}') + write( + os.path.join('fizz', 'DEPS'), + 'deps = {\n' + ' "fuzz": "/fuzz",\n' + '}') + write( + os.path.join('tar', 'DEPS'), + 'deps = {\n' + ' "taz": "/taz",\n' + '}') + + options, _ = gclient.OptionParser().parse_args([]) + obj = gclient.GClient.LoadCurrentConfig(options) + obj.RunOnDeps('None', []) + self.assertEquals( + [ + 'svn://example.com/foo', + 'svn://example.com/bar', + 'svn://example.com/tar', + 'svn://example.com/foo/bar', + 'svn://example.com/foo/bar/baz', + 'svn://example.com/foo/bar/baz/fizz', + ], + self._get_processed()) + + def testRecursionOverridesRecurselist(self): + """Verifies gclient respects |recursion| over |recurselist|. + + |recursion| is set in a top-level DEPS file. That value is meant + to affect how many subdeps are parsed via recursion. + + |recurselist| 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 |recurselist| 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 |recurselist| 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 |recurselist| are specified. When we see + |recursion|, we stop trying to use |recurselist|. + + There are 2 constructions of DEPS here that are key to this test: + + (1) In foo, if we used |recurselist| 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 |recurselist| 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' + 'recurselist = ["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' + 'recurselist = ["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( + [ + 'svn://example.com/foo', + 'svn://example.com/bar', + 'svn://example.com/foo/bar', + # Deps after this would have been skipped if we were obeying + # |recurselist|. + 'svn://example.com/foo/bar/baz', + 'svn://example.com/foo/bar/baz/fizz', + # And this dep would have been picked up if we were obeying + # |recurselist|. + # 'svn://example.com/foo/bar/baz/fuzz', + ], + self._get_processed()) + if __name__ == '__main__': sys.stdout = gclient_utils.MakeFileAutoFlush(sys.stdout)