You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@buildstream.apache.org by ak...@apache.org on 2024/03/23 14:57:06 UTC
(buildstream) branch abderrahim/mirror-plugins created (now 61b6b08f2)
This is an automated email from the ASF dual-hosted git repository.
akitouni pushed a change to branch abderrahim/mirror-plugins
in repository https://gitbox.apache.org/repos/asf/buildstream.git
at 61b6b08f2 Rework source mirrors
This branch includes the following new commits:
new 61b6b08f2 Rework source mirrors
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
(buildstream) 01/01: Rework source mirrors
Posted by ak...@apache.org.
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 <ab...@codethink.co.uk>
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: