Skip to content

Remove _vendor directory#288

Merged
jaraco merged 3 commits intopypa:mainfrom
abravalheri:remove-vendor
Sep 2, 2024
Merged

Remove _vendor directory#288
jaraco merged 3 commits intopypa:mainfrom
abravalheri:remove-vendor

Conversation

@abravalheri
Copy link
Copy Markdown
Contributor

@abravalheri abravalheri commented Aug 27, 2024

Setuptools recently changed its vendoring method to address the criticism regarding the proliferation of _vendor directories and multiple copies of the same files being distributed in the same wheel/sdist: pypa/setuptools#4455.

That change however do not address the existence of setuptools/_distutils/_vendor and therefore do not close the original issue 100%. I think there is not much point in distutils maintaining its own vendoring system, so this PR proposes simply removing it, and relying on the dependencies that come installed with setuptools.

In pypa/setuptools#4594, we also got a report regarding external tools expecting setuptools/_distutils/_vendor/*.dist-info folders to behave in a certain way (despite it being a private directory and not added to sys.path). Although that issue is not really a bug in setuptools (in my opinion), simply removing distutils/_vendor would also help with that.

Comment thread distutils/dist.py Outdated
Comment thread pyproject.toml Outdated
@jaraco jaraco closed this Sep 2, 2024
@jaraco jaraco reopened this Sep 2, 2024
@jaraco
Copy link
Copy Markdown
Member

jaraco commented Sep 2, 2024

The diffcov errors are weird, probably there due to the broken main brought about by a nitpicky change in ruff import sorting, which is now fixed in 7ee6a6e.

@jaraco jaraco merged commit de0bb44 into pypa:main Sep 2, 2024
@abravalheri
Copy link
Copy Markdown
Contributor Author

abravalheri commented Sep 2, 2024

Thank you very much for the review and improvements @jaraco.

Adding this comment pushes me a little closer to releasing distutils as its own package (with a caveat that it's not meant to be installed anywhere), and then including it as a vendored package in Setuptools (just like any other, with its dependencies). I'm not prepared to do that yet, but it's more tempting now that distutils has dependencies.

There is another approach as well which is to "just let setuptools do its thing" and remove this concern from distutils (instead, just do a weaker/simpler canonicalization that does not require 3rd party packages1).
Currently setuptools patches DistutilsMetadata because there is no mechanism for using a subclass when setuptools.dist.Distribution is instantiated... So for now we are stuck with this approach, and therefore adding dependencies to distutils has little pay off (patching is necessary anyway).

In the future we either start moving code from setuptools to distutils to remove the need for patching, or add a method that allows using DistutilsMetadata subclasses when instantiating setuptools.dist.Distribution2.

Footnotes

  1. We can safely assume that by the stage that we need the canonicalization, name and version have already been validated, so we could use some regex for the alpha|beta|rc => a|b|c and .|- => _ replacements.

  2. For example in https://github.com/pypa/distutils/blob/de0bb446b5a953e8fa48998980c07496533968bc/distutils/dist.py#L149, instead of having self.metadata = DistributionMetadata() we could do self.metadata = self._create_metdata(), so that we can extend DistutilsMetadata via subclassing instead of patching.

@abravalheri abravalheri deleted the remove-vendor branch September 2, 2024 13:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants