diff --git a/gclient.py b/gclient.py index b95979f20..35377d6b9 100755 --- a/gclient.py +++ b/gclient.py @@ -167,16 +167,13 @@ class Hook(object): def from_dict(d, variables=None, verbose=False, conditions=None): """Creates a Hook instance from a dict like in the DEPS file.""" # Merge any local and inherited conditions. - if conditions and d.get('condition'): - condition = '(%s) and (%s)' % (conditions, d['condition']) - else: - condition = conditions or d.get('condition') + gclient_eval.UpdateCondition(d, 'and', conditions) return Hook( d['action'], d.get('pattern'), d.get('name'), d.get('cwd'), - condition, + d.get('condition'), variables=variables, # Always print the header if not printing to a TTY. verbose=verbose or not setup_color.IS_TTY) @@ -392,8 +389,6 @@ class Dependency(gclient_utils.WorkItem, DependencySettings): # Calculates properties: self._dependencies = [] self._vars = {} - self._os_dependencies = {} - self._os_deps_hooks = {} # A cache of the files affected by the current operation, necessary for # hooks. @@ -597,52 +592,6 @@ class Dependency(gclient_utils.WorkItem, DependencySettings): return False return True - @staticmethod - def MergeWithOsDeps(deps, deps_os, target_os_list, process_all_deps): - """Returns a new "deps" structure that is the deps sent in updated - with information from deps_os (the deps_os section of the DEPS - file) that matches the list of target os.""" - new_deps = deps.copy() - for dep_os, os_deps in deps_os.iteritems(): - for key, value in os_deps.iteritems(): - if value is None: - # Make this condition very visible, so it's not a silent failure. - # It's unclear how to support None override in deps_os. - logging.error('Ignoring %r:%r in %r deps_os', key, value, dep_os) - continue - - # Normalize value to be a dict which contains |should_process| metadata. - if isinstance(value, basestring): - value = {'url': value} - assert isinstance(value, collections.Mapping), (key, value) - value['should_process'] = dep_os in target_os_list or process_all_deps - - # Handle collisions/overrides. - if key in new_deps and new_deps[key] != value: - # Normalize the existing new_deps entry. - if isinstance(new_deps[key], basestring): - new_deps[key] = {'url': new_deps[key]} - assert isinstance(new_deps[key], - collections.Mapping), (key, new_deps[key]) - - # It's OK if the "override" sets the key to the same value. - # This is mostly for legacy reasons to keep existing DEPS files - # working. Often mac/ios and unix/android will do this. - if value['url'] != new_deps[key]['url']: - raise gclient_utils.Error( - ('Value from deps_os (%r; %r: %r) conflicts with existing deps ' - 'entry (%r).') % (dep_os, key, value, new_deps[key])) - - # We'd otherwise overwrite |should_process| metadata, but a dep should - # be processed if _any_ of its references call for that. - value['should_process'] = ( - value['should_process'] or - new_deps[key].get('should_process', True)) - - new_deps[key] = value - - return new_deps - def _postprocess_deps(self, deps, rel_prefix): """Performs post-processing of deps compared to what's in the DEPS file.""" # Make sure the dict is mutable, e.g. in case it's frozen. @@ -653,6 +602,7 @@ class Dependency(gclient_utils.WorkItem, DependencySettings): for d in self.custom_deps: if d not in deps: deps[d] = self.custom_deps[d] + # Make child deps conditional on any parent conditions. This ensures that, # when flattened, recursed entries have the correct restrictions, even if # not explicitly set in the recursed DEPS file. For instance, if @@ -660,17 +610,8 @@ class Dependency(gclient_utils.WorkItem, DependencySettings): # recursively included by "src/ios_foo/DEPS" should also require # "checkout_ios=True". if self.condition: - for dname, dval in deps.iteritems(): - if isinstance(dval, basestring): - dval = {'url': dval} - deps[dname] = dval - else: - assert isinstance(dval, collections.Mapping) - if dval.get('condition'): - dval['condition'] = '(%s) and (%s)' % ( - dval['condition'], self.condition) - else: - dval['condition'] = self.condition + for value in deps.itervalues(): + gclient_eval.UpdateCondition(value, 'and', self.condition) if rel_prefix: logging.warning('use_relative_paths enabled.') @@ -697,21 +638,10 @@ class Dependency(gclient_utils.WorkItem, DependencySettings): if dep_value is None: continue - condition = None - condition_value = True - if isinstance(dep_value, basestring): - raw_url = dep_value - dep_type = None - else: - # This should be guaranteed by schema checking in gclient_eval. - assert isinstance(dep_value, collections.Mapping) - raw_url = dep_value.get('url') - # Take into account should_process metadata set by MergeWithOsDeps. - should_process = (should_process and - dep_value.get('should_process', True)) - condition = dep_value.get('condition') - dep_type = dep_value.get('dep_type') + condition = dep_value.get('condition') + dep_type = dep_value.get('dep_type') + condition_value = True if condition: condition_value = gclient_eval.EvaluateCondition( condition, self.get_vars()) @@ -732,6 +662,7 @@ class Dependency(gclient_utils.WorkItem, DependencySettings): self.custom_vars, should_process, use_relative_paths, condition, condition_value)) else: + raw_url = dep_value.get('url') url = raw_url.format(**self.get_vars()) if raw_url else None deps_to_add.append( GitDependency( @@ -824,10 +755,6 @@ class Dependency(gclient_utils.WorkItem, DependencySettings): elif self._relative: rel_prefix = os.path.dirname(self.name) - deps = {} - for key, value in local_scope.get('deps', {}).iteritems(): - deps[key.format(**self.get_vars())] = value - if 'recursion' in local_scope: self.recursion_override = local_scope.get('recursion') logging.warning( @@ -857,19 +784,8 @@ class Dependency(gclient_utils.WorkItem, DependencySettings): # 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'] - # load os specific dependencies if defined. these dependencies may - # override or extend the values defined by the 'deps' member. - target_os_list = self.target_os - if 'deps_os' in local_scope: - for dep_os, os_deps in local_scope['deps_os'].iteritems(): - self._os_dependencies[dep_os] = self._deps_to_objects( - self._postprocess_deps(os_deps, rel_prefix), use_relative_paths) - if target_os_list and not self._get_option( - 'do_not_merge_os_specific_entries', False): - deps = self.MergeWithOsDeps( - deps, local_scope['deps_os'], target_os_list, - self._get_option('process_all_deps', False)) + deps = local_scope.get('deps', {}) deps_to_add = self._deps_to_objects( self._postprocess_deps(deps, rel_prefix), use_relative_paths) @@ -879,21 +795,6 @@ class Dependency(gclient_utils.WorkItem, DependencySettings): for hook in local_scope.get('hooks', []): if hook.get('name', '') not in hook_names_to_suppress: hooks_to_run.append(hook) - if 'hooks_os' in local_scope and target_os_list: - hooks_os = local_scope['hooks_os'] - - # Keep original contents of hooks_os for flatten. - for hook_os, os_hooks in hooks_os.iteritems(): - self._os_deps_hooks[hook_os] = [ - Hook.from_dict(hook, variables=self.get_vars(), verbose=True, - conditions=self.condition) - for hook in os_hooks] - - # Specifically append these to ensure that hooks_os run after hooks. - if not self._get_option('do_not_merge_os_specific_entries', False): - for the_target_os in target_os_list: - the_target_os_hooks = hooks_os.get(the_target_os, []) - hooks_to_run.extend(the_target_os_hooks) # add the replacements and any additions for hook in self.custom_hooks: @@ -1196,21 +1097,11 @@ class Dependency(gclient_utils.WorkItem, DependencySettings): def dependencies(self): return tuple(self._dependencies) - @property - @gclient_utils.lockedmethod - def os_dependencies(self): - return dict(self._os_dependencies) - @property @gclient_utils.lockedmethod def deps_hooks(self): return tuple(self._deps_hooks) - @property - @gclient_utils.lockedmethod - def os_deps_hooks(self): - return dict(self._os_deps_hooks) - @property @gclient_utils.lockedmethod def pre_deps_hooks(self): @@ -2097,9 +1988,7 @@ class Flattener(object): self._allowed_hosts = set() self._deps = {} - self._deps_os = {} self._hooks = [] - self._hooks_os = {} self._pre_deps_hooks = [] self._vars = {} @@ -2146,10 +2035,6 @@ class Flattener(object): for dep in self._deps.itervalues(): self._pin_dep(dep) - for os_deps in self._deps_os.itervalues(): - for dep in os_deps.itervalues(): - self._pin_dep(dep) - def add_deps_file(dep): # Only include DEPS files referenced by recursedeps. if not (dep.parent is None or @@ -2168,9 +2053,6 @@ class Flattener(object): self._deps_files.add((dep.url, deps_file, dep.hierarchy_data())) for dep in self._deps.itervalues(): add_deps_file(dep) - for os_deps in self._deps_os.itervalues(): - for dep in os_deps.itervalues(): - add_deps_file(dep) gn_args_dep = self._deps.get(self._client.dependencies[0]._gn_args_from, self._client.dependencies[0]) @@ -2178,10 +2060,8 @@ class Flattener(object): _GNSettingsToLines(gn_args_dep._gn_args_file, gn_args_dep._gn_args) + _AllowedHostsToLines(self._allowed_hosts) + _DepsToLines(self._deps) + - _DepsOsToLines(self._deps_os) + _HooksToLines('hooks', self._hooks) + _HooksToLines('pre_deps_hooks', self._pre_deps_hooks) + - _HooksOsToLines(self._hooks_os) + _VarsToLines(self._vars) + ['# %s, %s' % (url, deps_file) for url, deps_file, _ in sorted(self._deps_files)] + @@ -2198,31 +2078,16 @@ class Flattener(object): if dep.url: self._deps[dep.name] = dep - def _add_os_dep(self, os_dep, dep_os): - """Helper to add an OS-specific dependency to flattened DEPS. - - Arguments: - os_dep (Dependency): dependency to add - dep_os (str): name of the OS - """ - assert ( - os_dep.name not in self._deps_os.get(dep_os, {}) or - self._deps_os.get(dep_os, {}).get(os_dep.name) == os_dep), ( - os_dep.name, self._deps_os.get(dep_os, {}).get(os_dep.name)) - if os_dep.url: - self._deps_os.setdefault(dep_os, {})[os_dep.name] = os_dep - - def _flatten_dep(self, dep, dep_os=None): + def _flatten_dep(self, dep): """Visits a dependency in order to flatten it (see CMDflatten). Arguments: dep (Dependency): dependency to process - dep_os (str or None): name of the OS |dep| is specific to """ - logging.debug('_flatten_dep(%s, %s)', dep.name, dep_os) + logging.debug('_flatten_dep(%s)', dep.name) - if not dep.deps_parsed: - dep.ParseDepsFile() + assert dep.deps_parsed, ( + "Attempted to flatten %s but it has not been processed." % dep.name) self._allowed_hosts.update(dep.allowed_hosts) @@ -2249,41 +2114,14 @@ class Flattener(object): self._vars[key] = (hierarchy + ' [custom_var override]', value) self._pre_deps_hooks.extend([(dep, hook) for hook in dep.pre_deps_hooks]) - - if dep_os: - if dep.deps_hooks: - self._hooks_os.setdefault(dep_os, []).extend( - [(dep, hook) for hook in dep.deps_hooks]) - else: - self._hooks.extend([(dep, hook) for hook in dep.deps_hooks]) + self._hooks.extend([(dep, hook) for hook in dep.deps_hooks]) for sub_dep in dep.dependencies: - if dep_os: - self._add_os_dep(sub_dep, dep_os) - else: - self._add_dep(sub_dep) - - for hook_os, os_hooks in dep.os_deps_hooks.iteritems(): - self._hooks_os.setdefault(hook_os, []).extend( - [(dep, hook) for hook in os_hooks]) - - for sub_dep_os, os_deps in dep.os_dependencies.iteritems(): - for os_dep in os_deps: - self._add_os_dep(os_dep, sub_dep_os) - - # Process recursedeps. |deps_by_name| is a map where keys are dependency - # names, and values are maps of OS names to |Dependency| instances. - # |None| in place of OS name means the dependency is not OS-specific. - deps_by_name = dict((d.name, {None: d}) for d in dep.dependencies) - for sub_dep_os, os_deps in dep.os_dependencies.iteritems(): - for os_dep in os_deps: - assert sub_dep_os not in deps_by_name.get(os_dep.name, {}), ( - os_dep.name, sub_dep_os) - deps_by_name.setdefault(os_dep.name, {})[sub_dep_os] = os_dep + self._add_dep(sub_dep) + + deps_by_name = {d.name: d for d in dep.dependencies} for recurse_dep_name in (dep.recursedeps or []): - dep_info = deps_by_name[recurse_dep_name] - for sub_dep_os, os_dep in dep_info.iteritems(): - self._flatten_dep(os_dep, dep_os=(sub_dep_os or dep_os)) + self._flatten_dep(deps_by_name[recurse_dep_name]) def CMDflatten(parser, args): @@ -2299,7 +2137,6 @@ def CMDflatten(parser, args): 'for checked out deps, NOT deps_os.')) options, args = parser.parse_args(args) - options.do_not_merge_os_specific_entries = True options.nohooks = True options.process_all_deps = True client = GClient.LoadCurrentConfig(options) @@ -2691,10 +2528,6 @@ def CMDsync(parser, args): help='override deps for the specified (comma-separated) ' 'platform(s); \'all\' will process all deps_os ' 'references') - # TODO(phajdan.jr): use argparse.SUPPRESS to hide internal flags. - parser.add_option('--do-not-merge-os-specific-entries', action='store_true', - help='INTERNAL ONLY - disables merging of deps_os and ' - 'hooks_os to dependencies and hooks') parser.add_option('--process-all-deps', action='store_true', help='Check out all deps, even for different OS-es, ' 'or with conditions evaluating to false') diff --git a/gclient_eval.py b/gclient_eval.py index 4d0b50730..5e2fd337c 100644 --- a/gclient_eval.py +++ b/gclient_eval.py @@ -5,8 +5,11 @@ import ast import cStringIO import collections +import logging import tokenize +import gclient_utils + from third_party import schema @@ -345,31 +348,9 @@ def Exec(content, expand_vars=True, filename='', vars_override=None): return _GCLIENT_SCHEMA.validate(local_scope) -def Parse(content, expand_vars, validate_syntax, filename, vars_override=None): - """Parses DEPS strings. - - Executes the Python-like string stored in content, resulting in a Python - dictionary specifyied by the schema above. Supports syntax validation and - variable expansion. - - Args: - content: str. DEPS file stored as a string. - expand_vars: bool. Whether variables should be expanded to their values. - validate_syntax: bool. Whether syntax should be validated using the schema - defined above. - filename: str. The name of the DEPS file, or a string describing the source - of the content, e.g. '', ''. - vars_override: dict, optional. A dictionary with overrides for the variables - defined by the DEPS file. - - Returns: - A Python dict with the parsed contents of the DEPS file, as specified by the - schema above. - """ - # TODO(ehmaldonado): Make validate_syntax = True the only case - if validate_syntax: - return Exec(content, expand_vars, filename, vars_override) - +def ExecLegacy(content, expand_vars=True, filename='', + vars_override=None): + """Executes a DEPS file |content| using exec.""" local_scope = {} global_scope = {'Var': lambda var_name: '{%s}' % var_name} @@ -409,6 +390,119 @@ def Parse(content, expand_vars, validate_syntax, filename, vars_override=None): return _DeepFormat(local_scope) +def _StandardizeDeps(deps_dict, vars_dict): + """"Standardizes the deps_dict. + + For each dependency: + - Expands the variable in the dependency name. + - Ensures the dependency is a dictionary. + - Set's the 'dep_type' to be 'git' by default. + """ + new_deps_dict = {} + for dep_name, dep_info in deps_dict.items(): + dep_name = dep_name.format(**vars_dict) + if not isinstance(dep_info, collections.Mapping): + dep_info = {'url': dep_info} + dep_info.setdefault('dep_type', 'git') + new_deps_dict[dep_name] = dep_info + return new_deps_dict + + +def _MergeDepsOs(deps_dict, os_deps_dict, os_name): + """Merges the deps in os_deps_dict into conditional dependencies in deps_dict. + + The dependencies in os_deps_dict are transformed into conditional dependencies + using |'checkout_' + os_name|. + If the dependency is already present, the URL and revision must coincide. + """ + for dep_name, dep_info in os_deps_dict.items(): + # Make this condition very visible, so it's not a silent failure. + # It's unclear how to support None override in deps_os. + if dep_info['url'] is None: + logging.error('Ignoring %r:%r in %r deps_os', dep_name, dep_info, os_name) + continue + + os_condition = 'checkout_' + (os_name if os_name != 'unix' else 'linux') + UpdateCondition(dep_info, 'and', os_condition) + + if dep_name in deps_dict: + if deps_dict[dep_name]['url'] != dep_info['url']: + raise gclient_utils.Error( + 'Value from deps_os (%r; %r: %r) conflicts with existing deps ' + 'entry (%r).' % ( + os_name, dep_name, dep_info, deps_dict[dep_name])) + + UpdateCondition(dep_info, 'or', deps_dict[dep_name].get('condition')) + + deps_dict[dep_name] = dep_info + + +def UpdateCondition(info_dict, op, new_condition): + """Updates info_dict's condition with |new_condition|. + + An absent value is treated as implicitly True. + """ + curr_condition = info_dict.get('condition') + # Easy case: Both are present. + if curr_condition and new_condition: + info_dict['condition'] = '(%s) %s (%s)' % ( + curr_condition, op, new_condition) + # If |op| == 'and', and at least one condition is present, then use it. + elif op == 'and' and (curr_condition or new_condition): + info_dict['condition'] = curr_condition or new_condition + # Otherwise, no condition should be set + elif curr_condition: + del info_dict['condition'] + + +def Parse(content, expand_vars, validate_syntax, filename, vars_override=None): + """Parses DEPS strings. + + Executes the Python-like string stored in content, resulting in a Python + dictionary specifyied by the schema above. Supports syntax validation and + variable expansion. + + Args: + content: str. DEPS file stored as a string. + expand_vars: bool. Whether variables should be expanded to their values. + validate_syntax: bool. Whether syntax should be validated using the schema + defined above. + filename: str. The name of the DEPS file, or a string describing the source + of the content, e.g. '', ''. + vars_override: dict, optional. A dictionary with overrides for the variables + defined by the DEPS file. + + Returns: + A Python dict with the parsed contents of the DEPS file, as specified by the + schema above. + """ + if validate_syntax: + result = Exec(content, expand_vars, filename, vars_override) + else: + result = ExecLegacy(content, expand_vars, filename, vars_override) + + vars_dict = result.get('vars', {}) + if 'deps' in result: + result['deps'] = _StandardizeDeps(result['deps'], vars_dict) + + if 'deps_os' in result: + deps = result.setdefault('deps', {}) + for os_name, os_deps in result['deps_os'].iteritems(): + os_deps = _StandardizeDeps(os_deps, vars_dict) + _MergeDepsOs(deps, os_deps, os_name) + del result['deps_os'] + + if 'hooks_os' in result: + hooks = result.setdefault('hooks', []) + for os_name, os_hooks in result['hooks_os'].iteritems(): + for hook in os_hooks: + UpdateCondition(hook, 'and', 'checkout_' + os_name) + hooks.extend(os_hooks) + del result['hooks_os'] + + return result + + def EvaluateCondition(condition, variables, referenced_variables=None): """Safely evaluates a boolean condition. Returns the result.""" if not referenced_variables: diff --git a/tests/gclient_eval_unittest.py b/tests/gclient_eval_unittest.py index f341c98e5..f29945cb8 100755 --- a/tests/gclient_eval_unittest.py +++ b/tests/gclient_eval_unittest.py @@ -186,6 +186,49 @@ class ExecTest(unittest.TestCase): str(cm.exception)) +class UpdateConditionTest(unittest.TestCase): + def test_both_present(self): + info = {'condition': 'foo'} + gclient_eval.UpdateCondition(info, 'and', 'bar') + self.assertEqual(info, {'condition': '(foo) and (bar)'}) + + info = {'condition': 'foo'} + gclient_eval.UpdateCondition(info, 'or', 'bar') + self.assertEqual(info, {'condition': '(foo) or (bar)'}) + + def test_one_present_and(self): + # If one of info's condition or new_condition is present, and |op| == 'and' + # then the the result must be the present condition. + info = {'condition': 'foo'} + gclient_eval.UpdateCondition(info, 'and', None) + self.assertEqual(info, {'condition': 'foo'}) + + info = {} + gclient_eval.UpdateCondition(info, 'and', 'bar') + self.assertEqual(info, {'condition': 'bar'}) + + def test_both_absent_and(self): + # Nothing happens + info = {} + gclient_eval.UpdateCondition(info, 'and', None) + self.assertEqual(info, {}) + + def test_or(self): + # If one of info's condition and new_condition is not present, then there + # shouldn't be a condition. An absent value is treated as implicitly True. + info = {'condition': 'foo'} + gclient_eval.UpdateCondition(info, 'or', None) + self.assertEqual(info, {}) + + info = {} + gclient_eval.UpdateCondition(info, 'or', 'bar') + self.assertEqual(info, {}) + + info = {} + gclient_eval.UpdateCondition(info, 'or', None) + self.assertEqual(info, {}) + + class EvaluateConditionTest(unittest.TestCase): def test_true(self): self.assertTrue(gclient_eval.EvaluateCondition('True', {})) @@ -573,8 +616,9 @@ class ParseTest(unittest.TestCase): for validate_syntax in True, False: local_scope = self.callParse(validate_syntax=validate_syntax) self.assertEqual({ - 'vars': collections.OrderedDict([('foo', 'bar')]), - 'deps': collections.OrderedDict([('a_dep', 'abarb')]), + 'vars': {'foo': 'bar'}, + 'deps': {'a_dep': {'url': 'abarb', + 'dep_type': 'git'}}, }, local_scope) def test_no_expands_vars(self): @@ -582,8 +626,9 @@ class ParseTest(unittest.TestCase): local_scope = self.callParse(False, validate_syntax=validate_syntax) self.assertEqual({ - 'vars': collections.OrderedDict([('foo', 'bar')]), - 'deps': collections.OrderedDict([('a_dep', 'a{foo}b')]), + 'vars': {'foo': 'bar'}, + 'deps': {'a_dep': {'url': 'a{foo}b', + 'dep_type': 'git'}}, }, local_scope) def test_overrides_vars(self): @@ -591,8 +636,9 @@ class ParseTest(unittest.TestCase): local_scope = self.callParse(validate_syntax=validate_syntax, vars_override={'foo': 'baz'}) self.assertEqual({ - 'vars': collections.OrderedDict([('foo', 'bar')]), - 'deps': collections.OrderedDict([('a_dep', 'abazb')]), + 'vars': {'foo': 'bar'}, + 'deps': {'a_dep': {'url': 'abazb', + 'dep_type': 'git'}}, }, local_scope) def test_no_extra_vars(self): @@ -618,6 +664,171 @@ class ParseTest(unittest.TestCase): '', {'baz': 'lalala'}) self.assertIn('baz', str(cm.exception)) + def test_standardizes_deps_string_dep(self): + for validate_syntax in True, False: + local_scope = gclient_eval.Parse('\n'.join([ + 'deps = {', + ' "a_dep": "a_url@a_rev",', + '}', + ]), False, validate_syntax, '') + self.assertEqual({ + 'deps': {'a_dep': {'url': 'a_url@a_rev', + 'dep_type': 'git'}}, + }, local_scope) + + def test_standardizes_deps_dict_dep(self): + for validate_syntax in True, False: + local_scope = gclient_eval.Parse('\n'.join([ + 'deps = {', + ' "a_dep": {', + ' "url": "a_url@a_rev",', + ' "condition": "checkout_android",', + ' },', + '}', + ]), False, validate_syntax, '') + self.assertEqual({ + 'deps': {'a_dep': {'url': 'a_url@a_rev', + 'dep_type': 'git', + 'condition': 'checkout_android'}}, + }, local_scope) + + def test_ignores_none_in_deps_os(self): + for validate_syntax in True, False: + local_scope = gclient_eval.Parse('\n'.join([ + 'deps = {', + ' "a_dep": "a_url@a_rev",', + '}', + 'deps_os = {', + ' "mac": {', + ' "a_dep": None,', + ' },', + '}', + ]), False, validate_syntax, '') + self.assertEqual({ + 'deps': {'a_dep': {'url': 'a_url@a_rev', + 'dep_type': 'git'}}, + }, local_scope) + + def test_merges_deps_os_extra_dep(self): + for validate_syntax in True, False: + local_scope = gclient_eval.Parse('\n'.join([ + 'deps = {', + ' "a_dep": "a_url@a_rev",', + '}', + 'deps_os = {', + ' "mac": {', + ' "b_dep": "b_url@b_rev"', + ' },', + '}', + ]), False, validate_syntax, '') + self.assertEqual({ + 'deps': {'a_dep': {'url': 'a_url@a_rev', + 'dep_type': 'git'}, + 'b_dep': {'url': 'b_url@b_rev', + 'dep_type': 'git', + 'condition': 'checkout_mac'}}, + }, local_scope) + + def test_merges_deps_os_existing_dep_with_no_condition(self): + for validate_syntax in True, False: + local_scope = gclient_eval.Parse('\n'.join([ + 'deps = {', + ' "a_dep": "a_url@a_rev",', + '}', + 'deps_os = {', + ' "mac": {', + ' "a_dep": "a_url@a_rev"', + ' },', + '}', + ]), False, validate_syntax, '') + self.assertEqual({ + 'deps': {'a_dep': {'url': 'a_url@a_rev', + 'dep_type': 'git'}}, + }, local_scope) + + def test_merges_deps_os_existing_dep_with_condition(self): + for validate_syntax in True, False: + local_scope = gclient_eval.Parse('\n'.join([ + 'deps = {', + ' "a_dep": {', + ' "url": "a_url@a_rev",', + ' "condition": "some_condition",', + ' },', + '}', + 'deps_os = {', + ' "mac": {', + ' "a_dep": "a_url@a_rev"', + ' },', + '}', + ]), False, validate_syntax, '') + self.assertEqual({ + 'deps': { + 'a_dep': {'url': 'a_url@a_rev', + 'dep_type': 'git', + 'condition': '(checkout_mac) or (some_condition)'}, + }, + }, local_scope) + + def test_merges_deps_os_multiple_os(self): + for validate_syntax in True, False: + local_scope = gclient_eval.Parse('\n'.join([ + 'deps_os = {', + ' "win": {' + ' "a_dep": "a_url@a_rev"', + ' },', + ' "mac": {', + ' "a_dep": "a_url@a_rev"', + ' },', + '}', + ]), False, validate_syntax, '') + self.assertEqual({ + 'deps': { + 'a_dep': {'url': 'a_url@a_rev', + 'dep_type': 'git', + 'condition': '(checkout_mac) or (checkout_win)'}, + }, + }, local_scope) + + def test_fails_to_merge_same_dep_with_different_revisions(self): + for validate_syntax in True, False: + with self.assertRaises(gclient_eval.gclient_utils.Error) as cm: + gclient_eval.Parse('\n'.join([ + 'deps = {', + ' "a_dep": {', + ' "url": "a_url@a_rev",', + ' "condition": "some_condition",', + ' },', + '}', + 'deps_os = {', + ' "mac": {', + ' "a_dep": "a_url@b_rev"', + ' },', + '}', + ]), False, validate_syntax, '') + self.assertIn('conflicts with existing deps', str(cm.exception)) + + def test_merges_hooks_os(self): + for validate_syntax in True, False: + local_scope = gclient_eval.Parse('\n'.join([ + 'hooks = [', + ' {', + ' "action": ["a", "action"],', + ' },', + ']', + 'hooks_os = {', + ' "mac": [', + ' {', + ' "action": ["b", "action"]', + ' },', + ' ]', + '}', + ]), False, validate_syntax, '') + self.assertEqual({ + "hooks": [{"action": ["a", "action"]}, + {"action": ["b", "action"], "condition": "checkout_mac"}], + }, local_scope) + + if __name__ == '__main__': level = logging.DEBUG if '-v' in sys.argv else logging.FATAL diff --git a/tests/gclient_smoketest.py b/tests/gclient_smoketest.py index b566556ca..751ffad80 100755 --- a/tests/gclient_smoketest.py +++ b/tests/gclient_smoketest.py @@ -875,6 +875,18 @@ class GClientSmokeGIT(GClientSmokeBase): ' "url": "git://127.0.0.1:20000/git/repo_6",', ' },', '', + ' # src -> src/mac_repo', + ' "src/mac_repo": {', + ' "url": "{repo5_var}",', + ' "condition": \'checkout_mac\',', + ' },', + '', + ' # src -> src/repo8 -> src/recursed_os_repo', + ' "src/recursed_os_repo": {', + ' "url": "/repo_5",', + ' "condition": \'(checkout_linux) or (checkout_mac)\',', + ' },', + '', ' # src -> src/repo2', ' "src/repo2": {', ' "url": "{git_base}repo_2@%s",' % ( @@ -893,41 +905,16 @@ class GClientSmokeGIT(GClientSmokeBase): ' "url": "/repo_8",', ' },', '', - '}', - '', - 'deps_os = {', - ' "mac": {', - ' # src -> src/mac_repo', - ' "src/mac_repo": {', - ' "url": "{repo5_var}",', - ' },', - '', - ' # src -> src/repo8 -> src/recursed_os_repo', - ' "src/recursed_os_repo": {', - ' "url": "/repo_5",', - ' },', - '', - ' },', - '', - ' "unix": {', - ' # src -> src/repo8 -> src/recursed_os_repo', - ' "src/recursed_os_repo": {', - ' "url": "/repo_5",', - ' },', - '', - ' # src -> src/unix_repo', - ' "src/unix_repo": {', - ' "url": "{repo5_var}",', - ' },', - '', + ' # src -> src/unix_repo', + ' "src/unix_repo": {', + ' "url": "{repo5_var}",', + ' "condition": \'checkout_linux\',', ' },', '', - ' "win": {', - ' # src -> src/win_repo', - ' "src/win_repo": {', - ' "url": "{repo5_var}",', - ' },', - '', + ' # src -> src/win_repo', + ' "src/win_repo": {', + ' "url": "{repo5_var}",', + ' "condition": \'checkout_win\',', ' },', '', '}', @@ -957,25 +944,20 @@ class GClientSmokeGIT(GClientSmokeBase): ' ]', ' },', '', - ']', - '', - 'hooks_os = {', - ' "mac": [', - ' # src', - ' {', - ' "pattern": ".",', - ' "cwd": ".",', - ' "action": [', - ' "python",', - ' "-c",', - ' "open(\'src/git_hooked_mac\', \'w\').write(' - '\'git_hooked_mac\')",', - ' ]', - ' },', - '', - ' ],', + ' # src', + ' {', + ' "pattern": ".",', + ' "condition": \'checkout_mac\',', + ' "cwd": ".",', + ' "action": [', + ' "python",', + ' "-c",', + ' "open(\'src/git_hooked_mac\', \'w\').write(' + '\'git_hooked_mac\')",', + ' ]', + ' },', '', - '}', + ']', '', 'vars = {', ' # src', @@ -1055,6 +1037,18 @@ class GClientSmokeGIT(GClientSmokeBase): self.githash('repo_6', 1)), ' },', '', + ' # src -> src/mac_repo', + ' "src/mac_repo": {', + ' "url": "{repo5_var}@%s",' % (self.githash('repo_5', 3)), + ' "condition": \'checkout_mac\',', + ' },', + '', + ' # src -> src/repo8 -> src/recursed_os_repo', + ' "src/recursed_os_repo": {', + ' "url": "/repo_5@%s",' % (self.githash('repo_5', 3)), + ' "condition": \'(checkout_linux) or (checkout_mac)\',', + ' },', + '', ' # src -> src/repo2', ' "src/repo2": {', ' "url": "{git_base}repo_2@%s",' % ( @@ -1073,41 +1067,16 @@ class GClientSmokeGIT(GClientSmokeBase): ' "url": "/repo_8@%s",' % (self.githash('repo_8', 1)), ' },', '', - '}', - '', - 'deps_os = {', - ' "mac": {', - ' # src -> src/mac_repo', - ' "src/mac_repo": {', - ' "url": "{repo5_var}@%s",' % (self.githash('repo_5', 3)), - ' },', - '', - ' # src -> src/repo8 -> src/recursed_os_repo', - ' "src/recursed_os_repo": {', - ' "url": "/repo_5@%s",' % (self.githash('repo_5', 3)), - ' },', - '', - ' },', - '', - ' "unix": {', - ' # src -> src/repo8 -> src/recursed_os_repo', - ' "src/recursed_os_repo": {', - ' "url": "/repo_5@%s",' % (self.githash('repo_5', 3)), - ' },', - '', - ' # src -> src/unix_repo', - ' "src/unix_repo": {', - ' "url": "{repo5_var}@%s",' % (self.githash('repo_5', 3)), - ' },', - '', + ' # src -> src/unix_repo', + ' "src/unix_repo": {', + ' "url": "{repo5_var}@%s",' % (self.githash('repo_5', 3)), + ' "condition": \'checkout_linux\',', ' },', '', - ' "win": {', - ' # src -> src/win_repo', - ' "src/win_repo": {', - ' "url": "{repo5_var}@%s",' % (self.githash('repo_5', 3)), - ' },', - '', + ' # src -> src/win_repo', + ' "src/win_repo": {', + ' "url": "{repo5_var}@%s",' % (self.githash('repo_5', 3)), + ' "condition": \'checkout_win\',', ' },', '', '}', @@ -1137,25 +1106,20 @@ class GClientSmokeGIT(GClientSmokeBase): ' ]', ' },', '', - ']', - '', - 'hooks_os = {', - ' "mac": [', - ' # src', - ' {', - ' "pattern": ".",', - ' "cwd": ".",', - ' "action": [', - ' "python",', - ' "-c",', - ' "open(\'src/git_hooked_mac\', \'w\').write(' - '\'git_hooked_mac\')",', - ' ]', - ' },', - '', - ' ],', + ' # src', + ' {', + ' "pattern": ".",', + ' "condition": \'checkout_mac\',', + ' "cwd": ".",', + ' "action": [', + ' "python",', + ' "-c",', + ' "open(\'src/git_hooked_mac\', \'w\').write(' + '\'git_hooked_mac\')",', + ' ]', + ' },', '', - '}', + ']', '', 'vars = {', ' # src', @@ -1218,6 +1182,7 @@ class GClientSmokeGIT(GClientSmokeBase): with open(output_deps) as f: deps_contents = f.read() + self.maxDiff = None self.assertEqual([ 'gclient_gn_args_file = "src/repo2/gclient.args"', "gclient_gn_args = ['str_var']", @@ -1227,6 +1192,30 @@ class GClientSmokeGIT(GClientSmokeBase): ' "url": "git://127.0.0.1:20000/git/repo_10",', ' },', '', + ' # src -> src/repo9 -> src/repo8 -> src/recursed_os_repo', + ' "src/recursed_os_repo": {', + ' "url": "/repo_5",', + ' "condition": \'(checkout_linux) or (checkout_mac)\',', + ' },', + '', + ' # src -> src/repo11', + ' "src/repo11": {', + ' "url": "/repo_11",', + ' "condition": \'(checkout_ios) or (checkout_mac)\',', + ' },', + '', + ' # src -> src/repo11 -> src/repo12', + ' "src/repo12": {', + ' "url": "/repo_12",', + ' "condition": \'(checkout_ios) or (checkout_mac)\',', + ' },', + '', + ' # src -> src/repo9 -> src/repo4', + ' "src/repo4": {', + ' "url": "/repo_4",', + ' "condition": \'checkout_android\',', + ' },', + '', ' # src -> src/repo6', ' "src/repo6": {', ' "url": "/repo_6",', @@ -1249,56 +1238,6 @@ class GClientSmokeGIT(GClientSmokeBase): '', '}', '', - 'deps_os = {', - ' "android": {', - ' # src -> src/repo9 -> src/repo4', - ' "src/repo4": {', - ' "url": "/repo_4",', - ' },', - '', - ' },', - '', - ' "ios": {', - ' # src -> src/repo11', - ' "src/repo11": {', - ' "url": "/repo_11",', - ' },', - '', - ' # src -> src/repo11 -> src/repo12', - ' "src/repo12": {', - ' "url": "/repo_12",', - ' },', - '', - ' },', - '', - ' "mac": {', - ' # src -> src/repo9 -> src/repo8 -> src/recursed_os_repo', - ' "src/recursed_os_repo": {', - ' "url": "/repo_5",', - ' },', - '', - ' # src -> src/repo11', - ' "src/repo11": {', - ' "url": "/repo_11",', - ' },', - '', - ' # src -> src/repo11 -> src/repo12', - ' "src/repo12": {', - ' "url": "/repo_12",', - ' },', - '', - ' },', - '', - ' "unix": {', - ' # src -> src/repo9 -> src/repo8 -> src/recursed_os_repo', - ' "src/recursed_os_repo": {', - ' "url": "/repo_5",', - ' },', - '', - ' },', - '', - '}', - '', 'vars = {', ' # src -> src/repo9', ' "str_var": \'xyz\',', diff --git a/tests/gclient_test.py b/tests/gclient_test.py index 996346dfc..c79755273 100755 --- a/tests/gclient_test.py +++ b/tests/gclient_test.py @@ -436,19 +436,10 @@ class GclientTest(trial_dir.TestCase): ' }]\n') write( os.path.join('foo', 'DEPS'), - 'target_os = ["baz"]\n' - 'deps_os = {\n' - ' "unix": { "foo/unix": "/unix", },\n' - ' "baz": { "foo/baz": "/baz", },\n' - ' "jaz": { "foo/jaz": "/jaz", },\n' - '}') + 'target_os = ["baz"]\n') write( os.path.join('bar', 'DEPS'), - 'deps_os = {\n' - ' "unix": { "bar/unix": "/unix", },\n' - ' "baz": { "bar/baz": "/baz", },\n' - ' "jaz": { "bar/jaz": "/jaz", },\n' - '}') + '') parser = gclient.OptionParser() options, _ = parser.parse_args(['--jobs', '1']) @@ -457,15 +448,11 @@ class GclientTest(trial_dir.TestCase): obj = gclient.GClient.LoadCurrentConfig(options) obj.RunOnDeps('None', []) self.assertEqual(['unix'], sorted(obj.enforced_os)) - self.assertEquals( - [ - ('bar', 'svn://example.com/bar'), - ('bar/unix', 'svn://example.com/unix'), - ('foo', 'svn://example.com/foo'), - ('foo/baz', 'svn://example.com/baz'), - ('foo/unix', 'svn://example.com/unix'), - ], - sorted(self._get_processed())) + self.assertEqual([('unix', 'baz'), ('unix',)], + [dep.target_os for dep in obj.dependencies]) + self.assertEqual([('foo', 'svn://example.com/foo'), + ('bar', 'svn://example.com/bar')], + self._get_processed()) def testTargetOsForHooksInDepsFile(self): """Verifies that specifying a target_os value in a DEPS file runs the right @@ -506,119 +493,15 @@ class GclientTest(trial_dir.TestCase): obj = gclient.GClient.LoadCurrentConfig(options) obj.RunOnDeps('None', args) self.assertEqual(['zippy'], sorted(obj.enforced_os)) - all_hooks = [h.action for h in obj.GetHooks(options)] - self.assertEquals( - [('.', 'svn://example.com/'),], - sorted(self._get_processed())) - self.assertEquals(all_hooks, - [('python', 'do_a')]) - - # Test for OS that has extra hooks in hooks_os. - parser = gclient.OptionParser() - options, args = parser.parse_args(['--jobs', '1']) - options.deps_os = 'blorp' - - obj = gclient.GClient.LoadCurrentConfig(options) - obj.RunOnDeps('None', args) - self.assertEqual(['blorp'], sorted(obj.enforced_os)) - all_hooks = [h.action for h in obj.GetHooks(options)] + all_hooks = obj.GetHooks(options) self.assertEquals( [('.', 'svn://example.com/'),], sorted(self._get_processed())) - self.assertEquals(all_hooks, + self.assertEquals([h.action for h in all_hooks], [('python', 'do_a'), ('python', 'do_b')]) - - - def testUpdateWithOsDeps(self): - """Verifies that complicated deps_os constructs result in the - correct data also with multple operating systems. Also see - testDepsOsOverrideDepsInDepsFile.""" - - test_data = [ - # Tuples of deps, deps_os, os_list and expected_deps. - ( - # OS with no overrides at all. - {'foo': 'default_foo'}, - {'os1': { 'foo': None } }, - ['os2'], - {'foo': 'default_foo'} - ), - ( - # One OS wants to add a module. - {'foo': 'default_foo'}, - {'os1': { 'bar': 'os1_bar' }}, - ['os1'], - {'foo': 'default_foo', - 'bar': {'should_process': True, 'url': 'os1_bar'}} - ), - ( - # One OS wants to add a module. One doesn't care. - {'foo': 'default_foo'}, - {'os1': { 'bar': 'os1_bar' }}, - ['os1', 'os2'], - {'foo': 'default_foo', - 'bar': {'should_process': True, 'url': 'os1_bar'}} - ), - ( - # Two OSes want to add a module with the same definition. - {'foo': 'default_foo'}, - {'os1': { 'bar': 'os12_bar' }, - 'os2': { 'bar': 'os12_bar' }}, - ['os1', 'os2'], - {'foo': 'default_foo', - 'bar': {'should_process': True, 'url': 'os12_bar'}} - ), - ( - # One OS doesn't need module, one OS wants the default. - {'foo': 'default_foo'}, - {'os1': { 'foo': None }, - 'os2': {}}, - ['os1', 'os2'], - {'foo': 'default_foo'} - ), - ( - # OS doesn't need module. - {'foo': 'default_foo'}, - {'os1': { 'foo': None } }, - ['os1'], - {'foo': 'default_foo'} - ), - ( - # No-op override. Regression test for http://crbug.com/735418 . - {'foo': 'default_foo'}, - {'os1': { 'foo': 'default_foo' } }, - [], - {'foo': {'should_process': True, 'url': 'default_foo'}} - ), - ] - for deps, deps_os, target_os_list, expected_deps in test_data: - orig_deps = copy.deepcopy(deps) - result = gclient.Dependency.MergeWithOsDeps( - deps, deps_os, target_os_list, False) - self.assertEqual(result, expected_deps) - self.assertEqual(deps, orig_deps) - - def testUpdateWithOsDepsInvalid(self): - test_data = [ - # Tuples of deps, deps_os, os_list. - ( - # OS wants a different version of module. - {'foo': 'default_foo'}, - {'os1': { 'foo': 'os1_foo'} }, - ['os1'], - ), - ( - # One OS doesn't need module, another OS wants a special version. - {'foo': 'default_foo'}, - {'os1': { 'foo': None }, - 'os2': { 'foo': 'os2_foo'}}, - ['os1', 'os2'], - ), - ] - for deps, deps_os, target_os_list in test_data: - with self.assertRaises(gclient_utils.Error): - gclient.Dependency.MergeWithOsDeps(deps, deps_os, target_os_list, False) + self.assertEquals([h.condition for h in all_hooks], + [None, 'checkout_blorp']) def testOverride(self): """Verifies expected behavior of OverrideURL."""