From 751c80d4938b7c3a798d056134f58844242939b3 Mon Sep 17 00:00:00 2001 From: Tristan van Berkom Date: Thu, 13 Aug 2020 19:01:00 +0900 Subject: Refactor: Lazily instantiate ProvenanceInformation objects As a rule, throughout the codebase we should not be using internal ProvenanceInformation objects in our APIs, but rather Node objects. This is because ProvenanceInformation is generated on the fly from a Node object, and it is needlessly expensive to instantiate one before it is absolutely needed. This patch unilaterally fixes the codebase to pass `provenance_node` Node objects around as arguments rather than `provenance` ProvenanceInformation objects. --- src/buildstream/_includes.py | 15 ++- src/buildstream/_loader/loadcontext.py | 2 +- src/buildstream/_loader/loadelement.pyi | 5 +- src/buildstream/_loader/loadelement.pyx | 36 ++----- src/buildstream/_loader/loader.py | 120 +++++++++++---------- src/buildstream/_loader/metasource.py | 1 - src/buildstream/_pluginfactory/elementfactory.py | 2 +- src/buildstream/_pluginfactory/pluginfactory.py | 17 +-- src/buildstream/_pluginfactory/pluginorigin.py | 4 +- .../_pluginfactory/pluginoriginjunction.py | 6 +- src/buildstream/_pluginfactory/pluginoriginpip.py | 12 ++- src/buildstream/_pluginfactory/sourcefactory.py | 2 +- src/buildstream/_project.py | 41 +++---- src/buildstream/element.py | 2 +- src/buildstream/node.pyi | 1 + src/buildstream/plugin.py | 10 +- src/buildstream/plugins/elements/junction.py | 15 ++- src/buildstream/plugins/elements/link.py | 7 +- src/buildstream/source.py | 3 +- src/buildstream/types.py | 6 +- 20 files changed, 140 insertions(+), 167 deletions(-) diff --git a/src/buildstream/_includes.py b/src/buildstream/_includes.py index 1556bc613..9542003ad 100644 --- a/src/buildstream/_includes.py +++ b/src/buildstream/_includes.py @@ -133,18 +133,17 @@ class Includes: # Can be prefixed with junctio name. # loader (Loader): Loader for the current project. def _include_file(self, include, loader): - provenance = include.get_provenance() - include = include.as_str() - shortname = include - if ":" in include: - junction, include = include.rsplit(":", 1) - current_loader = loader.get_loader(junction, provenance) + include_str = include.as_str() + shortname = include_str + if ":" in include_str: + junction, include_str = include_str.rsplit(":", 1) + current_loader = loader.get_loader(junction, include) current_loader.project.ensure_fully_loaded() else: current_loader = loader project = current_loader.project directory = project.directory - file_path = os.path.join(directory, include) + file_path = os.path.join(directory, include_str) key = (current_loader, file_path) if key not in self._loaded: try: @@ -152,7 +151,7 @@ class Includes: file_path, shortname=shortname, project=project, copy_tree=self._copy_tree ) except LoadError as e: - raise LoadError("{}: {}".format(provenance, e), e.reason, detail=e.detail) from e + raise LoadError("{}: {}".format(include.get_provenance(), e), e.reason, detail=e.detail) from e return self._loaded[key], file_path, current_loader diff --git a/src/buildstream/_loader/loadcontext.py b/src/buildstream/_loader/loadcontext.py index 4e6c9bca6..a9eb4a7b7 100644 --- a/src/buildstream/_loader/loadcontext.py +++ b/src/buildstream/_loader/loadcontext.py @@ -87,7 +87,7 @@ class ProjectLoaders: for loader in self._collect: duplicating, internalizing = self._search_project_relationships(loader) yield _ProjectInformation( - loader.project, loader.provenance, [str(l) for l in duplicating], [str(l) for l in internalizing] + loader.project, loader.provenance_node, [str(l) for l in duplicating], [str(l) for l in internalizing] ) # _search_project_relationships() diff --git a/src/buildstream/_loader/loadelement.pyi b/src/buildstream/_loader/loadelement.pyi index 717792bd5..b18a8def8 100644 --- a/src/buildstream/_loader/loadelement.pyi +++ b/src/buildstream/_loader/loadelement.pyi @@ -1,6 +1,6 @@ from typing import List -from ..node import Node, ProvenanceInformation +from ..node import Node, ScalarNode def extract_depends_from_node(node: Node) -> List[Dependency]: ... @@ -11,4 +11,5 @@ class LoadElement: first_pass: bool kind: str name: str - provenance: ProvenanceInformation + node: Node + link_target: ScalarNode diff --git a/src/buildstream/_loader/loadelement.pyx b/src/buildstream/_loader/loadelement.pyx index aef17bf48..60de948c8 100644 --- a/src/buildstream/_loader/loadelement.pyx +++ b/src/buildstream/_loader/loadelement.pyx @@ -84,8 +84,8 @@ cdef class Dependency: 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 cdef readonly list config_nodes # The custom config nodes for Element.configure_dependencies() + cdef readonly Node node # The original node of the dependency def __cinit__(self, LoadElement element = None, int dep_type = DependencyType.ALL): self.element = element @@ -94,16 +94,7 @@ cdef class Dependency: self.junction = None self.strict = False self.config_nodes = None - self._node = None - - # provenance - # - # A property to return the ProvenanceInformation for this - # dependency. - # - @property - def provenance(self): - return self._node.get_provenance() + self.node = None # path # @@ -148,8 +139,8 @@ cdef class Dependency: self.junction = junction self.name = name + self.node = dep self.element = None - self._node = dep if type(dep) is ScalarNode: self.dep_type = default_dep_type or DependencyType.ALL @@ -197,13 +188,13 @@ cdef class Dependency: .format(provenance), LoadErrorReason.INVALID_DATA) else: - raise LoadError("{}: Dependency is not specified as a string or a dictionary".format(self.provenance), + raise LoadError("{}: Dependency is not specified as a string or a dictionary".format(self.node.get_provenance()), LoadErrorReason.INVALID_DATA) # Only build dependencies are allowed to be strict # if self.strict and self.dep_type == DependencyType.RUNTIME: - raise LoadError("{}: Runtime dependency {} specified as `strict`.".format(self.provenance, self.name), + raise LoadError("{}: Runtime dependency {} specified as `strict`.".format(self.node.get_provenance(), self.name), LoadErrorReason.INVALID_DATA, detail="Only dependencies required at build time may be declared `strict`.") @@ -243,8 +234,7 @@ cdef class LoadElement: cdef int node_id cdef readonly bint first_pass cdef readonly object _loader - cdef readonly str link_target - cdef readonly ProvenanceInformation link_target_provenance + cdef readonly ScalarNode link_target # TODO: if/when pyroaring exports symbols, we could type this statically cdef object _dep_cache cdef readonly list dependencies @@ -259,8 +249,7 @@ cdef class LoadElement: self.name = filename # The element name self.full_name = None # The element full name (with associated junction) self.node_id = _next_synthetic_counter() - self.link_target = None # The target of a link element - self.link_target_provenance = None # The provenance of the link target + self.link_target = None # The target of a link element (ScalarNode) # # Private members @@ -306,8 +295,7 @@ cdef class LoadElement: LoadErrorReason.LINK_FORBIDDEN_DEPENDENCIES ) - self.link_target = element.target - self.link_target_provenance = element.target_provenance + self.link_target = element.target_node # We don't count progress for junction elements or link # as they do not represent real elements in the build graph. @@ -318,14 +306,6 @@ cdef class LoadElement: if self._loader.load_context.task and self.kind is not None and not self.first_pass: self._loader.load_context.task.add_current_progress() - # provenance - # - # A property reporting the ProvenanceInformation of the element - # - @property - def provenance(self): - return self.node.get_provenance() - # project # # A property reporting the Project in which this element resides. diff --git a/src/buildstream/_loader/loader.py b/src/buildstream/_loader/loader.py index 6ebf89a05..080ec0093 100644 --- a/src/buildstream/_loader/loader.py +++ b/src/buildstream/_loader/loader.py @@ -45,10 +45,10 @@ from .._message import Message, MessageType # Args: # project (Project): The toplevel Project object # parent (Loader): A parent Loader object, in the case this is a junctioned Loader -# provenance (ProvenanceInformation): The provenance of the reference to this project's junction +# provenance_node (Node): The provenance of the reference to this project's junction # class Loader: - def __init__(self, project, *, parent=None, provenance=None): + def __init__(self, project, *, parent=None, provenance_node=None): # Ensure we have an absolute path for the base directory basedir = project.element_path @@ -60,7 +60,7 @@ class Loader: # self.load_context = project.load_context # The LoadContext self.project = project # The associated Project - self.provenance = provenance # The provenance of whence this loader was instantiated + self.provenance_node = provenance_node # The provenance of whence this loader was instantiated self.loaded = None # The number of loaded Elements # @@ -75,7 +75,7 @@ class Loader: self._meta_elements = {} # Dict of resolved meta elements by name self._elements = {} # Dict of elements self._loaders = {} # Dict of junction loaders - self._loader_search_provenances = {} # Dictionary of provenances of ongoing child loader searches + self._loader_search_provenances = {} # Dictionary of provenance nodes of ongoing child loader searches self._includes = Includes(self, copy_tree=True) @@ -92,8 +92,8 @@ class Loader: if self.project.junction: junction_name = self.project.junction._get_full_name() - if self.provenance: - provenance = "({}): {}".format(junction_name, self.provenance) + if self.provenance_node: + provenance = "({}): {}".format(junction_name, self.provenance_node.get_provenance()) else: provenance = "({})".format(junction_name) else: @@ -185,34 +185,34 @@ class Loader: # # Args: # name (str): Name of junction, may have multiple `:` in the name - # provenance (ProvenanceInformation): The provenance + # provenance_node (Node): The provenance from where this loader was requested # load_subprojects (bool): Whether to load subprojects on demand # # Returns: # (Loader): loader for sub-project # - def get_loader(self, name, provenance, *, load_subprojects=True): + def get_loader(self, name, provenance_node, *, load_subprojects=True): junction_path = name.split(":") loader = self - circular_provenance = self._loader_search_provenances.get(name, None) - if circular_provenance: + circular_provenance_node = self._loader_search_provenances.get(name, None) + if circular_provenance_node: - assert provenance + assert provenance_node detail = None - if circular_provenance is not provenance: - detail = "Already searching for '{}' at: {}".format(name, circular_provenance) + if circular_provenance_node is not provenance_node: + detail = "Already searching for '{}' at: {}".format(name, circular_provenance_node.get_provenance()) raise LoadError( - "{}: Circular reference while searching for '{}'".format(provenance, name), + "{}: Circular reference while searching for '{}'".format(provenance_node.get_provenance(), name), LoadErrorReason.CIRCULAR_REFERENCE, detail=detail, ) - self._loader_search_provenances[name] = provenance + self._loader_search_provenances[name] = provenance_node for junction_name in junction_path: - loader = loader._get_loader(junction_name, provenance, load_subprojects=load_subprojects) + loader = loader._get_loader(junction_name, provenance_node, load_subprojects=load_subprojects) del self._loader_search_provenances[name] @@ -259,12 +259,12 @@ class Loader: # # Args: # filename (str): The element-path relative bst file - # provenance (Provenance): The location from where the file was referred to, or None + # provenance_node (Node): The location from where the file was referred to, or None # # Returns: # (LoadElement): A partially-loaded LoadElement # - def _load_file_no_deps(self, filename, provenance=None): + def _load_file_no_deps(self, filename, provenance_node=None): # Load the data and process any conditional statements therein fullpath = os.path.join(self._basedir, filename) try: @@ -281,8 +281,8 @@ class Loader: else: message = "Could not find element '{}' in elements directory '{}'".format(filename, self._basedir) - if provenance: - message = "{}: {}".format(provenance, message) + if provenance_node: + message = "{}: {}".format(provenance_node.get_provenance(), message) # If we can't find the file, try to suggest plausible # alternatives by stripping the element-path from the given @@ -299,8 +299,8 @@ class Loader: # If a .bst file exists in the element path, # let's suggest this as a plausible alternative. message = str(e) - if provenance: - message = "{}: {}".format(provenance, message) + if provenance_node: + message = "{}: {}".format(provenance_node.get_provenance(), message) detail = None if os.path.exists(os.path.join(self._basedir, filename + ".bst")): element_name = filename + ".bst" @@ -335,28 +335,27 @@ class Loader: # Args: # filename (str): The element-path relative bst file # load_subprojects (bool): Whether to load subprojects - # provenance (Provenance): The location from where the file was referred to, or None + # provenance_node (Node): The location from where the file was referred to, or None # # Returns: # (LoadElement): A loaded LoadElement # - def _load_file(self, filename, provenance, *, load_subprojects=True): + def _load_file(self, filename, provenance_node, *, load_subprojects=True): # Silently ignore already loaded files with suppress(KeyError): return self._elements[filename] - top_element = self._load_file_no_deps(filename, provenance) + top_element = self._load_file_no_deps(filename, provenance_node) # If this element is a link then we need to resolve it # and replace the dependency we've processed with this one if top_element.link_target is not None: + link_target = top_element.link_target.as_str() # pylint: disable=no-member _, filename, loader = self._parse_name( - top_element.link_target, top_element.link_target_provenance, load_subprojects=load_subprojects - ) - top_element = loader._load_file( - filename, top_element.link_target_provenance, load_subprojects=load_subprojects + link_target, top_element.link_target, load_subprojects=load_subprojects ) + top_element = loader._load_file(filename, top_element.link_target, load_subprojects=load_subprojects) dependencies = extract_depends_from_node(top_element.node) # The loader queue is a stack of tuples @@ -376,8 +375,8 @@ class Loader: current_element[2].append(dep.name) if dep.junction: - loader = self.get_loader(dep.junction, dep.provenance) - dep_element = loader._load_file(dep.name, dep.provenance) + loader = self.get_loader(dep.junction, dep.node) + dep_element = loader._load_file(dep.name, dep.node) else: dep_element = self._elements.get(dep.name) @@ -385,21 +384,22 @@ class Loader: # The loader does not have this available so we need to # 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, dep.provenance) + dep_element = self._load_file_no_deps(dep.name, dep.node) dep_deps = extract_depends_from_node(dep_element.node) loader_queue.append((dep_element, list(reversed(dep_deps)), [])) # Pylint is not very happy about Cython and can't understand 'node' is a 'MappingNode' if dep_element.node.get_str(Symbol.KIND) == "junction": # pylint: disable=no-member raise LoadError( - "{}: Cannot depend on junction".format(dep.provenance), LoadErrorReason.INVALID_DATA + "{}: Cannot depend on junction".format(dep.node.get_provenance()), + LoadErrorReason.INVALID_DATA, ) # If this dependency is a link then we need to resolve it # and replace the dependency we've processed with this one if dep_element.link_target: - _, filename, loader = self._parse_name(dep_element.link_target, dep_element.link_target_provenance) - dep_element = loader._load_file(filename, dep_element.link_target_provenance) + _, filename, loader = self._parse_name(dep_element.link_target.as_str(), dep_element.link_target) + dep_element = loader._load_file(filename, dep_element.link_target) # We've now resolved the element for this dependency, lets set the resolved # LoadElement on the dependency and append the dependency to the owning @@ -490,9 +490,9 @@ class Loader: overriding_loaders = [] while loader._parent: junction = loader.project.junction - override_filename, override_provenance = junction.overrides.get(override_path, (None, None)) - if override_filename: - overriding_loaders.append((loader._parent, override_filename, override_provenance)) + override_node = junction.overrides.get(override_path, None) + if override_node: + overriding_loaders.append((loader._parent, override_node)) override_path = junction.name + ":" + override_path loader = loader._parent @@ -501,8 +501,8 @@ class Loader: # the ancestry to lookup the loader for this project. # if overriding_loaders: - overriding_loader, override_filename, provenance = overriding_loaders[-1] - loader = overriding_loader.get_loader(override_filename, provenance) + overriding_loader, override_node = overriding_loaders[-1] + loader = overriding_loader.get_loader(override_node.as_str(), override_node) # # Record alternative loaders which were overridden. @@ -519,7 +519,7 @@ class Loader: loader._alternative_parents.append(self) del overriding_loaders[-1] - loader._alternative_parents.extend(l for l, _, _ in overriding_loaders) + loader._alternative_parents.extend(l for l, _ in overriding_loaders) return loader @@ -534,22 +534,25 @@ class Loader: # Args: # filename (str): Junction name # load_subprojects (bool): Whether to load subprojects - # provenance (Provenance): The location from where the file was referred to, or None + # provenance_node (Node): The location from where the file was referred to, or None # # Raises: LoadError # # Returns: A Loader or None if specified junction does not exist # - def _get_loader(self, filename, provenance, *, load_subprojects=True): + def _get_loader(self, filename, provenance_node, *, load_subprojects=True): loader = None - provenance_str = "" - if provenance is not None: - provenance_str = "{}: ".format(provenance) # return previously determined result if filename in self._loaders: return self._loaders[filename] + # Local function to conditionally resolve the provenance prefix string + def provenance_str(): + if provenance_node is not None: + return "{}: ".format(provenance_node.get_provenance()) + return "" + # # Search the ancestry for an overridden loader to use in place # of using the locally defined junction. @@ -562,7 +565,7 @@ class Loader: # # Load the junction file # - self._load_file(filename, provenance, load_subprojects=load_subprojects) + self._load_file(filename, provenance_node, load_subprojects=load_subprojects) # At this point we've loaded the LoadElement load_element = self._elements[filename] @@ -572,9 +575,9 @@ class Loader: # if load_element.link_target: _, filename, loader = self._parse_name( - load_element.link_target, load_element.link_target_provenance, load_subprojects=load_subprojects + load_element.link_target.as_str(), load_element.link_target, load_subprojects=load_subprojects ) - return loader.get_loader(filename, load_element.link_target_provenance, load_subprojects=load_subprojects) + return loader.get_loader(filename, load_element.link_target, load_subprojects=load_subprojects) # If we're only performing a lookup, we're done here. # @@ -583,7 +586,7 @@ class Loader: if load_element.kind != "junction": raise LoadError( - "{}{}: Expected junction but element kind is {}".format(provenance_str, filename, load_element.kind), + "{}{}: Expected junction but element kind is {}".format(provenance_str(), filename, load_element.kind), LoadErrorReason.INVALID_DATA, ) @@ -602,7 +605,7 @@ class Loader: # but since we haven't loaded those yet that's impossible. if load_element.dependencies: # Use the first dependency in the list as provenance - p = load_element.dependencies[0].provenance + p = load_element.dependencies[0].node.get_provenance() raise LoadError( "{}: Dependencies are forbidden for 'junction' elements".format(p), LoadErrorReason.INVALID_JUNCTION ) @@ -615,7 +618,7 @@ class Loader: if not element._has_all_sources_resolved(): detail = "Try tracking the junction element with `bst source track {}`".format(filename) raise LoadError( - "{}Subproject has no ref for junction: {}".format(provenance_str, filename), + "{}Subproject has no ref for junction: {}".format(provenance_str(), filename), LoadErrorReason.SUBPROJECT_INCONSISTENT, detail=detail, ) @@ -661,13 +664,12 @@ class Loader: junction=element, parent_loader=self, search_for_project=False, - provenance=provenance, + provenance_node=provenance_node, ) except LoadError as e: if e.reason == LoadErrorReason.MISSING_PROJECT_CONF: - message = ( - provenance_str + "Could not find the project.conf file in the project " - "referred to by junction element '{}'.".format(element.name) + message = provenance_str() + "Could not find the project.conf file in the project " "referred to by junction element '{}'.".format( + element.name ) if element.path: message += " Was expecting it at path '{}' in the junction's source.".format(element.path) @@ -687,7 +689,7 @@ class Loader: # # Args: # name (str): Name of target - # provenance (ProvenanceInformation): The provenance + # provenance_node (Node): The provenance node # load_subprojects (bool): Whether to load subprojects # # Returns: @@ -695,7 +697,7 @@ class Loader: # - (str): name of the element # - (Loader): loader for sub-project # - def _parse_name(self, name, provenance, *, load_subprojects=True): + def _parse_name(self, name, provenance_node, *, load_subprojects=True): # We allow to split only once since deep junctions names are forbidden. # Users who want to refer to elements in sub-sub-projects are required # to create junctions on the top level project. @@ -703,7 +705,7 @@ class Loader: if len(junction_path) == 1: return None, junction_path[-1], self else: - loader = self.get_loader(junction_path[-2], provenance, load_subprojects=load_subprojects) + loader = self.get_loader(junction_path[-2], provenance_node, load_subprojects=load_subprojects) return junction_path[-2], junction_path[-1], loader # Print a warning message, checks warning_token against project configuration diff --git a/src/buildstream/_loader/metasource.py b/src/buildstream/_loader/metasource.py index d49ae3d82..37a0a9406 100644 --- a/src/buildstream/_loader/metasource.py +++ b/src/buildstream/_loader/metasource.py @@ -40,4 +40,3 @@ class MetaSource: self.config = config self.directory = directory self.first_pass = first_pass - self.provenance = config.get_provenance() diff --git a/src/buildstream/_pluginfactory/elementfactory.py b/src/buildstream/_pluginfactory/elementfactory.py index 7d94c2541..f23ace809 100644 --- a/src/buildstream/_pluginfactory/elementfactory.py +++ b/src/buildstream/_pluginfactory/elementfactory.py @@ -48,6 +48,6 @@ class ElementFactory(PluginFactory): # LoadError (if the element itself took issue with the config) # def create(self, context, project, load_element): - element_type, default_config = self.lookup(context.messenger, load_element.kind, load_element.provenance) + element_type, default_config = self.lookup(context.messenger, load_element.kind, load_element.node) element = element_type(context, project, load_element, default_config) return element diff --git a/src/buildstream/_pluginfactory/pluginfactory.py b/src/buildstream/_pluginfactory/pluginfactory.py index f997d9017..fb5389b3e 100644 --- a/src/buildstream/_pluginfactory/pluginfactory.py +++ b/src/buildstream/_pluginfactory/pluginfactory.py @@ -26,7 +26,7 @@ from .. import _site from ..plugin import Plugin from ..source import Source from ..element import Element -from ..node import ProvenanceInformation +from ..node import Node from ..utils import UtilError from .._exceptions import PluginError from .._messenger import Messenger @@ -122,8 +122,7 @@ class PluginFactory: # Args: # messenger (Messenger): The messenger # kind (str): The kind of Plugin to create - # provenance (ProvenanceInformation): The provenance from where - # the plugin was referenced + # provenance_node (Node): The node from where the plugin was referenced # # Returns: # (type): The type associated with the given kind @@ -131,8 +130,8 @@ class PluginFactory: # # Raises: PluginError # - def lookup(self, messenger: Messenger, kind: str, provenance: ProvenanceInformation) -> Tuple[Type[Plugin], str]: - plugin_type, defaults = self._ensure_plugin(kind, provenance) + def lookup(self, messenger: Messenger, kind: str, provenance_node: Node) -> Tuple[Type[Plugin], str]: + plugin_type, defaults = self._ensure_plugin(kind, provenance_node) # We can be called with None for the messenger here in the # case that we've been pickled through the scheduler (see jobpickler.py), @@ -150,7 +149,7 @@ class PluginFactory: if plugin_type.BST_PLUGIN_DEPRECATED and not self._allow_deprecated[kind]: message = Message( MessageType.WARN, - "{}: Using deprecated plugin '{}'".format(provenance, kind), + "{}: Using deprecated plugin '{}'".format(provenance_node.get_provenance(), kind), detail=plugin_type.BST_PLUGIN_DEPRECATION_MESSAGE, ) messenger.message(message) @@ -214,7 +213,7 @@ class PluginFactory: # Raises: # (PluginError): In case something went wrong loading the plugin # - def _ensure_plugin(self, kind: str, provenance: ProvenanceInformation) -> Tuple[Type[Plugin], str]: + def _ensure_plugin(self, kind: str, provenance_node: Node) -> Tuple[Type[Plugin], str]: if kind not in self._types: @@ -239,7 +238,9 @@ class PluginFactory: # Try getting it from the core plugins if kind not in self._site_source.list_plugins(): raise PluginError( - "{}: No {} plugin registered for kind '{}'".format(provenance, self._plugin_type, kind), + "{}: No {} plugin registered for kind '{}'".format( + provenance_node.get_provenance(), self._plugin_type, kind + ), reason="plugin-not-found", ) diff --git a/src/buildstream/_pluginfactory/pluginorigin.py b/src/buildstream/_pluginfactory/pluginorigin.py index e75b8cb58..f464b22ea 100644 --- a/src/buildstream/_pluginfactory/pluginorigin.py +++ b/src/buildstream/_pluginfactory/pluginorigin.py @@ -79,7 +79,7 @@ class PluginOrigin: self.origin_type = origin_type # The PluginOriginType self.elements = {} # A dictionary of PluginConfiguration self.sources = {} # A dictionary of PluginConfiguration objects - self.provenance = None + self.provenance_node = None self.project = None # Private @@ -102,7 +102,7 @@ class PluginOrigin: # def initialize(self, project, origin_node): - self.provenance = origin_node.get_provenance() + self.provenance_node = origin_node self.project = project self.load_config(origin_node) diff --git a/src/buildstream/_pluginfactory/pluginoriginjunction.py b/src/buildstream/_pluginfactory/pluginoriginjunction.py index c32a7956c..55e6849a6 100644 --- a/src/buildstream/_pluginfactory/pluginoriginjunction.py +++ b/src/buildstream/_pluginfactory/pluginoriginjunction.py @@ -35,7 +35,7 @@ class PluginOriginJunction(PluginOrigin): # Get access to the project indicated by the junction, # possibly loading it as a side effect. # - loader = self.project.loader.get_loader(self._junction, self.provenance) + loader = self.project.loader.get_loader(self._junction, self.provenance_node) project = loader.project project.ensure_fully_loaded() @@ -54,7 +54,7 @@ class PluginOriginJunction(PluginOrigin): # raise PluginError( "{}: Error loading {} plugin '{}' from project '{}' referred to by junction '{}': {}".format( - self.provenance, plugin_type, kind, project.name, self._junction, e + self.provenance_node.get_provenance(), plugin_type, kind, project.name, self._junction, e ), reason="junction-plugin-load-error", detail=e.detail, @@ -69,7 +69,7 @@ class PluginOriginJunction(PluginOrigin): # raise PluginError( "{}: project '{}' referred to by junction '{}' does not declare any {} plugin kind: '{}'".format( - self.provenance, project.name, self._junction, plugin_type, kind + self.provenance_node.get_provenance(), project.name, self._junction, plugin_type, kind ), reason="junction-plugin-not-found", ) diff --git a/src/buildstream/_pluginfactory/pluginoriginpip.py b/src/buildstream/_pluginfactory/pluginoriginpip.py index 013cd67f3..59b5d5251 100644 --- a/src/buildstream/_pluginfactory/pluginoriginpip.py +++ b/src/buildstream/_pluginfactory/pluginoriginpip.py @@ -50,27 +50,31 @@ class PluginOriginPip(PluginOrigin): package = pkg_resources.get_entry_info(self._package_name, entrypoint_group, kind) except pkg_resources.DistributionNotFound as e: raise PluginError( - "{}: Failed to load {} plugin '{}': {}".format(self.provenance, plugin_type, kind, e), + "{}: Failed to load {} plugin '{}': {}".format( + self.provenance_node.get_provenance(), plugin_type, kind, e + ), reason="package-not-found", ) from e except pkg_resources.VersionConflict as e: raise PluginError( "{}: Version conflict encountered while loading {} plugin '{}'".format( - self.provenance, plugin_type, kind + self.provenance_node.get_provenance(), plugin_type, kind ), detail=e.report(), reason="package-version-conflict", ) from e except pkg_resources.RequirementParseError as e: raise PluginError( - "{}: Malformed package-name '{}' encountered: {}".format(self.provenance, self._package_name, e), + "{}: Malformed package-name '{}' encountered: {}".format( + self.provenance_node.get_provenance(), self._package_name, e + ), reason="package-malformed-requirement", ) from e if package is None: raise PluginError( "{}: Pip package {} does not contain a plugin named '{}'".format( - self.provenance, self._package_name, kind + self.provenance_node.get_provenance(), self._package_name, kind ), reason="plugin-not-found", ) diff --git a/src/buildstream/_pluginfactory/sourcefactory.py b/src/buildstream/_pluginfactory/sourcefactory.py index 2ed78f838..3375d182b 100644 --- a/src/buildstream/_pluginfactory/sourcefactory.py +++ b/src/buildstream/_pluginfactory/sourcefactory.py @@ -49,6 +49,6 @@ class SourceFactory(PluginFactory): # LoadError (if the source itself took issue with the config) # def create(self, context, project, meta, variables): - source_type, _ = self.lookup(context.messenger, meta.kind, meta.provenance) + source_type, _ = self.lookup(context.messenger, meta.kind, meta.config) source = source_type(context, project, meta, variables) return source diff --git a/src/buildstream/_project.py b/src/buildstream/_project.py index 209ba516c..6154c51c0 100644 --- a/src/buildstream/_project.py +++ b/src/buildstream/_project.py @@ -97,7 +97,7 @@ class Project: cli_options=None, default_mirror=None, parent_loader=None, - provenance=None, + provenance_node=None, search_for_project=True, ): @@ -159,14 +159,14 @@ class Project: self._sandbox = None self._splits = None - # This is a lookup table of dictionaries indexed by project, - # the child dictionaries are junction names as keys with their - # provenances as values + # This is a lookup table of lists indexed by project, + # the child dictionaries are lists of ScalarNodes indicating + # junction names self._junction_duplicates = {} - # A table of project relative junctions to consider as 'internal'. The values - # of the table are simply used to store ProvenanceInformation. - self._junction_internal = {} + # A list of project relative junctions to consider as 'internal', + # stored as ScalarNodes. + self._junction_internal = [] self._context.add_project(self) @@ -175,7 +175,7 @@ class Project: self._project_includes = None with PROFILER.profile(Topics.LOAD_PROJECT, self.directory.replace(os.sep, "-")): - self._load(parent_loader=parent_loader, provenance=provenance) + self._load(parent_loader=parent_loader, provenance_node=provenance_node) self._partially_loaded = True @@ -570,7 +570,7 @@ class Project: # def junction_is_duplicated(self, project_name, loader): - junction_dict = self._junction_duplicates.get(project_name, {}) + junctions = self._junction_duplicates.get(project_name, {}) # Iterate over all paths specified by this project and see # if we find a match for the specified loader. @@ -580,8 +580,8 @@ class Project: # to this project, regardless of any overrides or link elements # which might have been used in the project. # - for dup_path, dup_provenance in junction_dict.items(): - search = self.loader.get_loader(dup_path, dup_provenance, load_subprojects=False) + for dup_path in junctions: + search = self.loader.get_loader(dup_path.as_str(), dup_path, load_subprojects=False) if loader is search: return True @@ -608,8 +608,8 @@ class Project: # to this project, regardless of any overrides or link elements # which might have been used in the project. # - for internal_path, internal_provenance in self._junction_internal.items(): - search = self.loader.get_loader(internal_path, internal_provenance, load_subprojects=False) + for internal_path in self._junction_internal: + search = self.loader.get_loader(internal_path.as_str(), internal_path, load_subprojects=False) if loader is search: return True @@ -765,7 +765,7 @@ class Project: # # Raises: LoadError if there was a problem with the project.conf # - def _load(self, *, parent_loader=None, provenance=None): + def _load(self, *, parent_loader=None, provenance_node=None): # Load builtin default projectfile = os.path.join(self.directory, _PROJECT_CONF_FILE) @@ -819,19 +819,12 @@ class Project: # Parse duplicates junction_duplicates = junctions_node.get_mapping("duplicates", default={}) for project_name, junctions in junction_duplicates.items(): - # For each junction we preserve the provenance and the junction string, - # the provenance is used for lookups later on. - # - self._junction_duplicates[project_name] = junctions_dict = {} - for junction_node in junctions: - junctions_dict[junction_node.as_str()] = junction_node.get_provenance() + self._junction_duplicates[project_name] = junctions # Parse internal - junction_internal = junctions_node.get_sequence("internal", default=[]) - for junction_node in junction_internal: - self._junction_internal[junction_node.as_str()] = junction_node.get_provenance() + self._junction_internal = junctions_node.get_sequence("internal", default=[]) - self.loader = Loader(self, parent=parent_loader, provenance=provenance) + self.loader = Loader(self, parent=parent_loader, provenance_node=provenance_node) self._project_includes = Includes(self.loader, copy_tree=False) diff --git a/src/buildstream/element.py b/src/buildstream/element.py index 57aa37a85..81e55a01b 100644 --- a/src/buildstream/element.py +++ b/src/buildstream/element.py @@ -227,7 +227,7 @@ class Element(Plugin): self.__cache_key_dict = None # Dict for cache key calculation self.__cache_key = None # Our cached cache key - super().__init__(load_element.name, context, project, load_element.provenance, "element") + super().__init__(load_element.name, context, project, load_element.node, "element") # Ensure the project is fully loaded here rather than later on if not load_element.first_pass: diff --git a/src/buildstream/node.pyi b/src/buildstream/node.pyi index 57083602c..854f4bde6 100644 --- a/src/buildstream/node.pyi +++ b/src/buildstream/node.pyi @@ -9,6 +9,7 @@ class ProvenanceInformation: ... class Node: def clone(self) -> "Node": ... + def get_provenance(self) -> ProvenanceInformation: ... class MappingNode(Node, Generic[TNode]): def __init__(self, file_index: int, line: int, column: int, value: Mapping[str, TValidNodeValue]) -> None: ... diff --git a/src/buildstream/plugin.py b/src/buildstream/plugin.py index bc61ae7d6..84165c706 100644 --- a/src/buildstream/plugin.py +++ b/src/buildstream/plugin.py @@ -120,7 +120,7 @@ from weakref import WeakValueDictionary from . import utils from ._exceptions import PluginError, ImplError from ._message import Message, MessageType -from .node import MappingNode, ProvenanceInformation +from .node import Node, MappingNode from .types import CoreWarnings, SourceRef if TYPE_CHECKING: @@ -217,7 +217,7 @@ class Plugin: name: str, context: "Context", project: "Project", - provenance: ProvenanceInformation, + provenance_node: Node, type_tag: str, unique_id: Optional[int] = None, ): @@ -258,7 +258,7 @@ class Plugin: # sure to test plugin pickling if this reference is to be removed. self.__project = project # The Project object - self.__provenance = provenance # The Provenance information + self.__provenance_node = provenance_node # The originating YAML node self.__type_tag = type_tag # The type of plugin (element or source) self.__configuring = False # Whether we are currently configuring @@ -278,7 +278,7 @@ class Plugin: def __str__(self): return "{kind} {typetag} at {provenance}".format( - kind=self.__kind, typetag=self.__type_tag, provenance=self.__provenance + kind=self.__kind, typetag=self.__type_tag, provenance=self._get_provenance() ) ############################################################# @@ -629,7 +629,7 @@ class Plugin: # Fetch bst file, line and column of the entity # def _get_provenance(self): - return self.__provenance + return self.__provenance_node.get_provenance() # Context manager for getting the open file handle to this # plugin's log. Used in the child context to add stuff to diff --git a/src/buildstream/plugins/elements/junction.py b/src/buildstream/plugins/elements/junction.py index 425b917ef..8693313af 100644 --- a/src/buildstream/plugins/elements/junction.py +++ b/src/buildstream/plugins/elements/junction.py @@ -284,25 +284,22 @@ class JunctionElement(Element): self.ignore_junction_remotes = node.get_bool("ignore-junction-remotes", default=False) # The overrides dictionary has the target junction - # to override as a key, and a tuple consisting - # of the local overriding junction and the provenance - # of the override declaration. + # to override as a key, and the ScalarNode of the + # junction name as a value self.overrides = {} overrides_node = node.get_mapping("overrides", {}) - for key, value in overrides_node.items(): - junction_name = value.as_str() - provenance = value.get_provenance() + for key, junction_name in overrides_node.items(): # Cannot override a subproject with the project itself # - if junction_name == self.name: + if junction_name.as_str() == self.name: raise ElementError( "{}: Attempt to override subproject junction '{}' with the overriding junction '{}' itself".format( - provenance, key, junction_name + junction_name.get_provenance(), key, junction_name.as_str() ), reason="override-junction-with-self", ) - self.overrides[key] = (junction_name, provenance) + self.overrides[key] = junction_name def preflight(self): pass diff --git a/src/buildstream/plugins/elements/link.py b/src/buildstream/plugins/elements/link.py index e6d7f056e..e5e694579 100644 --- a/src/buildstream/plugins/elements/link.py +++ b/src/buildstream/plugins/elements/link.py @@ -59,12 +59,9 @@ class LinkElement(Element): node.validate_keys(["target"]) - # Hold onto the provenance of the specified target, - # allowing the loader to raise errors with better context. + # Hold onto the node, keep it around for provenance. # - target_node = node.get_scalar("target") - self.target = target_node.as_str() - self.target_provenance = target_node.get_provenance() + self.target_node = node.get_scalar("target") def preflight(self): pass diff --git a/src/buildstream/source.py b/src/buildstream/source.py index ce1cf8434..e0a2db45d 100644 --- a/src/buildstream/source.py +++ b/src/buildstream/source.py @@ -322,14 +322,13 @@ class Source(Plugin): alias_override: Optional[Tuple[str, str]] = None, unique_id: Optional[int] = None ): - provenance = meta.config.get_provenance() # Set element_name member before parent init, as needed for debug messaging self.__element_name = meta.element_name # The name of the element owning this source super().__init__( "{}-{}".format(meta.element_name, meta.element_index), context, project, - provenance, + meta.config, "source", unique_id=unique_id, ) diff --git a/src/buildstream/types.py b/src/buildstream/types.py index 48dd5de6d..a5cf7afb8 100644 --- a/src/buildstream/types.py +++ b/src/buildstream/types.py @@ -280,14 +280,14 @@ class _PipelineSelection(FastEnum): # # Args: # project (Project): The project instance -# provenance (ProvenanceInformation): The provenance information, if any +# provenance_node (Node): The provenance information, if any # duplicates (list): List of project descriptions which declared this project as a duplicate # internal (list): List of project descriptions which declared this project as internal # class _ProjectInformation: - def __init__(self, project, provenance, duplicates, internal): + def __init__(self, project, provenance_node, duplicates, internal): self.project = project - self.provenance = provenance + self.provenance = provenance_node.get_provenance() if provenance_node else None self.duplicates = duplicates self.internal = internal -- cgit v1.2.1