Ensure there are no duplicates in Elements.dependencies()chandan/duplicate-format-deps

When we are not recursing, `Element.dependencies()` uses a much more
light weight codepath since it just needs to print the direct
dependencies. However, this simple codepath was not accounting for
duplicates, in case something is both a build time and run time
dependency.

One way this manifested itself was in `bst show --format %{deps}`, but
it would also affect anything that was using this method to iterate on
the dependencies.

Fixes #1308.

3 files changed, 32 insertions, 8 deletions

diff --git a/src/buildstream/element.py b/src/buildstream/element.py
index fa39eba12..404cae5e7 100644
--- a/src/buildstream/element.py
+++ b/src/buildstream/element.py
@@ -83,7 +83,7 @@ from contextlib import contextmanager
 from functools import partial
 from itertools import chain
 import string
-from typing import cast, TYPE_CHECKING, Any, Dict, Iterator, List, Optional
+from typing import cast, TYPE_CHECKING, Any, Dict, Iterator, List, Optional, Set
 
 from pyroaring import BitMap  # pylint: disable=no-name-in-module
 
@@ -113,7 +113,7 @@ from .storage.directory import VirtualDirectoryError
 if TYPE_CHECKING:
     from .node import MappingNode, ScalarNode, SequenceNode
     from .types import SourceRef
-    from typing import Set, Tuple
+    from typing import Tuple
 
     # pylint: disable=cyclic-import
     from .sandbox import Sandbox
@@ -441,10 +441,17 @@ class Element(Plugin):
         # containing element that have been visited for the `Scope.BUILD` case
         # and the second one relating to the `Scope.RUN` case.
         if not recurse:
+            result: Set[Element] = set()
             if scope in (Scope.BUILD, Scope.ALL):
-                yield from self.__build_dependencies
+                for dep in self.__build_dependencies:
+                    if dep not in result:
+                        result.add(dep)
+                        yield dep
             if scope in (Scope.RUN, Scope.ALL):
-                yield from self.__runtime_dependencies
+                for dep in self.__runtime_dependencies:
+                    if dep not in result:
+                        result.add(dep)
+                        yield dep
         else:
 
             def visit(element, scope, visited):
diff --git a/tests/frontend/project/elements/format-deps.bst b/tests/frontend/project/elements/format-deps.bst
new file mode 100644
index 000000000..58a46891b
--- /dev/null
+++ b/tests/frontend/project/elements/format-deps.bst
@@ -0,0 +1,17 @@
+kind: import
+
+description: >
+  It is important that this element has both and build and runtime dependencies.
+  It is also important that it has a dependency that is needed at both build
+  time and runtime.
+
+sources:
+- kind: local
+  path: files/etc-files
+
+depends:
+- import-links.bst
+build-depends:
+- import-dev.bst
+runtime-depends:
+- import-bin.bst
diff --git a/tests/frontend/show.py b/tests/frontend/show.py
index 17931ffe3..4be4b72e9 100644
--- a/tests/frontend/show.py
+++ b/tests/frontend/show.py
@@ -399,14 +399,14 @@ def test_exceed_max_recursion_depth(cli, tmpdir, dependency_depth):
 @pytest.mark.parametrize(
     "dep_kind, expected_deps",
     [
-        ("%{deps}", "[import-dev.bst, import-bin.bst]"),
-        ("%{build-deps}", "[import-dev.bst]"),
-        ("%{runtime-deps}", "[import-bin.bst]"),
+        ("%{deps}", "[import-dev.bst, import-links.bst, import-bin.bst]"),
+        ("%{build-deps}", "[import-dev.bst, import-links.bst]"),
+        ("%{runtime-deps}", "[import-links.bst, import-bin.bst]"),
     ],
 )
 def test_format_deps(cli, datafiles, dep_kind, expected_deps):
     project = str(datafiles)
-    target = "checkout-deps.bst"
+    target = "format-deps.bst"
     result = cli.run(
         project=project, silent=True, args=["show", "--deps", "none", "--format", "%{name}: " + dep_kind, target]
     )