From 30fd1a21895f449a8d6073ebef0bb3fa4abd771a Mon Sep 17 00:00:00 2001 From: Vldimir Milyutin Date: Sat, 15 Aug 2026 11:56:43 +0300 Subject: [PATCH] _stream: use full element name in `artifact log`/`list-contents` `artifact_log()` and `artifact_list_contents()` were keying their result dicts by `obj.name`, which is just the bare, project-relative element name. For elements loaded across a junction, this drops the owning junction prefix, so e.g. `bst artifact list-contents element.bst nested.bst:element.bst` would only show one entry, since both targets resolved to the same key. Use `obj._get_full_name()` instead, which includes the junction prefix, falling back to the artifact ref for `ArtifactElement` targets (i.e. when an artifact is referred to by its ref rather than its element name) as before. Also drops a dead `obj.name = {ref: "No artifact cached"}` assignment in `artifact_list_contents()` that was immediately followed by `continue` and never used. Add tests covering `artifact log` and `artifact list-contents` with a cross-junction element alongside a same-named top-level element. --- src/buildstream/_artifactelement.py | 10 +++++++ src/buildstream/_stream.py | 7 ++--- tests/frontend/artifact_list_contents.py | 38 ++++++++++++++++++++++++ tests/frontend/artifact_log.py | 32 ++++++++++++++++++++ 4 files changed, 82 insertions(+), 5 deletions(-) diff --git a/src/buildstream/_artifactelement.py b/src/buildstream/_artifactelement.py index 7a1c336dd..56e81e15e 100644 --- a/src/buildstream/_artifactelement.py +++ b/src/buildstream/_artifactelement.py @@ -120,6 +120,16 @@ def _load_artifact(self, *, pull, strict=None): # pylint: disable=useless-super # Always operate in strict mode as artifact key has been specified explicitly. return super()._load_artifact(pull=pull, strict=True) + # An ArtifactElement is identified by its artifact name (which includes + # its cache key) rather than by its element name, since it may not even + # belong to a loaded project. Cache key is not yet known + # when this is first called from Plugin.__init__(), before our own + # __init__() has had a chance to set it via artifact_key. + def _get_full_name(self): + if self._get_cache_key() is None: + return super()._get_full_name() + return self.get_artifact_name() + # Once we've finished loading an artifact, we assume the # state of the loaded artifact. This is also used if the # artifact is loaded after pulling. diff --git a/src/buildstream/_stream.py b/src/buildstream/_stream.py index d326b9532..7ac10b5cf 100644 --- a/src/buildstream/_stream.py +++ b/src/buildstream/_stream.py @@ -844,7 +844,7 @@ def artifact_log(self, targets): self._context.messenger.warn("{} is cached without log files".format(ref)) continue - artifact_logs[obj.name] = obj._get_logs() + artifact_logs[obj._get_full_name()] = obj._get_logs() return artifact_logs @@ -869,15 +869,12 @@ def artifact_list_contents(self, targets): ref = obj.get_artifact_name() if not obj._cached(): self._context.messenger.warn("{} is not cached".format(ref)) - obj.name = {ref: "No artifact cached"} continue - if isinstance(obj, ArtifactElement): - obj.name = ref # Just hand over a Directory here artifact = obj._get_artifact() files = artifact.get_files() - elements_to_files[obj.name] = files + elements_to_files[obj._get_full_name()] = files return elements_to_files diff --git a/tests/frontend/artifact_list_contents.py b/tests/frontend/artifact_list_contents.py index d7d8397f8..580b8217c 100644 --- a/tests/frontend/artifact_list_contents.py +++ b/tests/frontend/artifact_list_contents.py @@ -21,12 +21,21 @@ from buildstream._testing import cli # pylint: disable=unused-import from buildstream.exceptions import ErrorDomain +from tests.testutils import generate_junction + # Project directory DATA_DIR = os.path.join( os.path.dirname(os.path.realpath(__file__)), "artifact_list_contents", ) +# Shared project directory, used for cross junction tests since it already +# has a subproject set up +CROSS_JUNCTION_DATA_DIR = os.path.join( + os.path.dirname(os.path.realpath(__file__)), + "project", +) + def prepare_symlink(project): # Create the link before running the tests. @@ -158,3 +167,32 @@ def test_artifact_list_exact_contents_glob(cli, datafiles): for artifact in expected_artifacts: assert artifact in result.output + + +@pytest.mark.datafiles(CROSS_JUNCTION_DATA_DIR) +def test_artifact_list_contents_cross_junction(cli, tmpdir, datafiles): + project = str(datafiles) + subproject_path = os.path.join(project, "files", "sub-project") + junction_path = os.path.join(project, "elements", "junction.bst") + + # Create a repo to hold the subproject and generate a junction element for it + generate_junction(tmpdir, subproject_path, junction_path) + + # Ensure we have artifacts to read + result = cli.run(project=project, args=["build", "import-bin.bst", "junction.bst:import-etc.bst"]) + result.assert_success() + + # List the contents of the cross junction element, alongside a regular + # top-level element which happens to share a basename with it. + result = cli.run( + project=project, args=["artifact", "list-contents", "import-bin.bst", "junction.bst:import-etc.bst"] + ) + result.assert_success() + + # The full element name, including the owning junction prefix, must be + # used as the header for the cross junction element's contents -- using + # only the bare element name would make it indistinguishable from an + # element of the same name in the top level project or another junction. + assert "import-bin.bst:\n" in result.output + assert "junction.bst:import-etc.bst:\n" in result.output + assert "etc/animal.conf" in result.output diff --git a/tests/frontend/artifact_log.py b/tests/frontend/artifact_log.py index db0123c81..b0168c004 100644 --- a/tests/frontend/artifact_log.py +++ b/tests/frontend/artifact_log.py @@ -21,6 +21,8 @@ from buildstream._testing import cli # pylint: disable=unused-import +from tests.testutils import generate_junction + # Project directory DATA_DIR = os.path.join( os.path.dirname(os.path.realpath(__file__)), @@ -101,3 +103,33 @@ def test_artifact_log_files(cli, datafiles): with open(import_bin, "r", encoding="utf-8") as f: data = f.read() assert len(re.findall(pattern, data, re.MULTILINE)) > 0 + + +@pytest.mark.datafiles(DATA_DIR) +def test_artifact_log_cross_junction(cli, tmpdir, datafiles): + project = str(datafiles) + subproject_path = os.path.join(project, "files", "sub-project") + junction_path = os.path.join(project, "elements", "junction.bst") + + # Create a repo to hold the subproject and generate a junction element for it + generate_junction(tmpdir, subproject_path, junction_path) + + # Ensure we have an artifact to read, alongside a regular top-level + # element which happens to share a basename with it. + result = cli.run(project=project, args=["build", "import-bin.bst", "junction.bst:import-etc.bst"]) + result.assert_success() + + logfiles = os.path.join(project, "logfiles") + + result = cli.run( + project=project, + args=["artifact", "log", "--out", logfiles, "import-bin.bst", "junction.bst:import-etc.bst"], + ) + result.assert_success() + + # The log file for the cross junction element must be named using the + # full element name, including the owning junction prefix, and not just + # the bare element name -- otherwise it would collide with the log file + # of a same-named element in the top level project or another junction. + assert os.path.exists(os.path.join(logfiles, "import-bin.log")) + assert os.path.exists(os.path.join(logfiles, "junction.bst:import-etc.log"))