Fix for handling of version ranges

[dimbleby]

don't include first prerelease in the allowed max for an exclusive range.

Fixes python-poetry/poetry#8475, python-poetry/poetry#8405, python-poetry/poetry#8202

(actually 8202 seems to have been resolved by changes in the available dependencies on pypi and so I can't reproduce it now: but I'm pretty sure these will all be the same)

Absurdly I can't find a simple testcase that takes two versions or version ranges and compares or intersects them or something to demonstrate the point. So I confess that I don't fully understand what was going wrong before this fix.

However I do understand that those issues are

• something to do with wildcard constraints (eg =2.*) which poetry interprets as having a lower bound that is the first dev-release (eg 2.dev0)
• and something to do with how they interact with regular but adjacent constraints like <2

so I've made this fix somewhat on gut: <2 certainly shouldn't allow 2.dev0 and so it's weird that allowed_max would return that value.

What can I say? I'm confused, but this looks like a good fix and it does resolve those issues...

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

[dimbleby]

I now think that the real problem - or anyway among the real problems - is ambiguity in poetry-core's treatment of the max in a strict inequality range.

• when dealing with a parsed value such as <2 the max is 2 but the allowed_max should be 2.dev0, because pre-releases are excluded by <
• however when dealing with a value calculated by operations between other versions or ranges, the max should be considered to be correct as it is, not to be revised by allowed_max
  • eg consider (>1,<3).difference(2). This should come out as >1,<2 || >2,<3: except that in the first part of that prereleases should be allowed and so subsequent calculations that treat this as though it were >1,<2.dev0 are wrong

A possible approach is to handle this at parsing and abandon allowed_max altogether. ie parse_single_constraint("<2") should construct the object with max=2.dev0 all along. Then never use allowed_max at all.

This also solves those issues reported in poetry - but causes carnage in the test scripts: the main (but probably not only) problem being that parsing and then printing <2 gives <2.dev0 - which should indeed be equivalent, but is not so pretty...

[radoering]

A possible approach is to handle this at parsing and abandon allowed_max altogether. ie parse_single_constraint("<2") should construct the object with max=2.dev0 all along. Then never use allowed_max at all.

This also solves those issues reported in poetry - but causes carnage in the test scripts: the main (but probably not only) problem being that parsing and then printing <2 gives <2.dev0 - which should indeed be equivalent, but is not so pretty...

If we went down this path, maybe we should also change __str__ so that <2 and <2.dev0 would always be printed as <2? I suppose explicit constraints like <2.dev0 are rare so that complaints that <2.dev0 gives <2 are neglectable.

[dimbleby]

that seems plausible and I have a branch in which I tried it. Among the chaos in the unit tests I see at least these wrinkles

• lots of failures where the unit tests compare a parsed value to an explicitly constructed Version and now get a different answer: to be expected, but will be tedious to work through
• there's a distinction between version ranges as used in version requirements, and versions as used in markers / python versions. So as not to break the world it's probably unwise to make changes for the latter two
• the good news is that printing <2.dev0 as just <2 is I think fine in the PEP440 context; because the rules about prereleases give these strings the same meaning
• this approach both fixes and introduces bugs when dealing with != specifiers
  • eg !=2 is represented internally as <2 || >2. Today, the allowed_max business re-interprets that as <2.dev0 || >2 ie it wrongly disallows the prereleases
  • eg after the change, (>1).union(!=2) for some reason becomes >1,<2 || >2. Without allowed_max that's correct as an internal representation, but becomes wrong once re-parsed - it really does want to be written eg in lockfiles as >1,!=2

probably there's more that I haven't yet spotted

This feels like the right direction conceptually but I've no idea when or whether I'm likely to find the time or energy to do anything about it