Simplify MANIFEST.in #7877
Closed
Simplify MANIFEST.in #7877
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
MANIFEST.in: Use 'graft' to include all resources relevant for the application and all tests more generically. Specifically include README.md and LICENSE.md. Follow-up to encode#7141 which only included tests partially.
Contributor
|
Let's take a pass on the code churn. Things are working as they are, and it's not super obvious to me exactly what the behaviour of graft would be here vs. our current more explicit approach. |
Contributor
Author
|
Currently the The changes in this pull request addressed this issue and used a more generic approach by using The file can in fact be even shorter: If docs should be excluded, which this PR did not address initially (I have updated the branch to reflect this), it can plainly be: This is because
Unless there is a specific need to do so, it seems questionable to add anything to the file but explicit global excludes or prune commands. With the proposed change I have the following files in the resulting sdist tarball (when using wheras with the current setup the sdist tarballs on pypi.org are still missing files: I would be very happy, if you'd reconsider this pull request. |
Contributor
|
We don't include tests in our sdist, and I've not seen any convincing reason for including them. Related #7141 Note also that I think any starting point here needs to be from a very specific POV, that explains exactly what you're attempting to do, with which tooling. I understand why an issue might start with "XYZ should be in the sdist" but it actually doesn't give a clear enough motivation. |
Contributor
Author
Sorry, but that is not correct: There are tests in the sdist tarball on pypi.org, but not all the test files are there.
I know. I opened that ticket. ;-)
I am aware of that. For packaging purposes we use
All of that is in fact already described in #7141. |
Contributor
Then I'd suggest a more tightly targeted pull request than this... "test files XYZ are not being included. Change Z is the lowest footprint change that's required to resolve that". |
Contributor
|
As a more general point here I'm not a big fan of the "our tooling requires that your workflow should be like this". From my perspective it ends being busy-work for us, larger package downloads, and no sufficiently obvious motivating payoff. |
Contributor
Author
I don't quite understand the motivation. Is this for the sake of not doing an improvement and only targetting a specific problem? If I add more complex include/exclude things, the file will get even more complex/unreadable.
I noticed this in #7141 already, and I observe it in this pull request as well: I don't quite understand the dismissive and hostile tone to an improvement, that helps others to test this software in their environment. At this point it might be worth asking the question of what exactly makes you believe, that "my" workflow is different from "your" workflow. This project runs pytest against the sources, when building the package on Arch we do the same to guard against breakage.
I don't understand why you deem it better to have incomplete and unusable tests in the sdist tarball (which is rarely used by the end user, as those usually e.g. install a wheel package via pip or a distribution package via their package manager). From my point of view the busy work now is not because of actual work or actual discussions, but because of discussions based on fabricated allegations towards me, which at this point does not make me "feel welcome" anymore. |
Contributor
|
What I'm trying to get to here are two things:
Having dug through which tests are missing after #7141, I'd suggest a minimal pull request, that fixes up the test cases...
That'd be a completely non-contentious change. Fixes something that's clearly in a mid-way broken state to something that's in a fixed state.
You're completely correct, it wasn't clear to me on initially reviewing this that part of the change resolved an issue there. |
Contributor
|
@dvzrv Honestly, I can understand from your side why this might feel like I'm batting this down in a hostile manner. It's genuinely not intentioned that way & your pull request is perfectly sensible option. When managing an ever oncoming avalanche of interaction I end up needing to be concise and brief, and always default to DONT-CHANGE-ANYTHING in order to keep things manageable. I don't always get the tone right for sure. Wishing you well. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
MANIFEST.in:
Use 'graft' to include all resources relevant for the application and
all tests more generically.
Specifically include README.md and LICENSE.md.
Follow-up to #7141 which only included tests partially.
Please make sure that this works as you expect it to work (e.g. by running
python setup sdist).