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

Unexpected handling of versions with "local" components by pip #275

Closed
ashb opened this issue Feb 19, 2020 · 14 comments
Closed

Unexpected handling of versions with "local" components by pip #275

ashb opened this issue Feb 19, 2020 · 14 comments

Comments

@ashb
Copy link

ashb commented Feb 19, 2020

Environment

  • pip version: pip 20.0.2 from /home/ash/.virtualenvs/clean/lib/python3.7/site-packages/pip (python 3.7)
  • Python version: 3.7
  • OS: linux

Description

I am investigating using version epochs for a local/private "fork" of a project, and want to push out updates without accidental upgrading to the upstream version.

So I have specified my requirement as apache-airflow>=1!1.10.7+local.6, and since there is an epoch in there (1! prefix) I should expect that to fail. As per PEP-440 Summary of permitted suffixes and relative ordering

The epoch segment of version identifiers MUST be sorted according to the numeric value of the given epoch. If no epoch segment is present, the implicit numeric value is 0.

However it appears that the implicit epoch is being treated as 1.

Collecting apache-airflow>=1!1.10.7+local.6
  Created temporary directory: /tmp/pip-unpack-jitssjpx
  Starting new HTTPS connection (1): files.pythonhosted.org:443
  https://files.pythonhosted.org:443 "GET /packages/d4/8e/109906b658256027467e34a9f2e0c82a133ff8b39b8f08c73ee03334b641/apache_airflow-1.10.9-py2.py3-none-any.whl HTTP/1.1" 200 4639908
  Downloading apache_airflow-1.10.9-py2.py3-none-any.whl (4.6 MB)

If I change the explicit epoch I required to 2! then it works as expected:

No matching distribution found for apache-airflow>=2!

(Yes I acknowledge I am abusing version epochs for this purpose, but this behaviour is still against what the spec says)

@uranusjr
Copy link
Member

This seems like a bug in packaging.specifiers. I can reproduce with:

from packaging import version
from packaging.specifiers import SpecifierSet

version.parse('1.10.9') in SpecifierSet('>=1!1.10.7+local.6')  # True

# Interestingly though, Version itself gets it right.
version.parse('1.10.9') >= version.parse('1!1.10.7+local.6')  # False

@uranusjr
Copy link
Member

uranusjr commented Feb 20, 2020

Hmm, I think the problem is not about epoch, but the local version identifier (+local.6) component in the specifier. The result is correct if you remove that part:

>>> version.parse('1.10.9') in specifiers.Specifier('>=1!1.10.7')
False

I found this paragraph in PEP 440:

Inclusive ordered comparison

[…] Local version identifiers are NOT permitted in this version specifier.

And indeed, packaging does not parse it as a PEP 440 specifier, but a “legacy” one:

>>> specifiers.Specifier('>=1!1.10.7+local.6')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "packaging/specifiers.py", line 111, in __init__
    raise InvalidSpecifier("Invalid specifier: '{0}'".format(spec))
packaging.specifiers.InvalidSpecifier: Invalid specifier: '>=1!1.10.7+local.6'
>>> list(specifiers.SpecifierSet('>=1!1.10.7+local.6'))
[<LegacySpecifier('>=1!1.10.7+local.6')>]

The confusing part is that packaging (and thus pip) uses literal comparison when dealing with legacy versions and specifiers, so you’re basically doing

'1.10.9' >= '1!1.10.7+local.6'

which would return True because . is U+0046, and ! is U+0033.

I’m not sure what the best resolution is here. We already promised to maintain the possibility to compare arbitrary strings, and can’t outright reject >=1!1.10.7+local.6 since there’s no way to tell whether the user actually has this legacy specifier, or in fact made a mistake when writing a PEP 440 (non-legacy) specifier. Should we accept this as an unsolvable edge case? I guess pip could print a message along the lines like hey you’re using a specifier incompatible to PEP 440; ignore me if this is actually your intention, otherwise heads-up?

@ashb
Copy link
Author

ashb commented Feb 20, 2020

Ohhh - I hit all the edge cases there :) I didn't read the full spec (clearly), but I assumed that it would only fallback to a string compare on the local part, not the full version specifier.

Hmm.

@ashb
Copy link
Author

ashb commented Feb 20, 2020

I read this bit of the spec about local releases:

The inclusion of the local version label makes it possible to differentiate upstream releases from potentially altered rebuilds by downstream integrators. The use of a local version identifier does not affect the kind of a release but, when applied to a source distribution, does indicate that it may not contain the exact same code as the corresponding upstream release.

And went "this is exactly what I'm doing"!

The spec says and then this a few paragraphs later:

Comparison and ordering of local versions considers each segment of the local version (divided by a .) separately. If a segment consists entirely of ASCII digits then that section should be considered an integer for comparison purposes and if a segment contains any ASCII letters then that segment is compared lexicographically with case insensitivity. When comparing a numeric and lexicographic segment, the numeric section always compares as greater than the lexicographic segment. Additionally a local version with a great number of segments will always compare as greater than a local version with fewer segments, as long as the shorter local version's segments match the beginning of the longer local version's segments exactly.

That bit about "Comparison and ordering of local versions" seems entirely pointless if the non-strict ordering comparisons (compatible, inclusive and exclusive ordered comparison) say local versions aren't permitted -- leaving only ==, != and ===

Sounds like a bug in the spec?, as disallowing local versions means that the structure of the local version is never checked.

@ashb
Copy link
Author

ashb commented Feb 20, 2020

I did all my testing of this with packaging.version, which is why I thought the bug was around epochs:

In [29]: v1 = packaging.version.parse('1!1.10.7+local.6')                                                                                                     

In [30]: v2 = packaging.version.parse('1.10.9')                                                                                                               

In [31]: v1 < v2                                                                                                                                              
Out[31]: False

In [32]: v2 < v1                                                                                                                                              
Out[32]: True

In [33]: v3 = packaging.version.parse('1!1.10.7+local.7')                                                                                                     

In [34]: v1 < v3                                                                                                                                              
Out[34]: True

In [35]: v3 < v1                                                                                                                                              
Out[35]: False

Which from the section about local versions was how I expected pip to behave too

@ashb
Copy link
Author

ashb commented Feb 20, 2020

And packaging.version does more than a string compare of local version parts:

In [29]: v1 = packaging.version.parse('1!1.10.7+local.6')                                                                                                     

In [36]: v3 = packaging.version.parse('1!1.10.7+local.10')                                                                                                    

In [37]: v3 < v1                                                                                                                                              
Out[37]: False

In [38]: v1 < v3                                                                                                                                              
Out[38]: True

@uranusjr
Copy link
Member

uranusjr commented Feb 20, 2020

That bit about "Comparison and ordering of local versions" seems entirely pointless if the non-strict ordering comparisons (compatible, inclusive and exclusive ordered comparison) say local versions aren't permitted -- leaving only ==, != and ===

I think the implication is that you can release multiple local versions, but you can’t make that show in the specifier. So you are allowed to release multiple local versions, and they are ordered (so local.10 is chosen instead of local.1 when you specify >=1.10.7), but you can’t say I want only local.2 or later in the specifier. I am not aware why the PEP thinks it is necessary to disallow local versions in the specifier, but IMO that does not make the local version ordering section pointless.

@uranusjr
Copy link
Member

Note: This thread probably should be moved to either pypa/packaging or pypa/packaging-problems to make it more visible for people more familiar with the spec.

/cc @pradyunsg

@pradyunsg pradyunsg transferred this issue from pypa/pip Feb 20, 2020
@pradyunsg
Copy link
Member

Taking a customary skim through this, ISTM that this is indeed a case of hitting a whole bunch of edge cases. :)

It'll probably take a bit more time to understand the nuances than I have right now. For now, I've transferred this to pypa/packaging.

@ashb
Copy link
Author

ashb commented Feb 20, 2020

I think the quick summary is:

Pip follows the PEP-440 spec as defined around local version comopnents, but I (naievly) assumed it would follow what ever packaging.version did with it's ordering/comparison of Version objects (and I checked they were "valid", not LegacyVersion after parsing)

@ashb ashb changed the title Implicit Version Epoch is being treated as 1, not 0. Unexpected handling of versions with "local" components by pip Feb 20, 2020
@pradyunsg
Copy link
Member

If pip's (and therefore packaging's) behaviour is consistent with pep 440, I don't think there's anything actionable here.

@ashb
Copy link
Author

ashb commented Feb 24, 2020

I'd like the PEP to be changed/relaxed if possible! It makes local versions much much harder to use, and makes them behave very differently to debian etc.

I'd like the spec to treat local versions how ever packaging.version is treating them.

Edit: at least for >= etc. Coming up with a definition that would work for ~=/compatible releases is hard, but the spec already defines how to sort local versions. I just want it to apply that more often.

@pradyunsg
Copy link
Member

@ashb I see -- in that case, consider clearly articulating why (and possibly how) the behavior should change at https://discuss.python.org/c/packaging, to make a case for changing the behavior.

Updates to such standards need a discussion for concensus-building (which may, or may not, be trivial) that there's a problem and then, how we want to solve it. :)

@pradyunsg
Copy link
Member

Closing since #321 is the approach we've taken here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants