_loader: Move extract_depends_from_node from loadelement to types

extract_depends_from_node is only depending on things declared in types,
loadelement doesn't have any dependency on it. It makes more sense to
have it in types.pyx.

Moreover, this allows us to cythonize the function, reducing its total
runtime impact.

- _yaml: expose node_del as public api for Cython

5 files changed, 49 insertions, 52 deletions

diff --git a/src/buildstream/_loader/loadelement.py b/src/buildstream/_loader/loadelement.py
index 24b4e7293..850b41a67 100644
--- a/src/buildstream/_loader/loadelement.py
+++ b/src/buildstream/_loader/loadelement.py
@@ -22,12 +22,8 @@ from itertools import count
 
 from pyroaring import BitMap, FrozenBitMap  # pylint: disable=no-name-in-module
 
-# BuildStream toplevel imports
 from .. import _yaml
 
-# Local package imports
-from .types import Symbol, Dependency
-
 
 # LoadElement():
 #
@@ -137,46 +133,3 @@ class LoadElement():
             self._dep_cache.update(elt._dep_cache)
 
         self._dep_cache = FrozenBitMap(self._dep_cache)
-
-
-# _extract_depends_from_node():
-#
-# Creates an array of Dependency objects from a given dict node 'node',
-# allows both strings and dicts for expressing the dependency and
-# throws a comprehensive LoadError in the case that the node is malformed.
-#
-# After extracting depends, the symbol is deleted from the node
-#
-# Args:
-#    node (dict): A YAML loaded dictionary
-#
-# Returns:
-#    (list): a list of Dependency objects
-#
-def _extract_depends_from_node(node, *, key=None):
-    if key is None:
-        build_depends = _extract_depends_from_node(node, key=Symbol.BUILD_DEPENDS)
-        runtime_depends = _extract_depends_from_node(node, key=Symbol.RUNTIME_DEPENDS)
-        depends = _extract_depends_from_node(node, key=Symbol.DEPENDS)
-        return build_depends + runtime_depends + depends
-    elif key == Symbol.BUILD_DEPENDS:
-        default_dep_type = Symbol.BUILD
-    elif key == Symbol.RUNTIME_DEPENDS:
-        default_dep_type = Symbol.RUNTIME
-    elif key == Symbol.DEPENDS:
-        default_dep_type = None
-    else:
-        assert False, "Unexpected value of key '{}'".format(key)
-
-    depends = _yaml.node_get(node, list, key, default_value=[])
-    output_deps = []
-
-    for index, dep in enumerate(depends):
-        dep_provenance = _yaml.node_get_provenance(node, key=key, indices=[index])
-        dependency = Dependency(dep, dep_provenance, default_dep_type=default_dep_type)
-        output_deps.append(dependency)
-
-    # Now delete the field, we dont want it anymore
-    _yaml.node_del(node, key, safe=True)
-
-    return output_deps
diff --git a/src/buildstream/_loader/loader.py b/src/buildstream/_loader/loader.py
index 5435e0d41..d52a8a72e 100644
--- a/src/buildstream/_loader/loader.py
+++ b/src/buildstream/_loader/loader.py
@@ -28,8 +28,8 @@ from .._profile import Topics, PROFILER
 from .._includes import Includes
 
 from ._loader import valid_chars_name
-from .types import Symbol
-from .loadelement import LoadElement, _extract_depends_from_node
+from .types import Symbol, extract_depends_from_node
+from .loadelement import LoadElement
 from .metaelement import MetaElement
 from .metasource import MetaSource
 from ..types import CoreWarnings
@@ -272,7 +272,7 @@ class Loader():
             ticker(filename)
 
         top_element = self._load_file_no_deps(filename, rewritable, provenance)
-        dependencies = _extract_depends_from_node(top_element.node)
+        dependencies = extract_depends_from_node(top_element.node)
         # The loader queue is a stack of tuples
         # [0] is the LoadElement instance
         # [1] is a stack of dependencies to load
@@ -306,7 +306,7 @@ class Loader():
                         # either recursively cause it to be loaded, or else we
                         # need to push this onto the loader queue in this loader
                         dep_element = self._load_file_no_deps(dep.name, rewritable, dep.provenance)
-                        dep_deps = _extract_depends_from_node(dep_element.node)
+                        dep_deps = extract_depends_from_node(dep_element.node)
                         loader_queue.append((dep_element, list(reversed(dep_deps)), []))
 
                         if _yaml.node_get(dep_element.node, str, Symbol.KIND) == 'junction':
diff --git a/src/buildstream/_loader/types.pyx b/src/buildstream/_loader/types.pyx
index d3cd06253..069d439a2 100644
--- a/src/buildstream/_loader/types.pyx
+++ b/src/buildstream/_loader/types.pyx
@@ -120,3 +120,46 @@ cdef class Dependency:
         # Attempt to split name if no junction was specified explicitly
         if not self.junction and self.name.count(':') == 1:
             self.junction, self.name = self.name.split(':')
+
+
+# extract_depends_from_node():
+#
+# Creates an array of Dependency objects from a given dict node 'node',
+# allows both strings and dicts for expressing the dependency and
+# throws a comprehensive LoadError in the case that the node is malformed.
+#
+# After extracting depends, the symbol is deleted from the node
+#
+# Args:
+#    node (dict): A YAML loaded dictionary
+#
+# Returns:
+#    (list): a list of Dependency objects
+#
+def extract_depends_from_node(node, *, key=None):
+    if key is None:
+        build_depends = extract_depends_from_node(node, key=Symbol.BUILD_DEPENDS)
+        runtime_depends = extract_depends_from_node(node, key=Symbol.RUNTIME_DEPENDS)
+        depends = extract_depends_from_node(node, key=Symbol.DEPENDS)
+        return build_depends + runtime_depends + depends
+    elif key == Symbol.BUILD_DEPENDS:
+        default_dep_type = Symbol.BUILD
+    elif key == Symbol.RUNTIME_DEPENDS:
+        default_dep_type = Symbol.RUNTIME
+    elif key == Symbol.DEPENDS:
+        default_dep_type = None
+    else:
+        assert False, "Unexpected value of key '{}'".format(key)
+
+    depends = _yaml.node_get(node, list, key, None, [])
+    output_deps = []
+
+    for index, dep in enumerate(depends):
+        dep_provenance = _yaml.node_get_provenance(node, key=key, indices=[index])
+        dependency = Dependency(dep, dep_provenance, default_dep_type=default_dep_type)
+        output_deps.append(dependency)
+
+    # Now delete the field, we dont want it anymore
+    _yaml.node_del(node, key, safe=True)
+
+    return output_deps
diff --git a/src/buildstream/_yaml.pxd b/src/buildstream/_yaml.pxd
index b6043e5a8..3cbad0d11 100644
--- a/src/buildstream/_yaml.pxd
+++ b/src/buildstream/_yaml.pxd
@@ -38,6 +38,7 @@ cdef class ProvenanceInformation:
     cdef public bint is_synthetic
 
 
+cpdef void node_del(Node node, str key, bint safe=*) except *
 cpdef object node_get(Node node, object expected_type, str key, list indices=*, object default_value=*, bint allow_none=*)
 cpdef void node_validate(Node node, list valid_keys) except *
 cpdef void node_set(Node node, object key, object value, list indices=*) except *
diff --git a/src/buildstream/_yaml.pyx b/src/buildstream/_yaml.pyx
index edad3fbee..2122dea3a 100644
--- a/src/buildstream/_yaml.pyx
+++ b/src/buildstream/_yaml.pyx
@@ -740,7 +740,7 @@ cpdef list node_keys(Node node):
 #    key (str): The key we want to remove
 #    safe (bool): Whether to raise a KeyError if unable
 #
-def node_del(Node node, str key, bint safe=False):
+cpdef void node_del(Node node, str key, bint safe=False) except *:
     try:
         del node.value[key]
     except KeyError: