Address #6197: use the ephemeral cache if we need to when autobuilding

news/6197.bugfix

@@ -0,0 +1,3 @@
+Fix a crash where PEP 517-based builds using ``--no-cache-dir`` would fail in
+some circumstances with an ``AssertionError`` due to not finalizing a build
+directory internally.

src/pip/_internal/wheel.py

@@ -48,7 +48,7 @@
     from pip._vendor.packaging.requirements import Requirement  # noqa: F401
     from pip._internal.req.req_install import InstallRequirement  # noqa: F401
     from pip._internal.download import PipSession  # noqa: F401
-    from pip._internal.index import PackageFinder  # noqa: F401
+    from pip._internal.index import FormatControl, PackageFinder  # noqa: F401
     from pip._internal.operations.prepare import (  # noqa: F401
         RequirementPreparer
     )
@@ -725,6 +725,60 @@ def _contains_egg_info(
     return bool(_egg_info_re.search(s))
 
 
+def should_use_ephemeral_cache(
+    req,  # type: InstallRequirement
+    format_control,  # type: FormatControl
+    autobuilding,  # type: bool
+    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
+        autobuilding=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.constraint:
+        return None
+    if req.is_wheel:
+        if not autobuilding:
+            logger.info(
+                'Skipping %s, due to already being wheel.', req.name,
+            )
+        return None
+    if not autobuilding:
+        return False
+
+    if req.editable or not req.source_dir:
+        return None
+
+    if req.link and not req.link.is_artifact:
+        # VCS checkout. Build wheel just for this run.
+        return True
+
+    if "binary" not in format_control.get_allowed_formats(
+            canonicalize_name(req.name)):
+        logger.info(
+            "Skipping bdist_wheel for %s, due to binaries "
+            "being disabled for it.", req.name,
+        )
+        return None
+
+    link = req.link
+    base, ext = link.splitext()
+    if cache_available and _contains_egg_info(base):
+        return False
+
+    # 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
+
+
 class WheelBuilder(object):
     """Build wheels from a RequirementSet."""
 
@@ -858,40 +912,20 @@ def build(
             newly built wheel, in preparation for installation.
         :return: True if all the wheels built correctly.
         """
-
         buildset = []
         format_control = self.finder.format_control
+        # Whether a cache directory is available for autobuilding=True.
+        cache_available = bool(self._wheel_dir or self.wheel_cache.cache_dir)
+
         for req in requirements:
-            if req.constraint:
+            ephem_cache = should_use_ephemeral_cache(
+                req, format_control=format_control, autobuilding=autobuilding,
+                cache_available=cache_available,
+            )
+            if ephem_cache is None:
                 continue
-            if req.is_wheel:
-                if not autobuilding:
-                    logger.info(
-                        'Skipping %s, due to already being wheel.', req.name,
-                    )
-            elif autobuilding and req.editable:
-                pass
-            elif autobuilding and not req.source_dir:
-                pass
-            elif autobuilding and req.link and not req.link.is_artifact:
-                # VCS checkout. Build wheel just for this run.
-                buildset.append((req, True))
-            else:
-                ephem_cache = False
-                if autobuilding:
-                    link = req.link
-                    base, ext = link.splitext()
-                    if not _contains_egg_info(base):
-                        # E.g. local directory. Build wheel just for this run.
-                        ephem_cache = True
-                    if "binary" not in format_control.get_allowed_formats(
-                            canonicalize_name(req.name)):
-                        logger.info(
-                            "Skipping bdist_wheel for %s, due to binaries "
-                            "being disabled for it.", req.name,
-                        )
-                        continue
-                buildset.append((req, ephem_cache))
+
+            buildset.append((req, ephem_cache))
 
         if not buildset:
             return []

tests/unit/test_wheel.py

@@ -10,6 +10,9 @@
 
 from pip._internal import pep425tags, wheel
 from pip._internal.exceptions import InvalidWheelFilename, UnsupportedWheel
+from pip._internal.index import FormatControl
+from pip._internal.models.link import Link
+from pip._internal.req.req_install import InstallRequirement
 from pip._internal.utils.compat import WINDOWS
 from pip._internal.utils.misc import unpack_file
 from tests.lib import DATA_DIR
@@ -35,6 +38,55 @@ def test_contains_egg_info(s, expected):
     assert result == expected
 
 
+@pytest.mark.parametrize(
+    "base_name, autobuilding, cache_available, expected",
+    [
+        ('pendulum-2.0.4', False, False, False),
+        # The following cases test autobuilding=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),
+    ],
+)
+def test_should_use_ephemeral_cache__issue_6197(
+    base_name, autobuilding, cache_available, expected,
+):
+    """
+    Regression test for: https://github.com/pypa/pip/issues/6197
+    """
+    req = Requirement('pendulum')
+    link_url = (
+        'https://files.pythonhosted.org/packages/aa/{base_name}.tar.gz'
+        '#sha256=cf535d36c063575d4752af36df928882b2e0e31541b4482c97d637527'
+        '85f9fcb'
+    ).format(base_name=base_name)
+    link = Link(
+        url=link_url,
+        comes_from='https://pypi.org/simple/pendulum/',
+        requires_python='>=2.7, !=3.0.*, !=3.1.*, !=3.2.*, !=3.3.*',
+    )
+    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_artifact
+
+    format_control = FormatControl()
+    ephem_cache = wheel.should_use_ephemeral_cache(
+        req, format_control=format_control, autobuilding=autobuilding,
+        cache_available=cache_available,
+    )
+    assert ephem_cache is expected
+
+
 @pytest.mark.parametrize("console_scripts",
                          ["pip = pip._internal.main:pip",
                           "pip:pip = pip._internal.main:pip"])