This is an automated email from the ASF dual-hosted git repository. akitouni pushed a commit to branch abderrahim/mirror-plugins in repository https://gitbox.apache.org/repos/asf/buildstream.git
commit 61b6b08f237fe89e002a4b92ba305f5c9003db4c Author: Abderrahim Kitouni <[email protected]> AuthorDate: Sat Mar 23 15:39:17 2024 +0100 Rework source mirrors * Allow the plugins to handle the entire configuration node * Split the default behaviour into a core plugin * Make the SourceMirrors return one URL per alias * Add an internal method to allow the legacy source mirror to provide multiple URLs --- .../_pluginfactory/sourcemirrorfactory.py | 6 +- src/buildstream/_project.py | 17 ++-- src/buildstream/plugins/sourcemirrors/legacy.py | 30 ++++++++ src/buildstream/source.py | 43 ++++------- src/buildstream/sourcemirror.py | 90 +++++++--------------- tests/frontend/mirror.py | 30 ++------ tests/frontend/project/sourcemirrors/mirror.py | 5 +- 7 files changed, 92 insertions(+), 129 deletions(-) diff --git a/src/buildstream/_pluginfactory/sourcemirrorfactory.py b/src/buildstream/_pluginfactory/sourcemirrorfactory.py index 5060fd71d..6a2acc4c1 100644 --- a/src/buildstream/_pluginfactory/sourcemirrorfactory.py +++ b/src/buildstream/_pluginfactory/sourcemirrorfactory.py @@ -62,9 +62,9 @@ class SourceMirrorFactory(PluginFactory): # kind = node.get_str("kind", None) if kind is None: - plugin_type = SourceMirror - else: - plugin_type, _ = self.lookup(context.messenger, kind, node) + kind = "legacy" + + plugin_type, _ = self.lookup(context.messenger, kind, node) source_mirror_type = cast(Type[SourceMirror], plugin_type) source_mirror = source_mirror_type(context, project, node) diff --git a/src/buildstream/_project.py b/src/buildstream/_project.py index 1877f4760..ca63d67b0 100644 --- a/src/buildstream/_project.py +++ b/src/buildstream/_project.py @@ -420,7 +420,7 @@ class Project: config = self.config if not alias or alias not in config._aliases: # pylint: disable=unsupported-membership-test - return [(None, None)] + return [None] uri_list: List[Tuple[Optional[SourceMirror], Optional[str]]] = [] policy = self._context.track_source if tracking else self._context.fetch_source @@ -429,18 +429,15 @@ class Project: policy == _SourceUriPolicy.USER and self._mirror_override ): for mirror_name, mirror in config.mirrors.items(): - mirror_uri_list = mirror._get_alias_uris(alias) - if mirror_uri_list: + list_to_add = mirror._get_alias_uris(alias) - list_to_add = [(mirror, uri) for uri in mirror_uri_list] - - if mirror_name == config.default_mirror: - uri_list = list_to_add + uri_list - else: - uri_list += list_to_add + if mirror_name == config.default_mirror: + uri_list = list_to_add + uri_list + else: + uri_list += list_to_add if policy in (_SourceUriPolicy.ALL, _SourceUriPolicy.ALIASES): - uri_list.append((None, config._aliases.get_str(alias))) + uri_list.append(config._aliases.get_str(alias)) return uri_list diff --git a/src/buildstream/plugins/sourcemirrors/legacy.py b/src/buildstream/plugins/sourcemirrors/legacy.py new file mode 100644 index 000000000..6ec028cad --- /dev/null +++ b/src/buildstream/plugins/sourcemirrors/legacy.py @@ -0,0 +1,30 @@ +from typing import Optional, Dict, List, Any, TYPE_CHECKING + +from buildstream import SourceMirror, MappingNode, SequenceNode, SourceMirrorError + + +class AliasesSourceMirror(SourceMirror): + BST_MIN_VERSION = "2.1" # temporary + + def configure(self, node: MappingNode) -> None: + node.validate_keys(SourceMirror.COMMON_CONFIG_KEYS + ["aliases"]) + self._aliases: Dict[str, List[str]] = self._load_aliases(node) + + def _get_alias_uris(self, alias): + if alias in self._aliases: + return self._aliases[alias] + + return [] + + def _load_aliases(self, node: MappingNode) -> Dict[str, List[str]]: + aliases: Dict[str, List[str]] = {} + alias_node: MappingNode = node.get_mapping("aliases") + + for alias, uris in alias_node.items(): + aliases[alias] = uris.as_str_list() + + return aliases + + +def setup(): + return AliasesSourceMirror diff --git a/src/buildstream/source.py b/src/buildstream/source.py index 9ca413645..d92aa244d 100644 --- a/src/buildstream/source.py +++ b/src/buildstream/source.py @@ -387,7 +387,6 @@ class Source(Plugin): meta: MetaSource, variables: Variables, *, - active_mirror: Optional[SourceMirror] = None, alias_override: Optional[Tuple[str, str]] = None, unique_id: Optional[int] = None, ): @@ -415,9 +414,6 @@ class Source(Plugin): # Set of marked download URLs self.__marked_urls: Set[str] = set() - # The active SourceMirror in context of fetch/track - self.__active_mirror: Optional[SourceMirror] = active_mirror - # Collect the composited element configuration and # ask the element to configure itself. self.__init_defaults(project, meta) @@ -754,16 +750,14 @@ class Source(Plugin): # Alias overriding can happen explicitly (by command-line) or # implicitly (the Source being constructed with an __alias_override). # - if self.__active_mirror is not None: - - assert alias_override or self.__alias_override + if alias_override or self.__alias_override: url_alias, url_body = url.split(utils._ALIAS_SEPARATOR, 1) project_alias_url = project.get_alias_url(url_alias, first_pass=self.__first_pass) if self.__alias_override is not None: override_alias = self.__alias_override[0] # type: ignore - override_url = self.__alias_override[1] # type: ignore + override_mirror = self.__alias_override[1] # type: ignore # Implicit alias overrides may only be done for one # specific alias, so that sources that fetch from multiple @@ -773,16 +767,20 @@ class Source(Plugin): if url_alias != override_alias: return url - alias_override = override_url + else: + override_mirror = alias_override + + assert override_mirror is not None, f"{alias_override} or {self.__alias_override}" + if isinstance(override_mirror, str): + return override_mirror + url_body # # Delegate the URL translation to the SourceMirror plugin # - return self.__active_mirror.translate_url( + return override_mirror.translate_url( project_name=project.name, alias=url_alias, alias_url=project_alias_url, - alias_substitute_url=alias_override, source_url=url_body, extra_data=extra_data, ) @@ -1386,7 +1384,7 @@ class Source(Plugin): # primary with either mark_download_url() or # translate_url(). # - def __clone_for_uri(self, mirror, uri): + def __clone_for_uri(self, mirror): project = self._get_project() context = self._get_context() alias = self._get_alias() @@ -1408,8 +1406,7 @@ class Source(Plugin): project, meta, self.__variables, - active_mirror=mirror, - alias_override=(alias, uri), + alias_override=(alias, mirror), unique_id=self._unique_id, ) @@ -1453,12 +1450,9 @@ class Source(Plugin): alias = fetcher._get_alias() last_error = None - for mirror, uri in project.get_alias_uris(alias, first_pass=self.__first_pass, tracking=False): - - self.__active_mirror = mirror - + for mirror in project.get_alias_uris(alias, first_pass=self.__first_pass, tracking=False): try: - fetcher.fetch(uri) + fetcher.fetch(mirror) # FIXME: Need to consider temporary vs. permanent failures, # and how this works with retries. except BstError as e: @@ -1466,12 +1460,10 @@ class Source(Plugin): continue # No error, we're done with this fetcher - self.__active_mirror = None break else: # No break occurred, raise the last detected error - self.__active_mirror = None raise last_error # Default codepath is to reinstantiate the Source @@ -1487,9 +1479,9 @@ class Source(Plugin): return last_error = None - for mirror, uri in project.get_alias_uris(alias, first_pass=self.__first_pass, tracking=False): + for mirror in project.get_alias_uris(alias, first_pass=self.__first_pass, tracking=False): - new_source = self.__clone_for_uri(mirror, uri) + new_source = self.__clone_for_uri(mirror) try: new_source.fetch(**kwargs) # FIXME: Need to consider temporary vs. permanent failures, @@ -1519,9 +1511,8 @@ class Source(Plugin): # NOTE: We are assuming here that tracking only requires substituting the # first alias used last_error = None - for mirror, uri in reversed(project.get_alias_uris(alias, first_pass=self.__first_pass, tracking=True)): - - new_source = self.__clone_for_uri(mirror, uri) + for mirror in reversed(project.get_alias_uris(alias, first_pass=self.__first_pass, tracking=True)): + new_source = self.__clone_for_uri(mirror) try: ref = new_source.track(**kwargs) # pylint: disable=assignment-from-none # FIXME: Need to consider temporary vs. permanent failures, diff --git a/src/buildstream/sourcemirror.py b/src/buildstream/sourcemirror.py index e55f7d8f2..4d9370995 100644 --- a/src/buildstream/sourcemirror.py +++ b/src/buildstream/sourcemirror.py @@ -93,6 +93,13 @@ class SourceMirror(Plugin): # The SourceMirror plugin type is only supported since BuildStream 2.2 BST_MIN_VERSION = "2.2" + COMMON_CONFIG_KEYS = ["name", "kind"] + """Common source config keys + + SourceMirror config keys that must not be accessed in configure(), and + should be checked for using node.validate_keys(). + """ + def __init__( self, context: "Context", @@ -103,22 +110,34 @@ class SourceMirror(Plugin): # the project level base variables, so there is no need # to expand them redundantly here. # - node.validate_keys(["name", "kind", "config", "aliases"]) # Do local base class parsing first name: str = node.get_str("name") - self.__aliases: Dict[str, List[str]] = self.__load_aliases(node) # Chain up to Plugin super().__init__(name, context, project, node, "source-mirror") + self.__aliases: List[str] = None # Plugin specific parsing - config = node.get_mapping("config", default={}) - self._configure(config) + self._configure(node) + + ########################################################## + # Internal API # + ########################################################## + def _get_alias_uris(self, alias: str) -> List: + assert self.__aliases is not None, "Didn't set aliases during configuring time" + if alias in self.__aliases: + return [self] ########################################################## # Public API # ########################################################## + + def set_supported_aliases(self, aliases: List[str]): + """Set the aliases for which `self` can translate urls.""" + assert self._get_configuring(), "Trying to set aliases after configure time" + self.__aliases = aliases + def translate_url( self, *, @@ -141,63 +160,6 @@ class SourceMirror(Plugin): source_url: The URL as specified by original source YAML, excluding the alias extra_data: An optional extra dictionary to return additional data """ - # - # Default implementation behaves in the same way we behaved before - # introducing the SourceMirror plugin. - # - assert alias_substitute_url is not None - - return alias_substitute_url + source_url - - ############################################################# - # Plugin API implementation # - ############################################################# - - # - # Provide a dummy implementation as the base class is used as a default - # - def configure(self, node: MappingNode) -> None: - pass - - ########################################################## - # Internal API # - ########################################################## - - # _get_alias_uris(): - # - # Get a list of URIs for the specified alias - # - # Args: - # alias: The alias to fetch URIs for - # - # Returns: - # A list of URIs for the given alias - # - def _get_alias_uris(self, alias: str) -> List[str]: - - aliases: List[str] - try: - aliases = self.__aliases[alias] - except KeyError: - aliases = [] - - return aliases - - ########################################################## - # Private API # - ########################################################## - def __load_aliases(self, node: MappingNode) -> Dict[str, List[str]]: - - aliases: Dict[str, List[str]] = {} - alias_node: MappingNode = node.get_mapping("aliases") - - for alias, uris in alias_node.items(): - if not isinstance(uris, SequenceNode): - raise LoadError( - "{}: Value of '{}' expected to be a list of strings".format(uris, alias), - LoadErrorReason.INVALID_DATA, - ) - - aliases[alias] = uris.as_str_list() - - return aliases + raise ImplError( + "source mirror plugin '{kind}' does not implement translate_url()".format(kind=self.get_kind()) + ) diff --git a/tests/frontend/mirror.py b/tests/frontend/mirror.py index ef391def8..b58942cf1 100644 --- a/tests/frontend/mirror.py +++ b/tests/frontend/mirror.py @@ -839,42 +839,24 @@ def test_source_mirror_plugin(cli, tmpdir): "name": "middle-earth", "kind": "mirror", "aliases": { - "foo": ["<invalid>"], - "bar": ["<invalid>"], - }, - "config": { - "aliases": { - "foo": ["OOF/"], - "bar": ["RAB/"], - }, + "foo": ["OOF/"], + "bar": ["RAB/"], }, }, { "name": "arrakis", "kind": "mirror", "aliases": { - "foo": ["<invalid>"], - "bar": ["<invalid>"], - }, - "config": { - "aliases": { - "foo": ["%{project-root}/OFO/"], - "bar": ["%{project-root}/RBA/"], - }, + "foo": ["%{project-root}/OFO/"], + "bar": ["%{project-root}/RBA/"], }, }, { "name": "oz", "kind": "mirror", "aliases": { - "foo": ["<invalid>"], - "bar": ["<invalid>"], - }, - "config": { - "aliases": { - "foo": ["ooF/"], - "bar": ["raB/"], - }, + "foo": ["ooF/"], + "bar": ["raB/"], }, }, ], diff --git a/tests/frontend/project/sourcemirrors/mirror.py b/tests/frontend/project/sourcemirrors/mirror.py index 211c736ef..17f0fd58c 100644 --- a/tests/frontend/project/sourcemirrors/mirror.py +++ b/tests/frontend/project/sourcemirrors/mirror.py @@ -11,7 +11,7 @@ class Sample(SourceMirror): BST_MIN_VERSION = "2.0" def configure(self, node): - node.validate_keys(["aliases"]) + node.validate_keys(SourceMirror.COMMON_CONFIG_KEYS + ["aliases"]) self.aliases = {} @@ -19,13 +19,14 @@ class Sample(SourceMirror): for alias_name, url_list in aliases.items(): self.aliases[alias_name] = url_list.as_str_list() + self.set_supported_aliases(self.aliases.keys()) + def translate_url( self, *, project_name: str, alias: str, alias_url: str, - alias_substitute_url: Optional[str], source_url: str, extra_data: Optional[Dict[str, Any]], ) -> str:
