From 4392664c500364a5850fe16ce6dd8facbf6e6854 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BCrg=20Billeter?= Date: Fri, 21 Nov 2025 14:55:30 +0100 Subject: [PATCH 1/3] _loader: Drop unused dependency list --- src/buildstream/_loader/loader.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/buildstream/_loader/loader.py b/src/buildstream/_loader/loader.py index b36dfe068..42a7eb4a4 100644 --- a/src/buildstream/_loader/loader.py +++ b/src/buildstream/_loader/loader.py @@ -495,8 +495,7 @@ 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)), [])] + loader_queue = [(top_element, list(reversed(dependencies)))] # Load all dependency files for the new LoadElement while loader_queue: @@ -505,8 +504,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 +526,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 From 9ed6aa573839c17cfa61c31e4e7efe4ef9cffe29 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BCrg=20Billeter?= Date: Fri, 21 Nov 2025 15:09:52 +0100 Subject: [PATCH 2/3] _loader: Merge duplicate dependencies via link elements Duplicate dependencies are normally merged already in `extract_depends_from_node()`. However, that only works for dependencies that are declared with the same filename. If a link element is in a dependency list, merging may still be required but can be done only after the link element has been loaded. The element state updates don't work correctly if there are duplicate dependencies that aren't merged. Fixes #2002. --- src/buildstream/_loader/loadelement.pyx | 2 +- src/buildstream/_loader/loader.py | 15 ++++++++++++--- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/src/buildstream/_loader/loadelement.pyx b/src/buildstream/_loader/loadelement.pyx index 52e9a8ecd..f4ebb673c 100644 --- a/src/buildstream/_loader/loadelement.pyx +++ b/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 diff --git a/src/buildstream/_loader/loader.py b/src/buildstream/_loader/loader.py index 42a7eb4a4..25548dd81 100644 --- a/src/buildstream/_loader/loader.py +++ b/src/buildstream/_loader/loader.py @@ -495,7 +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 - 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: @@ -526,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 @@ -539,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() From e90c2463b725a98fb26fc18067561628f659b807 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BCrg=20Billeter?= Date: Fri, 21 Nov 2025 10:28:02 +0100 Subject: [PATCH 3/3] tests/format/link.py: Add test for multiple links with the same target This is a regression test for #2002. --- tests/format/link.py | 20 +++++++++++++++++++ .../elements/hello-link-1.bst | 4 ++++ .../elements/hello-link-2.bst | 4 ++++ .../elements/hello.bst | 5 +++++ .../elements/target.bst | 5 +++++ .../files/hello.txt | 1 + .../multiple-links-same-target/project.conf | 4 ++++ 7 files changed, 43 insertions(+) create mode 100644 tests/format/link/multiple-links-same-target/elements/hello-link-1.bst create mode 100644 tests/format/link/multiple-links-same-target/elements/hello-link-2.bst create mode 100644 tests/format/link/multiple-links-same-target/elements/hello.bst create mode 100644 tests/format/link/multiple-links-same-target/elements/target.bst create mode 100644 tests/format/link/multiple-links-same-target/files/hello.txt create mode 100644 tests/format/link/multiple-links-same-target/project.conf diff --git a/tests/format/link.py b/tests/format/link.py index fee6de37b..ee8136bca 100644 --- a/tests/format/link.py +++ b/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")) diff --git a/tests/format/link/multiple-links-same-target/elements/hello-link-1.bst b/tests/format/link/multiple-links-same-target/elements/hello-link-1.bst new file mode 100644 index 000000000..83b0fbe46 --- /dev/null +++ b/tests/format/link/multiple-links-same-target/elements/hello-link-1.bst @@ -0,0 +1,4 @@ +kind: link + +config: + target: hello.bst diff --git a/tests/format/link/multiple-links-same-target/elements/hello-link-2.bst b/tests/format/link/multiple-links-same-target/elements/hello-link-2.bst new file mode 100644 index 000000000..83b0fbe46 --- /dev/null +++ b/tests/format/link/multiple-links-same-target/elements/hello-link-2.bst @@ -0,0 +1,4 @@ +kind: link + +config: + target: hello.bst diff --git a/tests/format/link/multiple-links-same-target/elements/hello.bst b/tests/format/link/multiple-links-same-target/elements/hello.bst new file mode 100644 index 000000000..a04a856cd --- /dev/null +++ b/tests/format/link/multiple-links-same-target/elements/hello.bst @@ -0,0 +1,5 @@ +kind: import + +sources: +- kind: local + path: files/hello.txt diff --git a/tests/format/link/multiple-links-same-target/elements/target.bst b/tests/format/link/multiple-links-same-target/elements/target.bst new file mode 100644 index 000000000..a547ef44b --- /dev/null +++ b/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 diff --git a/tests/format/link/multiple-links-same-target/files/hello.txt b/tests/format/link/multiple-links-same-target/files/hello.txt new file mode 100644 index 000000000..ce0136250 --- /dev/null +++ b/tests/format/link/multiple-links-same-target/files/hello.txt @@ -0,0 +1 @@ +hello diff --git a/tests/format/link/multiple-links-same-target/project.conf b/tests/format/link/multiple-links-same-target/project.conf new file mode 100644 index 000000000..12a1b2551 --- /dev/null +++ b/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