Merge branch '463-make-dependency-type-default-to-build-2' into 'master'

Resolve "Make dependency type default to build"

Closes #463

See merge request BuildStream/buildstream!633

13 files changed, 188 insertions, 25 deletions

diff --git a/NEWS b/NEWS
index 6d503370e..e1f5b9f0f 100644
--- a/NEWS
+++ b/NEWS
@@ -13,6 +13,10 @@ buildstream 1.3.1
     Downgrading after using this feature may result in workspaces
     not functioning correctly
 
+  o Elements may now specify 'build-depends' and 'runtime-depends' fields
+    to avoid having to specify the dependency type for every entry in
+    'depends'.
+
 
 =================
 buildstream 1.1.5
diff --git a/buildstream/_loader/loadelement.py b/buildstream/_loader/loadelement.py
index 065364a87..4104dfd59 100644
--- a/buildstream/_loader/loadelement.py
+++ b/buildstream/_loader/loadelement.py
@@ -71,6 +71,7 @@ class LoadElement():
             'kind', 'depends', 'sources', 'sandbox',
             'variables', 'environment', 'environment-nocache',
             'config', 'public', 'description',
+            'build-depends', 'runtime-depends',
         ])
 
         # Extract the Dependencies
@@ -127,28 +128,46 @@ class LoadElement():
 # Returns:
 #    (list): a list of Dependency objects
 #
-def _extract_depends_from_node(node):
-    depends = _yaml.node_get(node, list, Symbol.DEPENDS, default_value=[])
+def _extract_depends_from_node(node, *, key=None):
+    if key is None:
+        build_depends = _extract_depends_from_node(node, key=Symbol.BUILD_DEPENDS)
+        runtime_depends = _extract_depends_from_node(node, key=Symbol.RUNTIME_DEPENDS)
+        depends = _extract_depends_from_node(node, key=Symbol.DEPENDS)
+        return build_depends + runtime_depends + depends
+    elif key == Symbol.BUILD_DEPENDS:
+        default_dep_type = Symbol.BUILD
+    elif key == Symbol.RUNTIME_DEPENDS:
+        default_dep_type = Symbol.RUNTIME
+    elif key == Symbol.DEPENDS:
+        default_dep_type = None
+    else:
+        assert False, "Unexpected value of key '{}'".format(key)
+
+    depends = _yaml.node_get(node, list, key, default_value=[])
     output_deps = []
 
     for dep in depends:
-        dep_provenance = _yaml.node_get_provenance(node, key=Symbol.DEPENDS, indices=[depends.index(dep)])
+        dep_provenance = _yaml.node_get_provenance(node, key=key, indices=[depends.index(dep)])
 
         if isinstance(dep, str):
-            dependency = Dependency(dep, provenance=dep_provenance)
+            dependency = Dependency(dep, provenance=dep_provenance, dep_type=default_dep_type)
 
         elif isinstance(dep, Mapping):
-            _yaml.node_validate(dep, ['filename', 'type', 'junction'])
-
-            # Make type optional, for this we set it to None
-            dep_type = _yaml.node_get(dep, str, Symbol.TYPE, default_value=None)
-            if dep_type is None or dep_type == Symbol.ALL:
-                dep_type = None
-            elif dep_type not in [Symbol.BUILD, Symbol.RUNTIME]:
-                provenance = _yaml.node_get_provenance(dep, key=Symbol.TYPE)
-                raise LoadError(LoadErrorReason.INVALID_DATA,
-                                "{}: Dependency type '{}' is not 'build', 'runtime' or 'all'"
-                                .format(provenance, dep_type))
+            if default_dep_type:
+                _yaml.node_validate(dep, ['filename', 'junction'])
+                dep_type = default_dep_type
+            else:
+                _yaml.node_validate(dep, ['filename', 'type', 'junction'])
+
+                # Make type optional, for this we set it to None
+                dep_type = _yaml.node_get(dep, str, Symbol.TYPE, default_value=None)
+                if dep_type is None or dep_type == Symbol.ALL:
+                    dep_type = None
+                elif dep_type not in [Symbol.BUILD, Symbol.RUNTIME]:
+                    provenance = _yaml.node_get_provenance(dep, key=Symbol.TYPE)
+                    raise LoadError(LoadErrorReason.INVALID_DATA,
+                                    "{}: Dependency type '{}' is not 'build', 'runtime' or 'all'"
+                                    .format(provenance, dep_type))
 
             filename = _yaml.node_get(dep, str, Symbol.FILENAME)
             junction = _yaml.node_get(dep, str, Symbol.JUNCTION, default_value=None)
@@ -159,13 +178,13 @@ def _extract_depends_from_node(node):
 
         else:
             index = depends.index(dep)
-            p = _yaml.node_get_provenance(node, key=Symbol.DEPENDS, indices=[index])
+            p = _yaml.node_get_provenance(node, key=key, indices=[index])
             raise LoadError(LoadErrorReason.INVALID_DATA,
                             "{}: Dependency is not specified as a string or a dictionary".format(p))
 
         output_deps.append(dependency)
 
-    # Now delete "depends", we dont want it anymore
-    del node[Symbol.DEPENDS]
+    # Now delete the field, we dont want it anymore
+    del node[key]
 
     return output_deps
diff --git a/buildstream/_loader/types.py b/buildstream/_loader/types.py
index 000925a6e..25b785532 100644
--- a/buildstream/_loader/types.py
+++ b/buildstream/_loader/types.py
@@ -26,6 +26,8 @@ class Symbol():
     FILENAME = "filename"
     KIND = "kind"
     DEPENDS = "depends"
+    BUILD_DEPENDS = "build-depends"
+    RUNTIME_DEPENDS = "runtime-depends"
     SOURCES = "sources"
     CONFIG = "config"
     VARIABLES = "variables"
diff --git a/buildstream/_versions.py b/buildstream/_versions.py
index 6d5077a2a..3f32847ae 100644
--- a/buildstream/_versions.py
+++ b/buildstream/_versions.py
@@ -23,7 +23,7 @@
 # This version is bumped whenever enhancements are made
 # to the `project.conf` format or the core element format.
 #
-BST_FORMAT_VERSION = 13
+BST_FORMAT_VERSION = 14
 
 
 # The base BuildStream artifact version
diff --git a/doc/source/format_declaring.rst b/doc/source/format_declaring.rst
index 013e2c82c..38c107c1b 100644
--- a/doc/source/format_declaring.rst
+++ b/doc/source/format_declaring.rst
@@ -98,6 +98,68 @@ relative filename to the elements they depend on here.
 See :ref:`format_dependencies` for more information on the dependency model.
 
 
+.. _format_build_depends:
+
+Build-Depends
+~~~~~~~~~~~~~
+
+.. code:: yaml
+
+   # Specify some build-dependencies
+   build-depends:
+   - element1.bst
+   - element2.bst
+
+Build dependencies between elements can be specified with the ``build-depends`` attribute.
+The above code snippet is equivalent to:
+
+.. code:: yaml
+
+   # Specify some build-dependencies
+   depends:
+   - filename: element1.bst
+     type: build
+   - filename: element2.bst
+     type: build
+
+See :ref:`format_dependencies` for more information on the dependency model.
+
+.. note::
+
+   The ``build-depends`` configuration is available since :ref:`format version 14 <project_format_version>`
+
+
+.. _format_runtime_depends:
+
+Runtime-Depends
+~~~~~~~~~~~~~~~
+
+.. code:: yaml
+
+   # Specify some runtime-dependencies
+   runtime-depends:
+   - element1.bst
+   - element2.bst
+
+Runtime dependencies between elements can be specified with the ``runtime-depends`` attribute.
+The above code snippet is equivalent to:
+
+.. code:: yaml
+
+   # Specify some runtime-dependencies
+   depends:
+   - filename: element1.bst
+     type: runtime
+   - filename: element2.bst
+     type: runtime
+
+See :ref:`format_dependencies` for more information on the dependency model.
+
+.. note::
+
+   The ``runtime-depends`` configuration is available since :ref:`format version 14 <project_format_version>`
+
+
 .. _format_sources:
 
 Sources
@@ -276,8 +338,8 @@ attributes are suitable.
 
 .. note::
 
-   Note the order in which element dependencies are declared in the ``depends``
-   list is not meaningful.
+   Note the order in which element dependencies are declared in the ``depends``,
+   ``build-depends`` and ``runtime-depends`` lists are not meaningful.
 
 Dependency dictionary:
 
@@ -299,6 +361,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>`.
 
 * ``junction``
 
diff --git a/tests/loader/dependencies.py b/tests/loader/dependencies.py
index 4bb13a380..cb750fcb1 100644
--- a/tests/loader/dependencies.py
+++ b/tests/loader/dependencies.py
@@ -3,6 +3,7 @@ import pytest
 
 from buildstream._exceptions import LoadError, LoadErrorReason
 from buildstream._loader import Loader, MetaElement
+from tests.testutils import cli
 from . import make_loader
 
 DATA_DIR = os.path.join(
@@ -27,7 +28,7 @@ def test_two_files(datafiles):
     assert(len(element.dependencies) == 1)
     firstdep = element.dependencies[0]
     assert(isinstance(firstdep, MetaElement))
-    assert(firstdep.kind == 'thefirstdep')
+    assert(firstdep.kind == 'manual')
 
 
 @pytest.mark.datafiles(DATA_DIR)
@@ -47,7 +48,7 @@ def test_shared_dependency(datafiles):
     #
     firstdep = element.dependencies[0]
     assert(isinstance(firstdep, MetaElement))
-    assert(firstdep.kind == 'thefirstdep')
+    assert(firstdep.kind == 'manual')
     assert(len(firstdep.dependencies) == 0)
 
     # The second specified dependency is 'shareddep'
@@ -86,7 +87,7 @@ def test_dependency_dict(datafiles):
     assert(len(element.dependencies) == 1)
     firstdep = element.dependencies[0]
     assert(isinstance(firstdep, MetaElement))
-    assert(firstdep.kind == 'thefirstdep')
+    assert(firstdep.kind == 'manual')
 
 
 @pytest.mark.datafiles(DATA_DIR)
@@ -186,3 +187,49 @@ def test_all_dependency(datafiles):
     assert(isinstance(firstdep, MetaElement))
     firstbuilddep = element.build_dependencies[0]
     assert(firstdep == firstbuilddep)
+
+
+@pytest.mark.datafiles(DATA_DIR)
+def test_list_build_dependency(cli, datafiles):
+    project = str(datafiles)
+
+    # Check that the pipeline includes the build dependency
+    deps = cli.get_pipeline(project, ['elements/builddep-list.bst'], scope="build")
+    assert "elements/firstdep.bst" in deps
+
+
+@pytest.mark.datafiles(DATA_DIR)
+def test_list_runtime_dependency(cli, datafiles):
+    project = str(datafiles)
+
+    # Check that the pipeline includes the runtime dependency
+    deps = cli.get_pipeline(project, ['elements/runtimedep-list.bst'], scope="run")
+    assert "elements/firstdep.bst" in deps
+
+
+@pytest.mark.datafiles(DATA_DIR)
+def test_list_dependencies_combined(cli, datafiles):
+    project = str(datafiles)
+
+    # Check that runtime deps get combined
+    rundeps = cli.get_pipeline(project, ['elements/list-combine.bst'], scope="run")
+    assert "elements/firstdep.bst" not in rundeps
+    assert "elements/seconddep.bst" in rundeps
+    assert "elements/thirddep.bst" in rundeps
+
+    # Check that build deps get combined
+    builddeps = cli.get_pipeline(project, ['elements/list-combine.bst'], scope="build")
+    assert "elements/firstdep.bst" in builddeps
+    assert "elements/seconddep.bst" not in builddeps
+    assert "elements/thirddep.bst" in builddeps
+
+
+@pytest.mark.datafiles(DATA_DIR)
+def test_list_overlap(cli, datafiles):
+    project = str(datafiles)
+
+    # Check that dependencies get merged
+    rundeps = cli.get_pipeline(project, ['elements/list-overlap.bst'], scope="run")
+    assert "elements/firstdep.bst" in rundeps
+    builddeps = cli.get_pipeline(project, ['elements/list-overlap.bst'], scope="build")
+    assert "elements/firstdep.bst" in builddeps
diff --git a/tests/loader/dependencies/elements/builddep-list.bst b/tests/loader/dependencies/elements/builddep-list.bst
new file mode 100644
index 000000000..925de3afd
--- /dev/null
+++ b/tests/loader/dependencies/elements/builddep-list.bst
@@ -0,0 +1,4 @@
+kind: stack
+description: This element has a build-only dependency specified via build-depends
+build-depends:
+  - elements/firstdep.bst
diff --git a/tests/loader/dependencies/elements/firstdep.bst b/tests/loader/dependencies/elements/firstdep.bst
index 9b6a57824..5c9c1c1e6 100644
--- a/tests/loader/dependencies/elements/firstdep.bst
+++ b/tests/loader/dependencies/elements/firstdep.bst
@@ -1,2 +1,2 @@
-kind: thefirstdep
+kind: manual
 description: This is the first dependency
diff --git a/tests/loader/dependencies/elements/list-combine.bst b/tests/loader/dependencies/elements/list-combine.bst
new file mode 100644
index 000000000..2010d7008
--- /dev/null
+++ b/tests/loader/dependencies/elements/list-combine.bst
@@ -0,0 +1,8 @@
+kind: stack
+description: This element depends on three elements in different ways
+build-depends:
+- elements/firstdep.bst
+runtime-depends:
+- elements/seconddep.bst
+depends:
+- elements/thirddep.bst
diff --git a/tests/loader/dependencies/elements/list-overlap.bst b/tests/loader/dependencies/elements/list-overlap.bst
new file mode 100644
index 000000000..1e98a20a5
--- /dev/null
+++ b/tests/loader/dependencies/elements/list-overlap.bst
@@ -0,0 +1,7 @@
+kind: stack
+description: This element depends on two elements in different ways
+build-depends:
+- elements/firstdep.bst
+depends:
+- filename: elements/firstdep.bst
+  type: runtime
diff --git a/tests/loader/dependencies/elements/runtimedep-list.bst b/tests/loader/dependencies/elements/runtimedep-list.bst
new file mode 100644
index 000000000..790fa4d54
--- /dev/null
+++ b/tests/loader/dependencies/elements/runtimedep-list.bst
@@ -0,0 +1,4 @@
+kind: stack
+description: This element has a runtime-only dependency
+runtime-depends:
+  - elements/firstdep.bst
diff --git a/tests/loader/dependencies/elements/seconddep.bst b/tests/loader/dependencies/elements/seconddep.bst
new file mode 100644
index 000000000..93ded4359
--- /dev/null
+++ b/tests/loader/dependencies/elements/seconddep.bst
@@ -0,0 +1,2 @@
+kind: manual
+description: This is the second dependency
diff --git a/tests/loader/dependencies/elements/thirddep.bst b/tests/loader/dependencies/elements/thirddep.bst
new file mode 100644
index 000000000..39b58e57a
--- /dev/null
+++ b/tests/loader/dependencies/elements/thirddep.bst
@@ -0,0 +1,2 @@
+kind: manual
+description: This is the third dependency