Merge branch 'tristan/stack-require-depends-all' into 'master'

Require all stack dependencies to be both build & runtime dependencies

Closes #1075

See merge request BuildStream/buildstream!2113

19 files changed, 200 insertions, 35 deletions

diff --git a/NEWS b/NEWS
index 41046306a..ce93d29f4 100644
--- a/NEWS
+++ b/NEWS
@@ -11,6 +11,9 @@ Format
     BuildElement plugins to also stage dependencies into custom locations in
     the sandbox.
 
+  o BREAKING CHANGE: Stack element dependencies are now hard required to be
+    both build and runtime dependencies.
+
 Core
 ----
 
diff --git a/doc/source/format_declaring.rst b/doc/source/format_declaring.rst
index 380f367e0..1c6a5b3eb 100644
--- a/doc/source/format_declaring.rst
+++ b/doc/source/format_declaring.rst
@@ -450,8 +450,8 @@ Attributes:
 * ``type``
 
   This attribute is used to express the :ref:`dependency type <format_dependencies_types>`.
-  This field is not permitted in :ref:`Build-Depends <format_build_depends>` or
-  :ref:`Runtime-Depends <format_runtime_depends>`.
+  This field is not permitted in the :ref:`build-depends <format_build_depends>` or
+  :ref:`runtime-depends <format_runtime_depends>` lists.
 
 * ``strict``
 
@@ -540,12 +540,18 @@ There are three types which one can specify for a dependency:
   which has ``build`` dependencies will not implicitly depend on that element's
   ``build`` dependencies.
 
+  For convenience, these can be specified under the :ref:`build-depends <format_build_depends>`
+  list.
+
 * ``runtime``
 
   A ``runtime`` dependency type states that the given element's product
   must be present for the depending element to function. An element's
   ``runtime`` dependencies are not available to the element at build time.
 
+  For convenience, these can be specified under the :ref:`runtime-depends <format_runtime_depends>`
+  list.
+
 * ``all``
 
   An ``all`` dependency is the default dependency type. If ``all`` is specified,
diff --git a/doc/source/main_glossary.rst b/doc/source/main_glossary.rst
index aa0c6da1f..f1bf4cfaf 100644
--- a/doc/source/main_glossary.rst
+++ b/doc/source/main_glossary.rst
@@ -18,6 +18,10 @@ Glossary
        Artifacts can be built from :term:`Sources <Source>`, or pulled from a
        :term:`Remote Cache <Remote Cache>`, if available.
 
+   Artifact name
+       The :ref:`name of an artifact <artifact_names>`, which can be used
+       in various commands to operate directly on artifacts, without requiring
+       the use of a :term:`Project`.
 
    Cache
        BuildStream leverages various caching techniques in order to avoid
diff --git a/doc/source/using_commands.rst b/doc/source/using_commands.rst
index 27fe692d3..1162e6c44 100644
--- a/doc/source/using_commands.rst
+++ b/doc/source/using_commands.rst
@@ -77,6 +77,29 @@ Top-level commands
 Artifact subcommands
 --------------------
 
+
+.. _artifact_names:
+
+Artifact names
+~~~~~~~~~~~~~~
+Various artifact subcommands accept either :ref:`element names <format_element_names>`,
+which will operate on artifacts by deriving the artifact from local project state,
+or :term:`artifact names <Artifact name>` interchangeably as targets. Artifact names allow
+the user to operate directly on cached artifacts, without requiring local project data.
+
+An artifact name is composed of the following identifiers:
+
+* The :ref:`project name <project_format_name>`
+
+* The :ref:`element name <format_element_names>`, without any trailing ``.bst`` extension
+
+* The cache key of the element at the time it was built.
+
+To compose an artifact name, simply join these using a forward slash (``/``) character, like so: ``<project-name>/<element-name>/<cache-key>``.
+
+An artifact name might look like: ``project/target/788da21e7c1b5818b7e7b60f7eb75841057ff7e45d362cc223336c606fe47f27``
+
+
 .. _invoking_artifact_checkout:
 
 .. click:: buildstream._frontend.cli:artifact_checkout
diff --git a/src/buildstream/plugins/elements/stack.py b/src/buildstream/plugins/elements/stack.py
index b15f67073..bd914ed0a 100644
--- a/src/buildstream/plugins/elements/stack.py
+++ b/src/buildstream/plugins/elements/stack.py
@@ -1,5 +1,5 @@
 #
-#  Copyright (C) 2016 Codethink Limited
+#  Copyright (C) 2020 Codethink Limited
 #
 #  This program is free software; you can redistribute it and/or
 #  modify it under the terms of the GNU Lesser General Public
@@ -22,9 +22,85 @@ stack - Symbolic Element for dependency grouping
 ================================================
 Stack elements are simply a symbolic element used for representing
 a logical group of elements.
+
+All dependencies declared in stack elements must always be both
+:ref:`build and runtime dependencies <format_dependencies_types>`.
+
+**Example:**
+
+.. code:: yaml
+
+   kind: stack
+
+   # Declare all of your dependencies in the `depends` list.
+   depends:
+   - libc.bst
+   - coreutils.bst
+
+.. note::
+
+   Unlike other elements, whose cache keys are a unique identifier
+   of the contents of the artifacts they produce, stack elements do
+   not produce any artifact content. Instead, the cache key of an artifact
+   is a unique identifier for the assembly of its own dependencies.
+
+
+Using intermediate stacks
+-------------------------
+Using a stack element at intermediate levels of your build graph
+allows you to abstract away some parts of your project into logical
+subsystems which elements can more conveniently depend on as a whole.
+
+In addition to the added convenience, it will allow you to more
+easily change the implementation of a subsystem later on, without needing
+to update many reverse dependencies to depend on new elements, or even
+allow you to conditionally implement a subsystem with various implementations
+depending on what :ref:`project options <project_options>` were specified at
+build time.
+
+
+Using toplevel stacks
+---------------------
+Stack elements can also be useful as toplevel targets in your build graph
+to simply indicate all of the components which need to be built for a given
+system to be complete, or for your integration pipeline to be successful.
+
+
+Checking out and deploying toplevel stacks
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+In case that your software is built remotely, it is possible to checkout
+the built content of a stack on your own machine for the purposes of
+inspection or further deployment.
+
+To accomplish this, you will need to know the cache key of the stack element
+which was built remotely, possibly by inspecting the remote build log or by
+deriving it with an equally configured BuildStream project, and you will
+need read access to the artifact cache server which the build was uploaded to,
+this should be configured in your :ref:`user configuration file <config_artifacts>`.
+
+You can then checkout the remotely built stack using the
+:ref:`bst artifact checkout <invoking_artifact_checkout>` command and providing
+it with the :ref:`artifact name <artifact_names>`:
+
+**Example:**
+
+.. code:: shell
+
+   bst artifact checkout --deps build --pull --integrate \\
+       --directory `pwd`/checkout \\
+       project/stack/788da21e7c1b5818b7e7b60f7eb75841057ff7e45d362cc223336c606fe47f27
+
+.. note::
+
+   It is possible to checkout other elements in the same way, however stack
+   elements are uniquely suited to this purpose, as they cannot have
+   :ref:`runtime only dependencies <format_dependencies_types>`, and consequently
+   their cache keys are always a unique representation of their collective
+   dependencies.
 """
 
-from buildstream import Element
+from buildstream import Element, ElementError
+from buildstream.types import _Scope
 
 
 # Element implementation for the 'stack' kind.
@@ -46,7 +122,20 @@ class StackElement(Element):
         pass
 
     def preflight(self):
-        pass
+
+        # Assert that all dependencies are both build and runtime dependencies.
+        #
+        all_deps = list(self._dependencies(_Scope.ALL, recurse=False))
+        run_deps = list(self._dependencies(_Scope.RUN, recurse=False))
+        build_deps = list(self._dependencies(_Scope.BUILD, recurse=False))
+        if any(dep not in run_deps for dep in all_deps) or any(dep not in build_deps for dep in all_deps):
+            # There is no need to specify the `self` provenance here in preflight() errors, as the base class
+            # will take care of prefixing these for plugin author convenience.
+            raise ElementError(
+                "All dependencies of 'stack' elements must be both build and runtime dependencies",
+                detail="Make sure you declare all dependencies in the `depends` list, without specifying any `type`.",
+                reason="stack-requires-build-and-run",
+            )
 
     def get_unique_key(self):
         # We do not add anything to the build, only our dependencies
diff --git a/tests/format/dependencies1/elements/builddep-list.bst b/tests/format/dependencies1/elements/builddep-list.bst
index a0cbcaf23..eb216a127 100644
--- a/tests/format/dependencies1/elements/builddep-list.bst
+++ b/tests/format/dependencies1/elements/builddep-list.bst
@@ -1,4 +1,4 @@
-kind: stack
+kind: manual
 description: This element has a build-only dependency specified via build-depends
 build-depends:
-  - firstdep.bst
+- firstdep.bst
diff --git a/tests/format/dependencies1/elements/list-combine.bst b/tests/format/dependencies1/elements/list-combine.bst
index ed3452206..d39ddd38b 100644
--- a/tests/format/dependencies1/elements/list-combine.bst
+++ b/tests/format/dependencies1/elements/list-combine.bst
@@ -1,4 +1,4 @@
-kind: stack
+kind: manual
 description: This element depends on three elements in different ways
 build-depends:
 - firstdep.bst
diff --git a/tests/format/dependencies1/elements/runtimedep-list.bst b/tests/format/dependencies1/elements/runtimedep-list.bst
index 1207a492d..eaa0cd2e0 100644
--- a/tests/format/dependencies1/elements/runtimedep-list.bst
+++ b/tests/format/dependencies1/elements/runtimedep-list.bst
@@ -1,4 +1,4 @@
-kind: stack
+kind: manual
 description: This element has a runtime-only dependency
 runtime-depends:
-  - firstdep.bst
+- firstdep.bst
diff --git a/tests/format/stack.py b/tests/format/stack.py
new file mode 100644
index 000000000..b014e9b5d
--- /dev/null
+++ b/tests/format/stack.py
@@ -0,0 +1,22 @@
+# Pylint doesn't play well with fixtures and dependency injection from pytest
+# pylint: disable=redefined-outer-name
+
+import os
+import pytest
+
+from buildstream.exceptions import ErrorDomain
+from buildstream.testing import cli  # pylint: disable=unused-import
+
+DATA_DIR = os.path.join(os.path.dirname(os.path.realpath(__file__)), "stack")
+
+
+#
+# Assert that we have errors when trying to have runtime-only or
+# build-only dependencies.
+#
+@pytest.mark.datafiles(DATA_DIR)
+@pytest.mark.parametrize("target", ["build-only-stack.bst", "runtime-only-stack.bst",])
+def test_require_build_and_run(cli, datafiles, target):
+    project = str(datafiles)
+    result = cli.run(project=project, args=["show", target])
+    result.assert_main_error(ErrorDomain.ELEMENT, "stack-requires-build-and-run")
diff --git a/tests/format/stack/elements/build-only-stack.bst b/tests/format/stack/elements/build-only-stack.bst
new file mode 100644
index 000000000..92f1c80f8
--- /dev/null
+++ b/tests/format/stack/elements/build-only-stack.bst
@@ -0,0 +1,4 @@
+kind: stack
+
+build-depends:
+- dependency.bst
diff --git a/tests/format/stack/elements/dependency.bst b/tests/format/stack/elements/dependency.bst
new file mode 100644
index 000000000..8c231b6cf
--- /dev/null
+++ b/tests/format/stack/elements/dependency.bst
@@ -0,0 +1,2 @@
+kind: manual
+description: This is a dependency
diff --git a/tests/format/stack/elements/runtime-only-stack.bst b/tests/format/stack/elements/runtime-only-stack.bst
new file mode 100644
index 000000000..b5dd9a7ce
--- /dev/null
+++ b/tests/format/stack/elements/runtime-only-stack.bst
@@ -0,0 +1,4 @@
+kind: stack
+
+runtime-depends:
+- dependency.bst
diff --git a/tests/format/stack/project.conf b/tests/format/stack/project.conf
new file mode 100644
index 000000000..9a5f11ee1
--- /dev/null
+++ b/tests/format/stack/project.conf
@@ -0,0 +1,4 @@
+# Basic project
+name: test
+min-version: 2.0
+element-path: elements
diff --git a/tests/frontend/default-target/elements/dummy_stack.bst b/tests/frontend/default-target/elements/dummy_stack.bst
index 5f921667f..3ea51b00f 100644
--- a/tests/frontend/default-target/elements/dummy_stack.bst
+++ b/tests/frontend/default-target/elements/dummy_stack.bst
@@ -1,5 +1,5 @@
 kind: stack
 
-runtime-depends:
+depends:
 - dummy_1.bst
 - dummy_2.bst
diff --git a/tests/frontend/default_target.py b/tests/frontend/default_target.py
index bb7a49592..60578bb8e 100644
--- a/tests/frontend/default_target.py
+++ b/tests/frontend/default_target.py
@@ -13,27 +13,27 @@ from tests.testutils import create_artifact_share
 DATA_DIR = os.path.join(os.path.dirname(os.path.realpath(__file__)), "default-target",)
 
 
-###################################
-#      build/show operations      #
-###################################
-
-
+#
+# When no target is default, then expect all targets to be built
+#
 @pytest.mark.datafiles(DATA_DIR)
-@pytest.mark.parametrize("operation,expected_state", [("show", "buildable"), ("build", "cached")])
-def test_no_default(cli, datafiles, operation, expected_state):
+def test_no_default(cli, datafiles):
     project = str(datafiles)
     all_targets = ["dummy_1.bst", "dummy_2.bst", "dummy_3.bst", "dummy_stack.bst"]
 
-    result = cli.run(project=project, args=[operation])
+    result = cli.run(project=project, args=["build"])
     result.assert_success()
 
     states = cli.get_element_states(project, all_targets)
-    assert all(states[e] == expected_state for e in all_targets)
+    assert all(states[e] == "cached" for e in all_targets)
 
 
+#
+# When the stack is specified as the default target, then
+# expect only it and it's dependencies to be built
+#
 @pytest.mark.datafiles(DATA_DIR)
-@pytest.mark.parametrize("operation,expected_state", [("show", "buildable"), ("build", "cached")])
-def test_default_target(cli, datafiles, operation, expected_state):
+def test_default_target(cli, datafiles):
     project = str(datafiles)
     project_path = os.path.join(project, "project.conf")
 
@@ -49,19 +49,23 @@ def test_default_target(cli, datafiles, operation, expected_state):
     # dummy_stack only depends on dummy_1 and dummy_2, but not dummy_3
     all_targets = ["dummy_1.bst", "dummy_2.bst", "dummy_stack.bst"]
 
-    result = cli.run(project=project, args=[operation])
+    result = cli.run(project=project, args=["build"])
     result.assert_success()
 
     states = cli.get_element_states(project, all_targets)
-    assert all(states[e] == expected_state for e in all_targets)
+    assert all(states[e] == "cached" for e in all_targets)
 
     # assert that dummy_3 isn't included in the output
     assert "dummy_3.bst" not in states
 
 
+#
+# Even when there is a junction, expect that the elements in the
+# subproject referred to by the toplevel project are built when
+# calling `bst build` and no default is specified.
+#
 @pytest.mark.datafiles(DATA_DIR)
-@pytest.mark.parametrize("operation,expected_state", [("show", "buildable"), ("build", "cached")])
-def test_no_default_with_junction(cli, datafiles, operation, expected_state):
+def test_no_default_with_junction(cli, datafiles):
     project = str(datafiles)
     junction_path = os.path.join(project, "elements", "junction.bst")
     target_path = os.path.join(project, "elements", "junction-target.bst")
@@ -71,16 +75,16 @@ def test_no_default_with_junction(cli, datafiles, operation, expected_state):
     _yaml.roundtrip_dump(junction_config, junction_path)
 
     # Then, create a stack element with dependency on cross junction element
-    target_config = {"kind": "stack", "runtime-depends": ["junction.bst:dummy_subproject.bst"]}
+    target_config = {"kind": "stack", "depends": ["junction.bst:dummy_subproject.bst"]}
     _yaml.roundtrip_dump(target_config, target_path)
 
-    # Now try to perform the specified operation.
+    # Now try to perform a build
     # This should automatically fetch the junction at load time.
-    result = cli.run(project=project, args=[operation])
+    result = cli.run(project=project, args=["build"])
     result.assert_success()
 
-    assert cli.get_element_state(project, "junction.bst:dummy_subproject.bst") == expected_state
-    assert cli.get_element_state(project, "junction-target.bst") == expected_state
+    assert cli.get_element_state(project, "junction.bst:dummy_subproject.bst") == "cached"
+    assert cli.get_element_state(project, "junction-target.bst") == "cached"
 
 
 ###################################
diff --git a/tests/frontend/pull.py b/tests/frontend/pull.py
index c1fa76ab0..f873ad96e 100644
--- a/tests/frontend/pull.py
+++ b/tests/frontend/pull.py
@@ -338,7 +338,7 @@ def test_pull_missing_local_blob(cli, tmpdir, datafiles):
     _yaml.roundtrip_dump(input_config, input_file)
 
     depends_name = "depends.bst"
-    depends_config = {"kind": "stack", "depends": [{"filename": input_name, "type": "build"}]}
+    depends_config = {"kind": "stack", "depends": [input_name]}
     depends_file = os.path.join(element_dir, depends_name)
     _yaml.roundtrip_dump(depends_config, depends_file)
 
diff --git a/tests/frontend/strict-depends/elements/non-strict-depends.bst b/tests/frontend/strict-depends/elements/non-strict-depends.bst
index 9ab119bde..e0ed21ed6 100644
--- a/tests/frontend/strict-depends/elements/non-strict-depends.bst
+++ b/tests/frontend/strict-depends/elements/non-strict-depends.bst
@@ -1,4 +1,4 @@
 kind: stack
 
-build-depends:
+depends:
 - base.bst
diff --git a/tests/frontend/strict-depends/elements/strict-depends.bst b/tests/frontend/strict-depends/elements/strict-depends.bst
index 1e4e29414..10f3b9418 100644
--- a/tests/frontend/strict-depends/elements/strict-depends.bst
+++ b/tests/frontend/strict-depends/elements/strict-depends.bst
@@ -1,5 +1,5 @@
 kind: stack
 
-build-depends:
+depends:
 - filename: base.bst
   strict: true
diff --git a/tests/integration/project/elements/compose/test-integration.bst b/tests/integration/project/elements/compose/test-integration.bst
index 2f9faf170..ef47b3b58 100644
--- a/tests/integration/project/elements/compose/test-integration.bst
+++ b/tests/integration/project/elements/compose/test-integration.bst
@@ -1,6 +1,6 @@
 kind: stack
 
-runtime-depends:
+depends:
 - base.bst
 
 public: