Merge pull request #2097 from apache/jbilleter/links

Merge duplicate dependencies via link elements

Author: juergbi

src/buildstream/_loader/loadelement.pyx

@@ -202,7 +202,7 @@ cdef class Dependency:
     # Args:
     #    other (Dependency): The dependency to merge into this one
     #
-    cdef merge(self, Dependency other):
+    cpdef merge(self, Dependency other):
         self.dep_type = self.dep_type | other.dep_type
         self.strict = self.strict or other.strict
 

src/buildstream/_loader/loader.py

@@ -495,8 +495,8 @@ def _load_file(self, filename, provenance_node, *, load_subprojects=True):
         # The loader queue is a stack of tuples
         # [0] is the LoadElement instance
         # [1] is a stack of Dependency objects to load
-        # [2] is a list of dependency names used to warn when all deps are loaded
-        loader_queue = [(top_element, list(reversed(dependencies)), [])]
+        # [2] is a Dict[LoadElement, Dependency] of loaded dependencies
+        loader_queue = [(top_element, list(reversed(dependencies)), {})]
 
         # Load all dependency files for the new LoadElement
         while loader_queue:
@@ -505,8 +505,6 @@ def _load_file(self, filename, provenance_node, *, load_subprojects=True):
 
                 # Process the first dependency of the last loaded element
                 dep = current_element[1].pop()
-                # And record its name for checking later
-                current_element[2].append(dep.name)
 
                 if dep.junction:
                     loader = self.get_loader(dep.junction, dep.node)
@@ -529,7 +527,7 @@ def _load_file(self, filename, provenance_node, *, load_subprojects=True):
                         dep_element.mark_fully_loaded()
 
                         dep_deps = extract_depends_from_node(dep_element.node)
-                        loader_queue.append((dep_element, list(reversed(dep_deps)), []))
+                        loader_queue.append((dep_element, list(reversed(dep_deps)), {}))
 
                         # Pylint is not very happy about Cython and can't understand 'node' is a 'MappingNode'
                         if dep_element.node.get_str(Symbol.KIND) == "junction":  # pylint: disable=no-member
@@ -542,7 +540,15 @@ def _load_file(self, filename, provenance_node, *, load_subprojects=True):
                 # LoadElement on the dependency and append the dependency to the owning
                 # LoadElement dependency list.
                 dep.set_element(dep_element)
-                current_element[0].dependencies.append(dep)  # pylint: disable=no-member
+
+                dep_dict = current_element[2]
+                if dep.element in dep_dict:
+                    # Duplicate LoadElement in dependency list, this can happen if a dependency is
+                    # a link element that points to an element that is already a dependency.
+                    dep_dict[dep.element].merge(dep)
+                else:
+                    current_element[0].dependencies.append(dep)  # pylint: disable=no-member
+                    dep_dict[dep.element] = dep
             else:
                 # And pop the element off the queue
                 loader_queue.pop()

tests/format/link.py

@@ -198,3 +198,23 @@ def test_cross_link_junction_include(cli, tmpdir, datafiles):
     # Read back some of our project defaults from the env
     variables = _yaml.load_data(result.output)
     assert variables.get_str("test") == "the test"
+
+
+#
+# Test two links to the same element, both links are dependencies of the build target.
+#
+@pytest.mark.datafiles(DATA_DIR)
+def test_multiple_links_same_target(cli, tmpdir, datafiles):
+    project = os.path.join(str(datafiles), "multiple-links-same-target")
+    checkoutdir = os.path.join(str(tmpdir), "checkout")
+
+    target = "target.bst"
+
+    # Build, checkout
+    result = cli.run(project=project, args=["build", target])
+    result.assert_success()
+    result = cli.run(project=project, args=["artifact", "checkout", target, "--directory", checkoutdir])
+    result.assert_success()
+
+    # Check that the checkout contains the expected files from sub-sub-project
+    assert os.path.exists(os.path.join(checkoutdir, "hello.txt"))

tests/format/link/multiple-links-same-target/elements/hello-link-1.bst

@@ -0,0 +1,4 @@
+kind: link
+
+config:
+  target: hello.bst

tests/format/link/multiple-links-same-target/elements/hello-link-2.bst

@@ -0,0 +1,4 @@
+kind: link
+
+config:
+  target: hello.bst

tests/format/link/multiple-links-same-target/elements/hello.bst

@@ -0,0 +1,5 @@
+kind: import
+
+sources:
+- kind: local
+  path: files/hello.txt

tests/format/link/multiple-links-same-target/elements/target.bst

@@ -0,0 +1,5 @@
+kind: stack
+
+depends:
+- hello-link-1.bst
+- hello-link-2.bst

tests/format/link/multiple-links-same-target/files/hello.txt

@@ -0,0 +1 @@
+hello

tests/format/link/multiple-links-same-target/project.conf

@@ -0,0 +1,4 @@
+name: multiple-links-single-target
+min-version: 2.0
+
+element-path: elements