From d5dd149c2d9cf6507ce98127d37d9882c7bf1f97 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Bidoul=20=28ACSONE=29?=
 <stephane.bidoul@acsone.eu>
Date: Mon, 12 Aug 2019 23:11:19 +0200
Subject: [PATCH 1/9] Add test for pip wheel build cache

---
 tests/functional/test_wheel.py | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/tests/functional/test_wheel.py b/tests/functional/test_wheel.py
index 5ebc9ea4c21..00959f65dd6 100644
--- a/tests/functional/test_wheel.py
+++ b/tests/functional/test_wheel.py
@@ -61,6 +61,30 @@ def test_pip_wheel_success(script, data):
     assert "Successfully built simple" in result.stdout, result.stdout
 
 
+def test_pip_wheel_build_cache(script, data):
+    """
+    Test 'pip wheel' builds and caches.
+    """
+    result = script.pip(
+        'wheel', '--no-index', '-f', data.find_links,
+        'simple==3.0',
+    )
+    wheel_file_name = 'simple-3.0-py%s-none-any.whl' % pyversion[0]
+    wheel_file_path = script.scratch / wheel_file_name
+    assert wheel_file_path in result.files_created, result.stdout
+    assert "Successfully built simple" in result.stdout, result.stdout
+    # remove target file
+    (script.scratch_path / wheel_file_name).unlink()
+    # pip wheel again and test that no build occurs since
+    # we get the wheel from cache
+    result = script.pip(
+        'wheel', '--no-index', '-f', data.find_links,
+        'simple==3.0',
+    )
+    assert wheel_file_path in result.files_created, result.stdout
+    assert "Successfully built simple" not in result.stdout, result.stdout
+
+
 def test_basic_pip_wheel_downloads_wheels(script, data):
     """
     Test 'pip wheel' downloads wheels

From 51be2b6fb4b67c36eb0e00587f064e6d3c0a98c1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Bidoul=20=28ACSONE=29?=
 <stephane.bidoul@acsone.eu>
Date: Sat, 10 Aug 2019 18:53:07 +0200
Subject: [PATCH 2/9] Cache wheels that pip wheel built locally

Instead of building to the target wheel directory,
build to cache instead and then copy the result to the target directory.
This more closely matches what pip install does and makes the
cache more effective.
---
 news/6852.bugfix           |  2 ++
 src/pip/_internal/wheel.py | 48 +++++++++++++++++++++++---------------
 tests/unit/test_wheel.py   |  2 +-
 3 files changed, 32 insertions(+), 20 deletions(-)
 create mode 100644 news/6852.bugfix

diff --git a/news/6852.bugfix b/news/6852.bugfix
new file mode 100644
index 00000000000..f9ae543ada9
--- /dev/null
+++ b/news/6852.bugfix
@@ -0,0 +1,2 @@
+Cache wheels that ``pip wheel`` built locally after downloading
+them as sdist. This matches what ``pip install`` does.
diff --git a/src/pip/_internal/wheel.py b/src/pip/_internal/wheel.py
index bc0cdd260b1..3e93f6c3de0 100644
--- a/src/pip/_internal/wheel.py
+++ b/src/pip/_internal/wheel.py
@@ -793,24 +793,33 @@ def should_use_ephemeral_cache(
     if req.constraint:
         # never build requirements that are merely constraints
         return None
+
     if req.is_wheel:
         if not should_unpack:
             logger.info(
                 'Skipping %s, due to already being wheel.', req.name,
             )
+        # never build a requirement that is already a wheel
+        return None
+
+    if req.editable:
+        # if must build an editable, build to ephem cache,
+        # else do not build as install will egglink to it
+        if not should_unpack:
+            return True
         return None
-    if not should_unpack:
-        # i.e. pip wheel, not pip install;
-        # return False, knowing that the caller will never cache
-        # in this case anyway, so this return merely means "build it".
-        # TODO improve this behavior
-        return False
 
-    if req.editable or not req.source_dir:
+    if not req.source_dir:
+        # TODO explain what no source_dir means
+        if not should_unpack:
+            return True
         return None
 
     if "binary" not in format_control.get_allowed_formats(
             canonicalize_name(req.name)):
+        if not should_unpack:
+            # --no-binary has no effect as a pip wheel option
+            return True
         logger.info(
             "Skipping bdist_wheel for %s, due to binaries "
             "being disabled for it.", req.name,
@@ -1098,19 +1107,17 @@ def build(
                 python_tag = None
                 if should_unpack:
                     python_tag = pep425tags.implementation_tag
-                    if ephem:
-                        output_dir = _cache.get_ephem_path_for_link(req.link)
-                    else:
-                        output_dir = _cache.get_path_for_link(req.link)
-                    try:
-                        ensure_dir(output_dir)
-                    except OSError as e:
-                        logger.warning("Building wheel for %s failed: %s",
-                                       req.name, e)
-                        build_failure.append(req)
-                        continue
+                if ephem:
+                    output_dir = _cache.get_ephem_path_for_link(req.link)
                 else:
-                    output_dir = self._wheel_dir
+                    output_dir = _cache.get_path_for_link(req.link)
+                try:
+                    ensure_dir(output_dir)
+                except OSError as e:
+                    logger.warning("Building wheel for %s failed: %s",
+                                   req.name, e)
+                    build_failure.append(req)
+                    continue
                 wheel_file = self._build_one(
                     req, output_dir,
                     python_tag=python_tag,
@@ -1139,6 +1146,9 @@ def build(
                         assert req.link.is_wheel
                         # extract the wheel into the dir
                         unpack_file_url(link=req.link, location=req.source_dir)
+                    else:
+                        shutil.copy(wheel_file, self._wheel_dir)
+                        logger.info('Stored in directory: %s', self._wheel_dir)
                 else:
                     build_failure.append(req)
 
diff --git a/tests/unit/test_wheel.py b/tests/unit/test_wheel.py
index 449da28c199..f3d4e4510e8 100644
--- a/tests/unit/test_wheel.py
+++ b/tests/unit/test_wheel.py
@@ -80,7 +80,7 @@ def test_format_tag(file_tag, expected):
 @pytest.mark.parametrize(
     "base_name, should_unpack, cache_available, expected",
     [
-        ('pendulum-2.0.4', False, False, False),
+        ('pendulum-2.0.4', False, False, True),
         # The following cases test should_unpack=True.
         # Test _contains_egg_info() returning True.
         ('pendulum-2.0.4', True, True, False),

From a16e49cc83b81964e30092799fe7049c59f7f26b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Bidoul=20=28ACSONE=29?=
 <stephane.bidoul@acsone.eu>
Date: Fri, 16 Aug 2019 13:51:35 +0200
Subject: [PATCH 3/9] Extract must_build out of should_use_ephemeral_cache

---
 src/pip/_internal/wheel.py |  89 +++++++++++++++++-----------
 tests/unit/test_wheel.py   | 117 ++++++++++++++++++++-----------------
 2 files changed, 117 insertions(+), 89 deletions(-)

diff --git a/src/pip/_internal/wheel.py b/src/pip/_internal/wheel.py
index 3e93f6c3de0..c8d16410e4a 100644
--- a/src/pip/_internal/wheel.py
+++ b/src/pip/_internal/wheel.py
@@ -773,26 +773,18 @@ def _contains_egg_info(
     return bool(_egg_info_re.search(s))
 
 
-def should_use_ephemeral_cache(
+def should_build(
     req,  # type: InstallRequirement
-    format_control,  # type: FormatControl
     should_unpack,  # type: bool
-    cache_available  # type: bool
+    format_control,  # type: FormatControl
 ):
-    # type: (...) -> Optional[bool]
     """
-    Return whether to build an InstallRequirement object using the
-    ephemeral cache.
-
-    :param cache_available: whether a cache directory is available for the
-        should_unpack=True case.
-
-    :return: True or False to build the requirement with ephem_cache=True
-        or False, respectively; or None not to build the requirement.
+    Return whether a wheel should be built based on the requirement
+    characteristics, and if it should be ultimately unpacked or not.
     """
     if req.constraint:
         # never build requirements that are merely constraints
-        return None
+        return False
 
     if req.is_wheel:
         if not should_unpack:
@@ -800,31 +792,56 @@ def should_use_ephemeral_cache(
                 'Skipping %s, due to already being wheel.', req.name,
             )
         # never build a requirement that is already a wheel
-        return None
+        return False
 
     if req.editable:
-        # if must build an editable, build to ephem cache,
-        # else do not build as install will egglink to it
-        if not should_unpack:
-            return True
-        return None
+        # don't build an editable if it should be unpacked
+        return not should_unpack
 
     if not req.source_dir:
         # TODO explain what no source_dir means
-        if not should_unpack:
-            return True
-        return None
+        return not should_unpack
 
-    if "binary" not in format_control.get_allowed_formats(
+    if should_unpack and "binary" not in format_control.get_allowed_formats(
             canonicalize_name(req.name)):
-        if not should_unpack:
-            # --no-binary has no effect as a pip wheel option
-            return True
         logger.info(
             "Skipping bdist_wheel for %s, due to binaries "
             "being disabled for it.", req.name,
         )
-        return None
+        return False
+
+    return True
+
+
+def should_use_ephemeral_cache(
+    req,  # type: InstallRequirement
+    format_control,  # type: FormatControl
+    cache_available,  # type: bool
+):
+    # type: (...) -> Optional[bool]
+    """
+    Return whether to build an InstallRequirement object using the
+    ephemeral cache.
+
+    :param cache_available: whether a cache directory is available for the
+        should_unpack=True case.
+
+    :return: True or False to build the requirement with ephem_cache=True
+        or False, respectively; or None not to build the requirement.
+    """
+    if req.editable:
+        # don't cache editable requirements because they can
+        # be locally modified
+        return True
+
+    if not req.source_dir:
+        # TODO explain what no source_dir means
+        return True
+
+    if "binary" not in format_control.get_allowed_formats(
+            canonicalize_name(req.name)):
+        # TODO explain this
+        return True
 
     if req.link and req.link.is_vcs:
         # VCS checkout. Build wheel just for this run.
@@ -1071,16 +1088,18 @@ def build(
         cache_available = bool(self.wheel_cache.cache_dir)
 
         for req in requirements:
-            ephem_cache = should_use_ephemeral_cache(
+            if not should_build(
                 req,
-                format_control=format_control,
                 should_unpack=should_unpack,
+                format_control=format_control,
+            ):
+                continue
+            use_ephemeral_cache = should_use_ephemeral_cache(
+                req,
+                format_control=format_control,
                 cache_available=cache_available,
             )
-            if ephem_cache is None:
-                continue
-
-            buildset.append((req, ephem_cache))
+            buildset.append((req, use_ephemeral_cache))
 
         if not buildset:
             return []
@@ -1103,11 +1122,11 @@ def build(
         _cache = self.wheel_cache  # shorter name
         with indent_log():
             build_success, build_failure = [], []
-            for req, ephem in buildset:
+            for req, use_ephemeral_cache in buildset:
                 python_tag = None
                 if should_unpack:
                     python_tag = pep425tags.implementation_tag
-                if ephem:
+                if use_ephemeral_cache:
                     output_dir = _cache.get_ephem_path_for_link(req.link)
                 else:
                     output_dir = _cache.get_path_for_link(req.link)
diff --git a/tests/unit/test_wheel.py b/tests/unit/test_wheel.py
index f3d4e4510e8..4e49ac43285 100644
--- a/tests/unit/test_wheel.py
+++ b/tests/unit/test_wheel.py
@@ -38,6 +38,25 @@ def test_contains_egg_info(s, expected):
     assert result == expected
 
 
+class ReqMock:
+
+    def __init__(
+        self,
+        name="pendulum",
+        is_wheel=False,
+        editable=False,
+        link=None,
+        constraint=False,
+        source_dir="/tmp/pip-install-123/pendulum",
+    ):
+        self.name = name
+        self.is_wheel = is_wheel
+        self.editable = editable
+        self.link = link
+        self.constraint = constraint
+        self.source_dir = source_dir
+
+
 def make_test_install_req(base_name=None):
     """
     Return an InstallRequirement object for testing purposes.
@@ -78,76 +97,66 @@ def test_format_tag(file_tag, expected):
 
 
 @pytest.mark.parametrize(
-    "base_name, should_unpack, cache_available, expected",
+    "req, should_unpack, disallow_binaries, expected",
     [
-        ('pendulum-2.0.4', False, False, True),
-        # The following cases test should_unpack=True.
-        # Test _contains_egg_info() returning True.
-        ('pendulum-2.0.4', True, True, False),
-        ('pendulum-2.0.4', True, False, True),
-        # Test _contains_egg_info() returning False.
-        ('pendulum', True, True, True),
-        ('pendulum', True, False, True),
+        # pip wheel (should_unpack=False)
+        (ReqMock(), False, False, True),
+        (ReqMock(), False, True, True),
+        (ReqMock(constraint=True), False, False, False),
+        (ReqMock(is_wheel=True), False, False, False),
+        (ReqMock(editable=True), False, False, True),
+        (ReqMock(source_dir=None), False, False, True),
+        (ReqMock(link=Link("git+https://g.c/org/repo")), False, False, True),
+        (ReqMock(link=Link("git+https://g.c/org/repo")), False, True, True),
+        # pip install (should_unpack=True)
+        (ReqMock(), True, False, True),
+        (ReqMock(), True, True, False),
+        (ReqMock(constraint=True), True, False, False),
+        (ReqMock(is_wheel=True), True, False, False),
+        (ReqMock(editable=True), True, False, False),
+        (ReqMock(source_dir=None), True, False, False),
+        # By default (i.e. when binaries are allowed), VCS requirements
+        # should be built in install mode.
+        (ReqMock(link=Link("git+https://g.c/org/repo")), True, False, True),
+        # Disallowing binaries, however, should cause them not to be built.
+        (ReqMock(link=Link("git+https://g.c/org/repo")), True, True, False),
     ],
 )
-def test_should_use_ephemeral_cache__issue_6197(
-    base_name, should_unpack, cache_available, expected,
-):
-    """
-    Regression test for: https://github.com/pypa/pip/issues/6197
-    """
-    req = make_test_install_req(base_name=base_name)
-    assert not req.is_wheel
-    assert req.link.is_artifact
-
+def test_should_build(req, should_unpack, disallow_binaries, expected):
     format_control = FormatControl()
-    ephem_cache = wheel.should_use_ephemeral_cache(
-        req, format_control=format_control, should_unpack=should_unpack,
-        cache_available=cache_available,
+    if disallow_binaries:
+        format_control.disallow_binaries()
+    should_build = wheel.should_build(
+        req, should_unpack=should_unpack, format_control=format_control
     )
-    assert ephem_cache is expected
+    assert should_build is expected
 
 
 @pytest.mark.parametrize(
-    "disallow_binaries, expected",
+    "req, disallow_binaries, cache_available, expected",
     [
-        # By default (i.e. when binaries are allowed), VCS requirements
-        # should be built.
-        (False, True),
-        # Disallowing binaries, however, should cause them not to be built.
-        (True, None),
+        (ReqMock(editable=True), False, True, True),
+        (ReqMock(editable=True), False, False, True),
+        (ReqMock(source_dir=None), False, True, True),
+        (ReqMock(source_dir=None), False, False, True),
+        (ReqMock(link=Link("git+https://g.c/org/repo")), False, True, True),
+        (ReqMock(link=Link("git+https://g.c/org/repo")), False, False, True),
+        (ReqMock(link=Link("https://g.c/dist.tgz")), False, True, True),
+        (ReqMock(link=Link("https://g.c/dist.tgz")), False, False, True),
+        (ReqMock(link=Link("https://g.c/dist-2.0.4.tgz")), False, True, False),
+        (ReqMock(link=Link("https://g.c/dist-2.0.4.tgz")), False, False, True),
     ],
 )
-def test_should_use_ephemeral_cache__disallow_binaries_and_vcs_checkout(
-    disallow_binaries, expected,
+def test_should_use_ephemeral_cache(
+    req, disallow_binaries, cache_available, expected
 ):
-    """
-    Test that disallowing binaries (e.g. from passing --global-option)
-    causes should_use_ephemeral_cache() to return None for VCS checkouts.
-    """
-    req = Requirement('pendulum')
-    link = Link(url='git+https://git.example.com/pendulum.git')
-    req = InstallRequirement(
-        req=req,
-        comes_from=None,
-        constraint=False,
-        editable=False,
-        link=link,
-        source_dir='/tmp/pip-install-9py5m2z1/pendulum',
-    )
-    assert not req.is_wheel
-    assert req.link.is_vcs
-
     format_control = FormatControl()
     if disallow_binaries:
         format_control.disallow_binaries()
-
-    # The cache_available value doesn't matter for this test.
-    ephem_cache = wheel.should_use_ephemeral_cache(
-        req, format_control=format_control, should_unpack=True,
-        cache_available=True,
+    should_use_ephemeral_cache = wheel.should_use_ephemeral_cache(
+        req, format_control=format_control, cache_available=cache_available
     )
-    assert ephem_cache is expected
+    assert should_use_ephemeral_cache is expected
 
 
 def test_format_command_result__INFO(caplog):

From a1acf33adb05fb18e0a643af8f9a7351416e8a51 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Bidoul=20=28ACSONE=29?=
 <stephane.bidoul@acsone.eu>
Date: Fri, 16 Aug 2019 14:03:16 +0200
Subject: [PATCH 4/9] Add TODO for VCS caching

---
 src/pip/_internal/wheel.py | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/pip/_internal/wheel.py b/src/pip/_internal/wheel.py
index c8d16410e4a..ae8f3ea501d 100644
--- a/src/pip/_internal/wheel.py
+++ b/src/pip/_internal/wheel.py
@@ -844,7 +844,10 @@ def should_use_ephemeral_cache(
         return True
 
     if req.link and req.link.is_vcs:
-        # VCS checkout. Build wheel just for this run.
+        # VCS checkout. Build wheel just for this run
+        # unless it points to an immutable commit hash in which
+        # case it can be cached.
+        # TODO if req.link.is_vcs_immutable: return False
         return True
 
     link = req.link

From a7974d693f6ae2a1c0d84bb3b89cf2973f1746c8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Bidoul=20=28ACSONE=29?=
 <stephane.bidoul@acsone.eu>
Date: Sat, 24 Aug 2019 12:07:16 +0200
Subject: [PATCH 5/9] Simplify should_build

---
 src/pip/_internal/wheel.py | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/src/pip/_internal/wheel.py b/src/pip/_internal/wheel.py
index ae8f3ea501d..bf680ac9fec 100644
--- a/src/pip/_internal/wheel.py
+++ b/src/pip/_internal/wheel.py
@@ -794,15 +794,18 @@ def should_build(
         # never build a requirement that is already a wheel
         return False
 
+    if not should_unpack:
+        return True
+
     if req.editable:
-        # don't build an editable if it should be unpacked
-        return not should_unpack
+        # don't build an editable if it should ultimately be unpacked
+        return False
 
     if not req.source_dir:
         # TODO explain what no source_dir means
-        return not should_unpack
+        return False
 
-    if should_unpack and "binary" not in format_control.get_allowed_formats(
+    if "binary" not in format_control.get_allowed_formats(
             canonicalize_name(req.name)):
         logger.info(
             "Skipping bdist_wheel for %s, due to binaries "

From 3e4bc702abf2e6523c1dacc1e5f9b63cdc0b9aef Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Bidoul=20=28ACSONE=29?=
 <stephane.bidoul@acsone.eu>
Date: Sat, 24 Aug 2019 12:09:00 +0200
Subject: [PATCH 6/9] In should_build, use need_wheel instead of not
 should_unpack

---
 src/pip/_internal/wheel.py | 12 ++++++------
 tests/unit/test_wheel.py   | 36 ++++++++++++++++++------------------
 2 files changed, 24 insertions(+), 24 deletions(-)

diff --git a/src/pip/_internal/wheel.py b/src/pip/_internal/wheel.py
index bf680ac9fec..f354b71dade 100644
--- a/src/pip/_internal/wheel.py
+++ b/src/pip/_internal/wheel.py
@@ -775,30 +775,30 @@ def _contains_egg_info(
 
 def should_build(
     req,  # type: InstallRequirement
-    should_unpack,  # type: bool
+    need_wheel,  # type: bool
     format_control,  # type: FormatControl
 ):
     """
     Return whether a wheel should be built based on the requirement
-    characteristics, and if it should be ultimately unpacked or not.
+    characteristics, and if a wheel is ultimately needed or not.
     """
     if req.constraint:
         # never build requirements that are merely constraints
         return False
 
     if req.is_wheel:
-        if not should_unpack:
+        if need_wheel:
             logger.info(
                 'Skipping %s, due to already being wheel.', req.name,
             )
         # never build a requirement that is already a wheel
         return False
 
-    if not should_unpack:
+    if need_wheel:
         return True
 
     if req.editable:
-        # don't build an editable if it should ultimately be unpacked
+        # don't build an editable if no wheel is needed
         return False
 
     if not req.source_dir:
@@ -1096,7 +1096,7 @@ def build(
         for req in requirements:
             if not should_build(
                 req,
-                should_unpack=should_unpack,
+                need_wheel=not should_unpack,
                 format_control=format_control,
             ):
                 continue
diff --git a/tests/unit/test_wheel.py b/tests/unit/test_wheel.py
index 4e49ac43285..0756c44286f 100644
--- a/tests/unit/test_wheel.py
+++ b/tests/unit/test_wheel.py
@@ -97,37 +97,37 @@ def test_format_tag(file_tag, expected):
 
 
 @pytest.mark.parametrize(
-    "req, should_unpack, disallow_binaries, expected",
+    "req, need_wheel, disallow_binaries, expected",
     [
-        # pip wheel (should_unpack=False)
-        (ReqMock(), False, False, True),
-        (ReqMock(), False, True, True),
-        (ReqMock(constraint=True), False, False, False),
-        (ReqMock(is_wheel=True), False, False, False),
-        (ReqMock(editable=True), False, False, True),
-        (ReqMock(source_dir=None), False, False, True),
-        (ReqMock(link=Link("git+https://g.c/org/repo")), False, False, True),
-        (ReqMock(link=Link("git+https://g.c/org/repo")), False, True, True),
-        # pip install (should_unpack=True)
+        # pip wheel (need_wheel=True)
         (ReqMock(), True, False, True),
-        (ReqMock(), True, True, False),
+        (ReqMock(), True, True, True),
         (ReqMock(constraint=True), True, False, False),
         (ReqMock(is_wheel=True), True, False, False),
-        (ReqMock(editable=True), True, False, False),
-        (ReqMock(source_dir=None), True, False, False),
+        (ReqMock(editable=True), True, False, True),
+        (ReqMock(source_dir=None), True, False, True),
+        (ReqMock(link=Link("git+https://g.c/org/repo")), True, False, True),
+        (ReqMock(link=Link("git+https://g.c/org/repo")), True, True, True),
+        # pip install (need_wheel=False)
+        (ReqMock(), False, False, True),
+        (ReqMock(), False, True, False),
+        (ReqMock(constraint=True), False, False, False),
+        (ReqMock(is_wheel=True), False, False, False),
+        (ReqMock(editable=True), False, False, False),
+        (ReqMock(source_dir=None), False, False, False),
         # By default (i.e. when binaries are allowed), VCS requirements
         # should be built in install mode.
-        (ReqMock(link=Link("git+https://g.c/org/repo")), True, False, True),
+        (ReqMock(link=Link("git+https://g.c/org/repo")), False, False, True),
         # Disallowing binaries, however, should cause them not to be built.
-        (ReqMock(link=Link("git+https://g.c/org/repo")), True, True, False),
+        (ReqMock(link=Link("git+https://g.c/org/repo")), False, True, False),
     ],
 )
-def test_should_build(req, should_unpack, disallow_binaries, expected):
+def test_should_build(req, need_wheel, disallow_binaries, expected):
     format_control = FormatControl()
     if disallow_binaries:
         format_control.disallow_binaries()
     should_build = wheel.should_build(
-        req, should_unpack=should_unpack, format_control=format_control
+        req, need_wheel=need_wheel, format_control=format_control
     )
     assert should_build is expected
 

From 3844dce9c0b15d23047d87737519aa3df2e16943 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Bidoul=20=28ACSONE=29?=
 <stephane.bidoul@acsone.eu>
Date: Tue, 27 Aug 2019 18:49:10 +0200
Subject: [PATCH 7/9] rename should_user_ephemeral_cache to should_cache

For clarity
---
 src/pip/_internal/wheel.py | 26 +++++++++++++-------------
 tests/unit/test_wheel.py   |  2 +-
 2 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/src/pip/_internal/wheel.py b/src/pip/_internal/wheel.py
index f354b71dade..b1b91a85ef8 100644
--- a/src/pip/_internal/wheel.py
+++ b/src/pip/_internal/wheel.py
@@ -816,7 +816,7 @@ def should_build(
     return True
 
 
-def should_use_ephemeral_cache(
+def should_cache(
     req,  # type: InstallRequirement
     format_control,  # type: FormatControl
     cache_available,  # type: bool
@@ -824,44 +824,44 @@ def should_use_ephemeral_cache(
     # type: (...) -> Optional[bool]
     """
     Return whether to build an InstallRequirement object using the
-    ephemeral cache.
+    wheel cache.
 
     :param cache_available: whether a cache directory is available for the
         should_unpack=True case.
 
-    :return: True or False to build the requirement with ephem_cache=True
-        or False, respectively; or None not to build the requirement.
+    :return: True to use the persistent cache,
+        False to use the ephemeral cache.
     """
     if req.editable:
         # don't cache editable requirements because they can
         # be locally modified
-        return True
+        return False
 
     if not req.source_dir:
         # TODO explain what no source_dir means
-        return True
+        return False
 
     if "binary" not in format_control.get_allowed_formats(
             canonicalize_name(req.name)):
         # TODO explain this
-        return True
+        return False
 
     if req.link and req.link.is_vcs:
         # VCS checkout. Build wheel just for this run
         # unless it points to an immutable commit hash in which
         # case it can be cached.
-        # TODO if req.link.is_vcs_immutable: return False
-        return True
+        # TODO cache against the commit hash if we can.
+        return False
 
     link = req.link
     base, ext = link.splitext()
     if cache_available and _contains_egg_info(base):
-        return False
+        return True
 
     # Otherwise, build the wheel just for this run using the ephemeral
     # cache since we are either in the case of e.g. a local directory, or
     # no cache directory is available to use.
-    return True
+    return False
 
 
 def format_command_result(
@@ -1100,7 +1100,7 @@ def build(
                 format_control=format_control,
             ):
                 continue
-            use_ephemeral_cache = should_use_ephemeral_cache(
+            use_ephemeral_cache = not should_cache(
                 req,
                 format_control=format_control,
                 cache_available=cache_available,
@@ -1111,7 +1111,7 @@ def build(
             return []
 
         # Is any wheel build not using the ephemeral cache?
-        if any(not ephem_cache for _, ephem_cache in buildset):
+        if any(not use_ephemeral_cache for _, use_ephemeral_cache in buildset):
             have_directory_for_build = self._wheel_dir or (
                 should_unpack and self.wheel_cache.cache_dir
             )
diff --git a/tests/unit/test_wheel.py b/tests/unit/test_wheel.py
index 0756c44286f..c69624cf1ae 100644
--- a/tests/unit/test_wheel.py
+++ b/tests/unit/test_wheel.py
@@ -153,7 +153,7 @@ def test_should_use_ephemeral_cache(
     format_control = FormatControl()
     if disallow_binaries:
         format_control.disallow_binaries()
-    should_use_ephemeral_cache = wheel.should_use_ephemeral_cache(
+    should_use_ephemeral_cache = not wheel.should_cache(
         req, format_control=format_control, cache_available=cache_available
     )
     assert should_use_ephemeral_cache is expected

From d4b1b15c501ba4ef4511cfa9f8d7970d1f5f7ab5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Bidoul=20=28ACSONE=29?=
 <stephane.bidoul@acsone.eu>
Date: Tue, 27 Aug 2019 18:51:04 +0200
Subject: [PATCH 8/9] rename test_should_use_ephemeral_cache to
 test_should_cache

---
 tests/unit/test_wheel.py | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/tests/unit/test_wheel.py b/tests/unit/test_wheel.py
index c69624cf1ae..055f810b533 100644
--- a/tests/unit/test_wheel.py
+++ b/tests/unit/test_wheel.py
@@ -135,28 +135,28 @@ def test_should_build(req, need_wheel, disallow_binaries, expected):
 @pytest.mark.parametrize(
     "req, disallow_binaries, cache_available, expected",
     [
-        (ReqMock(editable=True), False, True, True),
-        (ReqMock(editable=True), False, False, True),
-        (ReqMock(source_dir=None), False, True, True),
-        (ReqMock(source_dir=None), False, False, True),
-        (ReqMock(link=Link("git+https://g.c/org/repo")), False, True, True),
-        (ReqMock(link=Link("git+https://g.c/org/repo")), False, False, True),
-        (ReqMock(link=Link("https://g.c/dist.tgz")), False, True, True),
-        (ReqMock(link=Link("https://g.c/dist.tgz")), False, False, True),
-        (ReqMock(link=Link("https://g.c/dist-2.0.4.tgz")), False, True, False),
-        (ReqMock(link=Link("https://g.c/dist-2.0.4.tgz")), False, False, True),
+        (ReqMock(editable=True), False, True, False),
+        (ReqMock(editable=True), False, False, False),
+        (ReqMock(source_dir=None), False, True, False),
+        (ReqMock(source_dir=None), False, False, False),
+        (ReqMock(link=Link("git+https://g.c/org/repo")), False, True, False),
+        (ReqMock(link=Link("git+https://g.c/org/repo")), False, False, False),
+        (ReqMock(link=Link("https://g.c/dist.tgz")), False, True, False),
+        (ReqMock(link=Link("https://g.c/dist.tgz")), False, False, False),
+        (ReqMock(link=Link("https://g.c/dist-2.0.4.tgz")), False, True, True),
+        (ReqMock(link=Link("https://g.c/dist-2.0.4.tgz")), False, False, False),
     ],
 )
-def test_should_use_ephemeral_cache(
+def test_should_cache(
     req, disallow_binaries, cache_available, expected
 ):
     format_control = FormatControl()
     if disallow_binaries:
         format_control.disallow_binaries()
-    should_use_ephemeral_cache = not wheel.should_cache(
+    should_cache = wheel.should_cache(
         req, format_control=format_control, cache_available=cache_available
     )
-    assert should_use_ephemeral_cache is expected
+    assert should_cache is expected
 
 
 def test_format_command_result__INFO(caplog):

From 19299a004a44d00fe7da449f2a1dad4c6a43b0b1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Bidoul=20=28ACSONE=29?=
 <stephane.bidoul@acsone.eu>
Date: Tue, 27 Aug 2019 19:01:04 +0200
Subject: [PATCH 9/9] Remove cache_available parameter of should_cache

For readbility: no need to ask if we can cache if we have no cache.
---
 src/pip/_internal/wheel.py | 21 ++++++---------------
 tests/unit/test_wheel.py   | 21 ++++++++-------------
 2 files changed, 14 insertions(+), 28 deletions(-)

diff --git a/src/pip/_internal/wheel.py b/src/pip/_internal/wheel.py
index b1b91a85ef8..e698cb371fe 100644
--- a/src/pip/_internal/wheel.py
+++ b/src/pip/_internal/wheel.py
@@ -819,18 +819,11 @@ def should_build(
 def should_cache(
     req,  # type: InstallRequirement
     format_control,  # type: FormatControl
-    cache_available,  # type: bool
 ):
     # type: (...) -> Optional[bool]
     """
     Return whether to build an InstallRequirement object using the
-    wheel cache.
-
-    :param cache_available: whether a cache directory is available for the
-        should_unpack=True case.
-
-    :return: True to use the persistent cache,
-        False to use the ephemeral cache.
+    wheel cache, assuming the wheel cache is available.
     """
     if req.editable:
         # don't cache editable requirements because they can
@@ -855,12 +848,11 @@ def should_cache(
 
     link = req.link
     base, ext = link.splitext()
-    if cache_available and _contains_egg_info(base):
+    if _contains_egg_info(base):
         return True
 
     # Otherwise, build the wheel just for this run using the ephemeral
-    # cache since we are either in the case of e.g. a local directory, or
-    # no cache directory is available to use.
+    # cache since we are either in the case of e.g. a local directory.
     return False
 
 
@@ -1100,10 +1092,9 @@ def build(
                 format_control=format_control,
             ):
                 continue
-            use_ephemeral_cache = not should_cache(
-                req,
-                format_control=format_control,
-                cache_available=cache_available,
+            use_ephemeral_cache = (
+                not cache_available or
+                not should_cache(req, format_control=format_control)
             )
             buildset.append((req, use_ephemeral_cache))
 
diff --git a/tests/unit/test_wheel.py b/tests/unit/test_wheel.py
index 055f810b533..c3accdf4e57 100644
--- a/tests/unit/test_wheel.py
+++ b/tests/unit/test_wheel.py
@@ -133,28 +133,23 @@ def test_should_build(req, need_wheel, disallow_binaries, expected):
 
 
 @pytest.mark.parametrize(
-    "req, disallow_binaries, cache_available, expected",
+    "req, disallow_binaries, expected",
     [
-        (ReqMock(editable=True), False, True, False),
-        (ReqMock(editable=True), False, False, False),
-        (ReqMock(source_dir=None), False, True, False),
-        (ReqMock(source_dir=None), False, False, False),
-        (ReqMock(link=Link("git+https://g.c/org/repo")), False, True, False),
-        (ReqMock(link=Link("git+https://g.c/org/repo")), False, False, False),
-        (ReqMock(link=Link("https://g.c/dist.tgz")), False, True, False),
-        (ReqMock(link=Link("https://g.c/dist.tgz")), False, False, False),
-        (ReqMock(link=Link("https://g.c/dist-2.0.4.tgz")), False, True, True),
-        (ReqMock(link=Link("https://g.c/dist-2.0.4.tgz")), False, False, False),
+        (ReqMock(editable=True), False, False),
+        (ReqMock(source_dir=None), False, False),
+        (ReqMock(link=Link("git+https://g.c/org/repo")), False, False),
+        (ReqMock(link=Link("https://g.c/dist.tgz")), False, False),
+        (ReqMock(link=Link("https://g.c/dist-2.0.4.tgz")), False, True),
     ],
 )
 def test_should_cache(
-    req, disallow_binaries, cache_available, expected
+    req, disallow_binaries, expected
 ):
     format_control = FormatControl()
     if disallow_binaries:
         format_control.disallow_binaries()
     should_cache = wheel.should_cache(
-        req, format_control=format_control, cache_available=cache_available
+        req, format_control=format_control,
     )
     assert should_cache is expected