gtristan commented on code in PR #2019:
URL: https://github.com/apache/buildstream/pull/2019#discussion_r2126136270
##########
src/buildstream/_pluginfactory/pluginoriginpip.py:
##########
@@ -72,7 +72,7 @@ def get_plugin_paths(self, kind, plugin_type):
reason="package-not-found",
) from e
- if dist.version not in package.specifier:
+ if not package.specifier.contains(dist.version, prereleases=True):
Review Comment:
I'm not sure that it makes sense to blanket accept prereleases.
Does this mean that the following would accept both
`buildstream-plugins-2.5.0` *and* any prerelease such as
`buildstream-plugins-2.5.0.dev0` or `buildstream-plugins-2.5.0.dev1` etc ?
```yaml
plugins:
origin: pip
package-name: buildstream-plugins==2.5.0
```
Looking at
https://docs.buildstream.build/master/format_project.html#pip-plugins, it seems
that we don't have sufficient configuration for proper specificity here right ?
Looking further at what is supported by the python version requirement
format which we purport to support, in
https://peps.python.org/pep-0440/#developmental-releases and in this SO post:
https://stackoverflow.com/questions/38762416/how-to-specify-python-requirements-by-allowing-prereleases
It would appear that we don't need an explicit API extension to support
correct behavior.
Rather, we should be able to specify the following:
```yaml
plugins:
origin: pip
package-name: buildstream-plugins>=2.5.0
```
To mean any stable release with 2.5.0 or greater (or use `~=` to specify any
`2.5.x` release).
And we should be able to specify:
```yaml
plugins:
origin: pip
package-name: buildstream-plugins>=2.5.0.dev0
```
To mean any `2.5.0` prerelease or greater, where `2.5.0` always sorts as
*newer* than any prerelease.
In this case it's unclear to me if `>=2.5.0.dev0` would allow `2.6.0.dev1`
to match, this probably needs clarification in our thinking in order to know
what pattern to recommend, possibly depending on a prerelease is more safe when
specifying `~=2.5.0.dev0`, meaning any `2.5.x` release *including prereleases*.
Lets get some clarity on the above question about whether depending on `>=`
prerelases includes *future prereleases of other versions*, and find out
whether this patch satisfies the python requirement expression format correctly.
--
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]
