Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Version matching does not conform to PEP440 when specifier sets contain pre-release versions #2271

Closed
3 tasks done
charmasaur opened this issue Apr 6, 2020 · 7 comments · Fixed by python-poetry/poetry-core#128
Labels
kind/bug Something isn't working as expected

Comments

@charmasaur
Copy link

  • I am on the latest Poetry version.
  • I have searched the issues of this repo and believe that this is not a duplicate.
  • If an exception occurs when executing a command, I executed it again in debug mode (-vvv option).

Issue

Poetry version matching doesn't seem to conform to PEP440, specifically for specifier sets containing pre-release versions and inequality constraints.

According to https://www.python.org/dev/peps/pep-0440/#exclusive-ordered-comparison: "The exclusive ordered comparison <V MUST NOT allow a pre-release of the specified version unless the specified version is itself a pre-release."

From the lock file generated from the pyproject.toml file in the gist above, we have:

<snip>
tensorflow-estimator = ">=2.1.0rc0,<2.2.0"
<snip>
[[package]]
category = "main"
description = "TensorFlow Estimator."
name = "tensorflow-estimator"
optional = false
python-versions = "*"
version = "2.2.0rc0"

That is, despite the constraint "<2.2.0" we're getting the version "2.2.0rc0".

The issue seems to be https://github.com/python-poetry/poetry/blob/master/poetry/semver/version_range.py#L63, which doesn't account for pre-release versions.

Incidentally, when trying to hunt down the cause of this issue I also came across https://github.com/python-poetry/poetry/blob/master/poetry/version/specifiers.py#L715, which doesn't seem to be used but does look to have a similar issue: if any of the specifiers in the set allow prereleases, the entire set is considered to allow prereleases, which will mean that a constraint like ">=2.1.0rc0,<2.2.0" is considered to allow prereleases and will thus erroneously allow "2.2.0rc0".

@az0uz
Copy link

az0uz commented Apr 15, 2020

I think the version constrain solver is at fault here:

from poetry.semver import parse_constraint, Version
parse_constraint("~=2.1.0").allows(Version.parse("2.2.0rc0"))

returns True
It seems poetry converts parse_constraint("~=2.1.0") to >=2.1.0,<2.2.0, which I guess is wrong because then we have 2.2.0rc0 < 2.2.0 (which is true)
parse_constraint("~=2.1.0") should be >=2.1.0, <2.2.0a0

@tomzx
Copy link
Contributor

tomzx commented Aug 7, 2020

After some investigation, it appears the cause is in poetry-core dependency.py.

https://github.com/python-poetry/poetry-core/blame/master/poetry/core/packages/dependency.py#L46-L49

Since the minimum version is a prerelease (in your case 2.2.0rc0), then it converts the dependency into a "allow-prereleases" dependency which will let it install any prerelease version. In your case, the interesting thing is that any prerelease of 2.2.0 are also considered as valid as @az0uz explained.

Here it seems that prerelease should convert the constraint into one that is similar to what @az0uz suggested since what the "prerelease" property seems to want to let you do is install prereleases that would match the same version as the minimum constraint but not the maximum version specified.

In the original example, you either have to remove the rc0 bit (>=2,1,0,<2.2.0) so it will not consider any prereleases of both 2.1.0 and 2.2.0 or replace the constraint with one mentioned by @az0uz (>=2.1.0rc0,<2.2.0a0).

@ghost
Copy link

ghost commented Aug 11, 2020

Can reproduce with package numba.

numba (0.50.1) depends on llvmlite (>=0.33.0.dev0,<0.34) but then 0.34.0rc1 gets installed.

test code

from poetry.core.semver import parse_constraint, Version
range = parse_constraint('>=0.33.0.dev0,<0.34')
rcver = Version.parse('0.34.0rc1')
range.allows(rcver)  # returns True

The problem seems to be that https://github.com/python-poetry/poetry-core/blob/28848f06e3201d511815f8b00e25a77b2ca82d37/poetry/core/semver/version_range.py#L75 doesn't account for the version being a pre-release.

@unimonkiez
Copy link

Can reproduce with package numba.

numba (0.50.1) depends on llvmlite (>=0.33.0.dev0,<0.34) but then 0.34.0rc1 gets installed.

test code

from poetry.core.semver import parse_constraint, Version
range = parse_constraint('>=0.33.0.dev0,<0.34')
rcver = Version.parse('0.34.0rc1')
range.allows(rcver)  # returns True

The problem seems to be that https://github.com/python-poetry/poetry-core/blob/28848f06e3201d511815f8b00e25a77b2ca82d37/poetry/core/semver/version_range.py#L75 doesn't account for the version being a pre-release.

Same thing happens in our project, I think setuputils has that problem too..

@finswimmer
Copy link
Member

For me this isn't looking like a bug. Some more paragraphs from PEP 440:

The exclusive ordered comparison >V MUST NOT allow a post-release of the given version unless V itself is a post release. You may mandate that releases are later than a particular post release, including additional post releases, by using >V.postN. For example, >1.7 will allow 1.7.1 but not 1.7.0.post1 and >1.7.post2 will allow 1.7.1 and 1.7.0.post3 but not 1.7.0.

The exclusive ordered comparison >V MUST NOT match a local version of the specified version.

The exclusive ordered comparison <V MUST NOT allow a pre-release of the specified version unless the specified version is itself a pre-release. Allowing pre-releases that are earlier than, but not equal to a specific pre-release may be accomplished by using <V.rc1 or similar.

My understanding of this is, that as soon there is a pre-release specified, any pre-release within the given range is allowed.

fin swimmer

@charmasaur
Copy link
Author

Personally I don't interpret the PEP that way. It also states The comma (",") is equivalent to a logical and operator: a candidate version must match all given version clauses in order to match the specifier as a whole., which to me suggests that the clauses are treated independently before eventually being ANDed. With that in mind, I don't think the pre-release in one clause affects the interpretation of the other. Also, The exclusive ordered comparison <V MUST NOT allow a pre-release of the specified version unless the specified version is itself a pre-release. seems unambiguous to me: if V is not a pre-release, then <V must not allow pre-releases of that version.

Copy link

github-actions bot commented Mar 2, 2024

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
kind/bug Something isn't working as expected
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants