From 42c5ee0a3a840756aae675a96b7b3c09e06b487d Mon Sep 17 00:00:00 2001 From: Tristan van Berkom Date: Sun, 9 Aug 2020 20:14:43 +0900 Subject: element.py: Adding new configure_dependencies() public API method 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. If elements implement this method, then a list of all dependencies are collected at element construction time and given to the implementing (depending) element. If a build dependency has more than one `config` specified, then it will be given to the Element twice, and if there is no `config` specified, then the tuple will still be given to the element with a Null `config`. Elements are provided via a new DependencyConfiguration type. --- src/buildstream/__init__.py | 2 +- src/buildstream/_loader/loadelement.pyx | 19 +++++- src/buildstream/element.py | 101 +++++++++++++++++++++++++++++++- src/buildstream/exceptions.py | 4 ++ 4 files changed, 122 insertions(+), 4 deletions(-) diff --git a/src/buildstream/__init__.py b/src/buildstream/__init__.py index 606353af0..c6722023f 100644 --- a/src/buildstream/__init__.py +++ b/src/buildstream/__init__.py @@ -35,7 +35,7 @@ if "_BST_COMPLETION" not in os.environ: from .plugin import Plugin from .source import Source, SourceError, SourceFetcher from .downloadablefilesource import DownloadableFileSource - from .element import Element, ElementError + from .element import Element, ElementError, DependencyConfiguration from .buildelement import BuildElement from .scriptelement import ScriptElement diff --git a/src/buildstream/_loader/loadelement.pyx b/src/buildstream/_loader/loadelement.pyx index 858334c7d..80b96cd2c 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 @@ -128,6 +130,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 @@ -140,10 +143,10 @@ cdef class Dependency: elif type(dep) is MappingNode: if default_dep_type: - ( dep).validate_keys(['filename', 'junction', 'strict']) + ( dep).validate_keys([Symbol.FILENAME, Symbol.JUNCTION, Symbol.STRICT, Symbol.CONFIG]) self.dep_type = default_dep_type else: - ( dep).validate_keys(['filename', 'type', 'junction', 'strict']) + ( dep).validate_keys([Symbol.FILENAME, Symbol.TYPE, Symbol.JUNCTION, Symbol.STRICT, Symbol.CONFIG]) # Resolve the DependencyType parsed_type = ( dep).get_str( Symbol.TYPE, None) @@ -162,6 +165,13 @@ cdef class Dependency: self.junction = ( dep).get_str( Symbol.JUNCTION, None) self.strict = ( dep).get_bool( Symbol.STRICT, False) + config_node = ( dep).get_mapping( 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 @@ -209,6 +219,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 08dd69fff..fd6e2da5c 100644 --- a/src/buildstream/element.py +++ b/src/buildstream/element.py @@ -115,9 +115,9 @@ from .storage._filebaseddirectory import FileBasedDirectory from .storage.directory import VirtualDirectoryError if TYPE_CHECKING: + from typing import Tuple from .node import MappingNode, ScalarNode, SequenceNode from .types import SourceRef - from typing import Tuple # pylint: disable=cyclic-import from .sandbox import Sandbox @@ -149,6 +149,26 @@ class ElementError(BstError): self.collect = collect +class DependencyConfiguration: + """An object representing the configuration of a dependency + + This is used to provide dependency configurations for elements which implement + :func:`Element.configure_dependencies() ` + """ + + def __init__(self, element: "Element", path: str, config: Optional["MappingNode"]): + + self.element = element # type: Element + """The dependency Element""" + + self.path = path # type: str + """The path used to refer to this dependency""" + + self.config = config # type: Optional[MappingNode] + """The custom :term:`dependency configuration `, or ``None`` + if no custom configuration was provided""" + + class Element(Plugin): """Element() @@ -336,6 +356,46 @@ class Element(Plugin): ############################################################# # Abstract Methods # ############################################################# + def configure_dependencies(self, dependencies: List[DependencyConfiguration]) -> 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 ` in a list. + + 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 ``dependencies`` list. + + Args: + dependencies (list): A list of :class:`DependencyConfiguration ` + objects + + Raises: + :class:`.ElementError`: When the element raises an error + + The format of the :class:`MappingNode ` provided as + :attr:`DependencyConfiguration.config + belongs to the implementing element, and as such the format should be documented by the plugin, + and the :func:`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 :term:`dependency configuration `. + """ + # 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 @@ -1031,6 +1091,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_configurations = [] + else: + custom_configurations = None + # Load the sources from the LoadElement element.__load_sources(load_element) @@ -1042,6 +1111,33 @@ 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_configurations is not None: + + # Create a proxy for the dependency + dep_proxy = cast("Element", ElementProxy(element, dependency)) + + # Class supports dependency configuration + if dep.config_nodes: + custom_configurations.extend( + [DependencyConfiguration(dep_proxy, dep.path, config) for config in dep.config_nodes] + ) + else: + custom_configurations.append(DependencyConfiguration(dep_proxy, dep.path, 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 element plugin '{}'".format( + provenance, element.get_kind() + ), + LoadErrorReason.INVALID_DEPENDENCY_CONFIG, + ) + if dep.dep_type & DependencyType.RUNTIME: element.__runtime_dependencies.append(dependency) dependency.__reverse_runtime_deps.add(element) @@ -1055,6 +1151,9 @@ class Element(Plugin): no_of_build_deps = len(element.__build_dependencies) element.__build_deps_uncached = no_of_build_deps + if custom_configurations is not None: + element.configure_dependencies(custom_configurations) + 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""" -- cgit v1.2.1