_yaml: Move 'node_composite' to a method on 'MappingNode'bschubert/new-node-compose

- Also take care of node_composite_move in the same way.
- Adapt all calling places

10 files changed, 64 insertions, 68 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 e470a7d50..32a39dfd4 100644
--- a/src/buildstream/_yaml.pxd
+++ b/src/buildstream/_yaml.pxd
@@ -38,6 +38,8 @@ cdef class Node:
 
 
 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=*)
diff --git a/src/buildstream/_yaml.pyx b/src/buildstream/_yaml.pyx
index d9f965521..6420474eb 100644
--- a/src/buildstream/_yaml.pyx
+++ b/src/buildstream/_yaml.pyx
@@ -232,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)
 
@@ -1202,49 +1239,6 @@ cdef Node __new_node_from_list(list inlist):
     return SequenceNode(ret, _SYNTHETIC_FILE_INDEX, 0, next_synthetic_counter())
 
 
-# composite()
-#
-# 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: LoadError
-#
-cpdef void composite(MappingNode target, MappingNode source) except *:
-    assert type(source.value) is dict
-    assert type(target.value) is dict
-
-    try:
-        source._composite(target, [])
-    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 ad8a09a33..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(base_config, project_config)
+            project_config.composite(base_config)
 
             _yaml.roundtrip_dump(base_config, project_filename)
 
diff --git a/tests/internals/yaml.py b/tests/internals/yaml.py
index 0432a0a84..9b3269d46 100644
--- a/tests/internals/yaml.py
+++ b/tests/internals/yaml.py
@@ -183,7 +183,7 @@ def test_composite_preserve_originals(datafiles):
     base = _yaml.load(filename)
     overlay = _yaml.load(overlayfile)
     base_copy = base.copy()
-    _yaml.composite(base_copy, overlay)
+    overlay.composite(base_copy)
 
     copy_extra = base_copy.get_mapping('extra')
     orig_extra = base.get_mapping('extra')
@@ -249,7 +249,7 @@ def test_list_composition(datafiles, filename, tmpdir,
     base = _yaml.load(base_file, 'basics.yaml')
     overlay = _yaml.load(overlay_file, shortname=filename)
 
-    _yaml.composite(base, overlay)
+    overlay.composite(base)
 
     children = base.get_sequence('children')
     assert len(children) == length
@@ -267,7 +267,7 @@ def test_list_deletion(datafiles):
 
     base = _yaml.load(base, shortname='basics.yaml')
     overlay = _yaml.load(overlay, shortname='listoverwriteempty.yaml')
-    _yaml.composite(base, overlay)
+    overlay.composite(base)
 
     children = base.get_sequence('children')
     assert not children
@@ -384,8 +384,8 @@ def test_list_composition_twice(datafiles, tmpdir, filename1, filename2,
     overlay1 = _yaml.load(file1, shortname=filename1)
     overlay2 = _yaml.load(file2, shortname=filename2)
 
-    _yaml.composite(base, overlay1)
-    _yaml.composite(base, overlay2)
+    overlay1.composite(base)
+    overlay2.composite(base)
 
     children = base.get_sequence('children')
     assert len(children) == length
@@ -401,8 +401,8 @@ def test_list_composition_twice(datafiles, tmpdir, filename1, filename2,
     overlay1 = _yaml.load(file1, shortname=filename1)
     overlay2 = _yaml.load(file2, shortname=filename2)
 
-    _yaml.composite(overlay1, overlay2)
-    _yaml.composite(base, overlay1)
+    overlay2.composite(overlay1)
+    overlay1.composite(base)
 
     children = base.get_sequence('children')
     assert len(children) == length