Merge branch 'jennis/update_checkout' into 'master'

Update artifact checkout to handle artifact refs

See merge request BuildStream/buildstream!1565

4 files changed, 81 insertions, 39 deletions

diff --git a/src/buildstream/_artifactelement.py b/src/buildstream/_artifactelement.py
index e3bffaf72..48c3d1769 100644
--- a/src/buildstream/_artifactelement.py
+++ b/src/buildstream/_artifactelement.py
@@ -42,6 +42,10 @@ class ArtifactElement(Element):
     # A hash of ArtifactElement by ref
     __instantiated_artifacts = {}   # type: Dict[str, ArtifactElement]
 
+    # ArtifactElement's require this as the sandbox will use a normal
+    # directory when we checkout
+    BST_VIRTUAL_DIRECTORY = True
+
     def __init__(self, context, ref):
         _, element, key = verify_artifact_ref(ref)
 
@@ -126,6 +130,22 @@ class ArtifactElement(Element):
         artifact = self._get_artifact()
         return artifact.get_dependency_refs(deps=scope)
 
+    # configure_sandbox()
+    #
+    # Configure a sandbox for installing artifacts into
+    #
+    # Args:
+    #    sandbox (Sandbox)
+    #
+    def configure_sandbox(self, sandbox):
+        install_root = self.get_variable('install-root')
+
+        # Tell the sandbox to mount the build root and install root
+        sandbox.mark_directory(install_root)
+
+        # Tell sandbox which directory is preserved in the finished artifact
+        sandbox.set_output_directory(install_root)
+
     # 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 132f7f71a..3fad1664d 100644
--- a/src/buildstream/_frontend/cli.py
+++ b/src/buildstream/_frontend/cli.py
@@ -181,7 +181,7 @@ def override_completions(orig_args, cmd, cmd_param, args, incomplete):
                 cmd_param.opts == ['--track'] or
                 cmd_param.opts == ['--track-except']):
             return complete_target(args, incomplete)
-        if cmd_param.name == 'artifacts':
+        if cmd_param.name == 'artifacts' or cmd_param.name == 'target':
             return complete_artifact(orig_args, args, incomplete)
 
     raise CompleteUnhandled()
@@ -1058,10 +1058,10 @@ def artifact_show(app, deps, artifacts):
 @click.option('--directory', default=None,
               type=click.Path(file_okay=False),
               help="The directory to checkout the artifact to")
-@click.argument('element', required=False,
+@click.argument('target', required=False,
                 type=click.Path(readable=False))
 @click.pass_obj
-def artifact_checkout(app, force, deps, integrate, hardlinks, tar, compression, pull_, directory, element):
+def artifact_checkout(app, force, deps, integrate, hardlinks, tar, compression, pull_, directory, target):
     """Checkout contents of an artifact
 
     When this command is executed from a workspace directory, the default
@@ -1083,7 +1083,7 @@ def artifact_checkout(app, force, deps, integrate, hardlinks, tar, compression,
             sys.exit(-1)
         else:
             if directory is None:
-                location = os.path.abspath(os.path.join(os.getcwd(), element))
+                location = os.path.abspath(os.path.join(os.getcwd(), target))
             else:
                 location = directory
             if location[-4:] == '.bst':
@@ -1102,23 +1102,17 @@ def artifact_checkout(app, force, deps, integrate, hardlinks, tar, compression,
         if not compression:
             compression = inferred_compression
 
-    if deps == "build":
-        scope = Scope.BUILD
-    elif deps == "none":
-        scope = Scope.NONE
-    else:
-        scope = Scope.RUN
-
     with app.initialized():
-        if not element:
-            element = app.project.get_default_target()
-            if not element:
+        if not target:
+            target = app.project.get_default_target()
+            if not target:
                 raise AppError('Missing argument "ELEMENT".')
 
-        app.stream.checkout(element,
+        scope = {'run': Scope.RUN, 'build': Scope.BUILD, 'none': Scope.NONE}
+        app.stream.checkout(target,
                             location=location,
                             force=force,
-                            scope=scope,
+                            scope=scope[deps],
                             integrate=True if integrate is None else integrate,
                             hardlinks=hardlinks,
                             pull=pull_,
diff --git a/src/buildstream/_stream.py b/src/buildstream/_stream.py
index 083dc1c43..b3e0a6183 100644
--- a/src/buildstream/_stream.py
+++ b/src/buildstream/_stream.py
@@ -29,7 +29,6 @@ import tempfile
 from contextlib import contextmanager, suppress
 from fnmatch import fnmatch
 
-from ._artifact import Artifact
 from ._artifactelement import verify_artifact_ref, ArtifactElement
 from ._exceptions import StreamError, ImplError, BstError, ArtifactElementError, ArtifactError
 from ._message import Message, MessageType
@@ -541,6 +540,10 @@ class Stream():
         elements, _ = self._load((target,), (), selection=selection, use_artifact_config=True, load_refs=True)
         target = elements[-1]
 
+        # Verify that --deps run has not been specified for an ArtifactElement
+        if isinstance(target, ArtifactElement) and scope == Scope.RUN:
+            raise StreamError("Unable to determine the runtime dependencies of an ArtifactElement")
+
         self._check_location_writable(location, force=force, tar=tar)
 
         uncached_elts = [elt for elt in elements if not elt._cached()]
@@ -551,27 +554,15 @@ class Stream():
             self._enqueue_plan(uncached_elts)
             self._run()
 
-        # Stage deps into a temporary sandbox first
-        if isinstance(target, ArtifactElement):
-            try:
-                key = target._get_cache_key()
-                artifact = Artifact(target, self._context, strong_key=key)
-                virdir = artifact.get_files()
+        try:
+            with target._prepare_sandbox(scope=scope, directory=None,
+                                         integrate=integrate) as sandbox:
+                # Copy or move the sandbox to the target directory
+                virdir = sandbox.get_virtual_directory()
                 self._export_artifact(tar, location, compression, target, hardlinks, virdir)
-            except AttributeError as e:
-                raise ArtifactError("Artifact reference '{}' seems to be invalid. "
-                                    "Note that an Element name can also be used."
-                                    .format(artifact._element.get_artifact_name())) from e
-        else:
-            try:
-                with target._prepare_sandbox(scope=scope, directory=None,
-                                             integrate=integrate) as sandbox:
-                    # Copy or move the sandbox to the target directory
-                    virdir = sandbox.get_virtual_directory()
-                    self._export_artifact(tar, location, compression, target, hardlinks, virdir)
-            except BstError as e:
-                raise StreamError("Error while staging dependencies into a sandbox"
-                                  ": '{}'".format(e), detail=e.detail, reason=e.reason) from e
+        except BstError as e:
+            raise StreamError("Error while staging dependencies into a sandbox"
+                              ": '{}'".format(e), detail=e.detail, reason=e.reason) from e
 
     def _export_artifact(self, tar, location, compression, target, hardlinks, virdir):
         if not tar:
diff --git a/tests/frontend/buildcheckout.py b/tests/frontend/buildcheckout.py
index 6281217b7..c9a42239a 100644
--- a/tests/frontend/buildcheckout.py
+++ b/tests/frontend/buildcheckout.py
@@ -311,7 +311,7 @@ def test_build_checkout_using_ref(datafiles, cli):
     result.assert_success()
 
     key = cli.get_element_key(project, 'checkout-deps.bst')
-    checkout_args = ['artifact', 'checkout', '--directory', checkout, 'test/checkout-deps/' + key]
+    checkout_args = ['artifact', 'checkout', '--directory', checkout, '--deps', 'none', 'test/checkout-deps/' + key]
 
     result = cli.run(project=project, args=checkout_args)
     result.assert_success()
@@ -343,6 +343,43 @@ def test_build_checkout_tarball_using_ref(datafiles, cli):
 
 
 @pytest.mark.datafiles(DATA_DIR)
+def test_build_checkout_build_deps_using_ref(datafiles, cli):
+    project = str(datafiles)
+    checkout = os.path.join(cli.directory, 'checkout')
+
+    result = cli.run(project=project, args=['build', 'checkout-deps.bst'])
+    result.assert_success()
+
+    key = cli.get_element_key(project, 'checkout-deps.bst')
+    checkout_args = ['artifact', 'checkout', '--directory', checkout, '--deps', 'build', 'test/checkout-deps/' + key]
+
+    result = cli.run(project=project, args=checkout_args)
+    result.assert_success()
+
+    build_dep_files = os.path.join(checkout, 'usr', 'include', 'pony.h')
+    runtime_dep_files = os.path.join(checkout, 'usr', 'bin', 'hello')
+    target_files = os.path.join(checkout, 'etc', 'buildstream', 'config')
+    assert os.path.exists(build_dep_files)
+    assert not os.path.exists(runtime_dep_files)
+    assert not os.path.exists(target_files)
+
+
+@pytest.mark.datafiles(DATA_DIR)
+def test_build_checkout_runtime_deps_using_ref_fails(datafiles, cli):
+    project = str(datafiles)
+    checkout = os.path.join(cli.directory, 'checkout')
+
+    result = cli.run(project=project, args=['build', 'checkout-deps.bst'])
+    result.assert_success()
+
+    key = cli.get_element_key(project, 'checkout-deps.bst')
+    checkout_args = ['artifact', 'checkout', '--directory', checkout, '--deps', 'run', 'test/checkout-deps/' + key]
+
+    result = cli.run(project=project, args=checkout_args)
+    result.assert_main_error(ErrorDomain.STREAM, None)
+
+
+@pytest.mark.datafiles(DATA_DIR)
 def test_build_checkout_invalid_ref(datafiles, cli):
     project = str(datafiles)
     checkout = os.path.join(cli.directory, 'checkout.tar')
@@ -358,7 +395,7 @@ def test_build_checkout_invalid_ref(datafiles, cli):
     checkout_args = ['artifact', 'checkout', '--deps', 'none', '--tar', checkout, non_existent_artifact]
     result = cli.run(project=project, args=checkout_args)
 
-    assert "Artifact reference '{}' seems to be invalid".format(non_existent_artifact) in result.stderr
+    assert "Error while staging dependencies into a sandbox: 'No artifacts to stage'" in result.stderr
 
 
 @pytest.mark.datafiles(DATA_DIR)