diff --git a/news/9264.bugfix.rst b/news/9264.bugfix.rst new file mode 100644 index 00000000000..0178ab197f0 --- /dev/null +++ b/news/9264.bugfix.rst @@ -0,0 +1 @@ +Revert "Skip candidate not providing valid metadata", as that caused pip to be overeager about downloading from the package index. diff --git a/src/pip/_internal/exceptions.py b/src/pip/_internal/exceptions.py index 3f2659e8792..56482caf77b 100644 --- a/src/pip/_internal/exceptions.py +++ b/src/pip/_internal/exceptions.py @@ -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""" diff --git a/src/pip/_internal/resolution/resolvelib/candidates.py b/src/pip/_internal/resolution/resolvelib/candidates.py index 83b6c98ab6a..cd1f188706f 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 = self._prepare() + self._dist = None # type: Optional[Distribution] def __str__(self): # type: () -> str @@ -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) @@ -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] @@ -251,6 +261,7 @@ 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 35345c5f0a1..b4c7bf11351 100644 --- a/src/pip/_internal/resolution/resolvelib/factory.py +++ b/src/pip/_internal/resolution/resolvelib/factory.py @@ -5,8 +5,6 @@ from pip._internal.exceptions import ( DistributionNotFound, InstallationError, - InstallationSubprocessError, - MetadataInconsistent, UnsupportedPythonVersion, UnsupportedWheel, ) @@ -35,7 +33,6 @@ ExplicitRequirement, RequiresPythonRequirement, SpecifierRequirement, - UnsatisfiableRequirement, ) if MYPY_CHECK_RUNNING: @@ -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 = { @@ -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 @@ -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, @@ -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): diff --git a/src/pip/_internal/resolution/resolvelib/requirements.py b/src/pip/_internal/resolution/resolvelib/requirements.py index 1229f353750..d926d0a0656 100644 --- a/src/pip/_internal/resolution/resolvelib/requirements.py +++ b/src/pip/_internal/resolution/resolvelib/requirements.py @@ -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 diff --git a/src/pip/_internal/utils/subprocess.py b/src/pip/_internal/utils/subprocess.py index 325897c8739..605e711e603 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 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 @@ -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', diff --git a/tests/functional/test_new_resolver.py b/tests/functional/test_new_resolver.py index 00d82fb95ee..b730b3cbdf9 100644 --- a/tests/functional/test_new_resolver.py +++ b/tests/functional/test_new_resolver.py @@ -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") diff --git a/tests/unit/test_utils_subprocess.py b/tests/unit/test_utils_subprocess.py index 1a031065114..b0de2bf578d 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 InstallationSubprocessError +from pip._internal.exceptions import InstallationError 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(InstallationSubprocessError) as exc: + with pytest.raises(InstallationError) 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, (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)), ]) @@ -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,