-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Fix extras_require and environment markers in sdist #2153
Conversation
23a5e6d
to
0d150e3
Compare
0d150e3
to
daafc2a
Compare
Fixes issue #2174 |
daafc2a
to
48ceb75
Compare
Still "Work In Progress" or ready for review? You removed WIP from title but not from initial comment. Probably an oversight? |
Ready for review :) |
48ceb75
to
6497dda
Compare
Remove now useless InstallRequirement.requirements method |
6497dda
to
d61c245
Compare
@@ -412,67 +412,45 @@ def prepare_files(self, finder): | |||
# ###################### # | |||
# # parse dependencies # # | |||
# ###################### # | |||
if (req_to_install.extras): | |||
logger.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.
I don't think we need this here, at the very least it should be logger.debug
not logger.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.
Well it was there for sdist so I've added it for wheel also.
But I'm ok to switch it to debug or remove it completely since the extras is likely to have been already logged through the req_install before.
Overall I think this looks good. I made one comment and I'd like to look at it again tomorrow morning before merging but I think this can land. |
extras and editable_options are two different things
use pkg_resources.Distribution.requires instead of Requirements.requirements to have environment markers parsing for free It also unifies a little the process for wheel and non-wheel installs closes pypa#2174
and use pkg_resources.Distribution instead
25e0b1b
to
1921ad1
Compare
@dstufft I switched the logging to debug. Did you spot any other issue ? |
Fix extras_require and environment markers in sdist
some nice cleanup in here! |
Currently, environment markers in extras_require (e.g. {':python_version=="2.6"': ['unittest2']}) in sdist do not work.
The plan is to use pkg_resources.Distribution which will give it for free.
First commit is to make sure we do not put editable_options in extras (parse_editable return types are currently merging the two).