_loader: Added DependencyTypetristan/dependency-type

This is a bit nicer than relying on strings in the Symbol enumeration,
and allows for some bitwise operations so we can test for BUILD or
RUNTIME.

4 files changed, 53 insertions, 30 deletions

diff --git a/src/buildstream/_loader/__init__.py b/src/buildstream/_loader/__init__.py
index a4be9cfe5..255150734 100644
--- a/src/buildstream/_loader/__init__.py
+++ b/src/buildstream/_loader/__init__.py
@@ -20,5 +20,6 @@
 from .types import Symbol
 from .metasource import MetaSource
 from .loadelement import LoadElement, Dependency
+from .loadelement import DependencyType  # type: ignore[attr-defined]
 from .loadcontext import LoadContext
 from .loader import Loader
diff --git a/src/buildstream/_loader/loadelement.pyx b/src/buildstream/_loader/loadelement.pyx
index 01334d124..31c54ced4 100644
--- a/src/buildstream/_loader/loadelement.pyx
+++ b/src/buildstream/_loader/loadelement.pyx
@@ -23,7 +23,6 @@ from pyroaring import BitMap, FrozenBitMap  # pylint: disable=no-name-in-module
 
 from .._exceptions import LoadError
 from ..exceptions import LoadErrorReason
-from ..element import Element
 from ..node cimport MappingNode, Node, ProvenanceInformation, ScalarNode, SequenceNode
 from .types import Symbol
 
@@ -37,6 +36,22 @@ cdef int _next_synthetic_counter():
     return _counter
 
 
+# DependencyType
+#
+# A bitfield to represent dependency types
+#
+cpdef enum DependencyType:
+
+    # A build dependency
+    BUILD = 0x001
+
+    # A runtime dependency
+    RUNTIME = 0x002
+
+    # Both build and runtime dependencies
+    ALL = 0x003
+
+
 # Dependency():
 #
 # Early stage data model for dependencies objects, the LoadElement has
@@ -50,17 +65,17 @@ cdef int _next_synthetic_counter():
 #
 # Args:
 #    element (LoadElement): a LoadElement on which there is a dependency
-#    dep_type (str): the type of dependency this dependency link is
+#    dep_type (DependencyType): the type of dependency this dependency link is
 #
 cdef class Dependency:
     cdef readonly LoadElement element  # The resolved LoadElement
-    cdef readonly str dep_type  # The dependency type (runtime or build or both)
+    cdef readonly int dep_type  # The dependency type (runtime or build or both)
     cdef readonly str name  # The project local dependency name
     cdef readonly str junction  # The junction path of the dependency name, if any
     cdef readonly bint strict  # Whether this is a strict dependency
     cdef Node _node  # The original node of the dependency
 
-    def __cinit__(self, element=None, dep_type=None):
+    def __cinit__(self, LoadElement element = None, int dep_type = DependencyType.ALL):
         self.element = element
         self.dep_type = dep_type
         self.name = None
@@ -98,38 +113,41 @@ cdef class Dependency:
     #
     # Args:
     #    dep (Node): A node to load the dependency from
-    #    default_dep_type (str): The default dependency type
+    #    default_dep_type (DependencyType): The default dependency type
     #
-    cdef load(self, Node dep, str default_dep_type):
-        cdef str dep_type
+    cdef load(self, Node dep, int default_dep_type):
+        cdef str parsed_type
 
         self._node = dep
         self.element = None
 
         if type(dep) is ScalarNode:
             self.name = dep.as_str()
-            self.dep_type = default_dep_type
+            self.dep_type = default_dep_type or DependencyType.ALL
             self.junction = None
             self.strict = False
 
         elif type(dep) is MappingNode:
             if default_dep_type:
                 (<MappingNode> dep).validate_keys(['filename', 'junction', 'strict'])
-                dep_type = default_dep_type
+                self.dep_type = default_dep_type
             else:
                 (<MappingNode> dep).validate_keys(['filename', 'type', 'junction', 'strict'])
 
-                # Make type optional, for this we set it to None
-                dep_type = (<MappingNode> dep).get_str(<str> Symbol.TYPE, None)
-                if dep_type is None or dep_type == <str> Symbol.ALL:
-                    dep_type = None
-                elif dep_type not in [Symbol.BUILD, Symbol.RUNTIME]:
+                # Resolve the DependencyType
+                parsed_type = (<MappingNode> dep).get_str(<str> Symbol.TYPE, None)
+                if parsed_type is None or parsed_type == <str> Symbol.ALL:
+                    self.dep_type = DependencyType.ALL
+                elif parsed_type == <str> Symbol.BUILD:
+                    self.dep_type = DependencyType.BUILD
+                elif parsed_type == <str> Symbol.RUNTIME:
+                    self.dep_type = DependencyType.RUNTIME
+                else:
                     provenance = dep.get_scalar(Symbol.TYPE).get_provenance()
                     raise LoadError("{}: Dependency type '{}' is not 'build', 'runtime' or 'all'"
-                                    .format(provenance, dep_type), LoadErrorReason.INVALID_DATA)
+                                    .format(provenance, parsed_type), LoadErrorReason.INVALID_DATA)
 
             self.name = (<MappingNode> dep).get_str(<str> Symbol.FILENAME)
-            self.dep_type = dep_type
             self.junction = (<MappingNode> dep).get_str(<str> Symbol.JUNCTION, None)
             self.strict = (<MappingNode> dep).get_bool(<str> Symbol.STRICT, False)
 
@@ -152,7 +170,7 @@ cdef class Dependency:
 
         # Only build dependencies are allowed to be strict
         #
-        if self.strict and self.dep_type == Symbol.RUNTIME:
+        if self.strict and self.dep_type == DependencyType.RUNTIME:
             raise LoadError("{}: Runtime dependency {} specified as `strict`.".format(self.provenance, self.name),
                             LoadErrorReason.INVALID_DATA,
                             detail="Only dependencies required at build time may be declared `strict`.")
@@ -242,6 +260,9 @@ cdef class LoadElement:
         # store the link target and provenance
         #
         if self.kind == 'link':
+            # Avoid cyclic import here
+            from ..element import Element
+
             element = Element._new_from_load_element(self)
             element._initialize_state()
 
@@ -345,9 +366,9 @@ def _dependency_cmp(Dependency dep_a, Dependency dep_b):
     # If there are no inter element dependencies, place
     # runtime only dependencies last
     if dep_a.dep_type != dep_b.dep_type:
-        if dep_a.dep_type == Symbol.RUNTIME:
+        if dep_a.dep_type == DependencyType.RUNTIME:
             return 1
-        elif dep_b.dep_type == Symbol.RUNTIME:
+        elif dep_b.dep_type == DependencyType.RUNTIME:
             return -1
 
     # All things being equal, string comparison.
@@ -424,12 +445,12 @@ def sort_dependencies(LoadElement element, set visited):
 # Args:
 #    node (Node): A YAML loaded dictionary
 #    key (str): the key on the Node corresponding to the dependency type
-#    default_dep_type (str): type to give to the dependency
+#    default_dep_type (DependencyType): type to give to the dependency
 #    acc (list): a list in which to add the loaded dependencies
 #    rundeps (dict): a dictionary mapping dependency (junction, name) to dependency for runtime deps
 #    builddeps (dict): a dictionary mapping dependency (junction, name) to dependency for build deps
 #
-cdef void _extract_depends_from_node(Node node, str key, str default_dep_type, list acc, dict rundeps, dict builddeps) except *:
+cdef void _extract_depends_from_node(Node node, str key, int default_dep_type, list acc, dict rundeps, dict builddeps) except *:
     cdef SequenceNode depends = node.get_sequence(key, [])
     cdef Node dep_node
     cdef tuple deptup
@@ -438,14 +459,14 @@ cdef void _extract_depends_from_node(Node node, str key, str default_dep_type, l
         dependency = Dependency()
         dependency.load(dep_node, default_dep_type)
         deptup = (dependency.junction, dependency.name)
-        if dependency.dep_type in [Symbol.BUILD, None]:
+        if dependency.dep_type & DependencyType.BUILD:
             if deptup in builddeps:
                 raise LoadError("{}: Duplicate build dependency found at {}."
                                 .format(dependency.provenance, builddeps[deptup].provenance),
                                 LoadErrorReason.DUPLICATE_DEPENDENCY)
             else:
                 builddeps[deptup] = dependency
-        if dependency.dep_type in [Symbol.RUNTIME, None]:
+        if dependency.dep_type & DependencyType.RUNTIME:
             if deptup in rundeps:
                 raise LoadError("{}: Duplicate runtime dependency found at {}."
                                 .format(dependency.provenance, rundeps[deptup].provenance),
@@ -476,7 +497,7 @@ def extract_depends_from_node(Node node):
     cdef list acc = []
     cdef dict rundeps = {}
     cdef dict builddeps = {}
-    _extract_depends_from_node(node, <str> Symbol.BUILD_DEPENDS, <str> Symbol.BUILD, acc, rundeps, builddeps)
-    _extract_depends_from_node(node, <str> Symbol.RUNTIME_DEPENDS, <str> Symbol.RUNTIME, acc, rundeps, builddeps)
-    _extract_depends_from_node(node, <str> Symbol.DEPENDS, None, acc, rundeps, builddeps)
+    _extract_depends_from_node(node, <str> Symbol.BUILD_DEPENDS, <int> DependencyType.BUILD, acc, rundeps, builddeps)
+    _extract_depends_from_node(node, <str> Symbol.RUNTIME_DEPENDS, <int> DependencyType.RUNTIME, acc, rundeps, builddeps)
+    _extract_depends_from_node(node, <str> Symbol.DEPENDS, <int> 0, acc, rundeps, builddeps)
     return acc
diff --git a/src/buildstream/_loader/loader.py b/src/buildstream/_loader/loader.py
index 94ee9078b..e22f9aa36 100644
--- a/src/buildstream/_loader/loader.py
+++ b/src/buildstream/_loader/loader.py
@@ -32,6 +32,7 @@ from ._loader import valid_chars_name
 from .types import Symbol
 from . import loadelement
 from .loadelement import LoadElement, Dependency, extract_depends_from_node
+from .loadelement import DependencyType  # type: ignore[attr-defined]
 from ..types import CoreWarnings, _KeyStrength
 from .._message import Message, MessageType
 
@@ -147,7 +148,7 @@ class Loader:
 
         # Pylint is not very happy with Cython and can't understand 'dependencies' is a list
         dummy_target.dependencies.extend(  # pylint: disable=no-member
-            Dependency(element, Symbol.RUNTIME) for element in target_elements
+            Dependency(element, DependencyType.RUNTIME) for element in target_elements
         )
 
         with PROFILER.profile(Topics.CIRCULAR_CHECK, "_".join(targets)):
diff --git a/src/buildstream/element.py b/src/buildstream/element.py
index 53468a2bb..0dedc85e0 100644
--- a/src/buildstream/element.py
+++ b/src/buildstream/element.py
@@ -107,7 +107,7 @@ from .types import CoreWarnings, _Scope, _CacheBuildTrees, _KeyStrength
 from ._artifact import Artifact
 from ._elementproxy import ElementProxy
 from ._elementsources import ElementSources
-from ._loader import Symbol, MetaSource
+from ._loader import Symbol, DependencyType, MetaSource
 
 from .storage.directory import Directory
 from .storage._filebaseddirectory import FileBasedDirectory
@@ -978,11 +978,11 @@ class Element(Plugin):
         for dep in load_element.dependencies:
             dependency = Element._new_from_load_element(dep.element, task)
 
-            if dep.dep_type != "runtime":
+            if dep.dep_type & DependencyType.BUILD:
                 element.__build_dependencies.append(dependency)
                 dependency.__reverse_build_deps.add(element)
 
-            if dep.dep_type != "build":
+            if dep.dep_type & DependencyType.RUNTIME:
                 element.__runtime_dependencies.append(dependency)
                 dependency.__reverse_runtime_deps.add(element)