pypa / setuptools

Official project repository for the Setuptools build system
https://pypi.org/project/setuptools/
MIT License
2.34k stars 1.14k forks source link

Eliminate double book keeping in `extern`, just using `_vendor` directly #4330

Closed abravalheri closed 2 weeks ago

abravalheri commented 2 weeks ago

Main idea:

This is an investigation on https://github.com/pypa/setuptools/pull/4320#pullrequestreview-2014802225

Summary of changes

Intentional changes:

Collateral changes:

Changes that were needed just to make the tests pass:

Closes

Pull Request Checklist

[PR docs]: https://setuptools.pypa.io/en/latest/development/developer-guide.html#making-a-pull-request


Drawbacks

The separation between the direct dependencies (vendored.in) and the automatic logging and documentation of the transitive dependencies (vendored.txt) is nice from some point-of-view.

However, it also requires that we run tox -e vendor using the lowest version of Python supported by setuptools (otherwise conditional environment markers may be ignored). This may be easy to forget.

abravalheri commented 2 weeks ago

NOTE:

There are some people out there importing extern: https://github.com/search?q=%2Ffrom+setuptools.extern%2F+language%3APython&type=code&l=Python

Maybe we should have a deprecation here? Although, I find it weird that a 3rd-party is to importing extern without at least a try-except.

abravalheri commented 2 weeks ago

@jaraco is this an approach you would be interested for us to follow in setuptools?

jaraco commented 2 weeks ago

Although, I find it weird that a 3rd-party is to importing extern without at least a try-except.

IIRC, my intention for extern was that it was providing the compatibility shim - if the vendored packages were present, they'd be imported, but if they were missing, the dependencies could be loaded from sys.path. That's what VendorImporter.search_path did. I wanted to make it trivially easy for a downstream distro to simply remove the _vendor directory and supply the dependencies and everything would just work. Hard-coding _vendor in the imports will break this assumption (and require distro packagers to re-write all of these imports).

My hope/expectation was that at some point the vendored packages would be removed in all but a few isolated scenarios and real dependencies would be used. That vision never panned out.

Maybe we should have a deprecation here?

Although I made extern a public-facing name, I never intended it to be used for anything except project specifically dependent on setuptools' dependencies (e.g. they need to catch the same exception from packaging). I never considered the backward compatibility implications here.

@jaraco is this an approach you would be interested for us to follow in setuptools?

My hope was that setuptools (and maybe pkg_resources) could simply import these dependencies instead of hard-coding the imports from their vendored location. I was thinking something like:

- from setuptools.extern import platformdirs
+ import platformdirs

And then early in the import of setuptools ensure that everything in setuptools._vendor appears in sys.path (such that the packages always prefer installed packages and only fall back to _vendor in bootstrap scenarios). I even have hopes to make the vendoring an exceptional case, where they're only installed (or added to sys.path) under special conditions that are known to need special bootstraping support, but otherwise rely entirely on standard packaging resolution. That is, under most circumstances, a PEP 517/518 installer will ensure dependencies are met before doing anything with setuptools... and only in source-only builds would there need to be a bootstrap step to break the cyclic dependency (e.g. setuptools --> packaging (from source dist) --> setuptools).

I'm not confident that transitioning to a hard-coded _vendor import will get us there.