diff --git a/news/9246.bugfix.rst b/news/9246.bugfix.rst new file mode 100644 index 00000000000..e7ebd398f3e --- /dev/null +++ b/news/9246.bugfix.rst @@ -0,0 +1,2 @@ +New resolver: Discard a candidate if it fails to provide metadata from source, +or if the provided metadata is inconsistent, instead of quitting outright. diff --git a/src/pip/_internal/exceptions.py b/src/pip/_internal/exceptions.py index 56482caf77b..3f2659e8792 100644 --- a/src/pip/_internal/exceptions.py +++ b/src/pip/_internal/exceptions.py @@ -151,6 +151,21 @@ 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""" diff --git a/src/pip/_internal/resolution/resolvelib/candidates.py b/src/pip/_internal/resolution/resolvelib/candidates.py index cd1f188706f..83b6c98ab6a 100644 --- a/src/pip/_internal/resolution/resolvelib/candidates.py +++ b/src/pip/_internal/resolution/resolvelib/candidates.py @@ -141,7 +141,7 @@ def __init__( self._ireq = ireq self._name = name self._version = version - self._dist = None # type: Optional[Distribution] + self.dist = self._prepare() def __str__(self): # type: () -> str @@ -209,8 +209,6 @@ 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) @@ -219,25 +217,17 @@ def _check_metadata_consistency(self, dist): raise MetadataInconsistent(self._ireq, "version", dist.version) def _prepare(self): - # type: () -> None - if self._dist is not None: - return + # type: () -> Distribution 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) - self._dist = dist - - @property - def dist(self): - # type: () -> Distribution - if self._dist is None: - self._prepare() - return self._dist + return dist def _get_requires_python_dependency(self): # type: () -> Optional[Requirement] @@ -261,7 +251,6 @@ def iter_dependencies(self, with_requires): def get_install_requirement(self): # type: () -> Optional[InstallRequirement] - self._prepare() return self._ireq diff --git a/src/pip/_internal/resolution/resolvelib/factory.py b/src/pip/_internal/resolution/resolvelib/factory.py index b4c7bf11351..35345c5f0a1 100644 --- a/src/pip/_internal/resolution/resolvelib/factory.py +++ b/src/pip/_internal/resolution/resolvelib/factory.py @@ -5,6 +5,8 @@ from pip._internal.exceptions import ( DistributionNotFound, InstallationError, + InstallationSubprocessError, + MetadataInconsistent, UnsupportedPythonVersion, UnsupportedWheel, ) @@ -33,6 +35,7 @@ ExplicitRequirement, RequiresPythonRequirement, SpecifierRequirement, + UnsatisfiableRequirement, ) if MYPY_CHECK_RUNNING: @@ -94,6 +97,7 @@ 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 = { @@ -136,21 +140,40 @@ def _make_candidate_from_link( name, # type: Optional[str] version, # type: Optional[_BaseVersion] ): - # type: (...) -> Candidate + # type: (...) -> Optional[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: - self._editable_candidate_cache[link] = EditableCandidate( - link, template, factory=self, name=name, version=version, - ) + 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 base = self._editable_candidate_cache[link] # type: BaseCandidate else: if link not in self._link_candidate_cache: - self._link_candidate_cache[link] = LinkCandidate( - link, template, factory=self, name=name, version=version, - ) + 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 base = self._link_candidate_cache[link] + if extras: return ExtrasCandidate(base, extras) return base @@ -210,13 +233,16 @@ def iter_index_candidates(): for ican in reversed(icans): if not all_yanked and ican.link.is_yanked: continue - yield self._make_candidate_from_link( + candidate = 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, @@ -280,6 +306,16 @@ 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): diff --git a/src/pip/_internal/resolution/resolvelib/requirements.py b/src/pip/_internal/resolution/resolvelib/requirements.py index d926d0a0656..1229f353750 100644 --- a/src/pip/_internal/resolution/resolvelib/requirements.py +++ b/src/pip/_internal/resolution/resolvelib/requirements.py @@ -158,3 +158,44 @@ 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 diff --git a/src/pip/_internal/utils/subprocess.py b/src/pip/_internal/utils/subprocess.py index 605e711e603..325897c8739 100644 --- a/src/pip/_internal/utils/subprocess.py +++ b/src/pip/_internal/utils/subprocess.py @@ -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 InstallationError +from pip._internal.exceptions import InstallationSubprocessError 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 @@ -233,11 +233,7 @@ def call_subprocess( exit_status=proc.returncode, ) subprocess_logger.error(msg) - exc_msg = ( - 'Command errored out with exit status {}: {} ' - 'Check the logs for full command output.' - ).format(proc.returncode, command_desc) - raise InstallationError(exc_msg) + raise InstallationSubprocessError(proc.returncode, command_desc) elif on_returncode == 'warn': subprocess_logger.warning( 'Command "%s" had error code %s in %s', diff --git a/tests/functional/test_new_resolver.py b/tests/functional/test_new_resolver.py index b730b3cbdf9..00d82fb95ee 100644 --- a/tests/functional/test_new_resolver.py +++ b/tests/functional/test_new_resolver.py @@ -1218,3 +1218,22 @@ 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") diff --git a/tests/unit/test_utils_subprocess.py b/tests/unit/test_utils_subprocess.py index b0de2bf578d..1a031065114 100644 --- a/tests/unit/test_utils_subprocess.py +++ b/tests/unit/test_utils_subprocess.py @@ -7,7 +7,7 @@ import pytest from pip._internal.cli.spinners import SpinnerInterface -from pip._internal.exceptions import InstallationError +from pip._internal.exceptions import InstallationSubprocessError from pip._internal.utils.misc import hide_value from pip._internal.utils.subprocess import ( call_subprocess, @@ -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(InstallationError) as exc: + with pytest.raises(InstallationSubprocessError) as exc: call_subprocess(args, spinner=spinner) result = None exc_message = str(exc.value) @@ -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, (InstallationError, 'error', 2)), + (3, False, None, INFO, (InstallationSubprocessError, 'error', 2)), # Test a non-zero exit status also in extra_ok_returncodes. (3, False, (3, ), INFO, (None, 'done', 2)), ]) @@ -396,7 +396,7 @@ def test_spinner_finish( assert spinner.spin_count == expected_spin_count def test_closes_stdin(self): - with pytest.raises(InstallationError): + with pytest.raises(InstallationSubprocessError): call_subprocess( [sys.executable, '-c', 'input()'], show_stdout=True,