_loader/loader.py: Reoganized public/private methodstristan/loader-api-fixup

  * 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).

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 = {}