Merge pull request #6253 from cjerdonek/issue-6252

Fix an IndexError crash when a legacy build of a wheel fails.

news/6252.bugfix

@@ -0,0 +1 @@
+Fix an ``IndexError`` crash when a legacy build of a wheel fails.

src/pip/_internal/wheel.py

@@ -779,6 +779,42 @@ def should_use_ephemeral_cache(
     return True
 
 
+def get_legacy_build_wheel_path(
+    names,  # type: List[str]
+    temp_dir,  # type: str
+    req,  # type: InstallRequirement
+    command_args,  # type: List[str]
+    command_output,  # type: str
+):
+    # type: (...) -> Optional[str]
+    """
+    Return the path to the wheel in the temporary build directory.
+    """
+    # Sort for determinism.
+    names = sorted(names)
+    if not names:
+        # call_subprocess() ensures that the command output ends in a newline.
+        msg = (
+            'Failed building wheel for {} with args: {}\n'
+            'Command output:\n{}'
+            '-----------------------------------------'
+        ).format(req.name, command_args, command_output)
+        logger.error(msg)
+        return None
+    if len(names) > 1:
+        # call_subprocess() ensures that the command output ends in a newline.
+        msg = (
+            'Found more than one file after building wheel for {} '
+            'with args: {}\n'
+            'Names: {}\n'
+            'Command output:\n{}'
+            '-----------------------------------------'
+        ).format(req.name, command_args, names, command_output)
+        logger.warning(msg)
+
+    return os.path.join(temp_dir, names[0])
+
+
 class WheelBuilder(object):
     """Build wheels from a RequirementSet."""
 
@@ -888,14 +924,21 @@ def _build_one_legacy(self, req, tempd, python_tag=None):
                 wheel_args += ["--python-tag", python_tag]
 
             try:
-                call_subprocess(wheel_args, cwd=req.setup_py_dir,
-                                show_stdout=False, spinner=spinner)
+                output = call_subprocess(wheel_args, cwd=req.setup_py_dir,
+                                         show_stdout=False, spinner=spinner)
             except Exception:
                 spinner.finish("error")
                 logger.error('Failed building wheel for %s', req.name)
                 return None
-            # listdir's return value is sorted to be deterministic
-            return os.path.join(tempd, sorted(os.listdir(tempd))[0])
+            names = os.listdir(tempd)
+            wheel_path = get_legacy_build_wheel_path(
+                names=names,
+                temp_dir=tempd,
+                req=req,
+                command_args=wheel_args,
+                command_output=output,
+            )
+            return wheel_path
 
     def _clean_one(self, req):
         base_args = self._base_setup_args(req)

tests/lib/__init__.py

@@ -21,6 +21,10 @@
 pyversion_tuple = sys.version_info
 
 
+def assert_paths_equal(actual, expected):
+    os.path.normpath(actual) == os.path.normpath(expected)
+
+
 def path_to_url(path):
     """
     Convert a path to URI. The path will be made absolute and

tests/unit/test_wheel.py

@@ -15,7 +15,7 @@
 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
+from tests.lib import DATA_DIR, assert_paths_equal
 
 
 @pytest.mark.parametrize(
@@ -38,25 +38,13 @@ 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,
-):
+def make_test_install_req(base_name=None):
     """
-    Regression test for: https://github.com/pypa/pip/issues/6197
+    Return an InstallRequirement object for testing purposes.
     """
+    if base_name is None:
+        base_name = 'pendulum-2.0.4'
+
     req = Requirement('pendulum')
     link_url = (
         'https://files.pythonhosted.org/packages/aa/{base_name}.tar.gz'
@@ -76,6 +64,30 @@ def test_should_use_ephemeral_cache__issue_6197(
         link=link,
         source_dir='/tmp/pip-install-9py5m2z1/pendulum',
     )
+
+    return req
+
+
+@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 = make_test_install_req(base_name=base_name)
     assert not req.is_wheel
     assert req.link.is_artifact
 
@@ -87,6 +99,59 @@ def test_should_use_ephemeral_cache__issue_6197(
     assert ephem_cache is expected
 
 
+def call_get_legacy_build_wheel_path(caplog, names):
+    req = make_test_install_req()
+    wheel_path = wheel.get_legacy_build_wheel_path(
+        names=names,
+        temp_dir='/tmp/abcd',
+        req=req,
+        command_args=['arg1', 'arg2'],
+        command_output='output line 1\noutput line 2\n',
+    )
+    return wheel_path
+
+
+def test_get_legacy_build_wheel_path(caplog):
+    actual = call_get_legacy_build_wheel_path(caplog, names=['name'])
+    assert_paths_equal(actual, '/tmp/abcd/name')
+    assert not caplog.records
+
+
+def test_get_legacy_build_wheel_path__no_names(caplog):
+    actual = call_get_legacy_build_wheel_path(caplog, names=[])
+    assert actual is None
+    assert len(caplog.records) == 1
+    record = caplog.records[0]
+    assert record.levelname == 'ERROR'
+    assert record.message.splitlines() == [
+        "Failed building wheel for pendulum with args: ['arg1', 'arg2']",
+        "Command output:",
+        "output line 1",
+        "output line 2",
+        "-----------------------------------------",
+    ]
+
+
+def test_get_legacy_build_wheel_path__multiple_names(caplog):
+    # Deliberately pass the names in non-sorted order.
+    actual = call_get_legacy_build_wheel_path(
+        caplog, names=['name2', 'name1'],
+    )
+    assert_paths_equal(actual, '/tmp/abcd/name1')
+    assert len(caplog.records) == 1
+    record = caplog.records[0]
+    assert record.levelname == 'WARNING'
+    assert record.message.splitlines() == [
+        ("Found more than one file after building wheel for pendulum "
+         "with args: ['arg1', 'arg2']"),
+        "Names: ['name1', 'name2']",
+        "Command output:",
+        "output line 1",
+        "output line 2",
+        "-----------------------------------------",
+    ]
+
+
 @pytest.mark.parametrize("console_scripts",
                          ["pip = pip._internal.main:pip",
                           "pip:pip = pip._internal.main:pip"])