gtristan commented on code in PR #1890:
URL: https://github.com/apache/buildstream/pull/1890#discussion_r1527095613
##########
tests/frontend/mirror.py:
##########
@@ -807,3 +807,93 @@ def test_mirror_expand_project_and_toplevel_root(cli,
tmpdir):
# Success if the expanded %{project-root} is found
assert foo_str in contents
assert bar_str in contents
+
+
+# Test a simple SourceMirror implementation which reads
+# plugin configuration and behaves in the same way as default
+# mirrors but using data in the plugin configuration instead.
+#
[email protected](DATA_DIR)
[email protected]("datafiles")
+def test_source_mirror_plugin(cli, tmpdir):
+ output_file = os.path.join(str(tmpdir), "output.txt")
+ project_dir = str(tmpdir)
+ element_dir = os.path.join(project_dir, "elements")
+ os.makedirs(element_dir, exist_ok=True)
+ element_name = "test.bst"
+ element_path = os.path.join(element_dir, element_name)
+ element = generate_element(output_file)
+ _yaml.roundtrip_dump(element, element_path)
+
+ project_file = os.path.join(project_dir, "project.conf")
+ project = {
+ "name": "test",
+ "min-version": "2.0",
+ "element-path": "elements",
+ "aliases": {
+ "foo": "FOO/",
+ "bar": "BAR/",
+ },
+ "mirrors": [
+ {
+ "name": "middle-earth",
+ "kind": "mirror",
+ "aliases": {
+ "foo": ["<invalid>"],
Review Comment:
I understand what you're saying now.
I would have rather expected:
```yaml
- name: freedesktop-sdk-raw-file-mirrors
kind: gitlab_lfs_mirror
aliases:
cpan: https://gitlab.com/freedesktop-sdk/mirrors/{alias}/raw-files
crates: https://gitlab.com/freedesktop-sdk/mirrors/{alias}/raw-files
ftp_gnu_org: https://gitlab.com/freedesktop-sdk/mirrors/{alias}/raw-files
...
```
I.e. we're still expecting the existing mirror configuration to be valuable,
and used in the default `SourceMirror` implementation (base class).
There are two paths I can see towards supporting this.
## Allow list-or-dict declaration of the aliases
In your example:
```yaml
- name: freedesktop-sdk-raw-file-mirrors
kind: gitlab_lfs_mirror
url: https://gitlab.com/
project: freedesktop-sdk/mirrors/{alias}/raw-files
aliases:
- cpan
- crates
- ftp_gnu_org
- github_files
```
This could be supported on the condition that the mirror is a user plugin,
and would result in passing `None` values for the configured URI passed through
`SourceMirror.translate_url()`.
## Support non aliased URIs
In your example:
```yaml
- name: freedesktop-sdk-raw-file-mirrors
kind: gitlab_lfs_mirror
config:
url: https://gitlab.com/
project: freedesktop-sdk/mirrors/{alias}/raw-files
aliases:
- cpan
- crates
- ftp_gnu_org
- github_files
```
This will require restructuring the `Source.__do_fetch()` and
`Source.__do_track()` loops and is a bit more involved.
I.e. we would want a mirror to be allowed to apply to _"any alias"_ and/or
_"no alias"_.
For the _"no alias"_ case it is a bit more clear cut, afaics there are
specific code blocks where we simply do not as the `Project` about the urls for
aliases and directly invoke the plugin implementation.
### For plugins who want to support _"any alias"_
For the _"any alias"_ case, maybe we would want a `None` URL with a
`SourceMirror` tuple to be returned in the list from `Project.get_alias_uris()`
(allowing the project to continue to respect the user configuration of
priorities).
Both cases will require that `Source.__clone_for_uri()` work with a `None`
uri and/or a `None` alias.
The last part, for blanket mirrors to not need to specify the alias/[urls],
it would make sense for `Source.translate_url()` to be allowed to _"not work"_
when given unrecognized aliases.
This could allow specification of multiple SourceMirror plugins which do
things in different ways, and can bail out on aliases/urls which that plugin
does not support, also it allows falling back to other mirrors or original
project alias if the user has configured `SourceUriPolicy.ALL` and the mirror
failed to produce a URI.
This is tricky to accomplish with the current code structure though. At
fetch/track time it could work by raising an exception, allowing the loop to
continue when the SourceMirror does:
```python
def translate_url(self, ... alias ...):
if i_dont_know_this(alias):
raise SourceMirrorUnhandled()
```
This would abort the plugin code calling into `Source.translate_url()`
without breaking existing plugin code, as long as mirrors are never used in
`Plugin.configure()` time and only at `Source.fetch()` and `Source.track()`
time (we would only end up throwing away cloned Sources in the case that a URI
could not be produced, and move on to the next mirror/uri).
--
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]
