Skip to content

C
Chromium_Depot_tools
Commit a68660d0 authored by Corentin Wallez's avatar Corentin Wallez Committed by Commit Bot
Browse files
Options

gclient: add use_relative_hooks 

When a recursive dependency has use_relative_paths it also makes sense
to have the hooks working directory by the dependency's directory.
Otherwise if a hook uses one of the relative dependencies it is impossible
to know which path prefix to use.

However we cannot change the behavior of hooks with use_relative_paths
because it would break existing projects that use_relative_paths but
hardcoded the prefix for hooks. Instead we add a second boolean,
use_relative_hooks that triggers the behavior.

Adds tests for the new behavior and a test for existing interactio
between hooks and recursedeps.

BUG=chromium:875245

Change-Id: Ie4c526baa425ff887b3be54e0feca7c597ededec
Reviewed-on: https://chromium-review.googlesource.com/1213327
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: 's avatarEdward Lesmes <ehmaldonado@chromium.org>
parent 25c380cf
Hide whitespace changes
Inline Side-by-side
Showing with 242 additions and 44 deletions
gclient.py
View file @ a68660d0
......@@ -151,7 +151,7 @@ class Hook(object):
"""Descriptor of command ran before/after sync or on demand."""
def __init__(self, action, pattern=None, name=None, cwd=None, condition=None,
variables=None, verbose=False):
variables=None, verbose=False, cwd_base=None):
"""Constructor.
Arguments:
......@@ -169,9 +169,11 @@ class Hook(object):
self._condition = condition
self._variables = variables
self._verbose = verbose
self._cwd_base = cwd_base
@staticmethod
def from_dict(d, variables=None, verbose=False, conditions=None):
def from_dict(d, variables=None, verbose=False, conditions=None,
cwd_base=None):
"""Creates a Hook instance from a dict like in the DEPS file."""
# Merge any local and inherited conditions.
gclient_eval.UpdateCondition(d, 'and', conditions)
......@@ -183,7 +185,8 @@ class Hook(object):
d.get('condition'),
variables=variables,
# Always print the header if not printing to a TTY.
verbose=verbose or not setup_color.IS_TTY)
verbose=verbose or not setup_color.IS_TTY,
cwd_base=cwd_base)
@property
def action(self):
......@@ -201,6 +204,13 @@ class Hook(object):
def condition(self):
return self._condition
@property
def effective_cwd(self):
cwd = self._cwd_base
if self._cwd:
cwd = os.path.join(cwd, self._cwd)
return cwd
def matches(self, file_list):
"""Returns true if the pattern matches any of files in the list."""
if not self._pattern:
......@@ -208,7 +218,7 @@ class Hook(object):
pattern = re.compile(self._pattern)
return bool([f for f in file_list if pattern.search(f)])
def run(self, root):
def run(self):
"""Executes the hook's command (provided the condition is met)."""
if (self._condition and
not gclient_eval.EvaluateCondition(self._condition, self._variables)):
......@@ -224,13 +234,10 @@ class Hook(object):
elif cmd[0] == 'vpython' and _detect_host_os() == 'win':
cmd[0] += '.bat'
cwd = root
if self._cwd:
cwd = os.path.join(cwd, self._cwd)
try:
start_time = time.time()
gclient_utils.CheckCallAndFilterAndHeader(
cmd, cwd=cwd, always=self._verbose)
cmd, cwd=self.effective_cwd, always=self._verbose)
except (gclient_utils.Error, subprocess2.CalledProcessError) as e:
# Use a discrete exit status code of 2 to indicate that a hook action
# failed. Users of this script may wish to treat hook action failures
......@@ -755,6 +762,18 @@ class Dependency(gclient_utils.WorkItem, DependencySettings):
deps_to_add = self._deps_to_objects(
self._postprocess_deps(deps, rel_prefix), use_relative_paths)
# compute which working directory should be used for hooks
use_relative_hooks = local_scope.get('use_relative_hooks', False)
hooks_cwd = self.root.root_dir
if use_relative_hooks:
if not use_relative_paths:
raise gclient_utils.Error(
'ParseDepsFile(%s): use_relative_hooks must be used with '
'use_relative_paths' % self.name)
hooks_cwd = os.path.join(hooks_cwd, self.name)
logging.warning('Updating hook base working directory to %s.',
hooks_cwd)
# override named sets of hooks by the custom hooks
hooks_to_run = []
hook_names_to_suppress = [c.get('name', '') for c in self.custom_hooks]
......@@ -770,11 +789,12 @@ class Dependency(gclient_utils.WorkItem, DependencySettings):
if self.should_recurse:
self._pre_deps_hooks = [
Hook.from_dict(hook, variables=self.get_vars(), verbose=True,
conditions=self.condition)
conditions=self.condition, cwd_base=hooks_cwd)
for hook in local_scope.get('pre_deps_hooks', [])
]
self.add_dependencies_and_close(deps_to_add, hooks_to_run)
self.add_dependencies_and_close(deps_to_add, hooks_to_run,
hooks_cwd=hooks_cwd)
logging.info('ParseDepsFile(%s) done' % self.name)
def _get_option(self, attr, default):
......@@ -783,15 +803,18 @@ class Dependency(gclient_utils.WorkItem, DependencySettings):
obj = obj.parent
return getattr(obj._options, attr, default)
def add_dependencies_and_close(self, deps_to_add, hooks):
def add_dependencies_and_close(self, deps_to_add, hooks, hooks_cwd=None):
"""Adds the dependencies, hooks and mark the parsing as done."""
if hooks_cwd == None:
hooks_cwd = self.root.root_dir
for dep in deps_to_add:
if dep.verify_validity():
self.add_dependency(dep)
self._mark_as_parsed([
Hook.from_dict(
h, variables=self.get_vars(), verbose=self.root._options.verbose,
conditions=self.condition)
conditions=self.condition, cwd_base=hooks_cwd)
for h in hooks
])
......@@ -1021,7 +1044,7 @@ class Dependency(gclient_utils.WorkItem, DependencySettings):
for hook in hooks:
if progress:
progress.update(extra=hook.name or '')
hook.run(self.root.root_dir)
hook.run()
if progress:
progress.end()
......@@ -1034,7 +1057,7 @@ class Dependency(gclient_utils.WorkItem, DependencySettings):
assert not s.processed
self._pre_deps_hooks_ran = True
for hook in self.pre_deps_hooks:
hook.run(self.root.root_dir)
hook.run()
def GetCipdRoot(self):
if self.root is self:
......@@ -2257,9 +2280,10 @@ def _HooksToLines(name, hooks):
s.append(' "pattern": "%s",' % hook.pattern)
if hook.condition is not None:
s.append(' "condition": %r,' % hook.condition)
# Flattened hooks need to be written relative to the root gclient dir
cwd = os.path.relpath(os.path.normpath(hook.effective_cwd))
s.extend(
# Hooks run in the parent directory of their dep.
[' "cwd": "%s",' % os.path.normpath(os.path.dirname(dep.name))] +
[' "cwd": "%s",' % cwd] +
[' "action": ['] +
[' "%s",' % arg for arg in hook.action] +
[' ]', ' },', '']
......@@ -2286,9 +2310,10 @@ def _HooksOsToLines(hooks_os):
s.append(' "pattern": "%s",' % hook.pattern)
if hook.condition is not None:
s.append(' "condition": %r,' % hook.condition)
# Flattened hooks need to be written relative to the root gclient dir
cwd = os.path.relpath(os.path.normpath(hook.effective_cwd))
s.extend(
# Hooks run in the parent directory of their dep.
[' "cwd": "%s",' % os.path.normpath(os.path.dirname(dep.name))] +
[' "cwd": "%s",' % cwd] +
[' "action": ['] +
[' "%s",' % arg for arg in hook.action] +
[' ]', ' },', '']
......
gclient_eval.py
View file @ a68660d0
......@@ -196,9 +196,13 @@ _GCLIENT_SCHEMA = schema.Schema(_NodeDictSchema({
schema.Optional('target_os'): [schema.Optional(basestring)],
# For recursed-upon sub-dependencies, check out their own dependencies
# relative to the paren't path, rather than relative to the .gclient file.
# relative to the parent's path, rather than relative to the .gclient file.
schema.Optional('use_relative_paths'): bool,
# For recursed-upon sub-dependencies, run their hooks relative to the
# parent's path instead of relative to the .gclient file.
schema.Optional('use_relative_hooks'): bool,
# Variables that can be referenced using Var() - see 'deps'.
schema.Optional('vars'): _NodeDictSchema({
schema.Optional(basestring): schema.Or(basestring, bool),
......
testing_support/fake_repos.py
View file @ a68660d0
......@@ -320,7 +320,7 @@ class FakeReposBase(object):
class FakeRepos(FakeReposBase):
"""Implements populateGit()."""
NB_GIT_REPOS = 14
NB_GIT_REPOS = 16
def populateGit(self):
# Testing:
......@@ -550,6 +550,8 @@ deps = {
'url': None,
},
'src/repo8': '/repo_8',
'src/repo15': '/repo_15',
'src/repo16': '/repo_16',
}
deps_os ={
'mac': {
......@@ -592,6 +594,8 @@ hooks_os = {
recursedeps = [
'src/repo2',
'src/repo8',
'src/repo15',
'src/repo16',
]""" % {
'git_base': self.git_base,
'hash': self.git_hashes['repo_2'][1][0][:7]
......@@ -778,6 +782,29 @@ deps = {
'origin': 'git/repo_14@2\n'
})
# A repo with a hook to be recursed in, without use_relative_hooks
self._commit_git('repo_15', {
'DEPS': textwrap.dedent("""\
hooks = [{
"name": "absolute_cwd",
"pattern": ".",
"action": ["python", "-c", "pass"]
}]"""),
'origin': 'git/repo_15@2\n'
})
# A repo with a hook to be recursed in, with use_relative_hooks
self._commit_git('repo_16', {
'DEPS': textwrap.dedent("""\
use_relative_paths=True
use_relative_hooks=True
hooks = [{
"name": "relative_cwd",
"pattern": ".",
"action": ["python", "relative.py"]
}]"""),
'relative.py': 'pass',
'origin': 'git/repo_16@2\n'
})
class FakeRepoSkiaDEPS(FakeReposBase):
"""Simulates the Skia DEPS transition in Chrome."""
......
tests/gclient_smoketest.py
View file @ a68660d0
......@@ -1008,6 +1008,16 @@ class GClientSmokeGIT(GClientSmokeBase):
' "condition": \'(checkout_linux) or (checkout_mac)\',',
' },',
'',
' # src -> src/repo15',
' "src/repo15": {',
' "url": "git://127.0.0.1:20000/git/repo_15",',
' },',
'',
' # src -> src/repo16',
' "src/repo16": {',
' "url": "git://127.0.0.1:20000/git/repo_16",',
' },',
'',
' # src -> src/repo2',
' "src/repo2": {',
' "url": "' + self.git_base + 'repo_2@%s",' % (
......@@ -1078,6 +1088,29 @@ class GClientSmokeGIT(GClientSmokeBase):
' ]',
' },',
'',
' # src -> src/repo15',
' {',
' "name": "absolute_cwd",',
' "pattern": ".",',
' "cwd": ".",',
' "action": [',
' "python",',
' "-c",',
' "pass",',
' ]',
' },',
'',
' # src -> src/repo16',
' {',
' "name": "relative_cwd",',
' "pattern": ".",',
' "cwd": "src/repo16",',
' "action": [',
' "python",',
' "relative.py",',
' ]',
' },',
'',
']',
'',
'vars = {',
......@@ -1116,6 +1149,8 @@ class GClientSmokeGIT(GClientSmokeBase):
'',
'}',
'',
'# ' + self.git_base + 'repo_15, DEPS',
'# ' + self.git_base + 'repo_16, DEPS',
'# ' + self.git_base + 'repo_2@%s, DEPS' % (
self.githash('repo_2', 1)[:7]),
'# ' + self.git_base + 'repo_6, DEPS',
......@@ -1174,6 +1209,18 @@ class GClientSmokeGIT(GClientSmokeBase):
' "condition": \'(checkout_linux) or (checkout_mac)\',',
' },',
'',
' # src -> src/repo15',
' "src/repo15": {',
' "url": "' + self.git_base + 'repo_15@%s",' % (
self.githash('repo_15', 1)),
' },',
'',
' # src -> src/repo16',
' "src/repo16": {',
' "url": "' + self.git_base + 'repo_16@%s",' % (
self.githash('repo_16', 1)),
' },',
'',
' # src -> src/repo2',
' "src/repo2": {',
' "url": "' + self.git_base + 'repo_2@%s",' % (
......@@ -1248,6 +1295,29 @@ class GClientSmokeGIT(GClientSmokeBase):
' ]',
' },',
'',
' # src -> src/repo15',
' {',
' "name": "absolute_cwd",',
' "pattern": ".",',
' "cwd": ".",',
' "action": [',
' "python",',
' "-c",',
' "pass",',
' ]',
' },',
'',
' # src -> src/repo16',
' {',
' "name": "relative_cwd",',
' "pattern": ".",',
' "cwd": "src/repo16",',
' "action": [',
' "python",',
' "relative.py",',
' ]',
' },',
'',
']',
'',
'vars = {',
......@@ -1286,6 +1356,10 @@ class GClientSmokeGIT(GClientSmokeBase):
'',
'}',
'',
'# ' + self.git_base + 'repo_15@%s, DEPS' % (
self.githash('repo_15', 1)),
'# ' + self.git_base + 'repo_16@%s, DEPS' % (
self.githash('repo_16', 1)),
'# ' + self.git_base + 'repo_2@%s, DEPS' % (
self.githash('repo_2', 1)),
'# ' + self.git_base + 'repo_6@%s, DEPS' % (
......
tests/gclient_test.py
View file @ a68660d0
......@@ -202,6 +202,21 @@ class GclientTest(trial_dir.TestCase):
pass
return items
def _get_hooks(self):
"""Retrieves the hooks that would be run"""
parser = gclient.OptionParser()
options, _ = parser.parse_args([])
options.force = True
client = gclient.GClient.LoadCurrentConfig(options)
work_queue = gclient_utils.ExecutionQueue(options.jobs, None, False)
for s in client.dependencies:
work_queue.enqueue(s)
work_queue.flush({}, None, [], options=options, patch_refs={},
target_branches={})
return client.GetHooks(options)
def testAutofix(self):
# Invalid urls causes pain when specifying requirements. Make sure it's
# auto-fixed.
......@@ -296,18 +311,8 @@ class GclientTest(trial_dir.TestCase):
write(os.path.join('top', 'fake.txt'),
"bogus content")
parser = gclient.OptionParser()
options, _ = parser.parse_args([])
options.force = True
client = gclient.GClient.LoadCurrentConfig(options)
work_queue = gclient_utils.ExecutionQueue(options.jobs, None, False)
for s in client.dependencies:
work_queue.enqueue(s)
work_queue.flush({}, None, [], options=options, patch_refs={},
target_branches={})
self.assertEqual(
[h.action for h in client.GetHooks(options)],
[h.action for h in self._get_hooks()],
[tuple(x['action']) for x in hooks])
def testCustomHooks(self):
......@@ -348,20 +353,83 @@ class GclientTest(trial_dir.TestCase):
write(os.path.join('bottom', 'fake.txt'),
"bogus content")
parser = gclient.OptionParser()
options, _ = parser.parse_args([])
options.force = True
client = gclient.GClient.LoadCurrentConfig(options)
work_queue = gclient_utils.ExecutionQueue(options.jobs, None, False)
for s in client.dependencies:
work_queue.enqueue(s)
work_queue.flush({}, None, [], options=options, patch_refs={},
target_branches={})
self.assertEqual(
[h.action for h in client.GetHooks(options)],
[h.action for h in self._get_hooks()],
[tuple(x['action']) for x in hooks + extra_hooks + sub_hooks])
def testRecurseDepsAndHooks(self):
"""Verifies that hooks in recursedeps are ran."""
write(
'.gclient',
'solutions = [\n'
' { "name": "foo", "url": "svn://example.com/foo" },\n'
']')
write(
os.path.join('foo', 'DEPS'),
'use_relative_paths = True\n'
'deps = {\n'
' "bar": "/bar",\n'
'}\n'
'recursedeps = ["bar"]')
write(
os.path.join('foo', 'bar', 'DEPS'),
'hooks = [{\n'
' "name": "toto",\n'
' "pattern": ".",\n'
' "action": ["tata", "titi"]\n'
'}]\n')
write(os.path.join('foo', 'bar', 'fake.txt'),
"bogus content")
self.assertEqual(
[h.action for h in self._get_hooks()],
[('tata', 'titi')])
def testRecurseDepsAndHooksCwd(self):
"""Verifies that hooks run in the correct directory with our without
use_relative_hooks"""
write(
'.gclient',
'solutions = [\n'
' { "name": "foo", "url": "svn://example.com/foo" },\n'
']')
write(
os.path.join('foo', 'DEPS'),
'use_relative_paths = True\n'
'deps = {\n'
' "bar": "/bar",\n'
' "baz": "/baz",\n'
'}\n'
'recursedeps = ["bar", "baz"]')
write(
os.path.join('foo', 'bar', 'DEPS'),
'hooks = [{\n'
' "name": "toto",\n'
' "pattern": ".",\n'
' "action": ["tata", "titi"]\n'
'}]\n')
write(os.path.join('foo', 'bar', 'fake.txt'),
"bogus content")
write(
os.path.join('foo', 'baz', 'DEPS'),
'use_relative_paths=True\n'
'use_relative_hooks=True\n'
'hooks = [{\n'
' "name": "lazors",\n'
' "pattern": ".",\n'
' "action": ["fire", "lazors"]\n'
'}]\n')
write(os.path.join('foo', 'baz', 'fake.txt'),
"bogus content")
self.assertEqual([(h.action, h.effective_cwd) for h in self._get_hooks()],
[
(('tata', 'titi'), self.root_dir),
(('fire', 'lazors'), os.path.join(self.root_dir, "foo", "baz"))
])
def testTargetOS(self):
"""Verifies that specifying a target_os pulls in all relevant dependencies.
......@@ -705,7 +773,7 @@ class GclientTest(trial_dir.TestCase):
'.gclient',
'solutions = [\n'
' { "name": "foo", "url": "svn://example.com/foo" },\n'
']')
']')
write(
os.path.join('foo', 'DEPS'),
'use_relative_paths = True\n'
......@@ -741,7 +809,7 @@ class GclientTest(trial_dir.TestCase):
'.gclient',
'solutions = [\n'
' { "name": "foo", "url": "svn://example.com/foo" },\n'
']')
']')
write(
os.path.join('foo', 'DEPS'),
'use_relative_paths = True\n'
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment