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

Removal of LegacyVersion and LegacySpecifier #530

Closed
pradyunsg opened this issue Apr 2, 2022 · 18 comments · Fixed by #407
Closed

Removal of LegacyVersion and LegacySpecifier #530

pradyunsg opened this issue Apr 2, 2022 · 18 comments · Fixed by #407

Comments

@pradyunsg
Copy link
Member

pradyunsg commented Apr 2, 2022

This issue is an obvious follow up to #321. I'm mostly filing this to serve as the issue that the changelog points to, for coalescing the discussion/planning for this change into a single location, and for having the PR close an issue. :)


>>> # before 22.0
>>> packaging.version.parse("This is a completely random string")
<LegacyVersion('This is a completely random string')>
>>> # after 22.0
>>> packaging.version.parse("This is a completely random string")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "[...]/.venv/lib/python3.10/site-packages/packaging/version.py", line 52, in parse
    return Version(version)
  File "[...]/.venv/lib/python3.10/site-packages/packaging/version.py", line 197, in __init__
    raise InvalidVersion(f"Invalid version: '{version}'")
packaging.version.InvalidVersion: Invalid version: 'This is a completely random string'

This "feature" has been deprecated (#321) for nearly two years as of April 2022.

PyPI has not permitted uploading packages with invalid versions for even more years. The latest versions of pip should be rejecting/erroring out on wheels with such versions as well. The stricter metadata validation helps pip's dependency resolver's logic, along with helping the broader ecology avoid needing to deal with outside-of-standard tooling/behaviours.

For users who might be impacted by this, they have hopefully caught this issue in (a) their test suites, (b) through the "warnings" presented during the execution of legacy code, during the deprecation period, or (c) as part of warnings presented by pip.

Users who encounter breakage due to this change can adopt the following broad strategy for dealing with it:

  1. Pin to an older version of packaging (or pip, if your error contains pip._vendor.packaging in the traceback), to deal with the immediate breakage.
  2. Modify the versions / specifiers that they use, to be compatible with PEP 440's specification. In practice, this means that if your Requirement/Version/Specifier can be parsed by the Requirement, Version or SpecifierSet classes from this package, it can be used going forward. Notably, you won't be able to compare between legacy versions outside of the === arbitrary-equality operator.
  3. Update their development/deployment processes to catch deprecation warnings before they become breakages in their workflows, or to mitigate the propagation of potentially-breaking-changes upstream making it into their development workflows.

If there's anything we can do to help you catch these issues earlier, please write your suggestion in a comment with a 💡 emoji in it (:bulb:). :)

@pradyunsg
Copy link
Member Author

pradyunsg commented Apr 2, 2022

This is what the pip warnings/errors should look like, to users:

  WARNING: Built wheel for awesome-test-package is invalid: Metadata 1.2 mandates PEP 440 version, but 'magickarp' is not

@uranusjr

This comment was marked as off-topic.

@pradyunsg

This comment was marked as off-topic.

@uranusjr

This comment was marked as off-topic.

@pradyunsg
Copy link
Member Author

Pinning this for visibility, since this has now been released.

@pombredanne
Copy link

While I agree this was a good idea, dropping LegacyVersion is still creating quite a bit of churn for several of my packages and the packages that depend on these. In particular tools that process PyPI packages in a generic way using packaging will fail for older packages that use legacy versions. An important aspect of such tool is to compare versions, and failing to process and sort the whole set of versions of a package is problematic. pytz is an example where old versions such as 2004d fail with v22 and up.

Basically, pip and other packaging tools will fail processing the whole PyPI package index and this is a use case where processing the legacy past is a feature.

For now I ended up coping with adding a https://github.com/nexB/pip-requirements-parser/blob/main/src/packaging_legacy_version.py for instance (as this would otherwise break pip-audit and scancode-toolkit) ... and I am considering either:

  • create a lib of this packaging_legacy_version.py
  • create a packaging2 library as a soft fork that tracks packaging but still support LegacyVersion

I am looking for help or ideas there 💡 !

@pradyunsg
Copy link
Member Author

pradyunsg commented Dec 22, 2022

I've said this elsewhere: folks are welcome to create a lnient-version library on PyPI that takes the LegacyVersion / LegacySpecifier objects from the last release as-is and makes it available for reuse.

@pombredanne

This comment was marked as resolved.

swastkk pushed a commit to swastkk/python-inspector that referenced this issue Jan 12, 2023
* Work around changes in packaging by replacing it with packvers
to avoid issues with missing packaging.version.LegacyVersion.

* Also bump the versions of dparse2 and pip-requirements-parser
with new versions that are not subject the LegcacyVersion issue.

* Adjust the code and tests accordingly.

Reference: nexB#108
Reference: pypa/packaging#530
Signed-off-by: swastik <swastkk@gmail.com>
swastkk pushed a commit to swastkk/python-inspector that referenced this issue Jan 13, 2023
* Work around changes in packaging by replacing it with packvers
to avoid issues with missing packaging.version.LegacyVersion.

* Also bump the versions of dparse2 and pip-requirements-parser
with new versions that are not subject the LegcacyVersion issue.

* Adjust the code and tests accordingly.

Reference: nexB#108
Reference: pypa/packaging#530
Signed-off-by: swastik <swastkk@gmail.com>
swastkk pushed a commit to swastkk/python-inspector that referenced this issue Jan 13, 2023
* Work around changes in packaging by replacing it with packvers
to avoid issues with missing packaging.version.LegacyVersion.

* Also bump the versions of dparse2 and pip-requirements-parser
with new versions that are not subject the LegcacyVersion issue.

* Adjust the code and tests accordingly.

Reference: nexB#108
Reference: pypa/packaging#530
Signed-off-by: swastik <swastkk@gmail.com>
swastkk pushed a commit to swastkk/python-inspector that referenced this issue Jan 13, 2023
* Work around changes in packaging by replacing it with packvers
to avoid issues with missing packaging.version.LegacyVersion.

* Also bump the versions of dparse2 and pip-requirements-parser
with new versions that are not subject the LegcacyVersion issue.

* Adjust the code and tests accordingly.

Reference: nexB#108
Reference: pypa/packaging#530
Signed-off-by: swastik <swastkk@gmail.com>
@pypa pypa locked as resolved and limited conversation to collaborators Jan 28, 2023
@pypa pypa unlocked this conversation Mar 6, 2023
@pypa pypa locked as resolved and limited conversation to collaborators Mar 6, 2023
@pypa pypa unlocked this conversation May 3, 2023
@pypa pypa locked as resolved and limited conversation to collaborators May 3, 2023
@pypa pypa unlocked this conversation Jul 15, 2023
shailshouryya added a commit to shailshouryya/save-thread-result that referenced this issue Aug 7, 2023
- this release does not add any new functionality nor modify existing functionality
- **SUMMARY**
  - see commit 21bd027 for the initial
  attempt at fixing the upload error
    - this change fixed the upload error, but changed the
    functionality of `python_requires` since any `3.0.N` version of
    python would become incompatible with this change
  - see commit d3e02a7 for the
  proper fix to the original upload error while maintaining
  compatibility for any `3.0.N` version of python
- **EXPLANATION (taken from pull request thread)**
After doing some digging, this is the likely culprit for what caused this problem:
- pypa/packaging#407
  - which was the result of pypa/packaging#566 (related: pypa/packaging#530 and pypa/packaging#321)
    - which in turn looks like the result of the discussion at https://discuss.python.org/t/how-to-pin-a-package-to-a-specific-major-version-or-lower/17077/8

It looks like this is the expected behavior as defined in PEP 440 under the [Inclusive ordered comparison section](https://peps.python.org/pep-0440/#inclusive-ordered-comparison):
> An inclusive ordered comparison clause includes a comparison operator and a version identifier, and will match any version where the comparison is correct based on the relative position of the candidate version and the specified version given the consistent ordering defined by the standard [Version scheme](https://peps.python.org/pep-0440/#version-scheme).

Following the link to the [Version scheme](https://peps.python.org/pep-0440/#version-scheme) section and looking at the specification under the [Public version identifiers](https://peps.python.org/pep-0440/#public-version-identifiers) section:
> The canonical public version identifiers MUST comply with the following scheme:
> `[N!]N(.N)*[{a|b|rc}N][.postN][.devN]`
> Public version identifiers MUST NOT include leading or trailing whitespace.
>
> Public version identifiers MUST be unique within a given distribution.
> ...

The last line included above seems to be the "loose implementation" of the version modifier that the issues and pull requests I linked to at the very top were discussing ("After doing some digging, this is the likely culprit for what caused this problem").

Once that "loose implementation" was fixed, any package that didn't follow the PEP 440 specification for version identifiers broke. In this package, the break occurred because of the `>=3.0.*` comparison, which IS NOT unique within a given distribution, as opposed to `>=3` (what commit d3e02a7 does), which IS unique within a given distribution.

To clarify: it looks like the problem we see in this issue is because of implementation fixes in the packaging tools to more closely follow PEP 440, specifically **"Public version identifiers MUST be unique within a given distribution."** Any package that relied on the previous implementation that loosely verified the PEP 440 specification but did not strictly follow PEP 440 broke after the implementation of the packaging tool(s) were fixed to more closely follow PEP 440. More explicitly (from [this comment](https://discuss.python.org/t/how-to-pin-a-package-to-a-specific-major-version-or-lower/17077/8) on the [How to pin a package to a specific major version or lower](https://discuss.python.org/t/how-to-pin-a-package-to-a-specific-major-version-or-lower/17077) discussion):
> Christopher already made the response I was going to make: for PEP 440 as written, using wildcards outside of “==” and “!=” comparisons isn’t valid.
>
> Allowing them for “>=” and “<=” would be reasonable, but it would involve a PEP to fix the spec. (It wasn’t a conscious choice to exclude them, we just didn’t notice at the time that the inclusive ordered comparison operators weren’t formally defined as combining an exclusive ordered comparison with a version match, so the tools have been written to ignore the wildcard instead of paying attention to it)
>
> Making a coherent definition wouldn’t be too hard: just ignore the wildcard for the exclusive ordered comparison part and keep it for the version matching part.

Here are some other posts that aren't directly relevant, but might be interesting tangents for anyone interested in packaging problems:
- https://stackoverflow.com/questions/19534896/enforcing-python-version-in-setup-py
  - https://packaging.python.org/en/latest/guides/distributing-packages-using-setuptools/#python-requires
  - https://packaging.python.org/en/latest/guides/distributing-packages-using-setuptools/#package-data
    - https://setuptools.pypa.io/en/latest/userguide/datafiles.html
      - https://peps.python.org/pep-0345/#requires-python
- https://stackoverflow.com/questions/8795617/how-to-pip-install-a-package-with-min-and-max-version-range
- https://python3statement.org/practicalities/
- https://discuss.python.org/t/requires-python-upper-limits/12663/20
- https://stackoverflow.com/questions/13924931/setup-py-restrict-the-allowable-version-of-the-python-interpreter/13925176#13925176
@Semnodime
Copy link

The lasting use of LeagacySpeifiers has detrimental impact on intuitive pipenv usability when parsing a Pipfile with a typo pypa/pipenv#4247.

@erdnaxeli When using an invalid version range like >=x=x,<y, Pipenv does not complain.

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

Successfully merging a pull request may close this issue.

4 participants