From f87dc2ddbf9d10178f93b45d5abc5c873051ea47 Mon Sep 17 00:00:00 2001 From: Harry Sarson Date: Wed, 13 Aug 2025 15:40:08 +0100 Subject: [PATCH 1/4] Adapt override-self test to show a nested-link bug This commit adds an additional level of nesting to the override-self buildstream test project by a new subproject and moving the self-junction into the subproject. Both the `link.bst` and the `nested-link.bst` elements point to the subproject's `target.bst` element but `nested-link.bst` does so via the self-junction, hence the "nesting". Running `bst show --deps none nested-link.bst` in the directory `tests/format/junctions/override-self` gives the following output: ``` 20:17:48 --:--:-- main:core activity START Loading elements 20:17:48 00:00:00 main:core activity FAILURE Loading elements project.conf [line 6 column 2]: Circular reference while searching for 'subproject.bst:self-junction.bst' Already searching for 'subproject.bst:self-junction.bst' at: nested-link.bst [line 4 column 10] ``` --- tests/format/junctions.py | 13 ++++++++++--- tests/format/junctions/override-self/link.bst | 4 ++++ .../format/junctions/override-self/nested-link.bst | 4 ++++ tests/format/junctions/override-self/project.conf | 6 +++--- tests/format/junctions/override-self/subproject.bst | 4 ++++ .../override-self/{ => subproject}/alternative.bst | 0 .../junctions/override-self/subproject/include.yml | 0 .../junctions/override-self/subproject/project.conf | 9 +++++++++ .../{ => subproject}/self-junction.bst | 1 + .../override-self/{ => subproject}/target.bst | 0 10 files changed, 35 insertions(+), 6 deletions(-) create mode 100644 tests/format/junctions/override-self/link.bst create mode 100644 tests/format/junctions/override-self/nested-link.bst create mode 100644 tests/format/junctions/override-self/subproject.bst rename tests/format/junctions/override-self/{ => subproject}/alternative.bst (100%) create mode 100644 tests/format/junctions/override-self/subproject/include.yml create mode 100644 tests/format/junctions/override-self/subproject/project.conf rename tests/format/junctions/override-self/{ => subproject}/self-junction.bst (99%) rename tests/format/junctions/override-self/{ => subproject}/target.bst (100%) diff --git a/tests/format/junctions.py b/tests/format/junctions.py index 458aa3a53..8b20b4d3e 100644 --- a/tests/format/junctions.py +++ b/tests/format/junctions.py @@ -593,10 +593,17 @@ def test_override_twice(cli, tmpdir, datafiles): @pytest.mark.parametrize( "target,expected_result", [ - ("target.bst", "pony"), - ("self-junction.bst:target.bst", "horsy"), + ("subproject.bst:target.bst", "pony"), + ("subproject.bst:self-junction.bst:target.bst", "horsy"), + ("link.bst", "pony"), + ("nested-link.bst", "horsy"), + ], + ids=[ + "direct-target", + "override-target", + "link-target", + "link-override-target", ], - ids=["direct-target", "override-target"], ) def test_override_self(cli, datafiles, target, expected_result): project = os.path.join(str(datafiles), "override-self") diff --git a/tests/format/junctions/override-self/link.bst b/tests/format/junctions/override-self/link.bst new file mode 100644 index 000000000..3da195d89 --- /dev/null +++ b/tests/format/junctions/override-self/link.bst @@ -0,0 +1,4 @@ +kind: link + +config: + target: subproject.bst:target.bst diff --git a/tests/format/junctions/override-self/nested-link.bst b/tests/format/junctions/override-self/nested-link.bst new file mode 100644 index 000000000..fa2b2b571 --- /dev/null +++ b/tests/format/junctions/override-self/nested-link.bst @@ -0,0 +1,4 @@ +kind: link + +config: + target: subproject.bst:self-junction.bst:target.bst diff --git a/tests/format/junctions/override-self/project.conf b/tests/format/junctions/override-self/project.conf index cb98a9a41..ed7370c4f 100644 --- a/tests/format/junctions/override-self/project.conf +++ b/tests/format/junctions/override-self/project.conf @@ -1,6 +1,6 @@ name: test min-version: 2.0 -junctions: - internal: - - self-junction.bst + +(@): +- subproject.bst:self-junction.bst:include.yml diff --git a/tests/format/junctions/override-self/subproject.bst b/tests/format/junctions/override-self/subproject.bst new file mode 100644 index 000000000..c88189cb0 --- /dev/null +++ b/tests/format/junctions/override-self/subproject.bst @@ -0,0 +1,4 @@ +kind: junction +sources: +- kind: local + path: subproject diff --git a/tests/format/junctions/override-self/alternative.bst b/tests/format/junctions/override-self/subproject/alternative.bst similarity index 100% rename from tests/format/junctions/override-self/alternative.bst rename to tests/format/junctions/override-self/subproject/alternative.bst diff --git a/tests/format/junctions/override-self/subproject/include.yml b/tests/format/junctions/override-self/subproject/include.yml new file mode 100644 index 000000000..e69de29bb diff --git a/tests/format/junctions/override-self/subproject/project.conf b/tests/format/junctions/override-self/subproject/project.conf new file mode 100644 index 000000000..d3f3752d3 --- /dev/null +++ b/tests/format/junctions/override-self/subproject/project.conf @@ -0,0 +1,9 @@ +name: subproject +min-version: 2.0 + +junctions: + internal: + - self-junction.bst + +# (@): +# - self-junction.bst:include.yml diff --git a/tests/format/junctions/override-self/self-junction.bst b/tests/format/junctions/override-self/subproject/self-junction.bst similarity index 99% rename from tests/format/junctions/override-self/self-junction.bst rename to tests/format/junctions/override-self/subproject/self-junction.bst index aae6eef39..52ea670a0 100644 --- a/tests/format/junctions/override-self/self-junction.bst +++ b/tests/format/junctions/override-self/subproject/self-junction.bst @@ -3,6 +3,7 @@ sources: - kind: local path: . + config: overrides: target.bst: alternative.bst diff --git a/tests/format/junctions/override-self/target.bst b/tests/format/junctions/override-self/subproject/target.bst similarity index 100% rename from tests/format/junctions/override-self/target.bst rename to tests/format/junctions/override-self/subproject/target.bst From b77c859b69cfe0f400ad7d281c90d1d9d0d202ad Mon Sep 17 00:00:00 2001 From: Harry Sarson Date: Wed, 13 Aug 2025 15:40:42 +0100 Subject: [PATCH 2/4] Still ensure fully loaded if reference is via link This commit load ensures the toplevel project is always loaded, even when addressing subproject elements via link. Commit cff58ec added a call to `Project.ensure_fully_loaded` to ensure the toplevel project is resolved first when addressing subproject elements on the command line. The commit assumes that whenever subproject elements are referenced on the command line, `Loader.get_loader` is called with `provenance_node == None`. However, if a link element that points to a subproject element is referenced on the command line then `get_loader` will be called with `provenance_node` referencing the link element. This means that `get_loader` skips the call to `ensure_fully_loaded` causing buildstream to crash. This commit fixes the crash by moving the call to `ensure_fully_loaded` up the call chain into `Loader.load` where we know that the element is addressed from the command line. --- src/buildstream/_loader/loader.py | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/buildstream/_loader/loader.py b/src/buildstream/_loader/loader.py index 25548dd81..153bc32c8 100644 --- a/src/buildstream/_loader/loader.py +++ b/src/buildstream/_loader/loader.py @@ -128,6 +128,15 @@ def load(self, targets): for target in targets: with PROFILER.profile(Topics.LOAD_PROJECT, target): + + # As we are attempting to load a subproject element via the + # command line ensure fully loaded. + # + # Other callers of `_parse_name` or `_load_file` that reference + # files through a project element or otherwise do not need to + # ensure fully loaded. + self.project.ensure_fully_loaded() + _junction, name, loader = self._parse_name(target, None) element = loader._load_file(name, None) target_elements.append(element) @@ -190,14 +199,6 @@ def get_loader(self, name, provenance_node, *, load_subprojects=True): junction_path = name.split(":") loader = self - # - # In this case we are attempting to load a subproject element via the - # command line instead of referencing the subproject through a project - # element or otherwise. - # - if provenance_node is None and load_subprojects: - self.project.ensure_fully_loaded() - circular_provenance_node = self._loader_search_provenances.get(name, None) if circular_provenance_node and load_subprojects: From 17a9b3d5aa4e11cb7b102e653333b56911e88c1f Mon Sep 17 00:00:00 2001 From: Harry Sarson Date: Wed, 13 Aug 2025 15:43:11 +0100 Subject: [PATCH 3/4] Include from nested junction in include-complex This commit moves subproject into a new intermediate project and the subproject.bst junction element is replaced by a link element. Including a file from a nested junction via a link element in the top level project.conf file tests more code paths in the loader. This change shows a bug in the loader, running `bst show --deps none target.bst` in the directory `tests/format/junctions/include-complex` gives the following output: ``` 20:02:32 --:--:-- main:core activity START Loading elements 20:02:32 00:00:00 main:core activity FAILURE Loading elements intermediate-project.bst:project.conf [line 5 column 2]: Circular reference while searching for 'subproject.bst' Already searching for 'subproject.bst' at: subproject.bst [line 4 column 10] ``` --- tests/format/junctions.py | 2 +- .../include-complex/alternative-placeholder.bst | 1 + .../include-complex/intermediate-project.bst | 12 ++++++++++++ .../intermediate-project/project.conf | 15 +++++++++++++++ .../intermediate-project/subproject.bst | 9 +++++++++ .../subproject/include.yml | 0 .../subproject/placeholder.bst | 1 + .../subproject/project.conf | 1 + .../subproject/target.bst | 0 .../junctions/include-complex/subproject.bst | 8 ++------ 10 files changed, 42 insertions(+), 7 deletions(-) create mode 100644 tests/format/junctions/include-complex/alternative-placeholder.bst create mode 100644 tests/format/junctions/include-complex/intermediate-project.bst create mode 100644 tests/format/junctions/include-complex/intermediate-project/project.conf create mode 100644 tests/format/junctions/include-complex/intermediate-project/subproject.bst rename tests/format/junctions/include-complex/{ => intermediate-project}/subproject/include.yml (100%) create mode 100644 tests/format/junctions/include-complex/intermediate-project/subproject/placeholder.bst rename tests/format/junctions/include-complex/{ => intermediate-project}/subproject/project.conf (85%) rename tests/format/junctions/include-complex/{ => intermediate-project}/subproject/target.bst (100%) diff --git a/tests/format/junctions.py b/tests/format/junctions.py index 8b20b4d3e..bf4919b94 100644 --- a/tests/format/junctions.py +++ b/tests/format/junctions.py @@ -929,7 +929,7 @@ def test_include_vars_optional(cli, datafiles, use_species, expected_result): @pytest.mark.datafiles(DATA_DIR) @pytest.mark.parametrize( "target", - ["target.bst", "subproject.bst:target.bst"], + ["target.bst", "subproject.bst:target.bst", "intermediate-project.bst:subproject.bst:target.bst"], ids=["toplevel-target", "subproject-target"], ) @pytest.mark.parametrize( diff --git a/tests/format/junctions/include-complex/alternative-placeholder.bst b/tests/format/junctions/include-complex/alternative-placeholder.bst new file mode 100644 index 000000000..066a03328 --- /dev/null +++ b/tests/format/junctions/include-complex/alternative-placeholder.bst @@ -0,0 +1 @@ +kind: stack diff --git a/tests/format/junctions/include-complex/intermediate-project.bst b/tests/format/junctions/include-complex/intermediate-project.bst new file mode 100644 index 000000000..25d9930f5 --- /dev/null +++ b/tests/format/junctions/include-complex/intermediate-project.bst @@ -0,0 +1,12 @@ +kind: junction + +sources: +- kind: local + path: intermediate-project + +config: + overrides: + subproject.bst:placeholder.bst: alternative-placeholder.bst + + options: + animal: '%{animal}' diff --git a/tests/format/junctions/include-complex/intermediate-project/project.conf b/tests/format/junctions/include-complex/intermediate-project/project.conf new file mode 100644 index 000000000..9a00c9e7f --- /dev/null +++ b/tests/format/junctions/include-complex/intermediate-project/project.conf @@ -0,0 +1,15 @@ +name: intermediate-project +min-version: 2.0 + +(@): +- subproject.bst:include.yml + +options: + animal: + type: enum + description: the animal + values: + - pony + - horsy + default: pony + variable: animal diff --git a/tests/format/junctions/include-complex/intermediate-project/subproject.bst b/tests/format/junctions/include-complex/intermediate-project/subproject.bst new file mode 100644 index 000000000..4d1041390 --- /dev/null +++ b/tests/format/junctions/include-complex/intermediate-project/subproject.bst @@ -0,0 +1,9 @@ +kind: junction +sources: +- kind: local + path: subproject + +config: + + options: + target_animal: '%{animal}' diff --git a/tests/format/junctions/include-complex/subproject/include.yml b/tests/format/junctions/include-complex/intermediate-project/subproject/include.yml similarity index 100% rename from tests/format/junctions/include-complex/subproject/include.yml rename to tests/format/junctions/include-complex/intermediate-project/subproject/include.yml diff --git a/tests/format/junctions/include-complex/intermediate-project/subproject/placeholder.bst b/tests/format/junctions/include-complex/intermediate-project/subproject/placeholder.bst new file mode 100644 index 000000000..066a03328 --- /dev/null +++ b/tests/format/junctions/include-complex/intermediate-project/subproject/placeholder.bst @@ -0,0 +1 @@ +kind: stack diff --git a/tests/format/junctions/include-complex/subproject/project.conf b/tests/format/junctions/include-complex/intermediate-project/subproject/project.conf similarity index 85% rename from tests/format/junctions/include-complex/subproject/project.conf rename to tests/format/junctions/include-complex/intermediate-project/subproject/project.conf index ad8a3b3de..700da1a30 100644 --- a/tests/format/junctions/include-complex/subproject/project.conf +++ b/tests/format/junctions/include-complex/intermediate-project/subproject/project.conf @@ -9,3 +9,4 @@ options: - pony - horsy default: pony + variable: target_animal diff --git a/tests/format/junctions/include-complex/subproject/target.bst b/tests/format/junctions/include-complex/intermediate-project/subproject/target.bst similarity index 100% rename from tests/format/junctions/include-complex/subproject/target.bst rename to tests/format/junctions/include-complex/intermediate-project/subproject/target.bst diff --git a/tests/format/junctions/include-complex/subproject.bst b/tests/format/junctions/include-complex/subproject.bst index daaea3091..191cd8051 100644 --- a/tests/format/junctions/include-complex/subproject.bst +++ b/tests/format/junctions/include-complex/subproject.bst @@ -1,8 +1,4 @@ -kind: junction -sources: -- kind: local - path: subproject +kind: link config: - options: - target_animal: '%{animal}' + target: intermediate-project.bst:subproject.bst From 98833a0f72fac0176b4874bc992145d6ae24df17 Mon Sep 17 00:00:00 2001 From: Harry Sarson Date: Wed, 13 Aug 2025 15:43:40 +0100 Subject: [PATCH 4/4] add fix for crash on project load bug --- src/buildstream/_loader/loader.py | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/buildstream/_loader/loader.py b/src/buildstream/_loader/loader.py index 153bc32c8..8f79eda53 100644 --- a/src/buildstream/_loader/loader.py +++ b/src/buildstream/_loader/loader.py @@ -797,15 +797,12 @@ def provenance_str(): # immediately and move on to the target. # if load_element.link_target: - _, filename, loader = self._parse_name( - load_element.link_target.as_str(), load_element.link_target, load_subprojects=load_subprojects + return self.get_loader( + load_element.link_target.as_str(), + load_element.link_target, + load_subprojects=load_subprojects, ) - if not loader: - # `loader` should never be None if `load_subprojects` is True - assert not load_subprojects - return None - return loader.get_loader(filename, load_element.link_target, load_subprojects=load_subprojects) # If we're only performing a lookup, we're done here. #