You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@buildstream.apache.org by "abderrahim (via GitHub)" <gi...@apache.org> on 2025/06/03 18:19:18 UTC
[PR] pluginoriginpip: allow prereleases [buildstream]
abderrahim opened a new pull request, #2019:
URL: https://github.com/apache/buildstream/pull/2019
This was a regression introduced by the move to use importlib and packaging instead of pkg_resources in 10b45216684a5de8f8770efbf1821f03f16b9e73
--
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: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] pluginoriginpip: allow prereleases [buildstream]
Posted by "abderrahim (via GitHub)" <gi...@apache.org>.
abderrahim commented on code in PR #2019:
URL: https://github.com/apache/buildstream/pull/2019#discussion_r2143194017
##########
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 would look at it from a different angle: I consider this a regression introduced by https://github.com/apache/buildstream/commit/10b45216684a5de8f8770efbf1821f03f16b9e73.
Before that commit (bst 2.3 and earlier) the expression
```yaml
plugins:
origin: pip
package-name: buildstream-plugins
```
would accept any version of buildstream-plugins, but now (2.4 and newer) it does not accept prereleases anymore. (This is the reason why buildstream-plugins-community testsuite currently fails with buildstream master).
Now I would argue that both interpretations of whether or not to accept pre-releases are correct. They correspond to what pip would do when passing `--pre` or not passing it, respectively.
What I consider to be different between pip and buildstream is that buildstream doesn't install these packages. So if the user has already installed a prerelease of buildstream-plugins-community, we should let them use it rather than tell them, unhelpfully,
```
buildstream-plugins-community 2.0.5.dev21+g07c4b9e is installed but buildstream_plugins_community is required
```
--
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: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] pluginoriginpip: allow prereleases [buildstream]
Posted by "gtristan (via GitHub)" <gi...@apache.org>.
gtristan commented on code in PR #2019:
URL: https://github.com/apache/buildstream/pull/2019#discussion_r2186699367
##########
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:
@abderrahim writes:
> What I consider to be different between pip and buildstream is that buildstream doesn't install these packages.
Agree.
> So if the user has already installed a prerelease of buildstream-plugins-community, we should let them use it
Disagree.
The purpose of the version string for plugins in the pip origin is for the project to make an assertion on it's requirements.
To avoid confusion, this version constraint should follow the pep and refuse to accept a prerelease if the version constraint expressed would not allow for a prerelease according to the pep 0440.
**If** it is the case that `buildstream-plugins-community` without any specific version allowed for prereleases in the past, but are not supposed to allow for that according to the pep 0440, then I think it would be good to fix that even if it changes buildstream behavior, and declare that this behavior was anyway broken until we now fixed it.
--
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: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] pluginoriginpip: allow prereleases [buildstream]
Posted by "gtristan (via GitHub)" <gi...@apache.org>.
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: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] pluginoriginpip: allow prereleases [buildstream]
Posted by "gtristan (via GitHub)" <gi...@apache.org>.
gtristan commented on code in PR #2019:
URL: https://github.com/apache/buildstream/pull/2019#discussion_r2186699367
##########
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:
@abderrahim writes:
> What I consider to be different between pip and buildstream is that buildstream doesn't install these packages.
Agree.
> So if the user has already installed a prerelease of buildstream-plugins-community, we should let them use it
Disagree.
The purpose of the version string for plugins in the pip origin is for the project to make an assertion on it's requirements.
To avoid confusion, this version constraint should follow the pep and refuse to accept a prerelease if the version constraint expressed would not allow for a prerelease according to the pep 0440.
**If** it is the case that `buildstream-plugins-community` without any specific version allowed for prereleases in the past, but are not supposed to allow for that according to the pep 0440, then I think it would be good to fix that even if it changes buildstream behavior, and declare that this behavior was anyway broken until we now fixed it.
As there has been discussion of API addition for a separate flag here, I would rather argue that it could make sense to have some way of forcing buildstream to ignore the version specifiers for pip loaded plugins. Possibly CLI flag or env var, e.g.: `BST_DANGEROUS_IGNORE_PIP_PLUGIN_VERSION_REQUIREMENTS_IM_TESTING_HERE=1`.
--
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: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] pluginoriginpip: allow prereleases [buildstream]
Posted by "juergbi (via GitHub)" <gi...@apache.org>.
juergbi commented on code in PR #2019:
URL: https://github.com/apache/buildstream/pull/2019#discussion_r2185492751
##########
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:
> What I consider to be different between pip and buildstream is that buildstream doesn't install these packages. So if the user has already installed a prerelease of buildstream-plugins-community, we should let them use it rather than tell them, unhelpfully,
That makes sense to me. If we were to install packages, I would definitely want to skip pre-releases by default. However, I don't think it's necessary to reject pre-releases installed by the user as long as they still fulfill versioning specifiers (if present). And as this was an unintentional change in behavior, I'm in favor of restoring the previous behavior.
An extra flag for this could be discussed if there is a real use case where the versioning specifiers are insufficient, but I don't consider that a blocker for this.
--
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: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] pluginoriginpip: allow prereleases [buildstream]
Posted by "abderrahim (via GitHub)" <gi...@apache.org>.
abderrahim commented on code in PR #2019:
URL: https://github.com/apache/buildstream/pull/2019#discussion_r2187049798
##########
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:
Look, I'm not trying to invent a new behaviour. This is how python tools work, and there is an option in the pypa `packaging` package I'm using that implements this behaviour. But if you want the spec, here is what PEP 440 says (emphasis mine):
> Pre-releases of any kind, including developmental releases, are implicitly excluded from all version specifiers, **unless they are already present on the system**, explicitly requested by the user, or if the only available version that satisfies the version specifier is a pre-release.
--
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: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] pluginoriginpip: allow prereleases [buildstream]
Posted by "gtristan (via GitHub)" <gi...@apache.org>.
gtristan commented on code in PR #2019:
URL: https://github.com/apache/buildstream/pull/2019#discussion_r2187965003
##########
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:
> Look, I'm not trying to invent a new behaviour.
Thanks for providing that excerpt from the pep and making this clear to me :)
I'm perfectly happy with this change with your clarifications.
--
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: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] pluginoriginpip: allow prereleases [buildstream]
Posted by "abderrahim (via GitHub)" <gi...@apache.org>.
abderrahim merged PR #2019:
URL: https://github.com/apache/buildstream/pull/2019
--
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: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] pluginoriginpip: allow prereleases [buildstream]
Posted by "gtristan (via GitHub)" <gi...@apache.org>.
gtristan commented on code in PR #2019:
URL: https://github.com/apache/buildstream/pull/2019#discussion_r2186699367
##########
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:
@abderrahim writes:
> What I consider to be different between pip and buildstream is that buildstream doesn't install these packages.
~~Agree~~. Disagree, this is not a defining property.
When I install a package with `pip install <package>` or `pip install .`, then pip will refuse the install with a sensible error when the required package is already installed in the target environment with a mismatched version - this is very similar to what BuildStream is doing.
> So if the user has already installed a prerelease of buildstream-plugins-community, we should let them use it
Disagree.
The purpose of the version string for plugins in the pip origin is for the project to make an assertion on it's requirements.
To avoid confusion, this version constraint should follow the pep and refuse to accept a prerelease if the version constraint expressed would not allow for a prerelease according to the pep 0440.
**If** it is the case that `buildstream-plugins-community` without any specific version allowed for prereleases in the past, but are not supposed to allow for that according to the pep 0440, then I think it would be good to fix that even if it changes buildstream behavior, and declare that this behavior was anyway broken until we now fixed it.
As there has been discussion of API addition for a separate flag here, I would rather argue that it could make sense to have some way of forcing buildstream to ignore the version specifiers for pip loaded plugins. Possibly CLI flag or env var, e.g.: `BST_DANGEROUS_IGNORE_PIP_PLUGIN_VERSION_REQUIREMENTS_IM_TESTING_HERE=1`.
--
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: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] pluginoriginpip: allow prereleases [buildstream]
Posted by "gtristan (via GitHub)" <gi...@apache.org>.
gtristan commented on code in PR #2019:
URL: https://github.com/apache/buildstream/pull/2019#discussion_r2186699367
##########
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:
@abderrahim writes:
> What I consider to be different between pip and buildstream is that buildstream doesn't install these packages.
~~Agree~~. Disagree, this is not a defining property.
When I install a package with `pip install <package>` or `pip install .`, then pip will refuse the install with a sensible error when a package dependency is already installed in the target environment with a mismatched version - this is very similar to what BuildStream is doing.
> So if the user has already installed a prerelease of buildstream-plugins-community, we should let them use it
Disagree.
The purpose of the version string for plugins in the pip origin is for the project to make an assertion on it's requirements.
To avoid confusion, this version constraint should follow the pep and refuse to accept a prerelease if the version constraint expressed would not allow for a prerelease according to the pep 0440.
**If** it is the case that `buildstream-plugins-community` without any specific version allowed for prereleases in the past, but are not supposed to allow for that according to the pep 0440, then I think it would be good to fix that even if it changes buildstream behavior, and declare that this behavior was anyway broken until we now fixed it.
As there has been discussion of API addition for a separate flag here, I would rather argue that it could make sense to have some way of forcing buildstream to ignore the version specifiers for pip loaded plugins. Possibly CLI flag or env var, e.g.: `BST_DANGEROUS_IGNORE_PIP_PLUGIN_VERSION_REQUIREMENTS_IM_TESTING_HERE=1`.
--
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: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org