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

Revert "Skip candidate not providing valid metadata" #9293

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
15 changes: 0 additions & 15 deletions src/pip/_internal/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -151,21 +151,6 @@ def __str__(self):
)


class InstallationSubprocessError(InstallationError):
"""A subprocess call failed during installation."""
def __init__(self, returncode, description):
# type: (int, str) -> None
self.returncode = returncode
self.description = description

def __str__(self):
# type: () -> str
return (
"Command errored out with exit status {}: {} "
"Check the logs for full command output."
).format(self.returncode, self.description)


class HashErrors(InstallationError):
"""Multiple HashError instances rolled into one for reporting"""

Expand Down
23 changes: 17 additions & 6 deletions src/pip/_internal/resolution/resolvelib/candidates.py
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ def __init__(
self._ireq = ireq
self._name = name
self._version = version
self.dist = self._prepare()
self._dist = None # type: Optional[Distribution]

def __str__(self):
# type: () -> str
Expand Down Expand Up @@ -209,6 +209,8 @@ def _prepare_distribution(self):
def _check_metadata_consistency(self, dist):
# type: (Distribution) -> None
"""Check for consistency of project name and version of dist."""
# TODO: (Longer term) Rather than abort, reject this candidate
# and backtrack. This would need resolvelib support.
name = canonicalize_name(dist.project_name)
if self._name is not None and self._name != name:
raise MetadataInconsistent(self._ireq, "name", dist.project_name)
Expand All @@ -217,17 +219,25 @@ def _check_metadata_consistency(self, dist):
raise MetadataInconsistent(self._ireq, "version", dist.version)

def _prepare(self):
# type: () -> Distribution
# type: () -> None
if self._dist is not None:
return
try:
dist = self._prepare_distribution()
except HashError as e:
# Provide HashError the underlying ireq that caused it. This
# provides context for the resulting error message to show the
# offending line to the user.
e.req = self._ireq
raise

assert dist is not None, "Distribution already installed"
self._check_metadata_consistency(dist)
return dist
self._dist = dist

@property
def dist(self):
# type: () -> Distribution
if self._dist is None:
self._prepare()
return self._dist

def _get_requires_python_dependency(self):
# type: () -> Optional[Requirement]
Expand All @@ -251,6 +261,7 @@ def iter_dependencies(self, with_requires):

def get_install_requirement(self):
# type: () -> Optional[InstallRequirement]
self._prepare()
return self._ireq


Expand Down
52 changes: 8 additions & 44 deletions src/pip/_internal/resolution/resolvelib/factory.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@
from pip._internal.exceptions import (
DistributionNotFound,
InstallationError,
InstallationSubprocessError,
MetadataInconsistent,
UnsupportedPythonVersion,
UnsupportedWheel,
)
Expand Down Expand Up @@ -35,7 +33,6 @@
ExplicitRequirement,
RequiresPythonRequirement,
SpecifierRequirement,
UnsatisfiableRequirement,
)

if MYPY_CHECK_RUNNING:
Expand Down Expand Up @@ -97,7 +94,6 @@ def __init__(
self._force_reinstall = force_reinstall
self._ignore_requires_python = ignore_requires_python

self._build_failures = {} # type: Cache[InstallationError]
self._link_candidate_cache = {} # type: Cache[LinkCandidate]
self._editable_candidate_cache = {} # type: Cache[EditableCandidate]
self._installed_candidate_cache = {
Expand Down Expand Up @@ -140,40 +136,21 @@ def _make_candidate_from_link(
name, # type: Optional[str]
version, # type: Optional[_BaseVersion]
):
# type: (...) -> Optional[Candidate]
# type: (...) -> Candidate
# TODO: Check already installed candidate, and use it if the link and
# editable flag match.

if link in self._build_failures:
# We already tried this candidate before, and it does not build.
# Don't bother trying again.
return None

if template.editable:
if link not in self._editable_candidate_cache:
try:
self._editable_candidate_cache[link] = EditableCandidate(
link, template, factory=self,
name=name, version=version,
)
except (InstallationSubprocessError, MetadataInconsistent) as e:
logger.warning("Discarding %s. %s", link, e)
self._build_failures[link] = e
return None
self._editable_candidate_cache[link] = EditableCandidate(
link, template, factory=self, name=name, version=version,
)
base = self._editable_candidate_cache[link] # type: BaseCandidate
else:
if link not in self._link_candidate_cache:
try:
self._link_candidate_cache[link] = LinkCandidate(
link, template, factory=self,
name=name, version=version,
)
except (InstallationSubprocessError, MetadataInconsistent) as e:
logger.warning("Discarding %s. %s", link, e)
self._build_failures[link] = e
return None
self._link_candidate_cache[link] = LinkCandidate(
link, template, factory=self, name=name, version=version,
)
base = self._link_candidate_cache[link]

if extras:
return ExtrasCandidate(base, extras)
return base
Expand Down Expand Up @@ -233,16 +210,13 @@ def iter_index_candidates():
for ican in reversed(icans):
if not all_yanked and ican.link.is_yanked:
continue
candidate = self._make_candidate_from_link(
yield self._make_candidate_from_link(
link=ican.link,
extras=extras,
template=template,
name=name,
version=ican.version,
)
if candidate is None:
continue
yield candidate

return FoundCandidates(
iter_index_candidates,
Expand Down Expand Up @@ -306,16 +280,6 @@ def make_requirement_from_install_req(self, ireq, requested_extras):
name=canonicalize_name(ireq.name) if ireq.name else None,
version=None,
)
if cand is None:
# There's no way we can satisfy a URL requirement if the underlying
# candidate fails to build. An unnamed URL must be user-supplied, so
# we fail eagerly. If the URL is named, an unsatisfiable requirement
# can make the resolver do the right thing, either backtrack (and
# maybe find some other requirement that's buildable) or raise a
# ResolutionImpossible eventually.
if not ireq.name:
raise self._build_failures[ireq.link]
return UnsatisfiableRequirement(canonicalize_name(ireq.name))
return self.make_requirement_from_candidate(cand)

def make_requirement_from_candidate(self, candidate):
Expand Down
41 changes: 0 additions & 41 deletions src/pip/_internal/resolution/resolvelib/requirements.py
Original file line number Diff line number Diff line change
Expand Up @@ -158,44 +158,3 @@ def is_satisfied_by(self, candidate):
# already implements the prerelease logic, and would have filtered out
# prerelease candidates if the user does not expect them.
return self.specifier.contains(candidate.version, prereleases=True)


class UnsatisfiableRequirement(Requirement):
"""A requirement that cannot be satisfied.
"""
def __init__(self, name):
# type: (str) -> None
self._name = name

def __str__(self):
# type: () -> str
return "{} (unavailable)".format(self._name)

def __repr__(self):
# type: () -> str
return "{class_name}({name!r})".format(
class_name=self.__class__.__name__,
name=str(self._name),
)

@property
def project_name(self):
# type: () -> str
return self._name

@property
def name(self):
# type: () -> str
return self._name

def format_for_error(self):
# type: () -> str
return str(self)

def get_candidate_lookup(self):
# type: () -> CandidateLookup
return None, None

def is_satisfied_by(self, candidate):
# type: (Candidate) -> bool
return False
8 changes: 6 additions & 2 deletions src/pip/_internal/utils/subprocess.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
from pip._vendor.six.moves import shlex_quote

from pip._internal.cli.spinners import SpinnerInterface, open_spinner
from pip._internal.exceptions import InstallationSubprocessError
from pip._internal.exceptions import InstallationError
from pip._internal.utils.compat import console_to_str, str_to_display
from pip._internal.utils.logging import subprocess_logger
from pip._internal.utils.misc import HiddenText, path_to_display
Expand Down Expand Up @@ -233,7 +233,11 @@ def call_subprocess(
exit_status=proc.returncode,
)
subprocess_logger.error(msg)
raise InstallationSubprocessError(proc.returncode, command_desc)
exc_msg = (
'Command errored out with exit status {}: {} '
'Check the logs for full command output.'
).format(proc.returncode, command_desc)
raise InstallationError(exc_msg)
elif on_returncode == 'warn':
subprocess_logger.warning(
'Command "%s" had error code %s in %s',
Expand Down
19 changes: 0 additions & 19 deletions tests/functional/test_new_resolver.py
Original file line number Diff line number Diff line change
Expand Up @@ -1218,22 +1218,3 @@ def test_new_resolver_does_not_reinstall_when_from_a_local_index(script):
assert "Installing collected packages: simple" not in result.stdout, str(result)
assert "Requirement already satisfied: simple" in result.stdout, str(result)
assert_installed(script, simple="0.1.0")


def test_new_resolver_skip_inconsistent_metadata(script):
create_basic_wheel_for_package(script, "A", "1")

a_2 = create_basic_wheel_for_package(script, "A", "2")
a_2.rename(a_2.parent.joinpath("a-3-py2.py3-none-any.whl"))

result = script.pip(
"install",
"--no-cache-dir", "--no-index",
"--find-links", script.scratch_path,
"--verbose",
"A",
allow_stderr_warning=True,
)

assert " different version in metadata: '2'" in result.stderr, str(result)
assert_installed(script, a="1")
8 changes: 4 additions & 4 deletions tests/unit/test_utils_subprocess.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import pytest

from pip._internal.cli.spinners import SpinnerInterface
from pip._internal.exceptions import InstallationSubprocessError
from pip._internal.exceptions import InstallationError
from pip._internal.utils.misc import hide_value
from pip._internal.utils.subprocess import (
call_subprocess,
Expand Down Expand Up @@ -276,7 +276,7 @@ def test_info_logging__subprocess_error(self, capfd, caplog):
command = 'print("Hello"); print("world"); exit("fail")'
args, spinner = self.prepare_call(caplog, log_level, command=command)

with pytest.raises(InstallationSubprocessError) as exc:
with pytest.raises(InstallationError) as exc:
call_subprocess(args, spinner=spinner)
result = None
exc_message = str(exc.value)
Expand Down Expand Up @@ -360,7 +360,7 @@ def test_info_logging_with_show_stdout_true(self, capfd, caplog):
# log level is only WARNING.
(0, True, None, WARNING, (None, 'done', 2)),
# Test a non-zero exit status.
(3, False, None, INFO, (InstallationSubprocessError, 'error', 2)),
(3, False, None, INFO, (InstallationError, 'error', 2)),
# Test a non-zero exit status also in extra_ok_returncodes.
(3, False, (3, ), INFO, (None, 'done', 2)),
])
Expand Down Expand Up @@ -396,7 +396,7 @@ def test_spinner_finish(
assert spinner.spin_count == expected_spin_count

def test_closes_stdin(self):
with pytest.raises(InstallationSubprocessError):
with pytest.raises(InstallationError):
call_subprocess(
[sys.executable, '-c', 'input()'],
show_stdout=True,
Expand Down