pip icon indicating copy to clipboard operation
pip copied to clipboard

Upgrade vendored packaging lib

Open sbidoul opened this issue 2 years ago • 24 comments

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

Closes #12063 Closes #11715 Closes #11445

sbidoul avatar Sep 29 '23 20:09 sbidoul

@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 https://github.com/pypa/pip/pull/12300/commits/920c4fcecd9ef2bc6fb2abe42245ee6a67d782c8. 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 avatar Sep 29 '23 21:09 sbidoul

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 avatar Sep 29 '23 21:09 sbidoul

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 avatar Oct 06 '23 19:10 pradyunsg

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. :/

pradyunsg avatar Oct 06 '23 19:10 pradyunsg

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.

pfmoore avatar Oct 06 '23 20:10 pfmoore

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 avatar Oct 07 '23 00:10 pradyunsg

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

pradyunsg avatar Oct 07 '23 19:10 pradyunsg

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.

pradyunsg avatar Oct 07 '23 19:10 pradyunsg

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...

sbidoul avatar Oct 08 '23 11:10 sbidoul

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: https://github.com/prefix-dev/rip/issues/27, https://github.com/prefix-dev/rip/issues/38, and https://github.com/prefix-dev/rip/issues/25)

FYI in PR https://github.com/pypa/pip/pull/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.

notatallshaw avatar Oct 08 '23 12:10 notatallshaw

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 https://github.com/pypa/pip/pull/12300#issuecomment-1752003872 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.

sbidoul avatar Oct 08 '23 15:10 sbidoul

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...

pfmoore avatar Oct 08 '23 15:10 pfmoore

I moved this PR and related issue to the 24.0 milestone, and opened #12332 to update the deprecation message.

sbidoul avatar Oct 08 '23 16:10 sbidoul

Cool. At some point, I might go through the discussion and raise specific issues for individual items (ignore invalid versions in indexes, ignore distributions with invalid versions in dependencies, handle invalid installed versions, ...) unless people think that's too much detail.

It might not be until next week, though - this week is quite busy for me.

pfmoore avatar Oct 08 '23 17:10 pfmoore

Neato, I'll move this back to draft then to avoid an accidental merge.

pradyunsg avatar Oct 08 '23 17:10 pradyunsg

Hi, same question as I was asking in #12063:

Hi, just upgrade to pip 23.3 with following message:

DEPRECATION: gpg 1.14.0-unknown has a non-standard version number. pip 23.3 will enforce this behaviour change. A possible replacement is to upgrade to a newer version of gpg or contact the author to suggest that they release a version with a conforming version number. Discussion can be found at https://github.com/pypa/pip/issues/12063 Installing collected packages: pip Attempting uninstall: pip Found existing installation: pip 23.2.1 Uninstalling pip-23.2.1: Successfully uninstalled pip-23.2.1 Successfully installed pip-23.3 gpg --version

gpg (GnuPG) 2.2.27 libgcrypt 1.8.8 Copyright (C) 2021 Free Software Foundation, Inc. License GNU GPL-3.0-or-later https://gnu.org/licenses/gpl.html This is free software: you are free to change and redistribute it. There is NO WARRANTY, to the extent permitted by law.

Home: /root/.gnupg Supported algorithms: Pubkey: RSA, ELG, DSA, ECDH, ECDSA, EDDSA Cipher: IDEA, 3DES, CAST5, BLOWFISH, AES, AES192, AES256, TWOFISH, CAMELLIA128, CAMELLIA192, CAMELLIA256 Hash: SHA1, RIPEMD160, SHA256, SHA384, SHA512, SHA224 Compression: Uncompressed, ZIP, ZLIB, BZIP2

Anything I need to update/upgrade/not working after that?

un99known99 avatar Oct 16 '23 06:10 un99known99

@un99known99 please don't post the same question in multiple places.

pradyunsg avatar Oct 16 '23 07:10 pradyunsg

It is very unlikely that I will have the time to drive this before 24.0. So if anyone wants to pick this up, don't hesitate.

If this helps, I can try to extract some commits in independent PR that don't require the packaging upgrade.

handle invalid installed versions

I have not tested but I suspect this could be problematic. At least we should make sure that pip can be downgraded by pip even if invalid versions are present in the environment, to ensure a pip upgrade will not irreversibly break an environment.

sbidoul avatar Oct 26 '23 13:10 sbidoul

Is anyone going to be able to pick this up for 24.0? The release will be in January, so I think that realistically we should be looking to get this merged into main very soon after the holiday period if we want to ensure we have time to address any follow-up issues before the release. Otherwise, we're looking at pushing it back again, to 24.1.

Personally, I think we should bite the bullet. With things like nogil support starting to become a real issue, I think we're just going to make things worse if we continue postponing this.

However, I'm afraid I won't have the bandwidth to do much on it myself.

pfmoore avatar Dec 14 '23 21:12 pfmoore

Personally, I think we should bite the bullet. With things like nogil support starting to become a real issue, I think we're just going to make things worse if we continue postponing this.

I agree with this sentiment. Please let me know if there's anything I can help with in order to satisfy this timeline.

ofek avatar Dec 19 '23 21:12 ofek

Is anyone going to be able to pick this up for 24.0?

I can try and make time for this in early January, which might be too late for 24.0 depending on how the RM feels.

TBH, I won't mind skipping January for 24.0 (i.e. cutting 24.0 in April) to give ourselves more time to do this. Looking at the changes since 23.3, I don't see anything that warrants a release.

pradyunsg avatar Dec 19 '23 21:12 pradyunsg

That seems reasonable. 23.3.2 having just been released, pushing 24.0 out barely a month later feels like a waste of effort.

But to be clear, as far as this issue is concerned, for me it's more important that the work gets done than that it hits any particular release. As far as I'm concerned the main reason for it being in any particular milestone is to ensure it doesn't fall off our radar. So if someone did feel the need to have a 24.0 in January for whatever reason, and we didn't hit that release, we could just reschedule to 24.1. And have this conversation again in March... 😉

pfmoore avatar Dec 19 '23 23:12 pfmoore

3rd time's the charm?

pradyunsg avatar Dec 20 '23 22:12 pradyunsg

Gentle ping on this. We have a few more months (as we seem to be skipping a January release) but IMO it would be prudent to try to get it done sooner rather than ending up pushed for time again in March. (And sorry, but my available time is still extremely limited, so I can't commit to working on this myself).

pfmoore avatar Jan 25 '24 21:01 pfmoore

It's be great if pip included packaging 24.0, either as part of this PR or subsequently, in order to enable support for the free-threaded (--disable-gil) CPython 3.13 build.

We're close to being able to test (and fix) Python packages with the free-threaded build, but I think some of that work will be blocked on a release of pip that includes packaging 24.0.

colesbury avatar Mar 21 '24 16:03 colesbury

I have rebased and resolved the conflicts.

If anyone wants to help here, the next things to do, I think, is writing additional (failing) tests for

  • [x] an index / --find-links page that provides legacy versions
  • [x] packages with legacy specifiers in dependencies
  • [ ] installation / upgrades in an environments containing installed distributions with legacy versions

sbidoul avatar Mar 27 '24 08:03 sbidoul

CI breaks becuase pip/_internal/resolution/resolvelib/factory.py still references CandidateVersion for a type hint, but CandidateVersion import was removed.

notatallshaw avatar Mar 30 '24 17:03 notatallshaw

I plan to provide some of the mentioned tests via packse but I don't have a timeline for completion (https://github.com/pypa/pip/pull/12580 and https://github.com/astral-sh/packse/issues/168)

notatallshaw avatar Mar 30 '24 17:03 notatallshaw

CI breaks becuase pip/_internal/resolution/resolvelib/factory.py still references CandidateVersion for a type hint, but CandidateVersion import was removed.

Thanks, I have fixed that.

I added a failing test for an index containing invalid versions, and a commit to ignore such versions. That one was easy. And it is probably the most common case.

I also added a test for an index containing a package with an invalid requirement in its dependencies. That one is harder to handle. What do we want to do with those? If we want to ignore them, we probably need to check if their dependencies are valid when constructing the Candidate. Would that have a performance impact?

sbidoul avatar Mar 30 '24 18:03 sbidoul

@pradyunsg this PR is not nearly ready, but before I forget, could you add a news fragment for your PEP 685 commits?

sbidoul avatar Mar 30 '24 19:03 sbidoul