-
Notifications
You must be signed in to change notification settings - Fork 3.1k
Add --platform and --supported-interpreter-version options to the pip install command #2965
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
Add --platform and --supported-interpreter-version options to the pip install command #2965
Conversation
871e46d
to
eaf0eac
Compare
Sorry, I didn't expect my force push to blow away the comments, but I addressed the indented block issue brought up by @jamezpolley. |
I love this idea, it's precisely what I wanted to use right now 😄 |
distlib is a vendored dependency of pip. There should be no modification to its code (except when bumping distlib to a new version, cf 320a07f). |
@xavfernandez, I got a little trigger happy when I was removing my calls to pdb.set_trace() after I finished testing. I just git grepped for all instances of that method call and deleted them. Do you think I should place those lines back in distlib, or do you think that maybe this is a small exception? I'm not sure if those debug lines were left in there on purpose. (This has been resolved. The changes to distlib have been reverted) |
The second option I added is called |
9431736
to
d68faf4
Compare
Is there anything else that needs to be done to get this change merged in? |
Hey, we're blocking on this making it into a release in order to release a related feature over at pantsbuild/pants. Would it be possible for someone to take a look and merge it in if everything is okay? |
It would be cleaner to first implement #2643 and then add those specific options to the new |
@xavfernandez Just did this in a different PR: #2995 |
) | ||
|
||
cmd_opts.add_option( | ||
'--supported-interpreter-version', |
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.
--interpreter-version
seems enough (and we are not using --supported-platform
)
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.
Agreed.
Something that is missing from this PR is the ability to download package for a different interpreter (i.e. downloading packages for PyPy from a CPython venv) |
@xavfernandez As for adding different python implementation support, I think it's doable but non-trivial (compared to adding different python version support -- the second thing I did in this PR). I'd rather save that for a different PR, after we get |
d68faf4
to
58a9610
Compare
It looks like you overrode |
58a9610
to
16c6c9f
Compare
… the user more control when downloading packages. The two new options are --platform and --supported-interpreter-version, and both enforce that the --download option is also specified, else a CommandError is raised. With the --platform option, a user can ask to download wheels of a different platform than that of the local machine running the command, which is designed as a utility for gathering dependencies and preparing a distribution. Because this option enforces that the --download option is also specified, it will never attempt to install wheels of an incorrect platform. With the --supported-interpreter-version option, a user can ask to download wheels that are explicitly compatible with a specific Python interpreter version, which is designed as a similar utility to the --package option.
16c6c9f
to
9e10fc6
Compare
@xavfernandez, good catch, I just fixed that by adding a |
The |
Hm, do you think it would be more clear to use the more-verbose As for implementing |
Ok, |
Accidentally closed this, reopening. Sorry! |
Oh no, you made me so happy there for a second xD What a tease, lol |
Hello! As part of an effort to ease the contribution process and adopt a more standard workflow pip has switched to doing development on the If you do nothing, this Pull Request will be automatically closed by @BrownTruck since it cannot be merged. If this pull request is still valid, please rebase it against If you choose to rebase/merge and resubmit this Pull Request, here is an example message that you can copy and paste:
|
This Pull Request was closed because it cannot be automatically reparented to the Please feel free to re-open it or re-submit it if it is still valid and you have rebased it onto |
This work started in PR #2911, and has been reproduced in a cleaner fashion here.
This allows a user to ask to download wheels of a different platform than that of the local machine running the command. This option enforces that the --download option is also specified, and will not attempt to install wheels of an incorrect platform.