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

Added --prefer-binary flag. #5370

Merged
merged 5 commits into from
May 11, 2018
Merged

Added --prefer-binary flag. #5370

merged 5 commits into from
May 11, 2018

Conversation

DanielShaulov
Copy link
Contributor

@DanielShaulov DanielShaulov commented May 5, 2018

Hi,

I ran into #3785, and I need this feature, because c/cpp compilation is cumbersome, so if there is a wheel, even an old one (but still valid for the requirements), i'd rather use it than attempt to compile the source package. I can't use --only-binary :all:, because then python-only package that provide only source distribution won't be installed.
This PR adds the needed feature.

Thanks.

Fixes #3785

The flag makes pip prefer an old but valid binary over a newer source package.
Fixes #3785.
@pfmoore
Copy link
Member

pfmoore commented May 5, 2018

In principle I like this idea (in some cases, compilation isn't merely cumbersome, it's simply not possible, due to for example lack of a compiler but there's no way for pip to know that in advance). I'd like some tests to be added, to clearly define the expected behaviour. Also, I'm a little concerned about the proliferation of flags here. It might be better to think about the UX across all the options in this area rather than just adding another flag - but that's something we could look at later if needed.

@DanielShaulov
Copy link
Contributor Author

I added 3 tests which should illustrate the needed behaviour and how it differs from regular and --only-binary :all: behaviour:
test_download_prefer_binary_when_tarball_higher_than_wheel - test the required feature, take the wheel, even if there is a tarball with a higher version.
The other 2 tests (only tarball, and tarball+wheel where the wheel doesn't satisfy requirements) are cases where --prefer-binary does nothing. They are interesting because those are the cases where --only-binary :all: falls short, and they are the reason why prefering binaries is better than forcing binaries.

About the UX issues - the only thing I can come up with to save a flag is --only-binary :prefer: instead of a separate flag. But I think it is way more complex to use and explain an str flag with magics, and using boolean flags is much easier.
If you're talking about some larger UX redesign of all thos flags, which requires breaking the existing ones, than the new flag can be broken as well in the redesign. I will really prefer that this feature doesn't get blocked for such a redesign.

Thanks

@pfmoore
Copy link
Member

pfmoore commented May 6, 2018

Agreed, we shouldn't block this on broader questions - I mostly just wanted to add the comment here as a reminder for later.

'--prefer-binary',
'--no-index',
'-f', data.packages,
'-d', '.', 'source>=0.9'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe make this "> 0.9"?

@pradyunsg
Copy link
Member

Is there some reason the test failed on Windows but not on Unix?

@DanielShaulov
Copy link
Contributor Author

So, it turns out that on windows only, scripttest runs the command with shell=True, because of some bug with PATH handling in subprocess.
So the >=0.9 part just makes the stdout go to a file named 0.9.

I'll try to fix it soon (Maybe I'll just use a requirements file instead of a cli param)

@DanielShaulov
Copy link
Contributor Author

Fixed :)

Copy link
Member

@pradyunsg pradyunsg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

If you could update the NEWS fragment, that would be nice (not a blocker). :)

@@ -0,0 +1 @@
Add the --prefer-binary flag, to prefer older wheels over newer source packages.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Introduce a new --prefer-binary flag, to prefer older wheels over newer source packages.

pradyunsg
pradyunsg approved these changes May 7, 2018
@DanielShaulov
Copy link
Contributor Author

So, can this be merged? (Changed the news fragment)

@pradyunsg pradyunsg added S: needs triage Issues/PRs that need to be triaged type: enhancement Improvements to functionality C: wheel The wheel format and 'pip wheel' command and removed S: needs triage Issues/PRs that need to be triaged labels May 11, 2018
@pradyunsg pradyunsg added this to the 18.0 milestone May 11, 2018
@pradyunsg
Copy link
Member

So, can this be merged? (Changed the news fragment)

Yes.

@pradyunsg pradyunsg merged commit d67d98d into pypa:master May 11, 2018
@DanielShaulov DanielShaulov deleted the feaute/add_prefer_binary branch May 11, 2018 08:15
@desaintmartin
Copy link

Would it be possible to have a new version of pip released with (at least) this feature? This is a nice improvement and would be helpful for a lot of people out there.

@pfmoore
Copy link
Member

pfmoore commented Jul 19, 2018

pip 18.0 is due for release this month.

@lock
Copy link

lock bot commented Jun 2, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot added the auto-locked Outdated issues that have been locked by automation label Jun 2, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Jun 2, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
auto-locked Outdated issues that have been locked by automation C: wheel The wheel format and 'pip wheel' command type: enhancement Improvements to functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Prefering wheel-based installation over source-based installation
4 participants