diff --git a/news/6252.bugfix b/news/6252.bugfix new file mode 100644 index 00000000000..d392bec16d0 --- /dev/null +++ b/news/6252.bugfix @@ -0,0 +1 @@ +Fix an ``IndexError`` crash when a legacy build of a wheel fails. diff --git a/src/pip/_internal/wheel.py b/src/pip/_internal/wheel.py index 700b180969e..1348f49da7c 100644 --- a/src/pip/_internal/wheel.py +++ b/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) diff --git a/tests/lib/__init__.py b/tests/lib/__init__.py index 5269b8409ed..837d982b6f8 100644 --- a/tests/lib/__init__.py +++ b/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 diff --git a/tests/unit/test_wheel.py b/tests/unit/test_wheel.py index 4a8a0bde727..b0648b0559c 100644 --- a/tests/unit/test_wheel.py +++ b/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"])