Merge branch 'tristan/junction-dep-names-bst-1' into 'bst-1'

Backport junction element shorthand

See merge request BuildStream/buildstream!1502

8 files changed, 238 insertions, 45 deletions

diff --git a/NEWS b/NEWS
index 0f8cd607e..baf32cce9 100644
--- a/NEWS
+++ b/NEWS
@@ -13,6 +13,9 @@ buildstream 1.3.1
     to avoid having to specify the dependency type for every entry in
     'depends'.
 
+  o Elements may now specify cross-junction dependencies as simple strings
+    using the format '{junction-name}:{element-name}'.
+
 =================
 buildstream 1.2.8
 =================
diff --git a/buildstream/_loader/loadelement.py b/buildstream/_loader/loadelement.py
index 4104dfd59..0b38cad65 100644
--- a/buildstream/_loader/loadelement.py
+++ b/buildstream/_loader/loadelement.py
@@ -18,10 +18,10 @@
 #        Tristan Van Berkom <tristan.vanberkom@codethink.co.uk>
 
 # System imports
-from collections import Mapping
+from itertools import count
+
 
 # BuildStream toplevel imports
-from .._exceptions import LoadError, LoadErrorReason
 from .. import _yaml
 
 # Local package imports
@@ -146,42 +146,9 @@ def _extract_depends_from_node(node, *, key=None):
     depends = _yaml.node_get(node, list, key, default_value=[])
     output_deps = []
 
-    for dep in depends:
-        dep_provenance = _yaml.node_get_provenance(node, key=key, indices=[depends.index(dep)])
-
-        if isinstance(dep, str):
-            dependency = Dependency(dep, provenance=dep_provenance, dep_type=default_dep_type)
-
-        elif isinstance(dep, Mapping):
-            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)
-            dependency = Dependency(filename,
-                                    dep_type=dep_type,
-                                    junction=junction,
-                                    provenance=dep_provenance)
-
-        else:
-            index = depends.index(dep)
-            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))
-
+    for index, dep in enumerate(depends):
+        dep_provenance = _yaml.node_get_provenance(node, key=key, indices=[index])
+        dependency = Dependency(dep, dep_provenance, default_dep_type=default_dep_type)
         output_deps.append(dependency)
 
     # Now delete the field, we dont want it anymore
diff --git a/buildstream/_loader/loader.py b/buildstream/_loader/loader.py
index 71b74c506..1605a823b 100644
--- a/buildstream/_loader/loader.py
+++ b/buildstream/_loader/loader.py
@@ -113,7 +113,7 @@ class Loader():
             junction, name, loader = self._parse_name(target, rewritable, ticker,
                                                       fetch_subprojects=fetch_subprojects)
             loader._load_file(name, rewritable, ticker, fetch_subprojects)
-            deps.append(Dependency(name, junction=junction))
+            deps.append(Dependency(target, provenance="[command line]"))
             profile_end(Topics.LOAD_PROJECT, target)
 
         #
diff --git a/buildstream/_loader/types.py b/buildstream/_loader/types.py
index 25b785532..eb6932b0b 100644
--- a/buildstream/_loader/types.py
+++ b/buildstream/_loader/types.py
@@ -17,6 +17,11 @@
 #  Authors:
 #        Tristan Van Berkom <tristan.vanberkom@codethink.co.uk>
 
+from collections.abc import Mapping
+
+from .._exceptions import LoadError, LoadErrorReason
+from .. import _yaml
+
 
 # Symbol():
 #
@@ -56,9 +61,54 @@ class Symbol():
 #                             dependency was declared
 #
 class Dependency():
-    def __init__(self, name,
-                 dep_type=None, junction=None, provenance=None):
-        self.name = name
-        self.dep_type = dep_type
-        self.junction = junction
+    def __init__(self, dep, provenance, default_dep_type=None):
         self.provenance = provenance
+
+        if isinstance(dep, str):
+            self.name = dep
+            self.dep_type = default_dep_type
+            self.junction = None
+
+        elif isinstance(dep, Mapping):
+            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))
+
+            self.name = _yaml.node_get(dep, str, Symbol.FILENAME)
+            self.dep_type = dep_type
+            self.junction = _yaml.node_get(dep, str, Symbol.JUNCTION, default_value=None)
+
+        else:
+            raise LoadError(LoadErrorReason.INVALID_DATA,
+                            "{}: Dependency is not specified as a string or a dictionary".format(provenance))
+
+        # `:` characters are not allowed in filename if a junction was
+        # explicitly specified
+        if self.junction and ':' in self.name:
+            raise LoadError(LoadErrorReason.INVALID_DATA,
+                            "{}: Dependency {} contains `:` in its name. "
+                            "`:` characters are not allowed in filename when "
+                            "junction attribute is specified.".format(self.provenance, self.name))
+
+        # Name of the element should never contain more than one `:` characters
+        if self.name.count(':') > 1:
+            raise LoadError(LoadErrorReason.INVALID_DATA,
+                            "{}: Dependency {} contains multiple `:` in its name. "
+                            "Recursive lookups for cross-junction elements is not "
+                            "allowed.".format(self.provenance, self.name))
+
+        # Attempt to split name if no junction was specified explicitly
+        if not self.junction and self.name.count(':') == 1:
+            self.junction, self.name = self.name.split(':')
diff --git a/buildstream/_versions.py b/buildstream/_versions.py
index 289f6cd06..e0ec4cdec 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 = 14
+BST_FORMAT_VERSION = 15
 
 
 # The base BuildStream artifact version
diff --git a/buildstream/plugins/elements/junction.py b/buildstream/plugins/elements/junction.py
index ee5ed24d5..d2c62fe48 100644
--- a/buildstream/plugins/elements/junction.py
+++ b/buildstream/plugins/elements/junction.py
@@ -109,6 +109,8 @@ Junctions can configure options of the linked project. Options are never
 implicitly inherited across junctions, however, variables can be used to
 explicitly assign the same value to a subproject option.
 
+.. _core_junction_nested:
+
 Nested Junctions
 ----------------
 Junctions can be nested. That is, subprojects are allowed to have junctions on
diff --git a/doc/source/format_declaring.rst b/doc/source/format_declaring.rst
index b86bbfb1c..f69d4a479 100644
--- a/doc/source/format_declaring.rst
+++ b/doc/source/format_declaring.rst
@@ -381,6 +381,41 @@ Attributes:
      The ``junction`` attribute is available since :ref:`format version 1 <project_format_version>`
 
 
+Cross-junction dependencies
+~~~~~~~~~~~~~~~~~~~~~~~~~~~
+As mentioned above, cross-junction dependencies can be specified using the
+``junction`` attribute. They can also be expressed as simple strings as a
+convenience shorthand. You can refer to cross-junction elements using the
+syntax ``{junction-name}:{element-name}``.
+
+For example, the following is logically same as the example above:
+
+.. code:: yaml
+
+   build-depends:
+     - baseproject.bst:foo.bst
+
+Similarly, you can also refer to cross-junction elements via the ``filename``
+attribute, like so:
+
+.. code:: yaml
+
+   depends:
+     - filename: baseproject.bst:foo.bst
+       type: build
+
+.. note::
+
+   BuildStream does not allow recursice lookups for junction elements. If a
+   filename contains more than one ``:`` (colon) character, an error will be
+   raised. See :ref:`nested junctions <core_junction_nested>` for more details
+   on nested junctions.
+
+.. note::
+
+   This shorthand is available since :ref:`format version 15 <project_format_version>`
+
+
 .. _format_dependencies_types:
 
 Dependency types
diff --git a/tests/frontend/buildcheckout.py b/tests/frontend/buildcheckout.py
index 10849b918..fb4ef72fe 100644
--- a/tests/frontend/buildcheckout.py
+++ b/tests/frontend/buildcheckout.py
@@ -558,3 +558,139 @@ def test_build_checkout_cross_junction(datafiles, cli, tmpdir):
 
     filename = os.path.join(checkout, 'etc', 'animal.conf')
     assert os.path.exists(filename)
+
+
+@pytest.mark.datafiles(DATA_DIR)
+def test_build_junction_short_notation(cli, tmpdir, datafiles):
+    project = os.path.join(datafiles.dirname, datafiles.basename)
+    subproject_path = os.path.join(project, 'files', 'sub-project')
+    junction_path = os.path.join(project, 'elements', 'junction.bst')
+    element_path = os.path.join(project, 'elements', 'junction-dep.bst')
+    workspace = os.path.join(cli.directory, 'workspace')
+    checkout = os.path.join(cli.directory, 'checkout')
+
+    # Create a repo to hold the subproject and generate a junction element for it
+    ref = generate_junction(tmpdir, subproject_path, junction_path)
+
+    # Create a stack element to depend on a cross junction element, using
+    # colon (:) as the separator
+    element = {
+        'kind': 'stack',
+        'depends': ['junction.bst:import-etc.bst']
+    }
+    _yaml.dump(element, element_path)
+
+    # Now try to build it, this should automatically result in fetching
+    # the junction itself at load time.
+    result = cli.run(project=project, args=['build', 'junction-dep.bst'])
+    result.assert_success()
+
+    # Assert that it's cached now
+    assert cli.get_element_state(project, 'junction-dep.bst') == 'cached'
+
+    # Now check it out
+    result = cli.run(project=project, args=[
+        'checkout', 'junction-dep.bst', checkout
+    ])
+    result.assert_success()
+
+    # Assert the content of /etc/animal.conf
+    filename = os.path.join(checkout, 'etc', 'animal.conf')
+    assert os.path.exists(filename)
+    with open(filename, 'r') as f:
+        contents = f.read()
+    assert contents == 'animal=Pony\n'
+
+
+@pytest.mark.datafiles(DATA_DIR)
+def test_build_junction_short_notation_filename(cli, tmpdir, datafiles):
+    project = os.path.join(datafiles.dirname, datafiles.basename)
+    subproject_path = os.path.join(project, 'files', 'sub-project')
+    junction_path = os.path.join(project, 'elements', 'junction.bst')
+    element_path = os.path.join(project, 'elements', 'junction-dep.bst')
+    checkout = os.path.join(cli.directory, 'checkout')
+
+    # Create a repo to hold the subproject and generate a junction element for it
+    ref = generate_junction(tmpdir, subproject_path, junction_path)
+
+    # Create a stack element to depend on a cross junction element, using
+    # colon (:) as the separator
+    element = {
+        'kind': 'stack',
+        'depends': [{'filename': 'junction.bst:import-etc.bst'}]
+    }
+    _yaml.dump(element, element_path)
+
+    # Now try to build it, this should automatically result in fetching
+    # the junction itself at load time.
+    result = cli.run(project=project, args=['build', 'junction-dep.bst'])
+    result.assert_success()
+
+    # Assert that it's cached now
+    assert cli.get_element_state(project, 'junction-dep.bst') == 'cached'
+
+    # Now check it out
+    result = cli.run(project=project, args=[
+        'checkout', 'junction-dep.bst', checkout
+    ])
+    result.assert_success()
+
+    # Assert the content of /etc/animal.conf
+    filename = os.path.join(checkout, 'etc', 'animal.conf')
+    assert os.path.exists(filename)
+    with open(filename, 'r') as f:
+        contents = f.read()
+    assert contents == 'animal=Pony\n'
+
+
+@pytest.mark.datafiles(DATA_DIR)
+def test_build_junction_short_notation_with_junction(cli, tmpdir, datafiles):
+    project = os.path.join(datafiles.dirname, datafiles.basename)
+    subproject_path = os.path.join(project, 'files', 'sub-project')
+    junction_path = os.path.join(project, 'elements', 'junction.bst')
+    element_path = os.path.join(project, 'elements', 'junction-dep.bst')
+    checkout = os.path.join(cli.directory, 'checkout')
+
+    # Create a repo to hold the subproject and generate a junction element for it
+    ref = generate_junction(tmpdir, subproject_path, junction_path)
+
+    # Create a stack element to depend on a cross junction element, using
+    # colon (:) as the separator
+    element = {
+        'kind': 'stack',
+        'depends': [{
+            'filename': 'junction.bst:import-etc.bst',
+            'junction': 'junction.bst',
+        }]
+    }
+    _yaml.dump(element, element_path)
+
+    # Now try to build it, this should fail as filenames should not contain
+    # `:` when junction is explicity specified
+    result = cli.run(project=project, args=['build', 'junction-dep.bst'])
+    result.assert_main_error(ErrorDomain.LOAD, LoadErrorReason.INVALID_DATA)
+
+
+@pytest.mark.datafiles(DATA_DIR)
+def test_build_junction_short_notation_with_junction(cli, tmpdir, datafiles):
+    project = os.path.join(datafiles.dirname, datafiles.basename)
+    subproject_path = os.path.join(project, 'files', 'sub-project')
+    junction_path = os.path.join(project, 'elements', 'junction.bst')
+    element_path = os.path.join(project, 'elements', 'junction-dep.bst')
+    checkout = os.path.join(cli.directory, 'checkout')
+
+    # Create a repo to hold the subproject and generate a junction element for it
+    ref = generate_junction(tmpdir, subproject_path, junction_path)
+
+    # Create a stack element to depend on a cross junction element, using
+    # colon (:) as the separator
+    element = {
+        'kind': 'stack',
+        'depends': ['junction.bst:import-etc.bst:foo.bst']
+    }
+    _yaml.dump(element, element_path)
+
+    # Now try to build it, this should fail as recursive lookups for
+    # cross-junction elements is not allowed.
+    result = cli.run(project=project, args=['build', 'junction-dep.bst'])
+    result.assert_main_error(ErrorDomain.LOAD, LoadErrorReason.INVALID_DATA)