From d8dc6bcf56b0ec73f1a5c954ccb184517e748ec5 Mon Sep 17 00:00:00 2001 From: Tristan van Berkom Date: Tue, 19 May 2020 19:15:51 +0900 Subject: _loader/loader.py: Reoganized public/private methods * This file had `clean_caches()` documented as public but the function was actually private `_clean_caches()`: Moved this to the end of the class in the private section. * The `_get_loader()` was marked as private but is in fact public, and used by the project to load cross junction include files. This patch also updates `_includes.py` to call the public `get_loader()` method instead of sneaking in and calling the private `_get_loader()` method (while also removing one redundant line of code from the same function). --- src/buildstream/_includes.py | 3 +- src/buildstream/_loader/loader.py | 391 +++++++++++++++++++------------------- 2 files changed, 197 insertions(+), 197 deletions(-) diff --git a/src/buildstream/_includes.py b/src/buildstream/_includes.py index 2ecb70a31..7f4863e52 100644 --- a/src/buildstream/_includes.py +++ b/src/buildstream/_includes.py @@ -151,8 +151,7 @@ class Includes: shortname = include if ":" in include: junction, include = include.split(":", 1) - junction_loader = loader._get_loader(junction) - current_loader = junction_loader + current_loader = loader.get_loader(junction) current_loader.project.ensure_fully_loaded() else: current_loader = loader diff --git a/src/buildstream/_loader/loader.py b/src/buildstream/_loader/loader.py index 3032f9036..3cfaf8be6 100644 --- a/src/buildstream/_loader/loader.py +++ b/src/buildstream/_loader/loader.py @@ -163,6 +163,183 @@ class Loader: return ret + # get_loader(): + # + # Return loader for specified junction + # + # Args: + # filename (str): Junction name + # + # Raises: LoadError + # + # Returns: A Loader or None if specified junction does not exist + # + def get_loader(self, filename, *, rewritable=False, ticker=None, level=0, provenance=None): + + provenance_str = "" + if provenance is not None: + provenance_str = "{}: ".format(provenance) + + # return previously determined result + if filename in self._loaders: + loader = self._loaders[filename] + + if loader is None: + # do not allow junctions with the same name in different + # subprojects + raise LoadError( + "{}Conflicting junction {} in subprojects, define junction in {}".format( + provenance_str, filename, self.project.name + ), + LoadErrorReason.CONFLICTING_JUNCTION, + ) + + return loader + + if self._parent: + # junctions in the parent take precedence over junctions defined + # in subprojects + loader = self._parent.get_loader( + filename, rewritable=rewritable, ticker=ticker, level=level + 1, provenance=provenance + ) + if loader: + self._loaders[filename] = loader + return loader + + try: + self._load_file(filename, rewritable, ticker) + except LoadError as e: + if e.reason != LoadErrorReason.MISSING_FILE: + # other load error + raise + + if level == 0: + # junction element not found in this or ancestor projects + raise + + # mark junction as not available to allow detection of + # conflicting junctions in subprojects + self._loaders[filename] = None + return None + + # meta junction element + # + # Note that junction elements are not allowed to have + # dependencies, so disabling progress reporting here should + # have no adverse effects - the junction element itself cannot + # be depended on, so it would be confusing for its load to + # show up in logs. + # + # Any task counting *inside* the junction will be handled by + # its loader. + meta_element = self._collect_element_no_deps(self._elements[filename], _NO_PROGRESS) + if meta_element.kind != "junction": + raise LoadError( + "{}{}: Expected junction but element kind is {}".format(provenance_str, filename, meta_element.kind), + LoadErrorReason.INVALID_DATA, + ) + + # We check that junctions have no dependencies a little + # early. This is cheating, since we don't technically know + # that junctions aren't allowed to have dependencies. + # + # However, this makes progress reporting more intuitive + # because we don't need to load dependencies of an element + # that shouldn't have any, and therefore don't need to + # duplicate the load count for elements that shouldn't be. + # + # We also fail slightly earlier (since we don't need to go + # through the entire loading process), which is nice UX. It + # would be nice if this could be done for *all* element types, + # but since we haven't loaded those yet that's impossible. + if self._elements[filename].dependencies: + raise LoadError("Dependencies are forbidden for 'junction' elements", LoadErrorReason.INVALID_JUNCTION) + + element = Element._new_from_meta(meta_element) + element._initialize_state() + + # If this junction element points to a sub-sub-project, we need to + # find loader for that project. + if element.target: + subproject_loader = self.get_loader( + element.target_junction, rewritable=rewritable, ticker=ticker, level=level, provenance=provenance + ) + loader = subproject_loader.get_loader( + element.target_element, rewritable=rewritable, ticker=ticker, level=level, provenance=provenance + ) + self._loaders[filename] = loader + return loader + + # Handle the case where a subproject has no ref + # + 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), + LoadErrorReason.SUBPROJECT_INCONSISTENT, + detail=detail, + ) + + # Handle the case where a subproject needs to be fetched + # + if not element._has_all_sources_in_source_cache(): + if ticker: + ticker(filename, "Fetching subproject") + self._fetch_subprojects([element]) + + sources = list(element.sources()) + if len(sources) == 1 and sources[0]._get_local_path(): + # Optimization for junctions with a single local source + basedir = sources[0]._get_local_path() + else: + # Note: We use _KeyStrength.WEAK here because junctions + # cannot have dependencies, therefore the keys are + # equivalent. + # + # Since the element has not necessarily been given a + # strong cache key at this point (in a non-strict build + # that is set *after* we complete building/pulling, which + # we haven't yet for this element), + # element._get_cache_key() can fail if used with the + # default _KeyStrength.STRONG. + basedir = os.path.join( + self.project.directory, ".bst", "staged-junctions", filename, element._get_cache_key(_KeyStrength.WEAK) + ) + if not os.path.exists(basedir): + os.makedirs(basedir, exist_ok=True) + element._stage_sources_at(basedir) + + # Load the project + project_dir = os.path.join(basedir, element.path) + try: + from .._project import Project # pylint: disable=cyclic-import + + project = Project( + project_dir, + self._context, + junction=element, + parent_loader=self, + search_for_project=False, + fetch_subprojects=self._fetch_subprojects, + ) + 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) + ) + if element.path: + message += " Was expecting it at path '{}' in the junction's source.".format(element.path) + raise LoadError(message=message, reason=LoadErrorReason.INVALID_JUNCTION) from e + + # Otherwise, we don't know the reason, so just raise + raise + + loader = project.loader + self._loaders[filename] = loader + + return loader + # get_state_for_child_job_pickling(self) # # Return data necessary to reconstruct this object in a child job process. @@ -193,23 +370,6 @@ class Loader: return state - # clean_caches() - # - # Clean internal loader caches, recursively - # - # When loading the elements, the loaders use caches in order to not load the - # same element twice. These are kept after loading and prevent garbage - # collection. Cleaning them explicitely is required. - # - def _clean_caches(self): - for loader in self._loaders.values(): - # value may be None with nested junctions without overrides - if loader is not None: - loader._clean_caches() - - self._meta_elements = {} - self._elements = {} - ########################################### # Private Methods # ########################################### @@ -335,7 +495,7 @@ class Loader: if dep.junction: self._load_file(dep.junction, rewritable, ticker, dep.provenance) - loader = self._get_loader( + loader = self.get_loader( dep.junction, rewritable=rewritable, ticker=ticker, provenance=dep.provenance ) dep_element = loader._load_file(dep.name, rewritable, ticker, dep.provenance) @@ -543,182 +703,6 @@ class Loader: return self._meta_elements[top_element.name] - # _get_loader(): - # - # Return loader for specified junction - # - # Args: - # filename (str): Junction name - # - # Raises: LoadError - # - # Returns: A Loader or None if specified junction does not exist - def _get_loader(self, filename, *, rewritable=False, ticker=None, level=0, provenance=None): - - provenance_str = "" - if provenance is not None: - provenance_str = "{}: ".format(provenance) - - # return previously determined result - if filename in self._loaders: - loader = self._loaders[filename] - - if loader is None: - # do not allow junctions with the same name in different - # subprojects - raise LoadError( - "{}Conflicting junction {} in subprojects, define junction in {}".format( - provenance_str, filename, self.project.name - ), - LoadErrorReason.CONFLICTING_JUNCTION, - ) - - return loader - - if self._parent: - # junctions in the parent take precedence over junctions defined - # in subprojects - loader = self._parent._get_loader( - filename, rewritable=rewritable, ticker=ticker, level=level + 1, provenance=provenance - ) - if loader: - self._loaders[filename] = loader - return loader - - try: - self._load_file(filename, rewritable, ticker) - except LoadError as e: - if e.reason != LoadErrorReason.MISSING_FILE: - # other load error - raise - - if level == 0: - # junction element not found in this or ancestor projects - raise - - # mark junction as not available to allow detection of - # conflicting junctions in subprojects - self._loaders[filename] = None - return None - - # meta junction element - # - # Note that junction elements are not allowed to have - # dependencies, so disabling progress reporting here should - # have no adverse effects - the junction element itself cannot - # be depended on, so it would be confusing for its load to - # show up in logs. - # - # Any task counting *inside* the junction will be handled by - # its loader. - meta_element = self._collect_element_no_deps(self._elements[filename], _NO_PROGRESS) - if meta_element.kind != "junction": - raise LoadError( - "{}{}: Expected junction but element kind is {}".format(provenance_str, filename, meta_element.kind), - LoadErrorReason.INVALID_DATA, - ) - - # We check that junctions have no dependencies a little - # early. This is cheating, since we don't technically know - # that junctions aren't allowed to have dependencies. - # - # However, this makes progress reporting more intuitive - # because we don't need to load dependencies of an element - # that shouldn't have any, and therefore don't need to - # duplicate the load count for elements that shouldn't be. - # - # We also fail slightly earlier (since we don't need to go - # through the entire loading process), which is nice UX. It - # would be nice if this could be done for *all* element types, - # but since we haven't loaded those yet that's impossible. - if self._elements[filename].dependencies: - raise LoadError("Dependencies are forbidden for 'junction' elements", LoadErrorReason.INVALID_JUNCTION) - - element = Element._new_from_meta(meta_element) - element._initialize_state() - - # If this junction element points to a sub-sub-project, we need to - # find loader for that project. - if element.target: - subproject_loader = self._get_loader( - element.target_junction, rewritable=rewritable, ticker=ticker, level=level, provenance=provenance - ) - loader = subproject_loader._get_loader( - element.target_element, rewritable=rewritable, ticker=ticker, level=level, provenance=provenance - ) - self._loaders[filename] = loader - return loader - - # Handle the case where a subproject has no ref - # - 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), - LoadErrorReason.SUBPROJECT_INCONSISTENT, - detail=detail, - ) - - # Handle the case where a subproject needs to be fetched - # - if not element._has_all_sources_in_source_cache(): - if ticker: - ticker(filename, "Fetching subproject") - self._fetch_subprojects([element]) - - sources = list(element.sources()) - if len(sources) == 1 and sources[0]._get_local_path(): - # Optimization for junctions with a single local source - basedir = sources[0]._get_local_path() - else: - # Note: We use _KeyStrength.WEAK here because junctions - # cannot have dependencies, therefore the keys are - # equivalent. - # - # Since the element has not necessarily been given a - # strong cache key at this point (in a non-strict build - # that is set *after* we complete building/pulling, which - # we haven't yet for this element), - # element._get_cache_key() can fail if used with the - # default _KeyStrength.STRONG. - basedir = os.path.join( - self.project.directory, ".bst", "staged-junctions", filename, element._get_cache_key(_KeyStrength.WEAK) - ) - if not os.path.exists(basedir): - os.makedirs(basedir, exist_ok=True) - element._stage_sources_at(basedir) - - # Load the project - project_dir = os.path.join(basedir, element.path) - try: - from .._project import Project # pylint: disable=cyclic-import - - project = Project( - project_dir, - self._context, - junction=element, - parent_loader=self, - search_for_project=False, - fetch_subprojects=self._fetch_subprojects, - ) - 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) - ) - if element.path: - message += " Was expecting it at path '{}' in the junction's source.".format(element.path) - raise LoadError(message=message, reason=LoadErrorReason.INVALID_JUNCTION) from e - - # Otherwise, we don't know the reason, so just raise - raise - - loader = project.loader - self._loaders[filename] = loader - - return loader - # _parse_name(): # # Get junction and base name of element along with loader for the sub-project @@ -743,7 +727,7 @@ class Loader: return None, junction_path[-1], self else: self._load_file(junction_path[-2], rewritable, ticker) - loader = self._get_loader(junction_path[-2], rewritable=rewritable, ticker=ticker) + loader = self.get_loader(junction_path[-2], rewritable=rewritable, ticker=ticker) return junction_path[-2], junction_path[-1], loader # Print a warning message, checks warning_token against project configuration @@ -805,3 +789,20 @@ class Loader: ), warning_token=CoreWarnings.BAD_CHARACTERS_IN_NAME, ) + + # _clean_caches() + # + # Clean internal loader caches, recursively + # + # When loading the elements, the loaders use caches in order to not load the + # same element twice. These are kept after loading and prevent garbage + # collection. Cleaning them explicitely is required. + # + def _clean_caches(self): + for loader in self._loaders.values(): + # value may be None with nested junctions without overrides + if loader is not None: + loader._clean_caches() + + self._meta_elements = {} + self._elements = {} -- cgit v1.2.1