diff options
author | Benjamin Schubert <contact@benschubert.me> | 2019-07-03 15:58:12 +0000 |
---|---|---|
committer | Benjamin Schubert <contact@benschubert.me> | 2019-07-03 15:58:12 +0000 |
commit | b4b0f79f6e09a89f1eee785702f47e5975e41bf7 (patch) | |
tree | 358dce8d3146891e18b0eda378ad56158416b393 /src/buildstream | |
parent | e89f5a27213831bb50d71ad66eaf6985fad999e0 (diff) | |
parent | 35fad402ef5d75d25589fdfd6cddde0306c263aa (diff) | |
download | buildstream-b4b0f79f6e09a89f1eee785702f47e5975e41bf7.tar.gz |
Merge branch 'bschubert/new-node-compose' into 'bschubert/new-node-api'
Rework node compose API
See merge request BuildStream/buildstream!1442
Diffstat (limited to 'src/buildstream')
-rw-r--r-- | src/buildstream/_context.py | 2 | ||||
-rw-r--r-- | src/buildstream/_includes.py | 2 | ||||
-rw-r--r-- | src/buildstream/_options/optionpool.py | 2 | ||||
-rw-r--r-- | src/buildstream/_project.py | 6 | ||||
-rw-r--r-- | src/buildstream/_yaml.pxd | 7 | ||||
-rw-r--r-- | src/buildstream/_yaml.pyx | 400 | ||||
-rw-r--r-- | src/buildstream/element.py | 20 | ||||
-rw-r--r-- | src/buildstream/source.py | 2 | ||||
-rw-r--r-- | src/buildstream/testing/runcli.py | 2 |
9 files changed, 218 insertions, 225 deletions
diff --git a/src/buildstream/_context.py b/src/buildstream/_context.py index 35ed95e11..524305177 100644 --- a/src/buildstream/_context.py +++ b/src/buildstream/_context.py @@ -201,7 +201,7 @@ class Context(): if config: self.config_origin = os.path.abspath(config) user_config = _yaml.load(config) - _yaml.composite(defaults, user_config) + user_config.composite(defaults) # Give obsoletion warnings if 'builddir' in defaults: diff --git a/src/buildstream/_includes.py b/src/buildstream/_includes.py index ea2bf484e..4c1eabb7e 100644 --- a/src/buildstream/_includes.py +++ b/src/buildstream/_includes.py @@ -81,7 +81,7 @@ class Includes: finally: included.remove(file_path) - _yaml.composite_and_move(node, include_node) + include_node.composite_under(node) for value in node.values(): self._process_value(value, diff --git a/src/buildstream/_options/optionpool.py b/src/buildstream/_options/optionpool.py index eae03f181..5155f62f4 100644 --- a/src/buildstream/_options/optionpool.py +++ b/src/buildstream/_options/optionpool.py @@ -290,7 +290,7 @@ class OptionPool(): # Apply the yaml fragment if its condition evaluates to true if apply_fragment: - _yaml.composite(node, value) + value.composite(node) return True diff --git a/src/buildstream/_project.py b/src/buildstream/_project.py index ba3b92207..1a22e0b06 100644 --- a/src/buildstream/_project.py +++ b/src/buildstream/_project.py @@ -570,7 +570,7 @@ class Project(): raise pre_config_node = self._default_config_node.copy() - _yaml.composite(pre_config_node, self._project_conf) + self._project_conf.composite(pre_config_node) # Assert project's format version early, before validating toplevel keys format_version = pre_config_node.get_int('format-version') @@ -616,7 +616,7 @@ class Project(): project_conf_first_pass = self._project_conf.copy() self._project_includes.process(project_conf_first_pass, only_local=True) config_no_include = self._default_config_node.copy() - _yaml.composite(config_no_include, project_conf_first_pass) + project_conf_first_pass.composite(config_no_include) self._load_pass(config_no_include, self.first_pass_config, ignore_unknown=True) @@ -640,7 +640,7 @@ class Project(): project_conf_second_pass = self._project_conf.copy() self._project_includes.process(project_conf_second_pass) config = self._default_config_node.copy() - _yaml.composite(config, project_conf_second_pass) + project_conf_second_pass.composite(config) self._load_pass(config, self.config) diff --git a/src/buildstream/_yaml.pxd b/src/buildstream/_yaml.pxd index 146f14a22..32a39dfd4 100644 --- a/src/buildstream/_yaml.pxd +++ b/src/buildstream/_yaml.pxd @@ -31,11 +31,15 @@ cdef class Node: cpdef object strip_node_info(self) cpdef void _assert_fully_composited(self) except * + cdef void _compose_on(self, str key, MappingNode target, list path) except * + cdef bint _is_composite_list(self) except * cdef bint _shares_position_with(self, Node target) cdef bint _walk_find(self, Node target, list path) except * cdef class MappingNode(Node): + cpdef void composite(self, MappingNode target) except * + cpdef void composite_under(self, MappingNode target) except * cdef Node get(self, str key, default, default_constructor) cpdef MappingNode get_mapping(self, str key, default=*) cpdef Node get_node(self, str key, list allowed_types=*, bint allow_none=*) @@ -49,6 +53,9 @@ cdef class MappingNode(Node): cpdef void safe_del(self, str key) cpdef object values(self) + cdef void _composite(self, MappingNode target, list path=*) except * + cdef void _compose_on_composite_dict(self, MappingNode target) + cdef void _compose_on_list(self, SequenceNode target) cpdef list _find(self, Node target) diff --git a/src/buildstream/_yaml.pyx b/src/buildstream/_yaml.pyx index 2c45057ee..6420474eb 100644 --- a/src/buildstream/_yaml.pyx +++ b/src/buildstream/_yaml.pyx @@ -101,6 +101,25 @@ cdef class Node: cpdef void _assert_fully_composited(self) except *: raise NotImplementedError() + # _is_composite_list + # + # Checks if the node is a Mapping with array composition + # directives. + # + # Returns: + # (bool): True if node was a Mapping containing only + # list composition directives + # + # Raises: + # (LoadError): If node was a mapping and contained a mix of + # list composition directives and other keys + # + cdef bint _is_composite_list(self) except *: + raise NotImplementedError() + + cdef void _compose_on(self, str key, MappingNode target, list path) except *: + raise NotImplementedError() + def __json__(self): raise ValueError("Nodes should not be allowed when jsonify-ing data", self) @@ -159,6 +178,20 @@ cdef class ScalarNode(Node): cpdef void _assert_fully_composited(self) except *: pass + cdef void _compose_on(self, str key, MappingNode target, list path) except *: + cdef Node target_value = target.value.get(key) + + if target_value is not None and type(target_value) is not ScalarNode: + raise CompositeError(path, + "{}: Cannot compose scalar on non-scalar at {}".format( + node_get_provenance(self), + node_get_provenance(target_value))) + + target.value[key] = self + + cdef bint _is_composite_list(self) except *: + return False + cdef bint _walk_find(self, Node target, list path) except *: return self._shares_position_with(target) @@ -199,6 +232,43 @@ cdef class MappingNode(Node): return path return None + # composite() + # + # Compose one mapping node onto another + # + # Args: + # target (Node): The target to compose into + # + # Raises: LoadError + # + cpdef void composite(self, MappingNode target) except *: + try: + self._composite(target, []) + except CompositeError as e: + source_provenance = node_get_provenance(self) + error_prefix = "" + if source_provenance: + error_prefix = "{}: ".format(source_provenance) + raise LoadError(LoadErrorReason.ILLEGAL_COMPOSITE, + "{}Failure composing {}: {}" + .format(error_prefix, + e.path, + e.message)) from e + + # Like composite(target, source), but where target overrides source instead. + # + cpdef void composite_under(self, MappingNode target) except *: + target.composite(self) + + cdef str key + cdef Node value + cdef list to_delete = [key for key in target.value.keys() if key not in self.value] + + for key, value in self.value.items(): + target.value[key] = value + for key in to_delete: + del target.value[key] + cdef Node get(self, str key, object default, object default_constructor): value = self.value.get(key, _sentinel) @@ -306,6 +376,111 @@ cdef class MappingNode(Node): return {key: value.strip_node_info() for key, value in self.value.items()} + cdef void _composite(self, MappingNode target, list path=None) except *: + cdef str key + cdef Node value + + for key, value in self.value.items(): + path.append(key) + value._compose_on(key, target, path) + path.pop() + + cdef void _compose_on(self, str key, MappingNode target, list path) except *: + cdef Node target_value + + if self._is_composite_list(): + if key not in target.value: + # Composite list clobbers empty space + target.value[key] = self + else: + target_value = target.value[key] + + if type(target_value) is SequenceNode: + # Composite list composes into a list + self._compose_on_list(target_value) + elif target_value._is_composite_list(): + # Composite list merges into composite list + self._compose_on_composite_dict(target_value) + else: + # Else composing on top of normal dict or a scalar, so raise... + raise CompositeError(path, + "{}: Cannot compose lists onto {}".format( + node_get_provenance(self), + node_get_provenance(target_value))) + else: + # We're composing a dict into target now + if key not in target.value: + # Target lacks a dict at that point, make a fresh one with + # the same provenance as the incoming dict + target.value[key] = MappingNode({}, self.file_index, self.line, self.column) + + self._composite(target.value[key], path) + + cdef void _compose_on_list(self, SequenceNode target): + cdef SequenceNode clobber = self.value.get("(=)") + cdef SequenceNode prefix = self.value.get("(<)") + cdef SequenceNode suffix = self.value.get("(>)") + + if clobber is not None: + target.value.clear() + target.value.extend(clobber.value) + if prefix is not None: + for v in reversed(prefix.value): + target.value.insert(0, v) + if suffix is not None: + target.value.extend(suffix.value) + + cdef void _compose_on_composite_dict(self, MappingNode target): + cdef SequenceNode clobber = self.value.get("(=)") + cdef SequenceNode prefix = self.value.get("(<)") + cdef SequenceNode suffix = self.value.get("(>)") + + if clobber is not None: + # We want to clobber the target list + # which basically means replacing the target list + # with ourselves + target.value["(=)"] = clobber + if prefix is not None: + target.value["(<)"] = prefix + elif "(<)" in target.value: + target.value["(<)"].value.clear() + if suffix is not None: + target.value["(>)"] = suffix + elif "(>)" in target.value: + target.value["(>)"].value.clear() + else: + # Not clobbering, so prefix the prefix and suffix the suffix + if prefix is not None: + if "(<)" in target.value: + for v in reversed(prefix.value): + target.value["(<)"].value.insert(0, v) + else: + target.value["(<)"] = prefix + if suffix is not None: + if "(>)" in target.value: + target.value["(>)"].value.extend(suffix.value) + else: + target.value["(>)"] = suffix + + cdef bint _is_composite_list(self) except *: + cdef bint has_directives = False + cdef bint has_keys = False + cdef str key + + for key in self.value.keys(): + if key in ['(>)', '(<)', '(=)']: + has_directives = True + else: + has_keys = True + + if has_keys and has_directives: + provenance = node_get_provenance(self) + raise LoadError(LoadErrorReason.INVALID_DATA, + "{}: Dictionary contains array composition directives and arbitrary keys" + .format(provenance)) + + return has_directives + def __delitem__(self, str key): del self.value[key] @@ -423,6 +598,24 @@ cdef class SequenceNode(Node): for value in self.value: value._assert_fully_composited() + cdef void _compose_on(self, str key, MappingNode target, list path) except *: + # List clobbers anything list-like + cdef Node target_value = target.value.get(key) + + if not (target_value is None or + type(target_value) is SequenceNode or + target_value._is_composite_list()): + raise CompositeError(path, + "{}: List cannot overwrite {} at: {}" + .format(node_get_provenance(self), + key, + node_get_provenance(target_value))) + # Looks good, clobber it + target.value[key] = self + + cdef bint _is_composite_list(self) except *: + return False + cdef bint _walk_find(self, Node target, list path) except *: cdef int i cdef Node v @@ -1046,213 +1239,6 @@ cdef Node __new_node_from_list(list inlist): return SequenceNode(ret, _SYNTHETIC_FILE_INDEX, 0, next_synthetic_counter()) -# _is_composite_list -# -# Checks if the given node is a Mapping with array composition -# directives. -# -# Args: -# node (value): Any node -# -# Returns: -# (bool): True if node was a Mapping containing only -# list composition directives -# -# Raises: -# (LoadError): If node was a mapping and contained a mix of -# list composition directives and other keys -# -cdef bint _is_composite_list(Node node): - cdef bint has_directives = False - cdef bint has_keys = False - cdef str key - - if type(node) is MappingNode: - for key in (<MappingNode> node).keys(): - if key in ['(>)', '(<)', '(=)']: # pylint: disable=simplifiable-if-statement - has_directives = True - else: - has_keys = True - - if has_keys and has_directives: - provenance = node_get_provenance(node) - raise LoadError(LoadErrorReason.INVALID_DATA, - "{}: Dictionary contains array composition directives and arbitrary keys" - .format(provenance)) - return has_directives - - return False - - -# _compose_composite_list() -# -# Composes a composite list (i.e. a dict with list composition directives) -# on top of a target list which is a composite list itself. -# -# Args: -# target (Node): A composite list -# source (Node): A composite list -# -cdef void _compose_composite_list(Node target, Node source): - clobber = source.value.get("(=)") - prefix = source.value.get("(<)") - suffix = source.value.get("(>)") - if clobber is not None: - # We want to clobber the target list - # which basically means replacing the target list - # with ourselves - target.value["(=)"] = clobber - if prefix is not None: - target.value["(<)"] = prefix - elif "(<)" in target.value: - target.value["(<)"].value.clear() - if suffix is not None: - target.value["(>)"] = suffix - elif "(>)" in target.value: - target.value["(>)"].value.clear() - else: - # Not clobbering, so prefix the prefix and suffix the suffix - if prefix is not None: - if "(<)" in target.value: - for v in reversed(prefix.value): - target.value["(<)"].value.insert(0, v) - else: - target.value["(<)"] = prefix - if suffix is not None: - if "(>)" in target.value: - target.value["(>)"].value.extend(suffix.value) - else: - target.value["(>)"] = suffix - - -# _compose_list() -# -# Compose a composite list (a dict with composition directives) on top of a -# simple list. -# -# Args: -# target (Node): The target list to be composed into -# source (Node): The composition list to be composed from -# -cdef void _compose_list(Node target, Node source): - clobber = source.value.get("(=)") - prefix = source.value.get("(<)") - suffix = source.value.get("(>)") - if clobber is not None: - target.value.clear() - target.value.extend(clobber.value) - if prefix is not None: - for v in reversed(prefix.value): - target.value.insert(0, v) - if suffix is not None: - target.value.extend(suffix.value) - - -# composite_dict() -# -# Compose one mapping node onto another -# -# Args: -# target (Node): The target to compose into -# source (Node): The source to compose from -# path (list): The path to the current composition node -# -# Raises: CompositeError -# -cpdef void composite_dict(Node target, Node source, list path=None) except *: - cdef str k - cdef Node v, target_value - - if path is None: - path = [] - for k, v in source.value.items(): - path.append(k) - if type(v.value) is list: - # List clobbers anything list-like - target_value = target.value.get(k) - if not (target_value is None or - type(target_value.value) is list or - _is_composite_list(target_value)): - raise CompositeError(path, - "{}: List cannot overwrite {} at: {}" - .format(node_get_provenance(source, k), - k, - node_get_provenance(target, k))) - # Looks good, clobber it - target.value[k] = v - elif _is_composite_list(v): - if k not in target.value: - # Composite list clobbers empty space - target.value[k] = v - elif type(target.value[k].value) is list: - # Composite list composes into a list - _compose_list(target.value[k], v) - elif _is_composite_list(target.value[k]): - # Composite list merges into composite list - _compose_composite_list(target.value[k], v) - else: - # Else composing on top of normal dict or a scalar, so raise... - raise CompositeError(path, - "{}: Cannot compose lists onto {}".format( - node_get_provenance(v), - node_get_provenance(target.value[k]))) - elif type(v.value) is dict: - # We're composing a dict into target now - if k not in target.value: - # Target lacks a dict at that point, make a fresh one with - # the same provenance as the incoming dict - target.value[k] = MappingNode({}, v.file_index, v.line, v.column) - if type(target.value) is not dict: - raise CompositeError(path, - "{}: Cannot compose dictionary onto {}".format( - node_get_provenance(v), - node_get_provenance(target.value[k]))) - composite_dict(target.value[k], v, path) - else: - target_value = target.value.get(k) - if target_value is not None and type(target_value.value) is not str: - raise CompositeError(path, - "{}: Cannot compose scalar on non-scalar at {}".format( - node_get_provenance(v), - node_get_provenance(target.value[k]))) - target.value[k] = v - path.pop() - - -# Like composite_dict(), but raises an all purpose LoadError for convenience -# -cpdef void composite(MappingNode target, MappingNode source) except *: - assert type(source.value) is dict - assert type(target.value) is dict - - try: - composite_dict(target, source) - except CompositeError as e: - source_provenance = node_get_provenance(source) - error_prefix = "" - if source_provenance: - error_prefix = "{}: ".format(source_provenance) - raise LoadError(LoadErrorReason.ILLEGAL_COMPOSITE, - "{}Failure composing {}: {}" - .format(error_prefix, - e.path, - e.message)) from e - - -# Like composite(target, source), but where target overrides source instead. -# -def composite_and_move(MappingNode target, MappingNode source): - composite(source, target) - - cdef str key - cdef Node value - cdef list to_delete = [key for key in target.value.keys() if key not in source.value] - for key, value in source.value.items(): - target.value[key] = value - for key in to_delete: - del target.value[key] - - # node_validate() # # Validate the node so as to ensure the user has not specified diff --git a/src/buildstream/element.py b/src/buildstream/element.py index a11ba21b9..a4496e192 100644 --- a/src/buildstream/element.py +++ b/src/buildstream/element.py @@ -2504,7 +2504,7 @@ class Element(Plugin): splits = project._splits.copy() # Extend project wide split rules with any split rules defined by the element - _yaml.composite(splits, element_splits) + element_splits.composite(splits) element_bst['split-rules'] = splits element_public['bst'] = element_bst @@ -2537,7 +2537,7 @@ class Element(Plugin): overrides = elements.get_mapping(kind, default=None) if overrides: - _yaml.composite(defaults, overrides) + overrides.composite(defaults) # Set the data class wide cls.__defaults = defaults @@ -2554,8 +2554,8 @@ class Element(Plugin): else: environment = project.base_environment.copy() - _yaml.composite(environment, default_env) - _yaml.composite(environment, meta.environment) + default_env.composite(environment) + meta.environment.composite(environment) environment._assert_fully_composited() return environment @@ -2600,8 +2600,8 @@ class Element(Plugin): else: variables = project.base_variables.copy() - _yaml.composite(variables, default_vars) - _yaml.composite(variables, meta.variables) + default_vars.composite(variables) + meta.variables.composite(variables) variables._assert_fully_composited() for var in ('project-name', 'element-name', 'max-jobs'): @@ -2623,7 +2623,7 @@ class Element(Plugin): config = cls.__defaults.get_mapping('config', default={}) config = config.copy() - _yaml.composite(config, meta.config) + meta.config.composite(config) config._assert_fully_composited() return config @@ -2649,8 +2649,8 @@ class Element(Plugin): sandbox_defaults = cls.__defaults.get_mapping('sandbox', default={}) sandbox_defaults = sandbox_defaults.copy() - _yaml.composite(sandbox_config, sandbox_defaults) - _yaml.composite(sandbox_config, meta.sandbox) + sandbox_defaults.composite(sandbox_config) + meta.sandbox.composite(sandbox_config) sandbox_config._assert_fully_composited() # Sandbox config, unlike others, has fixed members so we should validate them @@ -2685,7 +2685,7 @@ class Element(Plugin): # Allow elements to extend the default splits defined in their project or # element specific defaults - _yaml.composite(base_splits, element_splits) + element_splits.composite(base_splits) element_bst['split-rules'] = base_splits element_public['bst'] = element_bst diff --git a/src/buildstream/source.py b/src/buildstream/source.py index dfac3d4ec..7d27ac9a6 100644 --- a/src/buildstream/source.py +++ b/src/buildstream/source.py @@ -1291,7 +1291,7 @@ class Source(Plugin): config = cls.__defaults.get_mapping('config', default={}) config = config.copy() - _yaml.composite(config, meta.config) + meta.config.composite(config) config._assert_fully_composited() return config diff --git a/src/buildstream/testing/runcli.py b/src/buildstream/testing/runcli.py index 16593fa8e..016f8a83a 100644 --- a/src/buildstream/testing/runcli.py +++ b/src/buildstream/testing/runcli.py @@ -574,7 +574,7 @@ class CliIntegration(Cli): project_config = _yaml.load(temp_project) - _yaml.composite_dict(base_config, project_config) + project_config.composite(base_config) _yaml.roundtrip_dump(base_config, project_filename) |