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

Update pkg_resources (via setuptools) to 65.6.3 #11689

Merged
merged 3 commits into from
Feb 20, 2023

Conversation

pradyunsg
Copy link
Member

x-ref #11501

@pradyunsg pradyunsg added this to the 23.0 milestone Jan 3, 2023
@pradyunsg
Copy link
Member Author

Oh fun, a new dependency in the graph!

@uranusjr
Copy link
Member

uranusjr commented Jan 3, 2023

I’d rather include #11661 and declare pkg_resources dead on 3.12.

@pradyunsg
Copy link
Member Author

I think we still need the newer pkg_resources for unblocking the packaging update (it removes the reliance on LegacyVersion-style parsing). Besides, we should probably do an upgrade anyway.

@pradyunsg
Copy link
Member Author

pradyunsg commented Jan 9, 2023

Hmm... pkg_resources has a significantly bigger dependency cone, via @jaraco's various helper packages. It still depends on appdirs (pip's moved on to platformdirs) as well.

Well, I'm not sure what to do now -- pkg_resources has too many dependencies for me to be comfortable upgrading as-is (outside of the need for our vendoring support to handle namespace packages too), but unless we upgrade, we can't upgrade to packaging 22.0/23.0 in pip either, since those are incompatible with the older version of setuptools we're still on (and IDK if it'd work with setuptools either).

jaraco-text has a large set of dependencies as well: https://github.com/jaraco/jaraco.text/blob/ca494073a4b0a6a6fabac2fae150e3fc93e0ef5b/setup.cfg#L19

@pypa/setuptools-team Would you be willing to copy-over the three functions from jaraco.text that are actually used instead of needing downstream users to vendor all of jaraco.text and its dependency cone?

@pradyunsg
Copy link
Member Author

Unless I'm misunderstanding something, this is effectively blocking the implementation of PEP 685 and preventing us from benefitting from the improved startup times and error messages from the hand-written requirement parser in packaging 22.0+ in pip. Notably, it's also preventing the entire class of bugfixes from both packaging and pkg_resources from making it into pip as well (notably, muslinux fixes and macOS tags).

@uranusjr
Copy link
Member

uranusjr commented Jan 9, 2023

What are the functional changes of pkg_resources since setuptools 44.0? I wonder if it would actually be easier if we simply pull (or fork) the old pkg_resources code and apply (persumably uncommon) changes.

@pradyunsg
Copy link
Member Author

pradyunsg commented Jan 9, 2023

pypa/setuptools@v44.0.0...main#diff-e3d3d454fa -- doesn't seem like there's much?

@pradyunsg
Copy link
Member Author

TBH, I don't mind forking pkg_resources within pip at this point.

@uranusjr
Copy link
Member

uranusjr commented Jan 9, 2023

Yeah I looked at the history and there are only 10-ish relevant commits that changed any functionality, and a good portion of them are actually features/removals that we arguably might not want (remove support for zipped .egg, finding .egg-info in a zip file, etc.)

I’m starting to think this is a good idea.

@pradyunsg
Copy link
Member Author

Neato. Unless someone raises concerns, I'll spend some time exploring this.

I might also rename it internally to something other than pkg_resources to ensure that we don't trip people on a confusion due to this and to reflect that it's an implementation that can deviate.

@uranusjr
Copy link
Member

uranusjr commented Jan 9, 2023

We could move the file to pip/_internal/metadata/pkg_resources/impl.py

@pradyunsg
Copy link
Member Author

Hmm... pygments depends on pkg_resources to locate entrypoints on Python 3.7 (importlib.metadata is "New in version 3.8").

@jaraco
Copy link
Member

jaraco commented Jan 13, 2023

Hmm... pygments depends on pkg_resources to locate entrypoints on Python 3.7 (importlib.metadata is "New in version 3.8").

That's the reason importlib_metadata with support for Python 2.7 was created/maintained.

TBH, I don't mind forking pkg_resources within pip at this point.

It pains me greatly that pkg_resources is becoming more embedded in the implementation, given that I've spent the last several years working to create simpler and more viable replacement behavior in importlib.*. I plan to deprecate pkg_resources, and I already consider its use "discouraged" even if I haven't published that prominently. I'd urge you to consider porting to importlib metadata/resources in pip and other libraries. I'd be willing to help with the implementation. It probably would require pulling in the backports on older Pythons, and I'm willing to help navigate that complexity if it helps.

That said, if you wish to fork pkg_resources, that's certainly something you're welcome to do and may be a viable path. I and the Setuptools project is unwilling to support maintenance of pkg_resources in any expanded capacity. If you could limit the usage to Python<3.7 or <3.9 (for removal in just a few years), that would be preferable.

@pradyunsg
Copy link
Member Author

We have ported to importlib.metadata on newer versions of Python FWIW, but we've preserved the pkg_resources-backed implementation for handling metadata on < 3.10 (or < 3.11).

@pradyunsg
Copy link
Member Author

pradyunsg commented Jan 13, 2023

See https://github.com/pypa/pip/tree/main/src/pip/_internal/metadata for the implementation of this.

It's not more embedded and it's on its way out. The problem right now is that setuptools has created a large dependency graph for 3 utility functions to be used in pkg_resources. If you can trim or simplify the graph, as I've suggested above, that's unblock us to stay aligned with setuptools' pkg_resources.

As it stands, pkg_resources blocks us from being able to migrate to being stricter in what versions we accept (circa packaging's removal of LegacyVersion/LegacySpecifier). And, this blocks implementing PEPs that basically need a packaging upgrade (like 685).

The only motivation to fork would be to move quicker than setuptools can on this front, but if you're willing to move setuptools forward to unblock this, that would be amazing. <3

@pradyunsg
Copy link
Member Author

Kicking the can down the road for this.

@pradyunsg pradyunsg modified the milestones: 23.0, 23.1 Jan 28, 2023
@flying-sheep
Copy link

flying-sheep commented Feb 4, 2023

I’m happy I lobbied for some projects to get rid of pkg_resources like in pypa/setuptools-scm#513. @jaraco one thing you repeated here that I didn’t understand in that issue either: What’s the material difference between “deprecation” and “planning to deprecate”? 😄 Both have the same implications (migrating away is a good idea).

Thank you for maintaining it! I know how hard it is to invest into something that is on its way out.

@pradyunsg
Copy link
Member Author

pradyunsg commented Feb 4, 2023

Ok, I've taken a slightly different approach with the latest push: pulling in the 3 functions that pkg_resources actually uses from jaraco.text into a module within pip, which removes the need to vendor in the entire dependency cone of jaraco.text for ~15 lines of logic, and is a more tractable way of doing this than trying to convince mypy to be OK with namespace packages within pip._vendor.

@github-actions github-actions bot added the needs rebase or merge PR has conflicts with current master label Feb 7, 2023
@pypa-bot pypa-bot removed the needs rebase or merge PR has conflicts with current master label Feb 13, 2023
@pradyunsg pradyunsg marked this pull request as ready for review February 19, 2023 16:48
@jaraco
Copy link
Member

jaraco commented Feb 20, 2023

Given that you manage the namespace, you may be able to get away with adding an empty jaraco/__init__.py, but no problem if you’d prefer to hand vendor the functionality. I don’t be same in CPython:

@pradyunsg
Copy link
Member Author

pradyunsg commented Feb 20, 2023

I don’t be same in CPython:

Could you clarify what you mean by this?

@pradyunsg
Copy link
Member Author

I'm actually gonna go ahead and eagerly land this -- reverts are cheap and we can make additional changes in a follow up to this!

@pradyunsg pradyunsg merged commit 031d6ec into pypa:main Feb 20, 2023
@pradyunsg pradyunsg deleted the update-setuptools branch February 20, 2023 04:49
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 8, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants