Merge branch 'jennis/load_artifact_dependencies' into 'master'

Add the ability to load (build) deps from an artifact ref

See merge request BuildStream/buildstream!1553

9 files changed, 293 insertions, 16 deletions

diff --git a/src/buildstream/_artifact.py b/src/buildstream/_artifact.py
index 05d025427..1e3e39b8f 100644
--- a/src/buildstream/_artifact.py
+++ b/src/buildstream/_artifact.py
@@ -30,6 +30,7 @@ artifact composite interaction away from Element class
 
 import os
 
+from ._exceptions import ArtifactError
 from ._protos.buildstream.v2.artifact_pb2 import Artifact as ArtifactProto
 from . import _yaml
 from . import utils
@@ -349,6 +350,49 @@ class Artifact():
 
         return self._metadata_workspaced_dependencies
 
+    # get_dependency_refs()
+    #
+    # Retrieve the artifact refs of the artifact's dependencies
+    #
+    # Args:
+    #    deps (Scope): The scope of dependencies
+    #
+    # Returns:
+    #    (list [str]): A list of refs of all build dependencies in staging order.
+    #
+    def get_dependency_refs(self, deps=Scope.BUILD):
+        # XXX: The pylint disable is necessary due to upstream issue:
+        # https://github.com/PyCQA/pylint/issues/850
+        from .element import _get_normal_name  # pylint: disable=cyclic-import
+
+        # Extract the proto
+        artifact = self._get_proto()
+
+        if deps == Scope.BUILD:
+            try:
+                dependency_refs = [
+                    os.path.join(dep.project_name, _get_normal_name(dep.element_name), dep.cache_key)
+                    for dep in artifact.build_deps
+                ]
+            except AttributeError:
+                # If the artifact has no dependencies
+                dependency_refs = []
+        elif deps == Scope.NONE:
+            dependency_refs = [self._element.get_artifact_name()]
+        else:
+            # XXX: We can only support obtaining the build dependencies of
+            #      an artifact. This is because this is the only information we store
+            #      in the proto. If we were to add runtime deps to the proto, we'd need
+            #      to include these in cache key calculation.
+            #
+            #      This would have some undesirable side effects:
+            #          1. It might trigger unnecessary rebuilds.
+            #          2. It would be impossible to support cyclic runtime dependencies
+            #             in the future
+            raise ArtifactError("Dependency scope: {} is not supported for artifacts".format(deps))
+
+        return dependency_refs
+
     # cached():
     #
     # Check whether the artifact corresponding to the stored cache key is
diff --git a/src/buildstream/_artifactelement.py b/src/buildstream/_artifactelement.py
index d65d46173..cfd3c29c8 100644
--- a/src/buildstream/_artifactelement.py
+++ b/src/buildstream/_artifactelement.py
@@ -20,6 +20,7 @@ from . import Element
 from . import _cachekey
 from ._exceptions import ArtifactElementError
 from ._loader.metaelement import MetaElement
+from .types import Scope
 
 
 # ArtifactElement()
@@ -31,6 +32,9 @@ from ._loader.metaelement import MetaElement
 #    ref (str): The artifact ref
 #
 class ArtifactElement(Element):
+
+    __instantiated_artifacts = {}  # A hash of ArtifactElement by ref
+
     def __init__(self, context, ref):
         _, element, key = verify_artifact_ref(ref)
 
@@ -43,6 +47,52 @@ class ArtifactElement(Element):
 
         super().__init__(context, project, meta, plugin_conf)
 
+    # _new_from_artifact_ref():
+    #
+    # Recursively instantiate a new ArtifactElement instance, and its
+    # dependencies from an artifact ref
+    #
+    # Args:
+    #    ref (String): The artifact ref
+    #    context (Context): The Context object
+    #    task (Task): A task object to report progress to
+    #
+    # Returns:
+    #    (ArtifactElement): A newly created Element instance
+    #
+    @classmethod
+    def _new_from_artifact_ref(cls, ref, context, task=None):
+
+        if ref in cls.__instantiated_artifacts:
+            return cls.__instantiated_artifacts[ref]
+
+        artifact_element = ArtifactElement(context, ref)
+        # XXX: We need to call update state as it is responsible for
+        # initialising an Element/ArtifactElement's Artifact (__artifact)
+        artifact_element._update_state()
+        cls.__instantiated_artifacts[ref] = artifact_element
+
+        for dep_ref in artifact_element.get_dependency_refs(Scope.BUILD):
+            dependency = ArtifactElement._new_from_artifact_ref(dep_ref, context, task)
+            artifact_element._add_build_dependency(dependency)
+
+        return artifact_element
+
+    # _clear_artifact_refs_cache()
+    #
+    # Clear the internal artifact refs cache
+    #
+    # When loading ArtifactElements from artifact refs, we cache already
+    # instantiated ArtifactElements in order to not have to load the same
+    # ArtifactElements twice. This clears the cache.
+    #
+    # It should be called whenever we are done loading all artifacts in order
+    # to save memory.
+    #
+    @classmethod
+    def _clear_artifact_refs_cache(cls):
+        cls.__instantiated_artifacts = {}
+
     # Override Element.get_artifact_name()
     def get_artifact_name(self, key=None):
         return self._ref
@@ -55,6 +105,20 @@ class ArtifactElement(Element):
     def preflight(self):
         pass
 
+    # get_dependency_refs()
+    #
+    # Obtain the refs of a particular scope of dependencies
+    #
+    # Args:
+    #   scope (Scope): The scope of dependencies for which we want to obtain the refs
+    #
+    # Returns:
+    #   (list [str]): A list of artifact refs
+    #
+    def get_dependency_refs(self, scope=Scope.BUILD):
+        artifact = self._get_artifact()
+        return artifact.get_dependency_refs(deps=scope)
+
     # Override Element._calculate_cache_key
     def _calculate_cache_key(self, dependencies=None):
         return self._key
diff --git a/src/buildstream/_frontend/cli.py b/src/buildstream/_frontend/cli.py
index 1fcf85953..5b2e60d49 100644
--- a/src/buildstream/_frontend/cli.py
+++ b/src/buildstream/_frontend/cli.py
@@ -1233,12 +1233,15 @@ def artifact_list_contents(app, artifacts):
 #                     Artifact Delete Command                     #
 ###################################################################
 @artifact.command(name='delete', short_help="Remove artifacts from the local cache")
+@click.option('--deps', '-d', default='none',
+              type=click.Choice(['none', 'run', 'build', 'all']),
+              help="The dependencies to delete (default: none)")
 @click.argument('artifacts', type=click.Path(), nargs=-1)
 @click.pass_obj
-def artifact_delete(app, artifacts):
+def artifact_delete(app, artifacts, deps):
     """Remove artifacts from the local cache"""
     with app.initialized():
-        app.stream.artifact_delete(artifacts)
+        app.stream.artifact_delete(artifacts, selection=deps)
 
 
 ##################################################################
diff --git a/src/buildstream/_pipeline.py b/src/buildstream/_pipeline.py
index b6896b3de..7cf4abbe3 100644
--- a/src/buildstream/_pipeline.py
+++ b/src/buildstream/_pipeline.py
@@ -115,6 +115,21 @@ class Pipeline():
 
             return tuple(element_groups)
 
+    # load_artifacts()
+    #
+    # Loads ArtifactElements from target artifacts.
+    #
+    # Args:
+    #    target (list [str]): Target artifacts to load
+    #
+    # Returns:
+    #    (list [ArtifactElement]): A list of ArtifactElement objects
+    #
+    def load_artifacts(self, targets):
+        # XXX: This is not included as part of the "load-pipeline" profiler, we could move
+        #      the profiler to Stream?
+        return self._project.load_artifacts(targets)
+
     # resolve_elements()
     #
     # Resolve element state and cache keys.
diff --git a/src/buildstream/_project.py b/src/buildstream/_project.py
index d3bbc3939..00beebfab 100644
--- a/src/buildstream/_project.py
+++ b/src/buildstream/_project.py
@@ -463,6 +463,38 @@ class Project():
 
         return elements
 
+    # load_artifacts()
+    #
+    # Loads artifacts from target artifact refs
+    #
+    # Args:
+    #    targets (list): Target artifact refs
+    #
+    # Returns:
+    #    (list): A list of loaded ArtifactElement
+    #
+    def load_artifacts(self, targets):
+        with self._context.messenger.simple_task("Loading artifacts") as task:
+            # XXX: Here, we are explicitly checking for refs in the artifactdir
+            #      for two reasons:
+            #          1. The Project, or the Context, do not currently have
+            #             access to the ArtifactCache
+            #          2. The ArtifactCache.contains() method expects an Element
+            #             and a key, not a ref.
+            #
+            artifactdir = self._context.artifactdir
+            artifacts = []
+            for ref in targets:
+                if not os.path.exists(os.path.join(artifactdir, ref)):
+                    raise LoadError("{}\nis not present in the artifact cache ({})".format(ref, artifactdir),
+                                    LoadErrorReason.MISSING_FILE)
+
+                artifacts.append(ArtifactElement._new_from_artifact_ref(ref, self._context, task))
+
+        ArtifactElement._clear_artifact_refs_cache()
+
+        return artifacts
+
     # ensure_fully_loaded()
     #
     # Ensure project has finished loading. At first initialization, a
diff --git a/src/buildstream/_stream.py b/src/buildstream/_stream.py
index 167127cf2..58d4a0ebd 100644
--- a/src/buildstream/_stream.py
+++ b/src/buildstream/_stream.py
@@ -672,9 +672,10 @@ class Stream():
     # Args:
     #    targets (str): Targets to remove
     #
-    def artifact_delete(self, targets):
+    def artifact_delete(self, targets, *,
+                        selection=PipelineSelection.NONE):
         # Return list of Element and/or ArtifactElement objects
-        target_objects = self.load_selection(targets, selection=PipelineSelection.NONE, load_refs=True)
+        target_objects = self.load_selection(targets, selection=selection, load_refs=True)
 
         # Some of the targets may refer to the same key, so first obtain a
         # set of the refs to be removed.
@@ -1158,9 +1159,12 @@ class Stream():
         # Classify element and artifact strings
         target_elements, target_artifacts = self._classify_artifacts(targets)
 
-        if target_artifacts and not load_refs:
-            detail = '\n'.join(target_artifacts)
-            raise ArtifactElementError("Cannot perform this operation with artifact refs:", detail=detail)
+        if target_artifacts:
+            if not load_refs:
+                detail = '\n'.join(target_artifacts)
+                raise ArtifactElementError("Cannot perform this operation with artifact refs:", detail=detail)
+            if selection in (PipelineSelection.ALL, PipelineSelection.RUN):
+                raise StreamError("Error: '--deps {}' is not supported for artifact refs".format(selection))
 
         # Load rewritable if we have any tracking selection to make
         rewritable = False
@@ -1168,12 +1172,15 @@ class Stream():
             rewritable = True
 
         # Load all target elements
-        elements, except_elements, track_elements, track_except_elements = \
-            self._pipeline.load([target_elements, except_targets, track_targets, track_except_targets],
-                                rewritable=rewritable)
+        loadable = [target_elements, except_targets, track_targets, track_except_targets]
+        if any(loadable):
+            elements, except_elements, track_elements, track_except_elements = \
+                self._pipeline.load(loadable, rewritable=rewritable)
+        else:
+            elements, except_elements, track_elements, track_except_elements = [], [], [], []
 
-        # Obtain the ArtifactElement objects
-        artifacts = [self._project.create_artifact_element(ref) for ref in target_artifacts]
+        # Load all target artifacts
+        artifacts = self._pipeline.load_artifacts(target_artifacts) if target_artifacts else []
 
         # Optionally filter out junction elements
         if ignore_junction_targets:
diff --git a/src/buildstream/element.py b/src/buildstream/element.py
index bd5ca14e7..c44af942b 100644
--- a/src/buildstream/element.py
+++ b/src/buildstream/element.py
@@ -2355,6 +2355,27 @@ class Element(Plugin):
         casbd = self.__artifact.get_files()
         return [f for f in casbd.list_relative_paths()]
 
+    # _get_artifact()
+    #
+    # Return the Element's Artifact object
+    #
+    # Returns:
+    #    (Artifact): The Artifact object of the Element
+    #
+    def _get_artifact(self):
+        assert self.__artifact, "{}: has no Artifact object".format(self.name)
+        return self.__artifact
+
+    # _add_build_dependency()
+    #
+    # Add a build dependency to the Element
+    #
+    # Args:
+    #    (Element): The Element to add as a build dependency
+    #
+    def _add_build_dependency(self, dependency):
+        self.__build_dependencies.append(dependency)
+
     #############################################################
     #                   Private Local Methods                   #
     #############################################################
diff --git a/tests/frontend/artifact.py b/tests/frontend/artifact.py
index fed0b1aaa..f48807ef6 100644
--- a/tests/frontend/artifact.py
+++ b/tests/frontend/artifact.py
@@ -24,7 +24,9 @@
 import os
 import pytest
 
+from buildstream.element import _get_normal_name
 from buildstream.testing import cli  # pylint: disable=unused-import
+from buildstream._exceptions import ErrorDomain
 from tests.testutils import create_artifact_share
 
 
@@ -278,3 +280,92 @@ def test_artifact_delete_pulled_artifact_without_buildtree(cli, tmpdir, datafile
         result = cli.run(project=project, args=['artifact', 'delete', element])
         result.assert_success()
         assert cli.get_element_state(project, element) != 'cached'
+
+
+# Test that we can delete the build deps of an element
+@pytest.mark.datafiles(DATA_DIR)
+def test_artifact_delete_elements_build_deps(cli, tmpdir, datafiles):
+    project = str(datafiles)
+    element = 'target.bst'
+
+    # Build the element and ensure it's cached
+    result = cli.run(project=project, args=['build', element])
+    result.assert_success()
+
+    # Assert element and build deps are cached
+    assert cli.get_element_state(project, element) == 'cached'
+    bdep_states = cli.get_element_states(project, [element], deps='build')
+    for state in bdep_states.values():
+        assert state == 'cached'
+
+    result = cli.run(project=project, args=['artifact', 'delete', '--deps', 'build', element])
+    result.assert_success()
+
+    # Assert that the build deps have been deleted and that the artifact remains cached
+    assert cli.get_element_state(project, element) == 'cached'
+    bdep_states = cli.get_element_states(project, [element], deps='build')
+    for state in bdep_states.values():
+        assert state != 'cached'
+
+
+# Test that we can delete the build deps of an artifact by providing an artifact ref
+@pytest.mark.datafiles(DATA_DIR)
+def test_artifact_delete_artifacts_build_deps(cli, tmpdir, datafiles):
+    project = str(datafiles)
+    element = 'target.bst'
+
+    # Configure a local cache
+    local_cache = os.path.join(str(tmpdir), 'cache')
+    cli.configure({'cachedir': local_cache})
+
+    # First build an element so that we can find its artifact
+    result = cli.run(project=project, args=['build', element])
+    result.assert_success()
+
+    # Obtain the artifact ref
+    cache_key = cli.get_element_key(project, element)
+    artifact = os.path.join('test', os.path.splitext(element)[0], cache_key)
+
+    # Explicitly check that the ARTIFACT exists in the cache
+    assert os.path.exists(os.path.join(local_cache, 'artifacts', 'refs', artifact))
+
+    # get the artifact refs of the build dependencies
+    bdep_refs = []
+    bdep_states = cli.get_element_states(project, [element], deps='build')
+    for bdep in bdep_states.keys():
+        bdep_refs.append(os.path.join('test', _get_normal_name(bdep), cli.get_element_key(project, bdep)))
+
+    # Assert build dependencies are cached
+    for ref in bdep_refs:
+        assert os.path.exists(os.path.join(local_cache, 'artifacts', 'refs', ref))
+
+    # Delete the artifact
+    result = cli.run(project=project, args=['artifact', 'delete', '--deps', 'build', artifact])
+    result.assert_success()
+
+    # Check that the artifact's build deps are no longer in the cache
+    # Assert build dependencies have been deleted and that the artifact remains
+    for ref in bdep_refs:
+        assert not os.path.exists(os.path.join(local_cache, 'artifacts', 'refs', ref))
+    assert os.path.exists(os.path.join(local_cache, 'artifacts', 'refs', artifact))
+
+
+# Test that `--deps all` option fails if an artifact ref is specified
+@pytest.mark.datafiles(DATA_DIR)
+def test_artifact_delete_artifact_with_deps_all_fails(cli, tmpdir, datafiles):
+    project = str(datafiles)
+    element = 'target.bst'
+
+    # First build an element so that we can find its artifact
+    result = cli.run(project=project, args=['build', element])
+    result.assert_success()
+
+    # Obtain the artifact ref
+    cache_key = cli.get_element_key(project, element)
+    artifact = os.path.join('test', os.path.splitext(element)[0], cache_key)
+
+    # Try to delete the artifact with all of its dependencies
+    result = cli.run(project=project, args=['artifact', 'delete', '--deps', 'all', artifact])
+    result.assert_main_error(ErrorDomain.STREAM, None)
+
+    assert "Error: '--deps all' is not supported for artifact refs" in result.stderr
diff --git a/tests/frontend/buildcheckout.py b/tests/frontend/buildcheckout.py
index b83fc2f3d..98b179b9e 100644
--- a/tests/frontend/buildcheckout.py
+++ b/tests/frontend/buildcheckout.py
@@ -354,11 +354,11 @@ def test_build_checkout_invalid_ref(datafiles, cli):
     assert os.path.isdir(builddir)
     assert not os.listdir(builddir)
 
-    checkout_args = ['artifact', 'checkout', '--deps', 'none', '--tar', checkout,
-                     'test/checkout-deps/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa']
-
+    non_existent_artifact = 'test/checkout-deps/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'
+    checkout_args = ['artifact', 'checkout', '--deps', 'none', '--tar', checkout, non_existent_artifact]
     result = cli.run(project=project, args=checkout_args)
-    assert "seems to be invalid. Note that an Element name can also be used." in result.stderr
+
+    assert "{}\nis not present in the artifact cache".format(non_existent_artifact) in result.stderr
 
 
 @pytest.mark.datafiles(DATA_DIR)