diff --git a/src/buildstream/_project.py b/src/buildstream/_project.py index 334462df0..0af9b132b 100644 --- a/src/buildstream/_project.py +++ b/src/buildstream/_project.py @@ -125,6 +125,10 @@ def __init__( self.sandbox: Optional[MappingNode] = None self.splits: Optional[MappingNode] = None + self.source_provenance_attributes: Optional[ + MappingNode + ] = None # Source provenance attributes and their description + # # Private members # @@ -726,6 +730,7 @@ def _validate_toplevel_node(self, node, *, first_pass=False): "sources", "source-caches", "junctions", + "source-provenance-attributes", "(@)", "(?)", ] @@ -1006,6 +1011,13 @@ def _load_second_pass(self): self._shell_host_files.append(mount) + # We don't want to combine the source provenance attributes that a project defines with the defaults + # If the project config defines source-provenance-attributes use that, otherwise fall back to the defaults + # This is purely to maintain backwards compatibility with homepage and issue-tracker being available + self.source_provenance_attributes = project_conf_second_pass.get_mapping( + "source-provenance-attributes", None + ) or config.get_mapping("source-provenance-attributes") + # _load_pass(): # # Loads parts of the project configuration that are different diff --git a/src/buildstream/data/projectconfig.yaml b/src/buildstream/data/projectconfig.yaml index 4632d7a1d..f695428ca 100644 --- a/src/buildstream/data/projectconfig.yaml +++ b/src/buildstream/data/projectconfig.yaml @@ -175,6 +175,12 @@ shell: # command: [ 'sh', '-i' ] +# Define the set of fields accepted in `provenance` dictionaries of sources. +# +source-provenance-attributes: + homepage: "The project homepage URL" + issue-tracker: "The project's issue tracking URL" + # Defaults for bst commands # defaults: diff --git a/src/buildstream/element.py b/src/buildstream/element.py index fa79c2b44..aede8f1ea 100644 --- a/src/buildstream/element.py +++ b/src/buildstream/element.py @@ -85,12 +85,12 @@ from . import utils from . import _cachekey from . import _site -from .node import Node +from .node import Node, MappingNode, ScalarNode from .plugin import Plugin from .sandbox import _SandboxFlags, SandboxCommandError from .sandbox._config import SandboxConfig from .sandbox._sandboxremote import SandboxRemote -from .types import _Scope, _CacheBuildTrees, _KeyStrength, OverlapAction, _DisplayKey, _SourceProvenance +from .types import _Scope, _CacheBuildTrees, _KeyStrength, OverlapAction, _DisplayKey from ._artifact import Artifact from ._elementproxy import ElementProxy from ._elementsources import ElementSources @@ -102,7 +102,7 @@ if TYPE_CHECKING: from typing import Tuple - from .node import MappingNode, ScalarNode, SequenceNode + from .node import SequenceNode from .types import SourceRef # pylint: disable=cyclic-import @@ -2635,11 +2635,24 @@ def __load_sources(self, load_element): del source[Symbol.DIRECTORY] # Provenance is optional - provenance_node = source.get_mapping(Symbol.PROVENANCE, default=None) - provenance = None + provenance_node: MappingNode = source.get_mapping(Symbol.PROVENANCE, default=None) if provenance_node: del source[Symbol.PROVENANCE] - provenance = _SourceProvenance.new_from_node(provenance_node) + try: + provenance_node.validate_keys(project.source_provenance_attributes.keys()) + except LoadError as E: + raise LoadError( + f"Specified source provenance attribute not defined in project config\n {E}", + LoadErrorReason.UNDEFINED_SOURCE_PROVENANCE_ATTRIBUTE, + ) + + # make sure everything is a string + for key, value in provenance_node.items(): + if not isinstance(value, ScalarNode): + raise LoadError( + f"{value}: Expected string for the value of provenance attribute '{key}'", + LoadErrorReason.INVALID_DATA, + ) meta_source = MetaSource( self.name, @@ -2647,7 +2660,7 @@ def __load_sources(self, load_element): self.get_kind(), kind.as_str(), directory, - provenance, + provenance_node, source, load_element.first_pass, ) diff --git a/src/buildstream/exceptions.py b/src/buildstream/exceptions.py index 4806218e9..418e51eda 100644 --- a/src/buildstream/exceptions.py +++ b/src/buildstream/exceptions.py @@ -155,3 +155,8 @@ class LoadErrorReason(Enum): This warning will be produced when a filename for a target contains invalid characters in its name. """ + + UNDEFINED_SOURCE_PROVENANCE_ATTRIBUTE = 29 + """ + Thee source provenance attribute specified was not defined in the project config + """ diff --git a/src/buildstream/node.pyi b/src/buildstream/node.pyi index 253ba93b1..6fcb3719e 100644 --- a/src/buildstream/node.pyi +++ b/src/buildstream/node.pyi @@ -65,6 +65,7 @@ class MappingNode(Node, Generic[TNode]): def items(self) -> Iterable[Tuple[str, Any]]: ... def safe_del(self, key: str) -> None: ... def validate_keys(self, valid_keys: List[str]): ... + def values(self) -> List[Node]: ... @overload def get_scalar(self, key: str) -> ScalarNode: ... @overload diff --git a/src/buildstream/source.py b/src/buildstream/source.py index 44986290e..5eab8ee45 100644 --- a/src/buildstream/source.py +++ b/src/buildstream/source.py @@ -38,15 +38,15 @@ to provide additional source provenance related metadata which will later be reported in :class:`.SourceInfo` objects. - The ``provenance`` dictionary supports the following fields: + The ``provenance`` dictionary itself does not have any specific required keys. - * Homepage + Any attribute used in the ``provenance`` dictionary of a source must be + defined in the project.conf using the ``source-provenance-attributes`` dictionary + to define the attribute and its significance. - The ``homepage`` attribute can be used to specify the project homepage URL - - * Issue Tracker - - The ``issue-tracker`` attribute can be used to specify the project's issue tracking URL + Unless ``source-provenance-attributes`` is purposely defined as an empty dictionary, + BuildStream will default it's values to those found in + :ref:`builtin defaults `. *Since: 2.5* @@ -371,16 +371,26 @@ import os from contextlib import contextmanager -from typing import Iterable, Iterator, Optional, Tuple, Dict, Any, Set, TYPE_CHECKING, Union +from typing import ( + Iterable, + Iterator, + Optional, + Tuple, + Dict, + Any, + Set, + TYPE_CHECKING, + Union, +) from dataclasses import dataclass from . import _yaml, utils -from .node import MappingNode +from .node import MappingNode, ScalarNode from .plugin import Plugin from .sourcemirror import SourceMirror -from .types import SourceRef, CoreWarnings, FastEnum, _SourceProvenance -from ._exceptions import BstError, ImplError, PluginError -from .exceptions import ErrorDomain +from .types import SourceRef, CoreWarnings, FastEnum +from ._exceptions import BstError, ImplError, PluginError, LoadError +from .exceptions import ErrorDomain, LoadErrorReason from ._loader.metasource import MetaSource from ._projectrefs import ProjectRefStorage from ._cachekey import generate_key @@ -555,6 +565,7 @@ def __init__( url: str, homepage: Optional[str], issue_tracker: Optional[str], + provenance: Optional[MappingNode], medium: Union[SourceInfoMedium, str], version_type: Union[SourceVersionType, str], version: str, @@ -582,6 +593,11 @@ def __init__( The project issue tracking URL """ + self.provenance = provenance + """ + The optional YAML node with source provenance attributes + """ + self.medium: Union[SourceInfoMedium, str] = medium """ The :class:`.SourceInfoMedium` of the source input, or in the case @@ -642,10 +658,14 @@ def serialize(self) -> Dict[str, Union[str, Dict[str, str]]]: "url": self.url, } - if self.homepage is not None: - version_info["homepage"] = self.homepage - if self.issue_tracker is not None: - version_info["issue-tracker"] = self.issue_tracker + if self.provenance is not None: + # need to keep homepage/issue-tracker [also] at the top-level for backward compat + if (homepage := self.provenance.get_str("homepage", None)) is not None: + version_info["homepage"] = homepage + if (issue_tracker := self.provenance.get_str("issue-tracker", None)) is not None: + version_info["issue-tracker"] = issue_tracker + + version_info["provenance"] = self.provenance.strip_node_info() version_info["medium"] = medium_str version_info["version-type"] = version_type_str @@ -833,8 +853,8 @@ def __init__( self._directory = meta.directory # Staging relative directory self.__variables = variables # The variables used to resolve the source's config self.__provenance: Optional[ - _SourceProvenance - ] = meta.provenance # The _SourceProvenance for general user provided SourceInfo + MappingNode + ] = meta.provenance # The source provenance for general user provided SourceInfo if self.__provenance is not None and self.BST_CUSTOM_SOURCE_PROVENANCE: raise SourceError( @@ -1407,23 +1427,44 @@ def create_source_info( *Since: 2.5* """ - homepage = None - issue_tracker = None + project = self._get_project() + source_provenance: MappingNode | None if provenance_node is not None: - source_provenance: Optional[_SourceProvenance] = _SourceProvenance.new_from_node(provenance_node) + # Ensure provenance node keys are valid and values are all strings + try: + provenance_node.validate_keys(project.source_provenance_attributes.keys()) + except LoadError as E: + raise LoadError( + "Specified source provenance attribute not defined in project config\n {}".format(E), + LoadErrorReason.UNDEFINED_SOURCE_PROVENANCE_ATTRIBUTE, + ) + + # Make sure everything is a string + for key, value in provenance_node.items(): + if not isinstance(value, ScalarNode): + raise LoadError( + f"{value}: Expected string for the value of provenance attribute '{key}'", + LoadErrorReason.INVALID_DATA, + ) + + source_provenance = provenance_node else: source_provenance = self.__provenance + homepage = None + issue_tracker = None + if source_provenance is not None: - homepage = source_provenance.homepage - issue_tracker = source_provenance.issue_tracker + homepage = source_provenance.get_str("homepage", None) + issue_tracker = source_provenance.get_str("issue-tracker", None) return SourceInfo( self.get_kind(), url, homepage, issue_tracker, + source_provenance, medium, version_type, version, diff --git a/src/buildstream/types.py b/src/buildstream/types.py index d4d5e3cdb..0cc2106b3 100644 --- a/src/buildstream/types.py +++ b/src/buildstream/types.py @@ -390,42 +390,6 @@ def new_from_node(cls, node: MappingNode) -> "_SourceMirror": return cls(name, aliases) -# _SourceProvenance() -# -# A simple object describing user provided source provenance information -# -# Args: -# homepage: The project homepage URL -# issue_tracker: The project issue reporting URL -# -class _SourceProvenance: - def __init__(self, homepage: Optional[str], issue_tracker: Optional[str]): - self.homepage: Optional[str] = homepage - self.issue_tracker: Optional[str] = issue_tracker - - # new_from_node(): - # - # Creates a _SourceProvenance() from a YAML loaded node. - # - # Args: - # node: The configuration node describing the spec. - # - # Returns: - # The described _SourceProvenance instance. - # - # Raises: - # LoadError: If the node is malformed. - # - @classmethod - def new_from_node(cls, node: MappingNode) -> "_SourceProvenance": - node.validate_keys(["homepage", "issue-tracker"]) - - homepage: Optional[str] = node.get_str("homepage", None) - issue_tracker: Optional[str] = node.get_str("issue-tracker", None) - - return cls(homepage, issue_tracker) - - ######################################## # Type aliases # ######################################## diff --git a/tests/sources/source_provenance_attributes.py b/tests/sources/source_provenance_attributes.py index 65f28740a..a51e7c883 100644 --- a/tests/sources/source_provenance_attributes.py +++ b/tests/sources/source_provenance_attributes.py @@ -20,7 +20,7 @@ from buildstream._testing import generate_project, load_yaml from buildstream._testing import cli # pylint: disable=unused-import -from buildstream.exceptions import ErrorDomain +from buildstream.exceptions import ErrorDomain, LoadErrorReason DATA_DIR = os.path.join(os.path.dirname(os.path.realpath(__file__)), "source_provenance_attributes") @@ -40,6 +40,9 @@ def test_source_provenance_disallow_top_level(cli, datafiles): aliases = project_config.get_mapping("aliases") aliases["project_dir"] = "file://{}".format(project) + source_provenance_attrs = project_config.get_mapping("source-provenance-attributes") + source_provenance_attrs["homepage"] = "Testing" + generate_project(project, project_config) # Make sure disallowed usage of top-level source proveance fails @@ -49,3 +52,108 @@ def test_source_provenance_disallow_top_level(cli, datafiles): ) result.assert_main_error(ErrorDomain.SOURCE, "top-level-provenance-on-custom-implementation") + + +@pytest.mark.datafiles(DATA_DIR) +def test_source_provenance_no_defined_attributes(cli, datafiles): + project = str(datafiles) + + # Set the project_dir alias in project.conf to the path to the tested project + project_config_path = os.path.join(project, "project.conf") + project_config = load_yaml(project_config_path) + aliases = project_config.get_mapping("aliases") + aliases["project_dir"] = "file://{}".format(project) + + generate_project(project, project_config) + + # Make sure a non-default attribute fails + result = cli.run( + project=project, + args=["show", "--format", "%{source-info}", "target_a.bst"], + ) + result.assert_main_error(ErrorDomain.LOAD, LoadErrorReason.UNDEFINED_SOURCE_PROVENANCE_ATTRIBUTE) + + # Make sure a default attribute fails + result = cli.run( + project=project, + args=["show", "--format", "%{source-info}", "target_b.bst"], + ) + result.assert_main_error(ErrorDomain.LOAD, LoadErrorReason.UNDEFINED_SOURCE_PROVENANCE_ATTRIBUTE) + + +# Test that no defined source provenance attributes blocks all source provenance data +@pytest.mark.datafiles(DATA_DIR) +def test_source_provenance_default_attributes(cli, datafiles): + project = str(datafiles) + + # Set the project_dir alias in project.conf to the path to the tested project + project_config_path = os.path.join(project, "project.conf") + project_config = load_yaml(project_config_path) + aliases = project_config.get_mapping("aliases") + aliases["project_dir"] = "file://{}".format(project) + + # Edit config to fallback to default source provenance attributes + project_config.safe_del("source-provenance-attributes") + + generate_project(project, project_config) + + # Make sure defined attributes are available + result = cli.run( + project=project, + args=["show", "--format", "%{source-info}", "target_b.bst"], + ) + result.assert_success() + + provenance_result = "" + for line in result.output.split("\n"): + if "provenance:" in line or " " in line: + provenance_result += line + + assert provenance_result == " provenance: homepage: foo" + + # Make sure undefined attributes fail + result = cli.run( + project=project, + args=["show", "--format", "%{source-info}", "target_a.bst"], + ) + result.assert_main_error(ErrorDomain.LOAD, LoadErrorReason.UNDEFINED_SOURCE_PROVENANCE_ATTRIBUTE) + + +# Test that no defined source provenance attributes blocks all source provenance data +@pytest.mark.datafiles(DATA_DIR) +def test_source_provenance_project_defined_attributes(cli, datafiles): + project = str(datafiles) + + # Set the project_dir alias in project.conf to the path to the tested project + project_config_path = os.path.join(project, "project.conf") + project_config = load_yaml(project_config_path) + aliases = project_config.get_mapping("aliases") + aliases["project_dir"] = "file://{}".format(project) + + # Edit config to only use project specified source provenance attributes + + source_provenance_attrs = project_config.get_mapping("source-provenance-attributes") + source_provenance_attrs["originator"] = "Testing" + + generate_project(project, project_config) + + # Make sure defined attributes are available + result = cli.run( + project=project, + args=["show", "--format", "%{source-info}", "target_a.bst"], + ) + result.assert_success() + + provenance_result = "" + for line in result.output.split("\n"): + if "provenance:" in line or " " in line: + provenance_result += line + + assert provenance_result == " provenance: originator: bar" + + # Make sure undefined attributes fail + result = cli.run( + project=project, + args=["show", "--format", "%{source-info}", "target_b.bst"], + ) + result.assert_main_error(ErrorDomain.LOAD, LoadErrorReason.UNDEFINED_SOURCE_PROVENANCE_ATTRIBUTE) diff --git a/tests/sources/source_provenance_attributes/elements/target_a.bst b/tests/sources/source_provenance_attributes/elements/target_a.bst new file mode 100644 index 000000000..b46719284 --- /dev/null +++ b/tests/sources/source_provenance_attributes/elements/target_a.bst @@ -0,0 +1,7 @@ +kind: import + +sources: +- kind: remote + url: project_dir:/files/file + provenance: + originator: bar diff --git a/tests/sources/source_provenance_attributes/elements/target_b.bst b/tests/sources/source_provenance_attributes/elements/target_b.bst new file mode 100644 index 000000000..0587496d6 --- /dev/null +++ b/tests/sources/source_provenance_attributes/elements/target_b.bst @@ -0,0 +1,7 @@ +kind: import + +sources: +- kind: remote + url: project_dir:/files/file + provenance: + homepage: foo diff --git a/tests/sources/source_provenance_attributes/project.conf b/tests/sources/source_provenance_attributes/project.conf index e102c70cc..179308a76 100644 --- a/tests/sources/source_provenance_attributes/project.conf +++ b/tests/sources/source_provenance_attributes/project.conf @@ -12,3 +12,6 @@ plugins: path: plugins sources: - multisource-plugin + +# Don't allow any provenance +source-provenance-attributes: {}