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

Exclude top-level .tox|.nox|.venv from sdist #4603

Merged
merged 4 commits into from
Aug 27, 2024

Conversation

abravalheri
Copy link
Contributor

Summary of changes

This is a follow up on #4602: instead of doing an ad-hoc exclusion rule for setuptools' own MANIFEST.in, this PR proposes to always "prune" the .tox, .nox and .venv top-level directories.

This is done by extending the sdist.prune_file_list method from distutils using the same technique that the original method already uses to exclude directories like .git or .svn.

I am only excluding top-level .tox|.nox|.venv directories to be conservative and minimise Hyrum's law unexpected consequences.

This PR also removes the custom implementation on setuptools.command.egg_info:manifest_maker.prune_file_list and instead let the class just access the parent's class implementation via inheritance:

  • By comparing the removed lines with the distutils' implementation, I can see that manifest_maker is basically re-implementing the functionality.
  • I double checked that the implementation is present in CPython 3.8, so SETUPTOOLS_USE_DISTUTILS=stdlib should not affect this particular aspect.

Finally, it is important to notice that once the implementation of sdist prunes these directories, the user cannot add them back via MANIFEST.in. But I think that is OK.

I did a quick search on Github, and it seems that most of the users are trying to exclude the directories (on a quick/not-thorough inspection I did not see anyone intentionally including them).

Closes #4601

Pull Request Checklist

…ementation in sdist

It seems that `manifest_maker.prune_file_list` is basically
re-implementing the same exclusion patters as already implemented by
`distutils.command.sdist`.
So we should be able to remove this duplication.
@abravalheri abravalheri marked this pull request as ready for review August 27, 2024 16:53
@abravalheri abravalheri merged commit e90dfd5 into pypa:main Aug 27, 2024
23 checks passed
@abravalheri abravalheri deleted the issue-4601-general branch August 27, 2024 17:00
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.

[BUG] .tox directory is currently being distributed in sdist on PyPI
2 participants