Element.dependencies() now yields ElementProxy wrappers by default.tristan/element-proxy

This prepares the ground for policing the dependencies which are visible
to an Element plugin, such that plugins are only allowed to see the
elements in their Scope.BUILD scope, even if they call Element.dependencies()
on a dependency.

This commit does the following:

  * Element.dependencies() is now a user facing frontend which yields
    ElementProxy elements instead of Elements.

  * Various core codepaths have been updated to call the internal
    Element._dependencies() codepath which still returns Elements.

7 files changed, 134 insertions, 87 deletions

diff --git a/src/buildstream/_artifact.py b/src/buildstream/_artifact.py
index f74f3f9ff..1df665c14 100644
--- a/src/buildstream/_artifact.py
+++ b/src/buildstream/_artifact.py
@@ -181,7 +181,7 @@ class Artifact:
             size += public_data_digest.size_bytes
 
         # store build dependencies
-        for e in element.dependencies(Scope.BUILD):
+        for e in element._dependencies(Scope.BUILD):
             new_build = artifact.build_deps.add()
             new_build.project_name = e.project_name
             new_build.element_name = e.name
diff --git a/src/buildstream/_frontend/widget.py b/src/buildstream/_frontend/widget.py
index a4268f62b..dbe6b4337 100644
--- a/src/buildstream/_frontend/widget.py
+++ b/src/buildstream/_frontend/widget.py
@@ -413,17 +413,17 @@ class LogLine(Widget):
 
             # Dependencies
             if "%{deps" in format_:
-                deps = [e.name for e in element.dependencies(Scope.ALL, recurse=False)]
+                deps = [e.name for e in element._dependencies(Scope.ALL, recurse=False)]
                 line = p.fmt_subst(line, "deps", yaml.safe_dump(deps, default_style=None).rstrip("\n"))
 
             # Build Dependencies
             if "%{build-deps" in format_:
-                build_deps = [e.name for e in element.dependencies(Scope.BUILD, recurse=False)]
+                build_deps = [e.name for e in element._dependencies(Scope.BUILD, recurse=False)]
                 line = p.fmt_subst(line, "build-deps", yaml.safe_dump(build_deps, default_style=False).rstrip("\n"))
 
             # Runtime Dependencies
             if "%{runtime-deps" in format_:
-                runtime_deps = [e.name for e in element.dependencies(Scope.RUN, recurse=False)]
+                runtime_deps = [e.name for e in element._dependencies(Scope.RUN, recurse=False)]
                 line = p.fmt_subst(
                     line, "runtime-deps", yaml.safe_dump(runtime_deps, default_style=False).rstrip("\n")
                 )
diff --git a/src/buildstream/_pipeline.py b/src/buildstream/_pipeline.py
index 340c12b1a..0fb30e244 100644
--- a/src/buildstream/_pipeline.py
+++ b/src/buildstream/_pipeline.py
@@ -157,7 +157,7 @@ class Pipeline:
         visited = (BitMap(), BitMap())
 
         for target in targets:
-            for element in target.dependencies(scope, recurse=recurse, visited=visited):
+            for element in target._dependencies(scope, recurse=recurse, visited=visited):
                 yield element
 
     # plan()
@@ -251,7 +251,7 @@ class Pipeline:
             if element in targeted:
                 yield element
             else:
-                for dep in element.dependencies(Scope.ALL, recurse=False):
+                for dep in element._dependencies(Scope.ALL, recurse=False):
                     yield from find_intersection(dep)
 
         # Build a list of 'intersection' elements, i.e. the set of
@@ -272,7 +272,7 @@ class Pipeline:
                 continue
             visited.append(element)
 
-            queue.extend(element.dependencies(Scope.ALL, recurse=False))
+            queue.extend(element._dependencies(Scope.ALL, recurse=False))
 
         # That looks like a lot, but overall we only traverse (part
         # of) the graph twice. This could be reduced to once if we
@@ -474,12 +474,12 @@ class _Planner:
             return
 
         self.visiting_elements.add(element)
-        for dep in element.dependencies(Scope.RUN, recurse=False):
+        for dep in element._dependencies(Scope.RUN, recurse=False):
             self.plan_element(dep, depth)
 
         # Dont try to plan builds of elements that are cached already
         if not element._cached_success():
-            for dep in element.dependencies(Scope.BUILD, recurse=False):
+            for dep in element._dependencies(Scope.BUILD, recurse=False):
                 self.plan_element(dep, depth + 1)
 
         self.depth_map[element] = depth
diff --git a/src/buildstream/_stream.py b/src/buildstream/_stream.py
index e9bd60244..4665a05fd 100644
--- a/src/buildstream/_stream.py
+++ b/src/buildstream/_stream.py
@@ -818,7 +818,7 @@ class Stream:
         for target in elements:
 
             if not list(target.sources()):
-                build_depends = [x.name for x in target.dependencies(Scope.BUILD, recurse=False)]
+                build_depends = [x.name for x in target._dependencies(Scope.BUILD, recurse=False)]
                 if not build_depends:
                     raise StreamError("The element {}  has no sources".format(target.name))
                 detail = "Try opening a workspace on one of its dependencies instead:\n"
diff --git a/src/buildstream/element.py b/src/buildstream/element.py
index 4b53aa3dd..3953c7da9 100644
--- a/src/buildstream/element.py
+++ b/src/buildstream/element.py
@@ -105,6 +105,7 @@ from .sandbox._config import SandboxConfig
 from .sandbox._sandboxremote import SandboxRemote
 from .types import CoreWarnings, Scope, _CacheBuildTrees, _KeyStrength
 from ._artifact import Artifact
+from ._elementproxy import ElementProxy
 from ._elementsources import ElementSources
 from ._loader import Symbol, MetaSource
 
@@ -435,65 +436,13 @@ class Element(Plugin):
         Yields:
            The dependencies in `scope`, in deterministic staging order
         """
-        # The format of visited is (BitMap(), BitMap()), with the first BitMap
-        # containing element that have been visited for the `Scope.BUILD` case
-        # and the second one relating to the `Scope.RUN` case.
-        if not recurse:
-            result: Set[Element] = set()
-            if scope in (Scope.BUILD, Scope.ALL):
-                for dep in self.__build_dependencies:
-                    if dep not in result:
-                        result.add(dep)
-                        yield dep
-            if scope in (Scope.RUN, Scope.ALL):
-                for dep in self.__runtime_dependencies:
-                    if dep not in result:
-                        result.add(dep)
-                        yield dep
-        else:
-
-            def visit(element, scope, visited):
-                if scope == Scope.ALL:
-                    visited[0].add(element._unique_id)
-                    visited[1].add(element._unique_id)
-
-                    for dep in chain(element.__build_dependencies, element.__runtime_dependencies):
-                        if dep._unique_id not in visited[0] and dep._unique_id not in visited[1]:
-                            yield from visit(dep, Scope.ALL, visited)
-
-                    yield element
-                elif scope == Scope.BUILD:
-                    visited[0].add(element._unique_id)
-
-                    for dep in element.__build_dependencies:
-                        if dep._unique_id not in visited[1]:
-                            yield from visit(dep, Scope.RUN, visited)
-
-                elif scope == Scope.RUN:
-                    visited[1].add(element._unique_id)
-
-                    for dep in element.__runtime_dependencies:
-                        if dep._unique_id not in visited[1]:
-                            yield from visit(dep, Scope.RUN, visited)
-
-                    yield element
-                else:
-                    yield element
-
-            if visited is None:
-                # Visited is of the form (Visited for Scope.BUILD, Visited for Scope.RUN)
-                visited = (BitMap(), BitMap())
-            else:
-                # We have already a visited set passed. we might be able to short-circuit
-                if scope in (Scope.BUILD, Scope.ALL) and self._unique_id in visited[0]:
-                    return
-                if scope in (Scope.RUN, Scope.ALL) and self._unique_id in visited[1]:
-                    return
-
-            yield from visit(self, scope, visited)
+        for dep in self._dependencies(scope, recurse=recurse):
+            yield cast("Element", ElementProxy(self, dep))
 
     def search(self, scope: Scope, name: str) -> Optional["Element"]:
-        """Search for a dependency by name
+        """search(scope, *, name)
+
+        Search for a dependency by name
 
         Args:
            scope: The scope to search
@@ -502,9 +451,9 @@ class Element(Plugin):
         Returns:
            The dependency element, or None if not found.
         """
-        for dep in self.dependencies(scope):
-            if dep.name == name:
-                return dep
+        search = self._search(scope, name)
+        if search:
+            return cast("Element", ElementProxy(self, search))
 
         return None
 
@@ -732,7 +681,7 @@ class Element(Plugin):
                 # The bottom item overlaps nothing
                 overlapping_elements = elements[1:]
                 for elm in overlapping_elements:
-                    element = cast(Element, self.search(scope, elm))
+                    element = cast(Element, self._search(scope, elm))
                     if not element.__file_is_whitelisted(f):
                         overlap_warning_elements.append(elm)
                         overlap_warning = True
@@ -877,6 +826,101 @@ class Element(Plugin):
     #            Private Methods used in BuildStream            #
     #############################################################
 
+    # _dependencies()
+    #
+    # A generator function which yields the dependencies of the given element.
+    #
+    # If `recurse` is specified (the default), the full dependencies will be listed
+    # in deterministic staging order, starting with the basemost elements in the
+    # given `scope`. Otherwise, if `recurse` is not specified then only the direct
+    # dependencies in the given `scope` will be traversed, and the element itself
+    # will be omitted.
+    #
+    # Args:
+    #    scope (Scope): The scope to iterate in
+    #    recurse (bool): Whether to recurse
+    #
+    # Yields:
+    #    (Element): The dependencies in `scope`, in deterministic staging order
+    #
+    def _dependencies(self, scope: Scope, *, recurse: bool = True, visited=None) -> Iterator["Element"]:
+
+        # The format of visited is (BitMap(), BitMap()), with the first BitMap
+        # containing element that have been visited for the `Scope.BUILD` case
+        # and the second one relating to the `Scope.RUN` case.
+        if not recurse:
+            result: Set[Element] = set()
+            if scope in (Scope.BUILD, Scope.ALL):
+                for dep in self.__build_dependencies:
+                    if dep not in result:
+                        result.add(dep)
+                        yield dep
+            if scope in (Scope.RUN, Scope.ALL):
+                for dep in self.__runtime_dependencies:
+                    if dep not in result:
+                        result.add(dep)
+                        yield dep
+        else:
+
+            def visit(element, scope, visited):
+                if scope == Scope.ALL:
+                    visited[0].add(element._unique_id)
+                    visited[1].add(element._unique_id)
+
+                    for dep in chain(element.__build_dependencies, element.__runtime_dependencies):
+                        if dep._unique_id not in visited[0] and dep._unique_id not in visited[1]:
+                            yield from visit(dep, Scope.ALL, visited)
+
+                    yield element
+                elif scope == Scope.BUILD:
+                    visited[0].add(element._unique_id)
+
+                    for dep in element.__build_dependencies:
+                        if dep._unique_id not in visited[1]:
+                            yield from visit(dep, Scope.RUN, visited)
+
+                elif scope == Scope.RUN:
+                    visited[1].add(element._unique_id)
+
+                    for dep in element.__runtime_dependencies:
+                        if dep._unique_id not in visited[1]:
+                            yield from visit(dep, Scope.RUN, visited)
+
+                    yield element
+                else:
+                    yield element
+
+            if visited is None:
+                # Visited is of the form (Visited for Scope.BUILD, Visited for Scope.RUN)
+                visited = (BitMap(), BitMap())
+            else:
+                # We have already a visited set passed. we might be able to short-circuit
+                if scope in (Scope.BUILD, Scope.ALL) and self._unique_id in visited[0]:
+                    return
+                if scope in (Scope.RUN, Scope.ALL) and self._unique_id in visited[1]:
+                    return
+
+            yield from visit(self, scope, visited)
+
+    # _search()
+    #
+    # Search for a dependency by name
+    #
+    # Args:
+    #    scope (Scope): The scope to search
+    #    name (str): The dependency to search for
+    #
+    # Returns:
+    #    (Element): The dependency element, or None if not found.
+    #
+    def _search(self, scope: Scope, name: str) -> Optional["Element"]:
+
+        for dep in self._dependencies(scope):
+            if dep.name == name:
+                return dep
+
+        return None
+
     # _new_from_load_element():
     #
     # Recursively instantiate a new Element instance, its sources
@@ -1354,7 +1398,7 @@ class Element(Plugin):
         self.__required = True
 
         # Request artifacts of runtime dependencies
-        for dep in self.dependencies(Scope.RUN, recurse=False):
+        for dep in self._dependencies(Scope.RUN, recurse=False):
             dep._set_required()
 
         # When an element becomes required, it must be assembled for
@@ -1389,7 +1433,7 @@ class Element(Plugin):
         self.__artifact_files_required = True
 
         # Request artifact files of runtime dependencies
-        for dep in self.dependencies(scope, recurse=False):
+        for dep in self._dependencies(scope, recurse=False):
             dep._set_artifact_files_required(scope=scope)
 
     # _artifact_files_required():
@@ -1442,7 +1486,7 @@ class Element(Plugin):
         self.__assemble_scheduled = True
 
         # Requests artifacts of build dependencies
-        for dep in self.dependencies(Scope.BUILD, recurse=False):
+        for dep in self._dependencies(Scope.BUILD, recurse=False):
             dep._set_required()
 
         # Once we schedule an element for assembly, we know that our
@@ -2269,7 +2313,7 @@ class Element(Plugin):
     def __get_dependency_artifact_names(self):
         return [
             os.path.join(dep.project_name, _get_normal_name(dep.name), dep._get_cache_key())
-            for dep in self.dependencies(Scope.BUILD)
+            for dep in self._dependencies(Scope.BUILD)
         ]
 
     # __get_last_build_artifact()
@@ -2333,21 +2377,23 @@ class Element(Plugin):
     def __preflight(self):
 
         if self.BST_FORBID_RDEPENDS and self.BST_FORBID_BDEPENDS:
-            if any(self.dependencies(Scope.RUN, recurse=False)) or any(self.dependencies(Scope.BUILD, recurse=False)):
+            if any(self._dependencies(Scope.RUN, recurse=False)) or any(
+                self._dependencies(Scope.BUILD, recurse=False)
+            ):
                 raise ElementError(
                     "{}: Dependencies are forbidden for '{}' elements".format(self, self.get_kind()),
                     reason="element-forbidden-depends",
                 )
 
         if self.BST_FORBID_RDEPENDS:
-            if any(self.dependencies(Scope.RUN, recurse=False)):
+            if any(self._dependencies(Scope.RUN, recurse=False)):
                 raise ElementError(
                     "{}: Runtime dependencies are forbidden for '{}' elements".format(self, self.get_kind()),
                     reason="element-forbidden-rdepends",
                 )
 
         if self.BST_FORBID_BDEPENDS:
-            if any(self.dependencies(Scope.BUILD, recurse=False)):
+            if any(self._dependencies(Scope.BUILD, recurse=False)):
                 raise ElementError(
                     "{}: Build dependencies are forbidden for '{}' elements".format(self, self.get_kind()),
                     reason="element-forbidden-bdepends",
@@ -2904,7 +2950,7 @@ class Element(Plugin):
                 [e.project_name, e.name, e._get_cache_key(strength=_KeyStrength.WEAK)]
                 if self.BST_STRICT_REBUILD or e in self.__strict_dependencies
                 else [e.project_name, e.name]
-                for e in self.dependencies(Scope.BUILD)
+                for e in self._dependencies(Scope.BUILD)
             ]
 
             self.__weak_cache_key = self._calculate_cache_key(dependencies)
@@ -2917,7 +2963,7 @@ class Element(Plugin):
         if self.__strict_cache_key is None:
             dependencies = [
                 [e.project_name, e.name, e.__strict_cache_key] if e.__strict_cache_key is not None else None
-                for e in self.dependencies(Scope.BUILD)
+                for e in self._dependencies(Scope.BUILD)
             ]
             self.__strict_cache_key = self._calculate_cache_key(dependencies)
 
@@ -3007,7 +3053,7 @@ class Element(Plugin):
                 self.__cache_key = strong_key
             elif self.__assemble_scheduled or self.__assemble_done:
                 # Artifact will or has been built, not downloaded
-                dependencies = [[e.project_name, e.name, e._get_cache_key()] for e in self.dependencies(Scope.BUILD)]
+                dependencies = [[e.project_name, e.name, e._get_cache_key()] for e in self._dependencies(Scope.BUILD)]
                 self.__cache_key = self._calculate_cache_key(dependencies)
 
             if self.__cache_key is None:
diff --git a/src/buildstream/plugins/elements/filter.py b/src/buildstream/plugins/elements/filter.py
index 62c04e027..9647c7191 100644
--- a/src/buildstream/plugins/elements/filter.py
+++ b/src/buildstream/plugins/elements/filter.py
@@ -169,7 +169,7 @@ class FilterElement(Element):
 
     def preflight(self):
         # Exactly one build-depend is permitted
-        build_deps = list(self.dependencies(Scope.BUILD, recurse=False))
+        build_deps = list(self._dependencies(Scope.BUILD, recurse=False))
         if len(build_deps) != 1:
             detail = "Full list of build-depends:\n"
             deps_list = "  \n".join([x.name for x in build_deps])
@@ -183,7 +183,7 @@ class FilterElement(Element):
             )
 
         # That build-depend must not also be a runtime-depend
-        runtime_deps = list(self.dependencies(Scope.RUN, recurse=False))
+        runtime_deps = list(self._dependencies(Scope.RUN, recurse=False))
         if build_deps[0] in runtime_deps:
             detail = "Full list of runtime depends:\n"
             deps_list = "  \n".join([x.name for x in runtime_deps])
@@ -222,7 +222,7 @@ class FilterElement(Element):
 
     def assemble(self, sandbox):
         with self.timed_activity("Staging artifact", silent_nested=True):
-            for dep in self.dependencies(Scope.BUILD, recurse=False):
+            for dep in self._dependencies(Scope.BUILD, recurse=False):
                 # Check that all the included/excluded domains exist
                 pub_data = dep.get_public_data("bst")
                 split_rules = pub_data.get_mapping("split-rules", {})
@@ -253,14 +253,15 @@ class FilterElement(Element):
 
     def _get_source_element(self):
         # Filter elements act as proxies for their sole build-dependency
-        build_deps = list(self.dependencies(Scope.BUILD, recurse=False))
+        #
+        build_deps = list(self._dependencies(Scope.BUILD, recurse=False))
         assert len(build_deps) == 1
         output_elm = build_deps[0]._get_source_element()
         return output_elm
 
     def integrate(self, sandbox):
         if self.pass_integration:
-            for dep in self.dependencies(Scope.BUILD, recurse=False):
+            for dep in self._dependencies(Scope.BUILD, recurse=False):
                 dep.integrate(sandbox)
         super().integrate(sandbox)
 
diff --git a/tests/artifactcache/push.py b/tests/artifactcache/push.py
index 07f2c2560..87bc3b325 100644
--- a/tests/artifactcache/push.py
+++ b/tests/artifactcache/push.py
@@ -35,7 +35,7 @@ def _push(cli, cache_dir, project_dir, config_file, target):
         # Ensure the element's artifact memeber is initialised
         # This is duplicated from Pipeline.resolve_elements()
         # as this test does not use the cli frontend.
-        for e in element.dependencies(Scope.ALL):
+        for e in element._dependencies(Scope.ALL):
             e._initialize_state()
 
         # Manually setup the CAS remotes