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.
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
Support the metadata directory (as per PEP-517) for build_wheel #4647
Support the metadata directory (as per PEP-517) for build_wheel #4647
Changes from 5 commits
e74b501
bc82d73
283ce3b
bd615ee
eb81747
2f3b273
cf299fc
fc08e7e
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we know why egg info is still generated? Is it because other commands might have generated it? What happens if the line for
rmtree
is removed (will the tests fail)?While I'm okay with this approach, I'm also somewhat apprehensive, as it is perpetuating and reinforcing the expectation that egg-info is generated, even when it's not needed, making it harder in the future to correct this suboptimal approach. For example, this project also has the goal to someday obviate egg-info altogether, and to generate dist-info directly, and adding this additional handling of egg-info will likely complicate that work (though maybe not too much).
I wonder - how much do other commands from
bdist_wheel
depend on the generation of egg-info and could those commands get by without generating it ifbdist_wheel
could pass through the instruction to re-use existing dist-info?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like
install
callsinstall_egg_info
(a registered sub-command, which is automatically run), which then callsegg_info
. I wasn't brave enough to try to touch this.I've added a specific test for this. Previously the test would have passed.
Yes, I appreciate that. IMO, this change does add an additional place where you have to think about egg-info, but I believe it is already necessary to deal with
bdist_wheel
wrtegg-info
, and the added code-path now explicitly rejects its use - I think that ultimately makes that easier to remove egg-info in the future.