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

Trigger deprecation warning when find_packages() finds reserved name #3259

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mgorny
Copy link
Contributor

@mgorny mgorny commented Apr 11, 2022

Summary of changes

Trigger SetuptoolsDeprecationWarning whenever find_packages() finds
a top-level package whose name is found in reserved package name list,
e.g. "tests". Installing these packages is probably always wrong,
yet it is still a frequent mistake in packages using setuptools. This
warning should increase the chance of the mistake being caught before
releasing.

Closes #3243

Pull Request Checklist

Trigger SetuptoolsDeprecationWarning whenever find_packages() finds
a top-level package whose name is found in reserved package name list,
e.g. "tests".  Installing these packages is probably always wrong,
yet it is still a frequent mistake in packages using setuptools.  This
warning should increase the chance of the mistake being caught before
releasing.

Fixes pypa#3243
@mgorny mgorny force-pushed the find-package-deprecated branch from 0f937b1 to 3344ff4 Compare April 11, 2022 10:31
@mgorny
Copy link
Contributor Author

mgorny commented Apr 11, 2022

I have to admit that I went with the "minimal change" approach. If you prefer it done some other way, please let me know.

f"{top_package!r} is a reserved package name. "
"Please add it to find_package(exclude=...)",
setuptools.SetuptoolsDeprecationWarning,
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it really the case these patterns should be deprecated?

What if the user is the owner of the package tasks or changelog (those are perfectly valid entries for PyPI right?) and is explicitly using [options.packages.find] include = tasks*?

I am also not sure if this is the best place to implement the check..
This code path is traversed by the auto-discovery algorithm that will already error if the distribution contains multiple top-level packages... It is also traversed when the src-layout is being used (and my opinion is that if the src-layout is used, there should be no warning...).

I think that an alternative would be modifying setuptools.find_packages, setuptools.find_namespace_packages, setuptools.config.setupcfg.ConfigOptionsHandler._parse_packages and setuptools.pyprojecttoml._ConfigExpander._expand_packages to check for one of these patterns only if there is more than one top-level package.

@mgorny
Copy link
Contributor Author

mgorny commented Apr 16, 2022

I'm sorry but due to health issues I'll have to limit my computer activity for some time (apparently up to 4 weeks), so I won't be able to look into it any time soon.

@abravalheri
Copy link
Contributor

Sorry to hear that...
I wish you a good recovery.

@mgorny
Copy link
Contributor Author

mgorny commented Apr 16, 2022

Thank you very much.

@jaraco jaraco force-pushed the main branch 3 times, most recently from a85759c to 93ce5a0 Compare December 16, 2022 18:21
@jaraco
Copy link
Member

jaraco commented Jun 21, 2023

@mgorny I hope you're back to health. I thought I'd check in on this PR and see if you wanted to continue with it?

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.

[FR] Warn about top-level "tests" package in find_packages() by default
3 participants