junctions: Renaming `target` option to `link`.

The word `target` is confusing as by itself, it implies the opposite
of what the option actually does. Without additional context,
the word `target` seems to indicate "The target junction declaration
in a subproject to overwrite with this junction declaration", when
in fact we are inheriting the configuration rather than squashing it.

After discussing this, it appears the word `target` was selected
as it matches with filesystem terminology, e.g. the target specified
to the `ln` command when creating symbolic or hard links.

As such, this patch renames `target` to `link` instead, as this
should be less confusing, it indicates that we are creating a link
of two junctions; the parameter of the `link` can then be loosely
referred to as a `target`, but at least the word `link` is used.

Changes:

  * junction.py:
    - Rename of `target` to `link`, updating relevant documentation.
    - Enhanced documentation to link to the main dependencies section
      and use a less dated example of depending on an element through
      a junction
    - Updated preflight() method to:
      - Be a little more simplified (less branching)
      - Provide machine readable errors
      - Provide new errors in case other options are provided along
        with the link option, as these will be ignored.

  * _loader/loader.py: Updated to use the new `link` junction members instead of `target`

  * tests/format/junctions.py: Updated tests to:
    - Use new `link` terminology
    - Use machine readable errors instead of asserting strings from the UI
    - Improve coverage of junction misconfigurations, grouping the
      testing code under a single parameterized test

23 files changed, 139 insertions, 93 deletions

diff --git a/src/buildstream/_loader/loader.py b/src/buildstream/_loader/loader.py
index 3cfaf8be6..be1b7c274 100644
--- a/src/buildstream/_loader/loader.py
+++ b/src/buildstream/_loader/loader.py
@@ -260,12 +260,12 @@ class Loader:
 
         # If this junction element points to a sub-sub-project, we need to
         # find loader for that project.
-        if element.target:
+        if element.link:
             subproject_loader = self.get_loader(
-                element.target_junction, rewritable=rewritable, ticker=ticker, level=level, provenance=provenance
+                element.link_junction, rewritable=rewritable, ticker=ticker, level=level, provenance=provenance
             )
             loader = subproject_loader.get_loader(
-                element.target_element, rewritable=rewritable, ticker=ticker, level=level, provenance=provenance
+                element.link_element, rewritable=rewritable, ticker=ticker, level=level, provenance=provenance
             )
             self._loaders[filename] = loader
             return loader
diff --git a/src/buildstream/plugins/elements/junction.py b/src/buildstream/plugins/elements/junction.py
index 86d1de8f8..1a759084c 100644
--- a/src/buildstream/plugins/elements/junction.py
+++ b/src/buildstream/plugins/elements/junction.py
@@ -20,8 +20,9 @@
 """
 junction - Integrate subprojects
 ================================
-This element is a link to another BuildStream project. It allows integration
-of multiple projects into a single pipeline.
+This element connects another BuildStream project, allowing one to
+depend on elements in the *junctioned* project, and integrating
+multiple projects into a single pipeline.
 
 Overview
 --------
@@ -48,12 +49,12 @@ Overview
      # Optionally look in a subpath of the source repository for the project
      path: projects/hello
 
-     # Optionally specify another junction element to serve as a target for
-     # this element. Target should be defined using the syntax
+     # Optionally cause this junction to be a symbolic link to another
+     # junction. The link target should be defined using the syntax
      # ``{junction-name}:{element-name}``.
      #
-     # Note that this option cannot be used in conjunction with sources.
-     target: sub-project.bst:sub-sub-project.bst
+     # This option cannot be used in conjunction with other configurations.
+     link: sub-project.bst:sub-sub-project.bst
 
      # Optionally declare whether elements within the junction project
      # should interact with project remotes (default: False).
@@ -66,19 +67,16 @@ Overview
 
 .. note::
 
-   Junction elements may not specify any dependencies as they are simply
-   links to other projects and are not in the dependency graph on their own.
+   Junction elements may not specify any dependencies as they simply
+   connect to other projects and are not in the dependency graph on their own.
 
-With a junction element in place, local elements can depend on elements in
-the other BuildStream project using the additional ``junction`` attribute in the
-dependency dictionary:
+With a junction element in place, local elements can ref:`depend on elements <format_dependencies>`
+in the other BuildStream project:
 
 .. code:: yaml
 
-   depends:
-   - junction: toolchain.bst
-     filename: gcc.bst
-     type: build
+   build-depends:
+   - junction: toolchain.bst:gcc.bst
 
 While junctions are elements, only a limited set of element operations is
 supported. They can be tracked and fetched like other elements.
@@ -112,7 +110,7 @@ Options
      machine_arch: "%{machine_arch}"
      debug: True
 
-Junctions can configure options of the linked project. Options are never
+Junctions can configure options of the connected project. Options are never
 implicitly inherited across junctions, however, variables can be used to
 explicitly assign the same value to a subproject option.
 
@@ -125,21 +123,24 @@ their own. Nested junctions in different subprojects may point to the same
 project, however, in most use cases the same project should be loaded only once.
 BuildStream uses the junction element name as key to determine which junctions
 to merge. It is recommended that the name of a junction is set to the same as
-the name of the linked project.
+the name of the connected project.
 
 As the junctions may differ in source version and options, BuildStream cannot
 simply use one junction and ignore the others. Due to this, BuildStream requires
 the user to resolve possibly conflicting nested junctions by creating a junction
 with the same name in the top-level project, which then takes precedence.
 
-Targeting other junctions
-~~~~~~~~~~~~~~~~~~~~~~~~~
+Linking other junctions
+~~~~~~~~~~~~~~~~~~~~~~~
 When working with nested junctions, you can also create a junction element that
-targets another junction element in the sub-project. This can be useful if you
-need to ensure that both the top-level project and the sub-project are using
-the same version of the sub-sub-project.
+links to another junction element in a sub-project. Elements that your project depends
+on through a *linked junction* will be the same elements that your sub-project
+also depends on.
 
-This can be done using the ``target`` configuration option. See below for an
+This can be useful if you need to ensure that both the top-level project and the
+sub-project are using the same version of the sub-sub-project.
+
+This can be done using the ``link`` configuration option. See below for an
 example:
 
 .. code:: yaml
@@ -147,14 +148,11 @@ example:
    kind: junction
 
    config:
-     target: subproject.bst:subsubproject.bst
+     link: subproject.bst:subsubproject.bst
 
-In the above example, this junction element would be targeting the junction
+In the above example, this junction element becomes a link to the junction
 element named ``subsubproject.bst`` in the subproject referred to by
 ``subproject.bst``.
-
-Note that when targeting another junction, the names of the junction element
-must not be the same as the name of the target.
 """
 
 from buildstream import Element, ElementError
@@ -173,39 +171,64 @@ class JunctionElement(Element):
 
     def configure(self, node):
 
-        node.validate_keys(["path", "options", "target", "cache-junction-elements", "ignore-junction-remotes"])
+        node.validate_keys(["path", "options", "link", "cache-junction-elements", "ignore-junction-remotes"])
 
         self.path = node.get_str("path", default="")
         self.options = node.get_mapping("options", default={})
-        self.target = node.get_str("target", default=None)
-        self.target_element = None
-        self.target_junction = None
+        self.link = node.get_str("link", default=None)
+        self.link_element = None
+        self.link_junction = None
         self.cache_junction_elements = node.get_bool("cache-junction-elements", default=False)
         self.ignore_junction_remotes = node.get_bool("ignore-junction-remotes", default=False)
 
+        # Check if these parameters were specified for error reporting purposes
+        self.cache_junction_elements_specified = bool("cache-junction-elements" in node)
+        self.ignore_junction_remotes_specified = bool("ignore-junction-remotes" in node)
+
     def preflight(self):
-        # "target" cannot be used in conjunction with:
-        # 1. sources
-        # 2. config['options']
-        # 3. config['path']
-        if self.target and any(self.sources()):
-            raise ElementError("junction elements cannot define both 'sources' and 'target' config option")
-        if self.target and any(self.options.items()):
-            raise ElementError("junction elements cannot define both 'options' and 'target'")
-        if self.target and self.path:
-            raise ElementError("junction elements cannot define both 'path' and 'target'")
-
-        # Validate format of target, if defined
-        if self.target:
+        # The "link" option cannot be used in conjunction with any
+        # other defining options, because the junction becomes a symbolic
+        # link to it's link target.
+        if self.link:
+
+            if any(self.sources()):
+                raise ElementError(
+                    "junction elements cannot define both 'sources' and 'link' config option",
+                    reason="invalid-link-sources",
+                )
+            if any(self.options.items()):
+                raise ElementError(
+                    "junction elements cannot define both 'options' and 'link'", reason="invalid-link-options"
+                )
+            if self.path:
+                raise ElementError(
+                    "junction elements cannot define both 'path' and 'link'", reason="invalid-link-path"
+                )
+            if self.cache_junction_elements_specified:
+                raise ElementError(
+                    "junction elements cannot define both 'cache-junction-elements' and 'link'",
+                    reason="invalid-link-cache-junction-elements",
+                )
+            if self.ignore_junction_remotes_specified:
+                raise ElementError(
+                    "junction elements cannot define both 'ignore-junction-remotes' and 'link'",
+                    reason="invalid-link-ignore-junction-remotes",
+                )
+
+            # Validate format of the link target
             try:
-                self.target_junction, self.target_element = self.target.split(":")
+                self.link_junction, self.link_element = self.link.split(":")
             except ValueError:
-                raise ElementError("'target' option must be in format '{junction-name}:{element-name}'")
+                raise ElementError(
+                    "'link' option must be in format '{junction-name}:{element-name}'", reason="invalid-link-name"
+                )
 
-        # We cannot target a junction that has the same name as us, since that
+        # We cannot link to a junction that has the same name as us, since that
         # will cause an infinite recursion while trying to load it.
-        if self.name == self.target_element:
-            raise ElementError("junction elements cannot target an element with the same name")
+        if self.name == self.link_element:
+            raise ElementError(
+                "junction elements cannot link an element with the same name", reason="invalid-link-same-name"
+            )
 
     def get_unique_key(self):
         # Junctions do not produce artifacts. get_unique_key() implementation
diff --git a/tests/format/junctions.py b/tests/format/junctions.py
index 70572ee3e..1e6187740 100644
--- a/tests/format/junctions.py
+++ b/tests/format/junctions.py
@@ -411,8 +411,8 @@ def test_build_git_cross_junction_names(cli, tmpdir, datafiles):
 
 
 @pytest.mark.datafiles(DATA_DIR)
-def test_config_target(cli, tmpdir, datafiles):
-    project = os.path.join(str(datafiles), "config-target")
+def test_config_link(cli, tmpdir, datafiles):
+    project = os.path.join(str(datafiles), "config-link")
     checkoutdir = os.path.join(str(tmpdir), "checkout")
 
     # Build, checkout
@@ -426,18 +426,8 @@ def test_config_target(cli, tmpdir, datafiles):
 
 
 @pytest.mark.datafiles(DATA_DIR)
-def test_invalid_sources_and_target(cli, tmpdir, datafiles):
-    project = os.path.join(str(datafiles), "config-target")
-
-    result = cli.run(project=project, args=["show", "invalid-source-target.bst"])
-    result.assert_main_error(ErrorDomain.ELEMENT, None)
-
-    assert "junction elements cannot define both 'sources' and 'target' config option" in result.stderr
-
-
-@pytest.mark.datafiles(DATA_DIR)
-def test_invalid_target_name(cli, tmpdir, datafiles):
-    project = os.path.join(str(datafiles), "config-target")
+def test_invalid_link_same_name(cli, tmpdir, datafiles):
+    project = os.path.join(str(datafiles), "config-link")
 
     # Rename our junction element to the same name as its target
     old_path = os.path.join(project, "elements/subsubproject.bst")
@@ -446,22 +436,30 @@ def test_invalid_target_name(cli, tmpdir, datafiles):
 
     # This should fail now
     result = cli.run(project=project, args=["show", "subsubproject-junction.bst"])
-    result.assert_main_error(ErrorDomain.ELEMENT, None)
-
-    assert "junction elements cannot target an element with the same name" in result.stderr
-
-
-# We cannot exhaustively test all possible ways in which this can go wrong, so
-# test a couple of common ways in which we expect this to go wrong.
-@pytest.mark.parametrize("target", ["no-junction.bst", "nested-junction-target.bst"])
+    result.assert_main_error(ErrorDomain.ELEMENT, "invalid-link-same-name")
+
+
+@pytest.mark.parametrize(
+    "target,error",
+    [
+        ("no-junction.bst", "invalid-link-name"),
+        ("nested-junction-link.bst", "invalid-link-name"),
+        ("invalid-source-link.bst", "invalid-link-sources"),
+        ("invalid-path-link.bst", "invalid-link-path"),
+        ("invalid-cache-elements-link.bst", "invalid-link-cache-junction-elements"),
+        ("invalid-ignore-remotes-link.bst", "invalid-link-ignore-junction-remotes"),
+        # Also test if these parameters were explicitly set to their default
+        # value, ensure that an error is still issued since these would be ignored.
+        ("invalid-cache-elements-link-false.bst", "invalid-link-cache-junction-elements"),
+        ("invalid-ignore-remotes-link-false.bst", "invalid-link-ignore-junction-remotes"),
+    ],
+)
 @pytest.mark.datafiles(DATA_DIR)
-def test_invalid_target_format(cli, tmpdir, datafiles, target):
-    project = os.path.join(str(datafiles), "config-target")
+def test_invalid_link_params(cli, tmpdir, datafiles, target, error):
+    project = os.path.join(str(datafiles), "config-link")
 
     result = cli.run(project=project, args=["show", target])
-    result.assert_main_error(ErrorDomain.ELEMENT, None)
-
-    assert "'target' option must be in format '{junction-name}:{element-name}'" in result.stderr
+    result.assert_main_error(ErrorDomain.ELEMENT, error)
 
 
 @pytest.mark.datafiles(DATA_DIR)
diff --git a/tests/format/junctions/config-link/elements/invalid-cache-elements-link-false.bst b/tests/format/junctions/config-link/elements/invalid-cache-elements-link-false.bst
new file mode 100644
index 000000000..a0cd37916
--- /dev/null
+++ b/tests/format/junctions/config-link/elements/invalid-cache-elements-link-false.bst
@@ -0,0 +1,5 @@
+kind: junction
+
+config:
+  link: subproject.bst:subsubproject-junction.bst
+  cache-junction-elements: False
diff --git a/tests/format/junctions/config-link/elements/invalid-cache-elements-link.bst b/tests/format/junctions/config-link/elements/invalid-cache-elements-link.bst
new file mode 100644
index 000000000..b2241eeb9
--- /dev/null
+++ b/tests/format/junctions/config-link/elements/invalid-cache-elements-link.bst
@@ -0,0 +1,5 @@
+kind: junction
+
+config:
+  link: subproject.bst:subsubproject-junction.bst
+  cache-junction-elements: True
diff --git a/tests/format/junctions/config-link/elements/invalid-ignore-remotes-link-false.bst b/tests/format/junctions/config-link/elements/invalid-ignore-remotes-link-false.bst
new file mode 100644
index 000000000..1f974794e
--- /dev/null
+++ b/tests/format/junctions/config-link/elements/invalid-ignore-remotes-link-false.bst
@@ -0,0 +1,5 @@
+kind: junction
+
+config:
+  link: subproject.bst:subsubproject-junction.bst
+  ignore-junction-remotes: False
diff --git a/tests/format/junctions/config-link/elements/invalid-ignore-remotes-link.bst b/tests/format/junctions/config-link/elements/invalid-ignore-remotes-link.bst
new file mode 100644
index 000000000..513a3e9f6
--- /dev/null
+++ b/tests/format/junctions/config-link/elements/invalid-ignore-remotes-link.bst
@@ -0,0 +1,5 @@
+kind: junction
+
+config:
+  link: subproject.bst:subsubproject-junction.bst
+  ignore-junction-remotes: True
diff --git a/tests/format/junctions/config-link/elements/invalid-path-link.bst b/tests/format/junctions/config-link/elements/invalid-path-link.bst
new file mode 100644
index 000000000..f8dd33e77
--- /dev/null
+++ b/tests/format/junctions/config-link/elements/invalid-path-link.bst
@@ -0,0 +1,5 @@
+kind: junction
+
+config:
+  link: subproject.bst:subsubproject-junction.bst
+  path: subdir
diff --git a/tests/format/junctions/config-target/elements/invalid-source-target.bst b/tests/format/junctions/config-link/elements/invalid-source-link.bst
index b97d09034..55176ecd4 100644
--- a/tests/format/junctions/config-target/elements/invalid-source-target.bst
+++ b/tests/format/junctions/config-link/elements/invalid-source-link.bst
@@ -5,4 +5,4 @@ sources:
   path: subproject/subsubproject
 
 config:
-  target: subproject.bst:subsubproject-junction.bst
+  link: subproject.bst:subsubproject-junction.bst
diff --git a/tests/format/junctions/config-link/elements/nested-junction-link.bst b/tests/format/junctions/config-link/elements/nested-junction-link.bst
new file mode 100644
index 000000000..c1e618bd8
--- /dev/null
+++ b/tests/format/junctions/config-link/elements/nested-junction-link.bst
@@ -0,0 +1,4 @@
+kind: junction
+
+config:
+  link: subproject.bst:subsubproject.bst:hello.bst
diff --git a/tests/format/junctions/config-link/elements/no-junction.bst b/tests/format/junctions/config-link/elements/no-junction.bst
new file mode 100644
index 000000000..4f01509dc
--- /dev/null
+++ b/tests/format/junctions/config-link/elements/no-junction.bst
@@ -0,0 +1,4 @@
+kind: junction
+
+config:
+  link: subproject.bst
diff --git a/tests/format/junctions/config-target/elements/subproject.bst b/tests/format/junctions/config-link/elements/subproject.bst
index 6664eeec6..6664eeec6 100644
--- a/tests/format/junctions/config-target/elements/subproject.bst
+++ b/tests/format/junctions/config-link/elements/subproject.bst
diff --git a/tests/format/junctions/config-link/elements/subsubproject.bst b/tests/format/junctions/config-link/elements/subsubproject.bst
new file mode 100644
index 000000000..487101253
--- /dev/null
+++ b/tests/format/junctions/config-link/elements/subsubproject.bst
@@ -0,0 +1,4 @@
+kind: junction
+
+config:
+  link: subproject.bst:subsubproject-junction.bst
diff --git a/tests/format/junctions/config-target/elements/target.bst b/tests/format/junctions/config-link/elements/target.bst
index 50d74489a..50d74489a 100644
--- a/tests/format/junctions/config-target/elements/target.bst
+++ b/tests/format/junctions/config-link/elements/target.bst
diff --git a/tests/format/junctions/config-target/project.conf b/tests/format/junctions/config-link/project.conf
index d9e1d7a4f..d9e1d7a4f 100644
--- a/tests/format/junctions/config-target/project.conf
+++ b/tests/format/junctions/config-link/project.conf
diff --git a/tests/format/junctions/config-target/subproject/elements/subsubproject-junction.bst b/tests/format/junctions/config-link/subproject/elements/subsubproject-junction.bst
index 018fb8ec4..018fb8ec4 100644
--- a/tests/format/junctions/config-target/subproject/elements/subsubproject-junction.bst
+++ b/tests/format/junctions/config-link/subproject/elements/subsubproject-junction.bst
diff --git a/tests/format/junctions/config-target/subproject/project.conf b/tests/format/junctions/config-link/subproject/project.conf
index 1529ece04..1529ece04 100644
--- a/tests/format/junctions/config-target/subproject/project.conf
+++ b/tests/format/junctions/config-link/subproject/project.conf
diff --git a/tests/format/junctions/config-target/subproject/subsubproject/elements/hello.bst b/tests/format/junctions/config-link/subproject/subsubproject/elements/hello.bst
index a04a856cd..a04a856cd 100644
--- a/tests/format/junctions/config-target/subproject/subsubproject/elements/hello.bst
+++ b/tests/format/junctions/config-link/subproject/subsubproject/elements/hello.bst
diff --git a/tests/format/junctions/config-target/subproject/subsubproject/files/hello.txt b/tests/format/junctions/config-link/subproject/subsubproject/files/hello.txt
index ce0136250..ce0136250 100644
--- a/tests/format/junctions/config-target/subproject/subsubproject/files/hello.txt
+++ b/tests/format/junctions/config-link/subproject/subsubproject/files/hello.txt
diff --git a/tests/format/junctions/config-target/subproject/subsubproject/project.conf b/tests/format/junctions/config-link/subproject/subsubproject/project.conf
index 162143c80..162143c80 100644
--- a/tests/format/junctions/config-target/subproject/subsubproject/project.conf
+++ b/tests/format/junctions/config-link/subproject/subsubproject/project.conf
diff --git a/tests/format/junctions/config-target/elements/nested-junction-target.bst b/tests/format/junctions/config-target/elements/nested-junction-target.bst
deleted file mode 100644
index f76a264e5..000000000
--- a/tests/format/junctions/config-target/elements/nested-junction-target.bst
+++ /dev/null
@@ -1,4 +0,0 @@
-kind: junction
-
-config:
-  target: subproject.bst:subsubproject.bst:hello.bst
diff --git a/tests/format/junctions/config-target/elements/no-junction.bst b/tests/format/junctions/config-target/elements/no-junction.bst
deleted file mode 100644
index 15d1842f6..000000000
--- a/tests/format/junctions/config-target/elements/no-junction.bst
+++ /dev/null
@@ -1,4 +0,0 @@
-kind: junction
-
-config:
-  target: subproject.bst
diff --git a/tests/format/junctions/config-target/elements/subsubproject.bst b/tests/format/junctions/config-target/elements/subsubproject.bst
deleted file mode 100644
index 20dc4a0c4..000000000
--- a/tests/format/junctions/config-target/elements/subsubproject.bst
+++ /dev/null
@@ -1,4 +0,0 @@
-kind: junction
-
-config:
-  target: subproject.bst:subsubproject-junction.bst