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

Upgrade vendored packaging lib #12300

Merged
merged 43 commits into from
May 4, 2024
Merged

Upgrade vendored packaging lib #12300

merged 43 commits into from
May 4, 2024

Conversation

sbidoul
Copy link
Member

@sbidoul sbidoul commented Sep 29, 2023

Upgrade our vendored copy of packaging, remove all references to LegacyVersion and LegacySpecifier.

Closes #12063
Closes #11715
Closes #11445
Closes #7650
Closes #9613
Closes #10098

TODO

  • handle resolution with candidates with invalid versions
  • handle resolution with candidates with invalid requirements
  • handle environments containing installed distributions with legacy versions
  • handle environments containing installed distributions with invalid specifiers in their dependencies

@sbidoul
Copy link
Member Author

sbidoul commented Sep 29, 2023

@pypa/pip-committers this is a quick try at upgrading packaging.

The tests that need fixing may reveal a few interesting things.

In particular I wonder if we should worry about 920c4fc. When the pip-test-package-0.1.tar.gz sdist is in the find-links path, pip now fails with Invalid version test-package-0.1.

@sbidoul
Copy link
Member Author

sbidoul commented Sep 29, 2023

And the last failing test (test_install_special_extra[Hop_hOp-hoP-hop-hop-hop]) eludes me for now. It succeeds on 3.11 and 3.12 but fails on 3.10 and below.

@sbidoul sbidoul added this to the 23.3 milestone Sep 29, 2023
@mayeut mayeut mentioned this pull request Sep 30, 2023
@sbidoul sbidoul force-pushed the packaging-upgrade branch 2 times, most recently from af9fbb4 to fd89669 Compare October 1, 2023 12:31
@sbidoul sbidoul mentioned this pull request Oct 1, 2023
src/pip/_vendor/vendor.txt Outdated Show resolved Hide resolved
@sbidoul sbidoul force-pushed the packaging-upgrade branch 3 times, most recently from f731c58 to 62e9e9b Compare October 6, 2023 13:42
@pradyunsg
Copy link
Member

I just tried a couple of quick changes to the pkg_resources metadata backend to make it respect PEP 685 normalisation. A quick check reflects that this does behave correctly -- I'll wait for the CI check on this though, as well as a round of reviews before moving forward with it.

@pradyunsg
Copy link
Member

Quoting @pfmoore from the discussion above...

The problem is that setuptools.DistInfoDistribution makes the extras attribute a list of extras normalised by safe_extra. But pip has canonicalised the requested extra, and it can't re-constitute the non-canonical value that's in extras (nor should it). I don't think this is easily fixable without a fairly extensive rewrite of our extra handling (or a fix to pkg_resources to store normalised extra names, which, to be fair, isn't required by PEP 685).

Ugh, this is a nuance that I missed digging into this head-first. Indeed, dependency_map[safe_extra(ext)] is how pkg_resources handles names. :/

@pfmoore
Copy link
Member

pfmoore commented Oct 6, 2023

Nice! Looks like you may have fixed it @pradyunsg 🙂 I hadn't thought to go the whole way and manage getting the extras from the metadata directly (effectively copying the importlib.metadata approach for pkg_resources). That's a much more localised change than I'd feared we might need.

@pradyunsg
Copy link
Member

Ohkie, this last commit is an attempt at simplification -- we might want to leave it out if it isn't all-green, since it's unlikely I'd get a chance to look at this again before Monday morning.

@pradyunsg pradyunsg marked this pull request as ready for review October 7, 2023 19:29
@pradyunsg
Copy link
Member

I've taken the liberty to mark this as ready for review.

@pradyunsg
Copy link
Member

As I do that though... I'm realising that one thing that we ought to validate is whether there are suboptiomal behaviours/failure modes, when given a legacy version or specifier; coming via the index page, a built wheel, a project's metadata and various other data sources that pip interacts with.

@sbidoul
Copy link
Member Author

sbidoul commented Oct 8, 2023

This is not ready indeed. There is work to do to ignore invalid versions in indexes (e.g. pip install pytz fails because there are old invalid versions on PyPI), and ignore distributions with invalid version specifiers in their dependencies. And handle invalid installed versions. And probably more...

@notatallshaw
Copy link
Member

notatallshaw commented Oct 8, 2023

Other fails include pip install nltk, pip install apache-airflow[all], and pip install pandas (I raised similiar issues with rip recently which I beleive also vendors the latest version of Packaging: prefix-dev/rip#27, prefix-dev/rip#38, and prefix-dev/rip#25)

FYI in PR #12327 I am currently filtering out invalid file name and version name packages as Pip collects them in that particular scenario, not sure if that's the going to be the prefered way to do it.

@sbidoul
Copy link
Member Author

sbidoul commented Oct 8, 2023

Yeah, I now remember why I said I would not have time to do this before the release. I somehow had managed to forget about #12300 (comment) when I created this PR.

This seems to be way too far reaching to do so late in the release cycle. So I think I'll postpone it.

@pfmoore
Copy link
Member

pfmoore commented Oct 8, 2023

There is work to do to ignore invalid versions in indexes (e.g. pip install pytz fails because there are old invalid versions on PyPI), and ignore distributions with invalid version specifiers in their dependencies. And handle invalid installed versions. And probably more...

Are there issues tracking these (that we could put into the 24.0 milestone)? If we don't have some means of tracking what's needed to get this landed, I fear we're going to be in the same situation next release - in spite of no-one liking it, vendoring upgrades tend to get ignored until late in the release cycle...

@sbidoul sbidoul removed this from the 23.3 milestone Oct 8, 2023
sbidoul and others added 23 commits May 4, 2024 02:06
We do this by adding a version_str property to BaseDistribution,
which allows us to access the version without parsing it.
We do this to avoid needlessly building a sdist when
we have determined that a wheel has invalid metadata.
Co-authored-by: Tzu-ping Chung <[email protected]>
Dependencies should be more precise than requirements;
The pkg_ressources metadata backend does not suffer from invalid metadata in this test case.
@pradyunsg pradyunsg merged commit 4caa6c3 into pypa:main May 4, 2024
28 checks passed
@sbidoul sbidoul deleted the packaging-upgrade branch May 6, 2024 11:28
@tacaswell
Copy link

This breaks pip on the CPython maintenance branches as the version reported by Python ends with a + and nothing else

ERROR: Exception:
Traceback (most recent call last):
  File "/home/tcaswell/.virtualenvs/bleeding/lib/python3.12/site-packages/pip/_internal/cli/base_command.py", line 180, in exc_logging_wrapper
    status = run_func(*args)
             ^^^^^^^^^^^^^^^
  File "/home/tcaswell/.virtualenvs/bleeding/lib/python3.12/site-packages/pip/_internal/cli/req_command.py", line 247, in wrapper
    return func(self, options, args)
           ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/tcaswell/.virtualenvs/bleeding/lib/python3.12/site-packages/pip/_internal/commands/install.py", line 377, in run
    requirement_set = resolver.resolve(
                      ^^^^^^^^^^^^^^^^^
  File "/home/tcaswell/.virtualenvs/bleeding/lib/python3.12/site-packages/pip/_internal/resolution/resolvelib/resolver.py", line 95, in resolve
    result = self._result = resolver.resolve(
                            ^^^^^^^^^^^^^^^^^
  File "/home/tcaswell/.virtualenvs/bleeding/lib/python3.12/site-packages/pip/_vendor/resolvelib/resolvers.py", line 546, in resolve
    state = resolution.resolve(requirements, max_rounds=max_rounds)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/tcaswell/.virtualenvs/bleeding/lib/python3.12/site-packages/pip/_vendor/resolvelib/resolvers.py", line 427, in resolve
    failure_causes = self._attempt_to_pin_criterion(name)
                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/tcaswell/.virtualenvs/bleeding/lib/python3.12/site-packages/pip/_vendor/resolvelib/resolvers.py", line 239, in _attempt_to_pin_criterion
    criteria = self._get_updated_criteria(candidate)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/tcaswell/.virtualenvs/bleeding/lib/python3.12/site-packages/pip/_vendor/resolvelib/resolvers.py", line 229, in _get_updated_criteria
    for requirement in self._p.get_dependencies(candidate=candidate):
                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/tcaswell/.virtualenvs/bleeding/lib/python3.12/site-packages/pip/_internal/resolution/resolvelib/provider.py", line 247, in get_dependencies
    return [r for r in candidate.iter_dependencies(with_requires) if r is not None]
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/tcaswell/.virtualenvs/bleeding/lib/python3.12/site-packages/pip/_internal/resolution/resolvelib/candidates.py", line 243, in iter_dependencies
    for r in requires:
  File "/home/tcaswell/.virtualenvs/bleeding/lib/python3.12/site-packages/pip/_internal/metadata/importlib/_dists.py", line 222, in iter_dependencies
    elif not extras and req.marker.evaluate({"extra": ""}):
                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/tcaswell/.virtualenvs/bleeding/lib/python3.12/site-packages/pip/_vendor/packaging/markers.py", line 252, in evaluate
    return _evaluate_markers(self._markers, current_environment)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/tcaswell/.virtualenvs/bleeding/lib/python3.12/site-packages/pip/_vendor/packaging/markers.py", line 158, in _evaluate_markers
    groups[-1].append(_eval_op(lhs_value, op, rhs_value))
                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/tcaswell/.virtualenvs/bleeding/lib/python3.12/site-packages/pip/_vendor/packaging/markers.py", line 116, in _eval_op
    return spec.contains(lhs, prereleases=True)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/tcaswell/.virtualenvs/bleeding/lib/python3.12/site-packages/pip/_vendor/packaging/specifiers.py", line 568, in contains
    normalized_item = _coerce_version(item)
                      ^^^^^^^^^^^^^^^^^^^^^
  File "/home/tcaswell/.virtualenvs/bleeding/lib/python3.12/site-packages/pip/_vendor/packaging/specifiers.py", line 36, in _coerce_version
    version = Version(version)
              ^^^^^^^^^^^^^^^^
  File "/home/tcaswell/.virtualenvs/bleeding/lib/python3.12/site-packages/pip/_vendor/packaging/version.py", line 200, in __init__
    raise InvalidVersion(f"Invalid version: '{version}'")
pip._vendor.packaging.version.InvalidVersion: Invalid version: '3.12.3+'

Bisected to 47a8480 specifically.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 26, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bot:chronographer:provided type: deprecation Related to deprecation / removal.
Projects
None yet