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

Suggestions can be misleading #166

Open
AWhetter opened this issue Jul 24, 2023 · 1 comment
Open

Suggestions can be misleading #166

AWhetter opened this issue Jul 24, 2023 · 1 comment

Comments

@AWhetter
Copy link

At my place of work we make use of check-manifest extensively to ensure that our data files are configured correctly, particularly for juniors or those less familiar with the intricacies of Python packaging. We often find that, due to a lack of understanding of how setuptools works, users will follow the suggestions made by check-manifest blindly. For example, we'll end up with users appending to the MANIFEST.in with rules like include src/*.png for icon files that should be getting installed by the setup.py or include .cache-file-not-in-gitignore when in fact a file like this should be in the .gitignore rather than included by the MANIFEST.in. More knowledgeable developers then have to pick up on these things on code review.

On our side, there's an issue of our test coverage being too low to pick up these problems and there not being enough knowledge of setuptools to know what files should and shouldn't be included in the sdist.

However I also think that it's wrong for check-manifest to make any suggestions about rules to include in the MANIFEST.in. check-manifest has no knowledge of whether a file should be included or not, and the correct course of action for a file can be editing the setup.py, .gitignore, etc instead of editing the MANIFEST.in. Therefore the suggested rules can be misleading, and end up teaching the wrong course of action to users.

@mgedmin
Copy link
Owner

mgedmin commented Jul 25, 2023

A comment: src/*.png should be included in MANIFEST.in, otherwise when unpack the source distribution and run setup.py install, there will not be any icon files to install! (Assuming the unlikely case where the source distribution was built solely from the files included by MANIFEST.in, which is not the typical case nowadays, when build systems generally use version control metadata directly.)

The cache files thing is very true, but I don't know how to make check-manifest smarter about its suggestions. My expectation would be that this issue will be discovered when you git commit after testing the software and notice unwanted files being added, but not everyone is as particular about reviewing git status before every commit as I am.

check-manifest has no knowledge of whether a file should be included or not

check-manifest assumes that a file should be included in the source distribution if, and only if, it is included in the version control system. There are some exceptions, because where aren't any, but they are expected to be rare.

check-manifest is a thing that I hope will be obsolete in the far future, when Python build systems become smarter. I wrote it because setuptools made some unfortunate decisions (automatically include all versioned files, but only if you have the right plugin installed -- and if the plugin is missing, it will silently skip files instead of complaining).

check-manifest is also only important if you care about Python source distributions being complete. If you always build from a git checkout, then you don't need to care. Very few people use an extracted sdist as the preferred source of modification, in the sense that very few people extract an sdist, and then build a new sdist from the extracted tree. The other cases -- where the build system fails to discover existing version control metadata and thus fails to include all the right files -- are increasingly less likely in the modern Python ecosystem.

I'm sorry I"m rambling. I'm not sure where I was going with this comment.

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

No branches or pull requests

2 participants