Fix duplicate Dependency that appeared by switching from a flat list to a tree.
Instead of keeping the reference information in the parent, note for each Dependency if it should be processed with self.should_process. I had to hack a bit with the hooks to also enforce recursion_limit() to keep the old behavior, otherwise From() could cause hooks to run that weren't run before.
BUG=50015
TEST=hooks are run properly. Tested with webkit and pagespeed and buildbot master, fixes running the same checkout multiple times.
Review URL: http://codereview.chromium.org/3124017
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@55895 0039d316-1c4b-4281-b951-d872f2087c98
diff --git a/gclient.py b/gclient.py
index 4170f5f..4f27747 100644
--- a/gclient.py
+++ b/gclient.py
@@ -49,7 +49,7 @@
]
"""
-__version__ = "0.5.1"
+__version__ = "0.5.2"
import logging
import optparse
@@ -244,7 +244,7 @@
DEPS_FILE = 'DEPS'
def __init__(self, parent, name, url, safesync_url, custom_deps,
- custom_vars, deps_file):
+ custom_vars, deps_file, should_process):
GClientKeywords.__init__(self)
self.parent = parent
self.name = name
@@ -263,10 +263,8 @@
# If it is not set to True, the dependency wasn't processed for its child
# dependency, i.e. its DEPS wasn't read.
self.deps_parsed = False
- # A direct reference is dependency that is referenced by a deps, deps_os or
- # solution. A indirect one is one that was loaded with From() or that
- # exceeded recursion limit.
- self.direct_reference = False
+ # This dependency should be processed, i.e. checked out
+ self.should_process = should_process
# This dependency has been processed, i.e. checked out
self.processed = False
# This dependency had its hook run
@@ -295,6 +293,7 @@
Manages From() keyword accordingly. Do not touch self.parsed_url nor
self.url because it may called with other urls due to From()."""
+ assert self.parsed_url == None or not self.should_process, self.parsed_url
overriden_url = self.get_custom_deps(self.name, url)
if overriden_url != url:
logging.info('%s, %s was overriden to %s' % (self.name, url,
@@ -310,7 +309,7 @@
sub_target = url.sub_target_name or self.name
# Make sure the referenced dependency DEPS file is loaded and file the
# inner referenced dependency.
- ref.ParseDepsFile(False)
+ ref.ParseDepsFile()
found_dep = None
for d in ref.dependencies:
if d.name == sub_target:
@@ -351,12 +350,9 @@
else:
raise gclient_utils.Error('Unkown url type')
- def ParseDepsFile(self, direct_reference):
+ def ParseDepsFile(self):
"""Parses the DEPS file for this dependency."""
- if direct_reference:
- # Maybe it was referenced earlier by a From() keyword but it's now
- # directly referenced.
- self.direct_reference = direct_reference
+ assert self.processed == True
if self.deps_parsed:
logging.debug('%s was already parsed' % self.name)
return
@@ -423,14 +419,30 @@
raise gclient_utils.Error(
'The same name "%s" appears multiple times in the deps section' %
name)
+ should_process = self.recursion_limit() > 0 and self.should_process
+ if should_process:
+ tree = dict((d.name, d) for d in self.tree(False))
+ if name in tree:
+ if url == tree[name].url:
+ logging.info('Won\'t process duplicate dependency %s' % tree[name])
+ # In theory we could keep it as a shadow of the other one. In
+ # practice, simply ignore it.
+ #should_process = False
+ continue
+ else:
+ raise gclient_utils.Error(
+ 'Dependency %s specified more than once:\n %s\nvs\n %s' %
+ (name, tree[name].hierarchy(), self.hierarchy()))
self.dependencies.append(Dependency(self, name, url, None, None, None,
- None))
+ None, should_process))
logging.debug('Loaded: %s' % str(self))
def run(self, options, revision_overrides, command, args, work_queue):
"""Runs 'command' before parsing the DEPS in case it's a initial checkout
or a revert."""
assert self._file_list == []
+ if not self.should_process:
+ return
# When running runhooks, there's no need to consult the SCM.
# All known hooks are expected to run unconditionally regardless of working
# copy state, so skip the SCM status check.
@@ -455,9 +467,9 @@
for f in self._file_list]
options.revision = None
self.processed = True
- if self.recursion_limit():
+ if self.recursion_limit() > 0:
# Then we can parse the DEPS file.
- self.ParseDepsFile(True)
+ self.ParseDepsFile()
# Adjust the implicit dependency requirement; e.g. if a DEPS file contains
# both src/foo and src/foo/bar, src/foo/bar is implicitly dependent of
# src/foo. Yes, it's O(n^2)... It's important to do that before
@@ -476,9 +488,13 @@
def RunHooksRecursively(self, options):
"""Evaluates all hooks, running actions as needed. run()
must have been called before to load the DEPS."""
+ assert self.hooks_ran == False
+ if not self.should_process or self.recursion_limit() <= 0:
+ # Don't run the hook when it is above recursion_limit.
+ return
# If "--force" was specified, run all hooks regardless of what files have
# changed.
- if self.deps_hooks and self.direct_reference:
+ if self.deps_hooks:
# TODO(maruel): If the user is using git or git-svn, then we don't know
# what files have changed so we always run all hooks. It'd be nice to fix
# that.
@@ -513,9 +529,8 @@
matching_file_list = [f for f in file_list if pattern.search(f)]
if matching_file_list:
self._RunHookAction(hook_dict, matching_file_list)
- if self.recursion_limit():
- for s in self.dependencies:
- s.RunHooksRecursively(options)
+ for s in self.dependencies:
+ s.RunHooksRecursively(options)
def _RunHookAction(self, hook_dict, matching_file_list):
"""Runs the action from a single hook."""
@@ -554,13 +569,13 @@
return self.parent.tree(include_all)
def subtree(self, include_all):
+ """Breadth first"""
result = []
- # Add breadth-first.
- if self.direct_reference or include_all:
- for d in self.dependencies:
+ for d in self.dependencies:
+ if d.should_process or include_all:
result.append(d)
- for d in self.dependencies:
- result.extend(d.subtree(include_all))
+ for d in self.dependencies:
+ result.extend(d.subtree(include_all))
return result
def get_custom_deps(self, name, url):
@@ -579,8 +594,8 @@
def __str__(self):
out = []
for i in ('name', 'url', 'parsed_url', 'safesync_url', 'custom_deps',
- 'custom_vars', 'deps_hooks', '_file_list', 'processed',
- 'hooks_ran', 'deps_parsed', 'requirements', 'direct_reference'):
+ 'custom_vars', 'deps_hooks', '_file_list', 'should_process',
+ 'processed', 'hooks_ran', 'deps_parsed', 'requirements'):
# 'deps_file'
if self.__dict__[i]:
out.append('%s: %s' % (i, self.__dict__[i]))
@@ -655,7 +670,7 @@
# Do not change previous behavior. Only solution level and immediate DEPS
# are processed.
self._recursion_limit = 2
- Dependency.__init__(self, None, None, None, None, None, None, None)
+ Dependency.__init__(self, None, None, None, None, None, None, None, True)
self._options = options
if options.deps_os:
enforced_os = options.deps_os.split(',')
@@ -677,12 +692,17 @@
gclient_utils.SyntaxErrorToError('.gclient', e)
for s in config_dict.get('solutions', []):
try:
+ tree = dict((d.name, d) for d in self.tree(False))
+ if s['name'] in tree:
+ raise gclient_utils.Error(
+ 'Dependency %s specified more than once in .gclient' % s['name'])
self.dependencies.append(Dependency(
self, s['name'], s['url'],
s.get('safesync_url', None),
s.get('custom_deps', {}),
s.get('custom_vars', {}),
- None))
+ None,
+ True))
except KeyError:
raise gclient_utils.Error('Invalid .gclient file. Solution is '
'incomplete: %s' % s)
@@ -900,7 +920,7 @@
print line
logging.info(str(self))
- def ParseDepsFile(self, direct_reference):
+ def ParseDepsFile(self):
"""No DEPS to parse for a .gclient file."""
raise gclient_utils.Error('Internal error')