From f69860bdbf90ea5c42b610c88d07f087b6c93b96 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Hajdan=2C=20Jr?= Date: Mon, 5 Jun 2017 20:24:28 +0200 Subject: [PATCH] gclient: evaluate conditions for deps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug: 570091 Change-Id: Id95be31767d302c5eb9f7de96d31e32cc4dec1a4 Reviewed-on: https://chromium-review.googlesource.com/524492 Reviewed-by: Dirk Pranke Commit-Queue: Paweł Hajdan Jr. --- gclient.py | 44 +++++++++++++++++++++++++++++------ gclient_eval.py | 6 +++++ testing_support/fake_repos.py | 15 +++++++++++- tests/gclient_smoketest.py | 21 +++++++++++++---- tests/gclient_test.py | 12 ++++++---- 5 files changed, 81 insertions(+), 17 deletions(-) diff --git a/gclient.py b/gclient.py index 59c899dfc..e651c6cb2 100755 --- a/gclient.py +++ b/gclient.py @@ -176,13 +176,18 @@ class DependencySettings(GClientKeywords): """Immutable configuration settings.""" def __init__( self, parent, url, managed, custom_deps, custom_vars, - custom_hooks, deps_file, should_process, relative): + custom_hooks, deps_file, should_process, relative, + condition, condition_value): GClientKeywords.__init__(self) # These are not mutable: self._parent = parent self._deps_file = deps_file self._url = url + # The condition as string (or None). Useful to keep e.g. for flatten. + self._condition = condition + # Boolean value of the condition. If there's no condition, just True. + self._condition_value = condition_value # 'managed' determines whether or not this dependency is synced/updated by # gclient after gclient checks it out initially. The difference between # 'managed' and 'should_process' is that the user specifies 'managed' via @@ -259,6 +264,14 @@ class DependencySettings(GClientKeywords): def url(self): return self._url + @property + def condition(self): + return self._condition + + @property + def condition_value(self): + return self._condition_value + @property def target_os(self): if self.local_target_os is not None: @@ -279,11 +292,12 @@ class Dependency(gclient_utils.WorkItem, DependencySettings): def __init__(self, parent, name, url, managed, custom_deps, custom_vars, custom_hooks, deps_file, should_process, - relative): + relative, condition, condition_value): gclient_utils.WorkItem.__init__(self, name) DependencySettings.__init__( self, parent, url, managed, custom_deps, custom_vars, - custom_hooks, deps_file, should_process, relative) + custom_hooks, deps_file, should_process, relative, + condition, condition_value) # This is in both .gclient and DEPS files: self._deps_hooks = [] @@ -654,15 +668,24 @@ class Dependency(gclient_utils.WorkItem, DependencySettings): deps_file = ent['deps_file'] if dep_value is None: continue + condition = None + condition_value = True if isinstance(dep_value, basestring): url = dep_value else: # This should be guaranteed by schema checking in gclient_eval. assert isinstance(dep_value, dict) url = dep_value['url'] + condition = dep_value.get('condition') + if condition: + # TODO(phajdan.jr): should we also take custom vars into account? + condition_value = gclient_eval.EvaluateCondition( + condition, local_scope.get('vars', {})) + should_process = should_process and condition_value deps_to_add.append(Dependency( self, name, url, None, None, self.custom_vars, None, - deps_file, should_process, use_relative_paths)) + deps_file, should_process, use_relative_paths, condition, + condition_value)) deps_to_add.sort(key=lambda x: x.name) # override named sets of hooks by the custom hooks @@ -1114,7 +1137,7 @@ solutions = [ # are processed. self._recursion_limit = 2 Dependency.__init__(self, None, None, None, True, None, None, None, - 'unused', True, None) + 'unused', True, None, None, True) self._options = options if options.deps_os: enforced_os = options.deps_os.split(',') @@ -1203,7 +1226,9 @@ it or fix the checkout. s.get('custom_hooks', []), s.get('deps_file', 'DEPS'), True, - None)) + None, + None, + True)) except KeyError: raise gclient_utils.Error('Invalid .gclient file. Solution is ' 'incomplete: %s' % s) @@ -1734,9 +1759,14 @@ def _DepsToLines(deps): """Converts |deps| dict to list of lines for output.""" s = ['deps = {'] for name, dep in sorted(deps.iteritems()): + condition_part = ([' "condition": "%s",' % dep.condition] + if dep.condition else []) s.extend([ ' # %s' % dep.hierarchy(include_url=False), - ' "%s": "%s",' % (name, dep.url), + ' "%s": {' % (name,), + ' "url": "%s",' % (dep.url,), + ] + condition_part + [ + ' },', '', ]) s.extend(['}', '']) diff --git a/gclient_eval.py b/gclient_eval.py index 5066a2df4..81930b0d0 100644 --- a/gclient_eval.py +++ b/gclient_eval.py @@ -40,7 +40,13 @@ _GCLIENT_SCHEMA = schema.Schema({ schema.Optional(basestring): schema.Or( basestring, { + # Repo and revision to check out under the path + # (same as if no dict was used). 'url': basestring, + + # Optional condition string. The dep will only be processed + # if the condition evaluates to True. + schema.Optional('condition'): basestring, }, ), }, diff --git a/testing_support/fake_repos.py b/testing_support/fake_repos.py index 764d85cec..16ba5bac2 100755 --- a/testing_support/fake_repos.py +++ b/testing_support/fake_repos.py @@ -325,8 +325,16 @@ vars = { 'DummyVariable': 'repo', } deps = { - 'src/repo2': '%(git_base)srepo_2', + 'src/repo2': { + 'url': '%(git_base)srepo_2', + 'condition': 'True', + }, 'src/repo2/repo3': '/' + Var('DummyVariable') + '_3@%(hash3)s', + # Test that deps where condition evaluates to False are skipped. + 'src/repo5': { + 'url': '/repo_5', + 'condition': 'False', + }, } deps_os = { 'mac': { @@ -449,6 +457,11 @@ pre_deps_hooks = [ deps = { 'src/repo2': { 'url': '%(git_base)srepo_2@%(hash)s', + 'condition': 'True', + }, + 'src/repo4': { + 'url': '/repo_4', + 'condition': 'False', }, } deps_os ={ diff --git a/tests/gclient_smoketest.py b/tests/gclient_smoketest.py index 5deaaebc3..0677f08b7 100755 --- a/tests/gclient_smoketest.py +++ b/tests/gclient_smoketest.py @@ -552,14 +552,27 @@ class GClientSmokeGIT(GClientSmokeBase): self.assertEqual([ 'deps = {', ' # src -> src/repo2 -> foo/bar', - ' "foo/bar": "/repo_3",', + ' "foo/bar": {', + ' "url": "/repo_3",', + ' },', '', ' # src', - ' "src": "git://127.0.0.1:20000/git/repo_6",', + ' "src": {', + ' "url": "git://127.0.0.1:20000/git/repo_6",', + ' },', '', ' # src -> src/repo2', - ' "src/repo2": "git://127.0.0.1:20000/git/repo_2@%s",' % ( - self.githash('repo_2', 1)[:7]), + ' "src/repo2": {', + ' "url": "git://127.0.0.1:20000/git/repo_2@%s",' % ( + self.githash('repo_2', 1)[:7]), + ' "condition": "True",', + ' },', + '', + ' # src -> src/repo4', + ' "src/repo4": {', + ' "url": "/repo_4",', + ' "condition": "False",', + ' },', '', '}', '', diff --git a/tests/gclient_test.py b/tests/gclient_test.py index cde2c9826..35e65277f 100755 --- a/tests/gclient_test.py +++ b/tests/gclient_test.py @@ -202,7 +202,7 @@ class GclientTest(trial_dir.TestCase): # auto-fixed. d = gclient.Dependency( None, 'name', 'proto://host/path/@revision', None, None, None, - None, '', True, False) + None, '', True, False, None, True) self.assertEquals('proto://host/path@revision', d.url) def testStr(self): @@ -212,16 +212,18 @@ class GclientTest(trial_dir.TestCase): obj.add_dependencies_and_close( [ gclient.Dependency( - obj, 'foo', 'url', None, None, None, None, 'DEPS', True, False), + obj, 'foo', 'url', None, None, None, None, 'DEPS', True, False, + None, True), gclient.Dependency( - obj, 'bar', 'url', None, None, None, None, 'DEPS', True, False), + obj, 'bar', 'url', None, None, None, None, 'DEPS', True, False, + None, True), ], []) obj.dependencies[0].add_dependencies_and_close( [ gclient.Dependency( obj.dependencies[0], 'foo/dir1', 'url', None, None, None, - None, 'DEPS', True, False), + None, 'DEPS', True, False, None, True), ], []) # Make sure __str__() works fine. @@ -604,7 +606,7 @@ class GclientTest(trial_dir.TestCase): """Verifies expected behavior of LateOverride.""" url = "git@github.com:dart-lang/spark.git" d = gclient.Dependency(None, 'name', 'url', - None, None, None, None, '', True, False) + None, None, None, None, '', True, False, None, True) late_url = d.LateOverride(url) self.assertEquals(url, late_url)