element.py: Adding new configure_dependencies() public API methodtristan/configure-dependencies

This patch implements the essentials of the proposal to extend the
dependency attributes:

   https://lists.apache.org/thread.html/r850aeb270200daade44261f16dbad403bf762e553b89dcafa0848fe7%40%3Cdev.buildstream.apache.org%3E

And essentially this will obsolete issue #931 by providing a more
suitable replacement for Element.search().

Summary of changes:

  * _loader/loadelement.pyx: The Dependency object now loads the `config` node,
    and raises an error if the `config` node is specified on a runtime-only
    dependency.

  * element.py: Created the new Element.configure_dependencies() virtual method.

    This new method is given only the build dependencies on which a
    config node was specified by the user, and not called at all if there
    were no dependencies with a config node specified.

    Element plugins which do not support any special config node parsing for
    their dependencies do not need to implement this method.

3 files changed, 100 insertions, 4 deletions

diff --git a/src/buildstream/_loader/loadelement.pyx b/src/buildstream/_loader/loadelement.pyx
index c7d6ec88d..a515a1211 100644
--- a/src/buildstream/_loader/loadelement.pyx
+++ b/src/buildstream/_loader/loadelement.pyx
@@ -74,6 +74,7 @@ cdef class Dependency:
     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
+    cdef readonly list config_nodes  # The custom config nodes for Element.configure_dependencies()
 
     def __cinit__(self, LoadElement element = None, int dep_type = DependencyType.ALL):
         self.element = element
@@ -81,6 +82,7 @@ cdef class Dependency:
         self.name = None
         self.junction = None
         self.strict = False
+        self.config_nodes = None
         self._node = None
 
     # provenance
@@ -117,6 +119,7 @@ cdef class Dependency:
     #
     cdef load(self, Node dep, int default_dep_type):
         cdef str parsed_type
+        cdef MappingNode config_node
 
         self._node = dep
         self.element = None
@@ -129,10 +132,10 @@ cdef class Dependency:
 
         elif type(dep) is MappingNode:
             if default_dep_type:
-                (<MappingNode> dep).validate_keys(['filename', 'junction', 'strict'])
+                (<MappingNode> dep).validate_keys([Symbol.FILENAME, Symbol.JUNCTION, Symbol.STRICT, Symbol.CONFIG])
                 self.dep_type = default_dep_type
             else:
-                (<MappingNode> dep).validate_keys(['filename', 'type', 'junction', 'strict'])
+                (<MappingNode> dep).validate_keys([Symbol.FILENAME, Symbol.TYPE, Symbol.JUNCTION, Symbol.STRICT, Symbol.CONFIG])
 
                 # Resolve the DependencyType
                 parsed_type = (<MappingNode> dep).get_str(<str> Symbol.TYPE, None)
@@ -151,6 +154,13 @@ cdef class Dependency:
             self.junction = (<MappingNode> dep).get_str(<str> Symbol.JUNCTION, None)
             self.strict = (<MappingNode> dep).get_bool(<str> Symbol.STRICT, False)
 
+            config_node = (<MappingNode> dep).get_mapping(<str> Symbol.CONFIG, None)
+            if config_node:
+                if self.dep_type == DependencyType.RUNTIME:
+                    raise LoadError("{}: Specifying 'config' for a runtime dependency is not allowed"
+                                    .format(config_node.get_provenance()), LoadErrorReason.INVALID_DATA)
+                self.config_nodes = [config_node]
+
             # Here we disallow explicitly setting 'strict' to False.
             #
             # This is in order to keep the door open to allowing the project.conf
@@ -198,6 +208,11 @@ cdef class Dependency:
         self.dep_type = self.dep_type | other.dep_type
         self.strict = self.strict or other.strict
 
+        if self.config_nodes and other.config_nodes:
+            self.config_nodes.extend(other.config_nodes)
+        else:
+            self.config_nodes = self.config_nodes or other.config_nodes
+
 
 # LoadElement():
 #
diff --git a/src/buildstream/element.py b/src/buildstream/element.py
index 0dedc85e0..9c172a046 100644
--- a/src/buildstream/element.py
+++ b/src/buildstream/element.py
@@ -84,7 +84,7 @@ from contextlib import contextmanager, suppress
 from functools import partial
 from itertools import chain
 import string
-from typing import cast, TYPE_CHECKING, Any, Dict, Iterator, List, Optional, Set, Sequence
+from typing import cast, TYPE_CHECKING, Any, Dict, Iterator, List, Tuple, Optional, Set, Sequence
 
 from pyroaring import BitMap  # pylint: disable=no-name-in-module
 from ruamel import yaml
@@ -116,7 +116,6 @@ from .storage.directory import VirtualDirectoryError
 if TYPE_CHECKING:
     from .node import MappingNode, ScalarNode, SequenceNode
     from .types import SourceRef
-    from typing import Tuple
 
     # pylint: disable=cyclic-import
     from .sandbox import Sandbox
@@ -321,6 +320,51 @@ class Element(Plugin):
     #############################################################
     #                      Abstract Methods                     #
     #############################################################
+    def configure_dependencies(self, config_list: List[Tuple["Element", "MappingNode"]]) -> None:
+        """Configure the Element with regards to it's build dependencies
+
+        Elements can use this method to parse custom configuration which define their
+        relationship to their build dependencies.
+
+        If this method is implemented, then it will be called with all direct build dependencies
+        specified in their :ref:`element declaration <format_dependencies>` in a list of tuples.
+
+        If the dependency was declared with custom configuration, it will be provided along
+        with the dependency element, otherwise `None` will be passed with dependencies which
+        do not have any additional configuration.
+
+        If the user has specified the same build dependency multiple times with differing
+        configurations, then those build dependencies will be provided multiple times
+        in the ``config_list``.
+
+        Args:
+           config_list (list): A list of (:class:`.Element`, :class:`MappingNode <buildstream.node.MappingNode>`)
+                               tuples, where the :class:`MappingNode <buildstream.node.MappingNode>` can be ``None``.
+
+        Raises:
+           :class:`.ElementError`: When the element raises an error
+
+        The :class:`MappingNode <buildstream.node.MappingNode>` format belongs to the implementing
+        element, and as such the format should be documented by the plugin, and the
+        :func:`MappingNode.validate_keys() <buildstream.node.MappingNode.validate_keys>`
+        method should be called by the implementing plugin in order to validate it.
+
+        .. note::
+
+           It is unnecessary to implement this method if the plugin does not support
+           any custom ``config`` parsing for its dependencies.
+
+           If the plugin derives from an abstract class in BuildStream, like :class:`BuildElement`
+           or :class:`ScriptElement`, then the documentation of those classes should be observed
+           in order to determine whether it is necessary to invoke the parent class implementation,
+           and how to validate the nodes properly with
+           :func:`MappingNode.validate_keys() <buildstream.node.MappingNode.validate_keys>`
+        """
+        # This method is not called on plugins which do not implement it, so it would
+        # be a bug if this accidentally gets called.
+        #
+        assert False, "Code should not be reached"
+
     def configure_sandbox(self, sandbox: "Sandbox") -> None:
         """Configures the the sandbox for execution
 
@@ -971,6 +1015,15 @@ class Element(Plugin):
         element = load_element.project.create_element(load_element)
         cls.__instantiated_elements[load_element] = element
 
+        # If the element implements configure_dependencies(), we will collect
+        # the dependency configurations for it, otherwise we will consider
+        # it an error to specify `config` on dependencies.
+        #
+        if element.configure_dependencies.__func__ is not Element.configure_dependencies:
+            custom_config_nodes = []
+        else:
+            custom_config_nodes = None
+
         # Load the sources from the LoadElement
         element.__load_sources(load_element)
 
@@ -982,6 +1035,27 @@ class Element(Plugin):
                 element.__build_dependencies.append(dependency)
                 dependency.__reverse_build_deps.add(element)
 
+                # Configuration data is only collected for build dependencies,
+                # if configuration data is specified on a runtime dependency
+                # then the assertion will be raised by the LoadElement.
+                #
+                if custom_config_nodes is not None:
+                    # Class supports dependency configuration
+                    if dep.config_nodes:
+                        custom_config_nodes.extend([(dependency, config) for config in dep.config_nodes])
+                    else:
+                        custom_config_nodes.append((dependency, None))
+
+                elif dep.config_nodes:
+                    # Class does not support dependency configuration
+                    provenance = dep.config_nodes[0].get_provenance()
+                    raise LoadError(
+                        "{}: Custom dependency configuration is not supported by {} plugin '{}'".format(
+                            provenance, element.__type_tag, element.get_kind()
+                        ),
+                        LoadErrorReason.INVALID_DEPENDENCY_CONFIG,
+                    )
+
             if dep.dep_type & DependencyType.RUNTIME:
                 element.__runtime_dependencies.append(dependency)
                 dependency.__reverse_runtime_deps.add(element)
@@ -999,6 +1073,9 @@ class Element(Plugin):
         element.__build_deps_without_cache_key = no_of_build_deps
         element.__build_deps_uncached = no_of_build_deps
 
+        if custom_config_nodes is not None:
+            element.configure_dependencies(custom_config_nodes)
+
         element.__preflight()
 
         if task:
diff --git a/src/buildstream/exceptions.py b/src/buildstream/exceptions.py
index 0ce1956e0..4b9118978 100644
--- a/src/buildstream/exceptions.py
+++ b/src/buildstream/exceptions.py
@@ -137,6 +137,10 @@ class LoadErrorReason(Enum):
     PROTECTED_VARIABLE_REDEFINED = 23
     """An attempt was made to set the value of a protected variable"""
 
+    INVALID_DEPENDENCY_CONFIG = 24
+    """An attempt was made to specify dependency configuration on an element
+    which does not support custom dependency configuration"""
+
     LINK_FORBIDDEN_DEPENDENCIES = 25
     """A link element declared dependencies"""