Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Feb 11, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions news/6252.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix an ``IndexError`` crash when a legacy build of a wheel fails.
51 changes: 47 additions & 4 deletions src/pip/_internal/wheel.py
Original file line number Diff line number Diff line change
Expand Up @@ -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."""

Expand Down Expand Up @@ -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)
Expand Down
4 changes: 4 additions & 0 deletions tests/lib/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
101 changes: 83 additions & 18 deletions tests/unit/test_wheel.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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'
Expand All @@ -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

Expand All @@ -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"])
Expand Down