_yaml: Remove 'node_del' and support `del mapping[key]`bschubert/node-api-nodel

- Also add a convenience method 'safe_del' catching the exception
when we don't care if the value was there or not.

8 files changed, 20 insertions, 28 deletions

diff --git a/src/buildstream/_includes.py b/src/buildstream/_includes.py
index 2831c9765..db343018b 100644
--- a/src/buildstream/_includes.py
+++ b/src/buildstream/_includes.py
@@ -44,7 +44,7 @@ class Includes:
                 includes = includes_node.as_str_list()
 
             include_provenance = _yaml.node_get_provenance(node, key='(@)')
-            _yaml.node_del(node, '(@)')
+            del node['(@)']
 
             for include in reversed(includes):
                 if only_local and ':' in include:
diff --git a/src/buildstream/_loader/loader.py b/src/buildstream/_loader/loader.py
index 1bad5fe0b..73da5434a 100644
--- a/src/buildstream/_loader/loader.py
+++ b/src/buildstream/_loader/loader.py
@@ -471,12 +471,12 @@ class Loader():
 
         for index, source in enumerate(sources):
             kind = source.get_str(Symbol.KIND)
-            _yaml.node_del(source, Symbol.KIND)
+            del source[Symbol.KIND]
 
             # Directory is optional
             directory = source.get_str(Symbol.DIRECTORY, default=None)
             if directory:
-                _yaml.node_del(source, Symbol.DIRECTORY)
+                del source[Symbol.DIRECTORY]
 
             meta_source = MetaSource(element.name, index, element_kind, kind, source, directory)
             meta_sources.append(meta_source)
diff --git a/src/buildstream/_loader/types.pyx b/src/buildstream/_loader/types.pyx
index dfd8f9046..3a415afc9 100644
--- a/src/buildstream/_loader/types.pyx
+++ b/src/buildstream/_loader/types.pyx
@@ -150,7 +150,7 @@ cdef void _extract_depends_from_node(_yaml.Node node, str key, str default_dep_t
         acc.append(dependency)
 
     # Now delete the field, we dont want it anymore
-    _yaml.node_del(node, key, safe=True)
+    node.safe_del(key)
 
 
 # extract_depends_from_node():
diff --git a/src/buildstream/_options/optionpool.py b/src/buildstream/_options/optionpool.py
index 9a4a88c36..713c9b056 100644
--- a/src/buildstream/_options/optionpool.py
+++ b/src/buildstream/_options/optionpool.py
@@ -267,7 +267,7 @@ class OptionPool():
                 _yaml.node_get_provenance(node, '(?)', indices=[i])
                 for i in range(len(conditions))
             ]
-            _yaml.node_del(node, '(?)')
+            del node['(?)']
 
             for condition, p in zip(conditions, provenance):
                 tuples = list(_yaml.node_items(condition))
diff --git a/src/buildstream/_project.py b/src/buildstream/_project.py
index 424af133e..90de6ba08 100644
--- a/src/buildstream/_project.py
+++ b/src/buildstream/_project.py
@@ -753,8 +753,8 @@ class Project():
         # assertion after.
         output.element_overrides = config.get_mapping('elements', default={})
         output.source_overrides = config.get_mapping('sources', default={})
-        _yaml.node_del(config, 'elements', safe=True)
-        _yaml.node_del(config, 'sources', safe=True)
+        config.safe_del('elements')
+        config.safe_del('sources')
         _yaml.node_final_assertions(config)
 
         self._load_plugin_factories(config, output)
@@ -943,7 +943,7 @@ class Project():
             _yaml.node_set(origin_node, 'plugins', [k for k in _yaml.node_keys(plugins)])
             for group in expected_groups:
                 if group in origin_node:
-                    _yaml.node_del(origin_node, group)
+                    del origin_node[group]
 
             if origin_node.get_str('origin') == 'local':
                 path = self.get_path_from_node(origin, 'path',
diff --git a/src/buildstream/_yaml.pxd b/src/buildstream/_yaml.pxd
index 8deb17a78..e883a232a 100644
--- a/src/buildstream/_yaml.pxd
+++ b/src/buildstream/_yaml.pxd
@@ -37,6 +37,8 @@ cdef class MappingNode(Node):
     cpdef bint get_bool(self, str key, default=*) except *
     cpdef int get_int(self, str key, default=*) except *
     cpdef str get_str(self, str key, object default=*)
+    cpdef void safe_del(self, str key)
+
 
 
 cdef class ScalarNode(Node):
@@ -62,7 +64,6 @@ cdef class ProvenanceInformation:
     cdef public bint is_synthetic
 
 
-cpdef void node_del(Node node, str key, bint safe=*) except *
 cpdef void node_validate(Node node, list valid_keys) except *
 cpdef void node_set(Node node, object key, object value, list indices=*) except *
 cpdef list node_keys(Node node)
diff --git a/src/buildstream/_yaml.pyx b/src/buildstream/_yaml.pyx
index 4a49a9832..8734878bb 100644
--- a/src/buildstream/_yaml.pyx
+++ b/src/buildstream/_yaml.pyx
@@ -215,6 +215,15 @@ cdef class MappingNode(Node):
         cdef ScalarNode scalar = self.get_scalar(key, default)
         return scalar.as_str()
 
+    cpdef void safe_del(self, str key):
+        try:
+            del self.value[key]
+        except KeyError:
+            pass
+
+    def __delitem__(self, str key):
+        del self.value[key]
+
 
 cdef class SequenceNode(Node):
     def __init__(self, list value, int file_index, int line, int column):
@@ -832,24 +841,6 @@ cpdef list node_keys(Node node):
     return list(node.value.keys())
 
 
-# node_del()
-#
-# A convenience generator for iterating over loaded key/value
-# tuples in a dictionary loaded from project YAML.
-#
-# Args:
-#    node (dict): The dictionary node
-#    key (str): The key we want to remove
-#    safe (bool): Whether to raise a KeyError if unable
-#
-cpdef void node_del(Node node, str key, bint safe=False) except *:
-    try:
-        del node.value[key]
-    except KeyError:
-        if not safe:
-            raise
-
-
 # is_node()
 #
 # A test method which returns whether or not the passed in value
diff --git a/tests/frontend/workspace.py b/tests/frontend/workspace.py
index b240e5728..6b71fbc08 100644
--- a/tests/frontend/workspace.py
+++ b/tests/frontend/workspace.py
@@ -1132,7 +1132,7 @@ def test_external_track(cli, datafiles, tmpdir_factory, guess_element):
     # Delete the ref from the source so that we can detect if the
     # element has been tracked
     element_contents = _yaml.load(element_file)
-    _yaml.node_del(element_contents.get_sequence('sources').mapping_at(0), 'ref')
+    del element_contents.get_sequence('sources').mapping_at(0)['ref']
     _yaml.dump(element_contents, element_file)
 
     result = cli.run(project=project, args=['-C', workspace, 'source', 'track', *arg_elm])