Repository: pypa/pip
Pull request: Added --prefer-binary flag.
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

Problem statement:
Prefering wheel-based installation over source-based installation
> @pfmoore said:
> If we had a --prefer-binary option, that said "only use binaries, unless that means there are no candidates, in which case retry allowing source", I suspect many of my concerns that over-eager upgrading would result in breakage might be alleviated.

This issue serves as a starting point for further discussion on this proposed `--prefer-binary` flag.

Hints from issue discussion:
I agree. Binary (wheel-based) installations mostly "just-work". Plus, they are the preferred format for packaging and publishing to PyPI (at least from pip's point-of-view).

I think that making it the default would catalyse the adoption of wheels (not sure if it's needed though). Even in the long term, this seems to be a preferable.

For completeness,

> @dstufft said:
> Likely bundled with a warning when that ends up not installing the latest version that would otherwise have been installed.

I want to bother @dstufft and @pfmoore for this one. Thanks for keeping the discussions going.

Would you want to discuss this now or after the install-as-upgrade work is implemented?

---

From what I understand about the current situation,
- Some source installations require a C/C++/some-compiled-language compiler.
- Some source installations require extra configuration for proper building/installation of the package.
- Some source installations take a long time to complete.

While all this is fine and expected, the issue is pip does not know whether a source installation fits one of these, or any other case I missed. 

Thus, source installations have to be treated differently from wheel installations. wheels are superior if it supports your platform. (Otherwise, it's only source builds. Those platforms won't be affected by this issue anyway) So, preferring them over source builds is indeed favourable. This is already the case for the selected (to-be-downloaded-and-installed) version.

However, there are situations when the user might be okay with getting a slightly older version of a package as long as it's a wheel, to simplify their life. The proposed `--prefer-binary` is a flag to provide this behaviour to the user.

Just a drive-by comment here: I have been bitten several times by projects uploading a new release and the wheels do not come for a little while later. This leaves the new sdist in a state where it is the default for pip. This can cause the build to fail until the wheel is finally uploaded.

Ideally `--prefer-binary` would be the default and if pip is not installing the latest version it can emit a warning. This only leaves cases where a project changed their build process such that they used to build pure-python wheels or wheels for a certain platform that they've now decided to stop building. The user could then use the warning to pin their package to `foo > version-that-changed-build-process` to avoid the binary logic falling back to that super old version.
@mmerickel I don't think it should be default. Anyone installing a package would expect the latest version to be installed, unless they do something to the defaults (like passing this flag).
Is there any update on this issue? This would be a useful feature help us avoid errors with missing C/C++ compilers and save installation time.
Hi @kendallc!

> Is there any update on this issue?

None yet. It's on my list but it's fairly low in it.

This would be nice.

I have a case where I need to pre-download all dependencies for a package for fully offline installation on a different architecture; I can't specify the other architecture without specifying `--only-binary=:all:`, but that makes pip ignore source packages altogether, so not all packages get downloaded. I wouldn't like to have to sift through the list of dependencies (and transitive dependencies) to try and figure out which are binary and which aren't...
Considering this case (https://github.com/pypa/wheel/issues/215), I think pip can't make the decision on the client side only.

Here is a plan for precedence logic of downloading format:

`pip_options_specified or uploader_recommended or wheel_preferred`

ref: https://github.com/pypa/warehouse/issues/3106

Commit message: tests: Added tests for the --prefer-binary flag

Patch to review:
diff --git a/news/3785.feature b/news/3785.feature
new file mode 100644
index 00000000000..fa13f58a02c
--- /dev/null
+++ b/news/3785.feature
@@ -0,0 +1 @@
+Add the --prefer-binary flag, to prefer older wheels over newer source packages.
diff --git a/src/pip/_internal/basecommand.py b/src/pip/_internal/basecommand.py
index 38bdff24a04..cfd5d67577b 100644
--- a/src/pip/_internal/basecommand.py
+++ b/src/pip/_internal/basecommand.py
@@ -370,4 +370,5 @@ def _build_package_finder(self, options, session,
             versions=python_versions,
             abi=abi,
             implementation=implementation,
+            prefer_binary=options.prefer_binary,
         )
diff --git a/src/pip/_internal/cmdoptions.py b/src/pip/_internal/cmdoptions.py
index 6319995d4e9..eb113457f30 100644
--- a/src/pip/_internal/cmdoptions.py
+++ b/src/pip/_internal/cmdoptions.py
@@ -406,6 +406,16 @@ def only_binary():
     )
 
 
+def prefer_binary():
+    return Option(
+        "--prefer-binary",
+        dest="prefer_binary",
+        action="store_true",
+        default=False,
+        help="Prefer older binary packages over newer source packages."
+    )
+
+
 cache_dir = partial(
     Option,
     "--cache-dir",
diff --git a/src/pip/_internal/commands/download.py b/src/pip/_internal/commands/download.py
index 5713d07ce5d..66bcbd5c822 100644
--- a/src/pip/_internal/commands/download.py
+++ b/src/pip/_internal/commands/download.py
@@ -52,6 +52,7 @@ def __init__(self, *args, **kw):
         cmd_opts.add_option(cmdoptions.global_options())
         cmd_opts.add_option(cmdoptions.no_binary())
         cmd_opts.add_option(cmdoptions.only_binary())
+        cmd_opts.add_option(cmdoptions.prefer_binary())
         cmd_opts.add_option(cmdoptions.src())
         cmd_opts.add_option(cmdoptions.pre())
         cmd_opts.add_option(cmdoptions.no_clean())
diff --git a/src/pip/_internal/commands/install.py b/src/pip/_internal/commands/install.py
index 9138683acdd..aa0988c43ad 100644
--- a/src/pip/_internal/commands/install.py
+++ b/src/pip/_internal/commands/install.py
@@ -183,6 +183,7 @@ def __init__(self, *args, **kw):
 
         cmd_opts.add_option(cmdoptions.no_binary())
         cmd_opts.add_option(cmdoptions.only_binary())
+        cmd_opts.add_option(cmdoptions.prefer_binary())
         cmd_opts.add_option(cmdoptions.no_clean())
         cmd_opts.add_option(cmdoptions.require_hashes())
         cmd_opts.add_option(cmdoptions.progress_bar())
diff --git a/src/pip/_internal/commands/wheel.py b/src/pip/_internal/commands/wheel.py
index ac55f91e62e..0fb72c1a92b 100644
--- a/src/pip/_internal/commands/wheel.py
+++ b/src/pip/_internal/commands/wheel.py
@@ -57,6 +57,7 @@ def __init__(self, *args, **kw):
         )
         cmd_opts.add_option(cmdoptions.no_binary())
         cmd_opts.add_option(cmdoptions.only_binary())
+        cmd_opts.add_option(cmdoptions.prefer_binary())
         cmd_opts.add_option(
             '--build-option',
             dest='build_options',
diff --git a/src/pip/_internal/index.py b/src/pip/_internal/index.py
index 3c3a92b7c1a..17aee7246c4 100644
--- a/src/pip/_internal/index.py
+++ b/src/pip/_internal/index.py
@@ -108,7 +108,8 @@ class PackageFinder(object):
     def __init__(self, find_links, index_urls, allow_all_prereleases=False,
                  trusted_hosts=None, process_dependency_links=False,
                  session=None, format_control=None, platform=None,
-                 versions=None, abi=None, implementation=None):
+                 versions=None, abi=None, implementation=None,
+                 prefer_binary=False):
         """Create a PackageFinder.
 
         :param format_control: A FormatControl object or None. Used to control
@@ -176,6 +177,9 @@ def __init__(self, find_links, index_urls, allow_all_prereleases=False,
             impl=implementation,
         )
 
+        # Do we prefer old, but valid, binary dist over new source dist
+        self.prefer_binary = prefer_binary
+
         # If we don't have TLS enabled, then WARN if anyplace we're looking
         # relies on TLS.
         if not HAS_TLS:
@@ -275,12 +279,14 @@ def _candidate_sort_key(self, candidate):
           1. existing installs
           2. wheels ordered via Wheel.support_index_min(self.valid_tags)
           3. source archives
+        If prefer_binary was set, then all wheels are sorted above sources.
         Note: it was considered to embed this logic into the Link
               comparison operators, but then different sdist links
               with the same version, would have to be considered equal
         """
         support_num = len(self.valid_tags)
         build_tag = tuple()
+        binary_preference = 0
         if candidate.location.is_wheel:
             # can raise InvalidWheelFilename
             wheel = Wheel(candidate.location.filename)
@@ -289,6 +295,8 @@ def _candidate_sort_key(self, candidate):
                     "%s is not a supported wheel for this platform. It "
                     "can't be sorted." % wheel.filename
                 )
+            if self.prefer_binary:
+                binary_preference = 1
             pri = -(wheel.support_index_min(self.valid_tags))
             if wheel.build_tag is not None:
                 match = re.match(r'^(\d+)(.*)$', wheel.build_tag)
@@ -296,7 +304,7 @@ def _candidate_sort_key(self, candidate):
                 build_tag = (int(build_tag_groups[0]), build_tag_groups[1])
         else:  # sdist
             pri = -(support_num)
-        return (candidate.version, build_tag, pri)
+        return (binary_preference, candidate.version, build_tag, pri)
 
     def _validate_secure_origin(self, logger, location):
         # Determine if this url used a secure transport mechanism
diff --git a/tests/functional/test_download.py b/tests/functional/test_download.py
index bdb46523d34..829d144aa87 100644
--- a/tests/functional/test_download.py
+++ b/tests/functional/test_download.py
@@ -602,3 +602,55 @@ def test_download_exit_status_code_when_blank_requirements_file(script):
     """
     script.scratch_path.join("blank.txt").write("\n")
     script.pip('download', '-r', 'blank.txt')
+
+
+def test_download_prefer_binary_when_tarball_higher_than_wheel(script, data):
+    fake_wheel(data, 'source-0.8-py2.py3-none-any.whl')
+    result = script.pip(
+        'download',
+        '--prefer-binary',
+        '--no-index',
+        '-f', data.packages,
+        '-d', '.', 'source'
+    )
+    assert (
+        Path('scratch') / 'source-0.8-py2.py3-none-any.whl'
+        in result.files_created
+    )
+    assert (
+        Path('scratch') / 'source-1.0.tar.gz'
+        not in result.files_created
+    )
+
+
+def test_download_prefer_binary_when_wheel_doesnt_satisfy_req(script, data):
+    fake_wheel(data, 'source-0.8-py2.py3-none-any.whl')
+    result = script.pip(
+        'download',
+        '--prefer-binary',
+        '--no-index',
+        '-f', data.packages,
+        '-d', '.', 'source>=0.9'
+    )
+    assert (
+        Path('scratch') / 'source-1.0.tar.gz'
+        in result.files_created
+    )
+    assert (
+        Path('scratch') / 'source-0.8-py2.py3-none-any.whl'
+        not in result.files_created
+    )
+
+
+def test_download_prefer_binary_when_only_tarball_exists(script, data):
+    result = script.pip(
+        'download',
+        '--prefer-binary',
+        '--no-index',
+        '-f', data.packages,
+        '-d', '.', 'source'
+    )
+    assert (
+        Path('scratch') / 'source-1.0.tar.gz'
+        in result.files_created
+    )


Code review comments:
diff_hunk: @@ -602,3 +602,55 @@ def test_download_exit_status_code_when_blank_requirements_file(script):
     """
     script.scratch_path.join("blank.txt").write("\n")
     script.pip('download', '-r', 'blank.txt')
+
+
+def test_download_prefer_binary_when_tarball_higher_than_wheel(script, data):
+    fake_wheel(data, 'source-0.8-py2.py3-none-any.whl')
+    result = script.pip(
+        'download',
+        '--prefer-binary',
+        '--no-index',
+        '-f', data.packages,
+        '-d', '.', 'source'
+    )
+    assert (
+        Path('scratch') / 'source-0.8-py2.py3-none-any.whl'
+        in result.files_created
+    )
+    assert (
+        Path('scratch') / 'source-1.0.tar.gz'
+        not in result.files_created
+    )
+
+
+def test_download_prefer_binary_when_wheel_doesnt_satisfy_req(script, data):
+    fake_wheel(data, 'source-0.8-py2.py3-none-any.whl')
+    result = script.pip(
+        'download',
+        '--prefer-binary',
+        '--no-index',
+        '-f', data.packages,
+        '-d', '.', 'source>=0.9'
path: tests/functional/test_download.py
text: @user1:
Maybe make this "> 0.9"?