abderrahim commented on code in PR #2098:
URL: https://github.com/apache/buildstream/pull/2098#discussion_r2717062528
##########
tests/frontend/show.py:
##########
@@ -814,3 +814,33 @@ def test_source_info_workspace(cli, datafiles, tmpdir):
# There is no version guessing for a workspace
assert source_info.get_str("version-guess", None) is None
+
+
[email protected](os.path.join(DATA_DIR, "source-info"))
+def test_multi_source_info(cli, datafiles):
+ project = str(datafiles)
+ result = cli.run(
+ project=project, silent=True, args=["show", "--format",
"%{name}:\n%{source-info}", "multisource.bst"]
+ )
+ result.assert_success()
+
+ loaded = _yaml.load_data(result.output)
+ sources = loaded.get_sequence("multisource.bst")
+
+ source_info = sources.mapping_at(0)
+ assert source_info.get_str("kind") == "multisource"
+ assert source_info.get_str("url") == "http://ponyfarm.com/ponies";
+ assert source_info.get_str("medium") == "pony-ride"
+ assert source_info.get_str("version-type") == "pony-age"
+ assert source_info.get_str("version") == "1234567"
+ assert source_info.get_str("version-guess", None) == "12"
+ assert source_info.get_str("homepage") ==
"https://flying-ponies.com/index.html";
Review Comment:
Is this test up to date? Shouldn't the homepage here be unset?
(also should we test for the warning/error condition?)
##########
src/buildstream/source.py:
##########
@@ -759,6 +759,14 @@ class Source(Plugin):
# The defaults from the project
__defaults: Optional[Dict[str, Any]] = None
+ BST_MULTI_SOURCE_PROVENANCE = False
Review Comment:
I would call this CUSTOM or something. While the main use is for sources
that have multiple source fetchers that might want to have different source
provenance, the *effect* of setting this flag is to allow the source to set its
own source provenance programatically and prevent the buildstream core from
parsing / using the provenance information from the top-level source
configuration.
##########
src/buildstream/source.py:
##########
@@ -1387,14 +1396,32 @@ def create_source_info(
version: A string which represents a unique version of this source
input
version_guess: An optional string representing the guessed human
readable version
extra_data: Additional plugin defined key/values
+ provenance_node: The optional :class:`Node <buildstream.node.Node>`
with source provenance attributes,
Review Comment:
nit: The -> An
##########
src/buildstream/source.py:
##########
@@ -1387,14 +1396,32 @@ def create_source_info(
version: A string which represents a unique version of this source
input
version_guess: An optional string representing the guessed human
readable version
extra_data: Additional plugin defined key/values
+ provenance_node: The optional :class:`Node <buildstream.node.Node>`
with source provenance attributes,
+ defaults to the provenance specified at the top
level of the source.
*Since: 2.5*
"""
homepage = None
issue_tracker = None
- if self.__provenance is not None:
- homepage = self.__provenance.homepage
- issue_tracker = self.__provenance.issue_tracker
+
+ provenance = None
+ if provenance_node is not None:
+ provenance: Optional[_SourceProvenance] =
_SourceProvenance.new_from_node(provenance_node)
+ # if the source uses multiple sources but the individual source_info
has
+ # no specified provenance data, skip it. We don't want to use top level
+ # data as a fallback because it'll just be wrong
+ elif self.BST_MULTI_SOURCE_PROVENANCE:
+ if self.__provenance is not None:
+ # This warning is printed for each occurrence of missing
provenance
+ # for each sub-source, maybe it should be done just the once
per
+ # multi-source?
+ self.warn("Multi source plugin: Not using top level source
provenance")
Review Comment:
As a follow-up to my comment above, this shouldn't call them "Multi source
plugin".
Also I'm not sure if this should be a warning or an error. I feel we should
be strict with this, especially if this check runs early on in the pipeline
(i.e. it's not only run if are looking for source info)
And please use the provenance (as in the `ProvenanceInformation`, not the
`SourceProvenance`) in the error/warning message.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
