Merge branch 'danielsilverstone-ct/load-reject-duplicates' into 'master'

Reject elements which have duplicated dependencies

Closes #1077

See merge request BuildStream/buildstream!1528

11 files changed, 121 insertions, 4 deletions

diff --git a/src/buildstream/_exceptions.py b/src/buildstream/_exceptions.py
index d5b87a85e..648742dbb 100644
--- a/src/buildstream/_exceptions.py
+++ b/src/buildstream/_exceptions.py
@@ -225,6 +225,9 @@ class LoadErrorReason(Enum):
     # An attempt so set the value of a protected variable
     PROTECTED_VARIABLE_REDEFINED = 23
 
+    # A duplicate dependency was detected
+    DUPLICATE_DEPENDENCY = 24
+
 
 # LoadError
 #
diff --git a/src/buildstream/_loader/types.pyx b/src/buildstream/_loader/types.pyx
index db5004f20..0a223f9a9 100644
--- a/src/buildstream/_loader/types.pyx
+++ b/src/buildstream/_loader/types.pyx
@@ -133,13 +133,31 @@ cdef class Dependency:
 #    key (str): the key on the Node corresponding to the dependency type
 #    default_dep_type (str): type to give to the dependency
 #    acc (list): a list in which to add the loaded dependencies
+#    rundeps (dict): a dictionary mapping dependency (junction, name) to dependency for runtime deps
+#    builddeps (dict): a dictionary mapping dependency (junction, name) to dependency for build deps
 #
-cdef void _extract_depends_from_node(Node node, str key, str default_dep_type, list acc) except *:
+cdef void _extract_depends_from_node(Node node, str key, str default_dep_type, list acc, dict rundeps, dict builddeps) except *:
     cdef SequenceNode depends = node.get_sequence(key, [])
     cdef Node dep_node
+    cdef tuple deptup
 
     for dep_node in depends:
         dependency = Dependency(dep_node, default_dep_type=default_dep_type)
+        deptup = (dependency.junction, dependency.name)
+        if dependency.dep_type in [Symbol.BUILD, None]:
+            if deptup in builddeps:
+                raise LoadError("{}: Duplicate build dependency found at {}."
+                                .format(dependency.provenance, builddeps[deptup].provenance),
+                                LoadErrorReason.DUPLICATE_DEPENDENCY)
+            else:
+                builddeps[deptup] = dependency
+        if dependency.dep_type in [Symbol.RUNTIME, None]:
+            if deptup in rundeps:
+                raise LoadError("{}: Duplicate runtime dependency found at {}."
+                                .format(dependency.provenance, rundeps[deptup].provenance),
+                                LoadErrorReason.DUPLICATE_DEPENDENCY)
+            else:
+                rundeps[deptup] = dependency
         acc.append(dependency)
 
     # Now delete the field, we dont want it anymore
@@ -162,7 +180,9 @@ cdef void _extract_depends_from_node(Node node, str key, str default_dep_type, l
 #
 def extract_depends_from_node(Node node):
     cdef list acc = []
-    _extract_depends_from_node(node, <str> Symbol.BUILD_DEPENDS, <str> Symbol.BUILD, acc)
-    _extract_depends_from_node(node, <str> Symbol.RUNTIME_DEPENDS, <str> Symbol.RUNTIME, acc)
-    _extract_depends_from_node(node, <str> Symbol.DEPENDS, None, acc)
+    cdef dict rundeps = {}
+    cdef dict builddeps = {}
+    _extract_depends_from_node(node, <str> Symbol.BUILD_DEPENDS, <str> Symbol.BUILD, acc, rundeps, builddeps)
+    _extract_depends_from_node(node, <str> Symbol.RUNTIME_DEPENDS, <str> Symbol.RUNTIME, acc, rundeps, builddeps)
+    _extract_depends_from_node(node, <str> Symbol.DEPENDS, None, acc, rundeps, builddeps)
     return acc
diff --git a/tests/format/dependencies.py b/tests/format/dependencies.py
index 15b6e1f4a..e8c50d3bc 100644
--- a/tests/format/dependencies.py
+++ b/tests/format/dependencies.py
@@ -218,3 +218,26 @@ def test_no_recurse(cli, datafiles):
         'dep-two.bst',
         'target.bst',
     ]
+
+
+@pytest.mark.datafiles(DATA_DIR)
+@pytest.mark.parametrize(("element", "asserts"), [
+    ('build-runtime', False),
+    ('build-build', True),
+    ('build-all', True),
+    ('runtime-runtime', True),
+    ('runtime-all', True),
+    ('all-all', True),
+])
+def test_duplicate_deps(cli, datafiles, element, asserts):
+    project = os.path.join(str(datafiles), 'dependencies3')
+
+    result = cli.run(project=project, args=['show', '{}.bst'.format(element)])
+
+    if asserts:
+        result.assert_main_error(ErrorDomain.LOAD,
+                                 LoadErrorReason.DUPLICATE_DEPENDENCY)
+        assert '[line 10 column 2]' in result.stderr
+        assert '[line 8 column 2]' in result.stderr
+    else:
+        result.assert_success()
diff --git a/tests/format/dependencies3/all-all.bst b/tests/format/dependencies3/all-all.bst
new file mode 100644
index 000000000..98122472d
--- /dev/null
+++ b/tests/format/dependencies3/all-all.bst
@@ -0,0 +1,11 @@
+kind: import
+
+sources:
+- kind: local
+  path: all-all.bst
+
+depends:
+- filename: dep.bst
+  type: all
+- filename: dep.bst
+  type: all
diff --git a/tests/format/dependencies3/build-all.bst b/tests/format/dependencies3/build-all.bst
new file mode 100644
index 000000000..4c66524e7
--- /dev/null
+++ b/tests/format/dependencies3/build-all.bst
@@ -0,0 +1,11 @@
+kind: import
+
+sources:
+- kind: local
+  path: all-all.bst
+
+depends:
+- filename: dep.bst
+  type: build
+- filename: dep.bst
+  type: all
diff --git a/tests/format/dependencies3/build-build.bst b/tests/format/dependencies3/build-build.bst
new file mode 100644
index 000000000..2a813b3ab
--- /dev/null
+++ b/tests/format/dependencies3/build-build.bst
@@ -0,0 +1,11 @@
+kind: import
+
+sources:
+- kind: local
+  path: all-all.bst
+
+depends:
+- filename: dep.bst
+  type: build
+- filename: dep.bst
+  type: build
diff --git a/tests/format/dependencies3/build-runtime.bst b/tests/format/dependencies3/build-runtime.bst
new file mode 100644
index 000000000..f740736d8
--- /dev/null
+++ b/tests/format/dependencies3/build-runtime.bst
@@ -0,0 +1,11 @@
+kind: import
+
+sources:
+- kind: local
+  path: all-all.bst
+
+depends:
+- filename: dep.bst
+  type: build
+- filename: dep.bst
+  type: runtime
diff --git a/tests/format/dependencies3/dep.bst b/tests/format/dependencies3/dep.bst
new file mode 100644
index 000000000..f4f9f6862
--- /dev/null
+++ b/tests/format/dependencies3/dep.bst
@@ -0,0 +1,4 @@
+kind: import
+sources:
+- kind: local
+  path: project.conf
diff --git a/tests/format/dependencies3/project.conf b/tests/format/dependencies3/project.conf
new file mode 100644
index 000000000..d9f7e08cb
--- /dev/null
+++ b/tests/format/dependencies3/project.conf
@@ -0,0 +1 @@
+name: dup-dup-checker
diff --git a/tests/format/dependencies3/runtime-all.bst b/tests/format/dependencies3/runtime-all.bst
new file mode 100644
index 000000000..c08594623
--- /dev/null
+++ b/tests/format/dependencies3/runtime-all.bst
@@ -0,0 +1,11 @@
+kind: import
+
+sources:
+- kind: local
+  path: all-all.bst
+
+depends:
+- filename: dep.bst
+  type: runtime
+- filename: dep.bst
+  type: all
diff --git a/tests/format/dependencies3/runtime-runtime.bst b/tests/format/dependencies3/runtime-runtime.bst
new file mode 100644
index 000000000..d01181f9b
--- /dev/null
+++ b/tests/format/dependencies3/runtime-runtime.bst
@@ -0,0 +1,11 @@
+kind: import
+
+sources:
+- kind: local
+  path: all-all.bst
+
+depends:
+- filename: dep.bst
+  type: runtime
+- filename: dep.bst
+  type: runtime