loader: Allow dependencies to use ":" to refer to junctioned elements

This will allow cross-junction dependencies to be listed as strings on a
single line.

As part of this, some logic around initializing `Dependency()` objects
have been moved out of `_extract_depends_from_node()` method into the
constructor of `Dependency()` class, to keep all related code in one
place.

* _loader/types.py: While initializing `Dependency` objects, attempt to
                    split filenames, only if no `junction` was specified
                    explicitly. If a `junction` was specified, then
                    filenames with `:` in their names will result in an
                    error.

* _loader/loadelement.py: Refactor logic to initialize `Dependency()`
                          objects to move it to the `Dependency()`
                          constructor.

* tests/frontend/buildcheckout.py: Add tests to ensure the above.

* _versions.py: Bump BST_FORMAT_VERSION.

Fixes #809.

4 files changed, 197 insertions, 44 deletions

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/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/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)