Skip to content

Commit 9405d00

Browse files
committed
Skip candidate not providing valid metadata
This is done by catching InstallationError from the underlying distribution preparation logic. There are three cases to catch: 1. Candidates from indexes. These are simply ignored since we can potentially satisfy the requirement with other candidates. 2. Candidates from URLs with a dist name (PEP 508 or #egg=). A new UnsatisfiableRequirement class is introduced to represent this; it is like an ExplicitRequirement without an underlying candidate. As the name suggests, an instance of this can never be satisfied, and will cause eventual backtracking. 3. Candidates from URLs without a dist name. This is only possible for top-level user requirements, and no recourse is possible for them. So we error out eagerly. The InstallationError raised during distribution preparation is cached in the factory, like successfully prepared candidates, since we don't want to repeatedly try to build a candidate if we already know it'd fail. Plus pip's preparation logic also does not allow packages to be built multiple times anyway.
1 parent 643217b commit 9405d00

File tree

7 files changed

+115
-31
lines changed

7 files changed

+115
-31
lines changed

Diff for: news/9246.bugfix.rst

+2
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
New resolver: Discard a candidate if it fails to provide metadata from source,
2+
or if the provided metadata is inconsistent, instead of quitting outright.

Diff for: src/pip/_internal/exceptions.py

+15
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,21 @@ def __str__(self):
151151
)
152152

153153

154+
class InstallationSubprocessError(InstallationError):
155+
"""A subprocess call failed during installation."""
156+
def __init__(self, returncode, description):
157+
# type: (int, str) -> None
158+
self.returncode = returncode
159+
self.description = description
160+
161+
def __str__(self):
162+
# type: () -> str
163+
return (
164+
"Command errored out with exit status {}: {} "
165+
"Check the logs for full command output."
166+
).format(self.returncode, self.description)
167+
168+
154169
class HashErrors(InstallationError):
155170
"""Multiple HashError instances rolled into one for reporting"""
156171

Diff for: src/pip/_internal/resolution/resolvelib/candidates.py

+3-17
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@ def __init__(
141141
self._ireq = ireq
142142
self._name = name
143143
self._version = version
144-
self._dist = None # type: Optional[Distribution]
144+
self.dist = self._prepare()
145145

146146
def __str__(self):
147147
# type: () -> str
@@ -209,8 +209,6 @@ def _prepare_distribution(self):
209209
def _check_metadata_consistency(self, dist):
210210
# type: (Distribution) -> None
211211
"""Check for consistency of project name and version of dist."""
212-
# TODO: (Longer term) Rather than abort, reject this candidate
213-
# and backtrack. This would need resolvelib support.
214212
name = canonicalize_name(dist.project_name)
215213
if self._name is not None and self._name != name:
216214
raise MetadataInconsistent(self._ireq, "name", dist.project_name)
@@ -219,25 +217,14 @@ def _check_metadata_consistency(self, dist):
219217
raise MetadataInconsistent(self._ireq, "version", dist.version)
220218

221219
def _prepare(self):
222-
# type: () -> None
223-
if self._dist is not None:
224-
return
220+
# type: () -> Distribution
225221
try:
226222
dist = self._prepare_distribution()
227223
except HashError as e:
228224
e.req = self._ireq
229225
raise
230-
231-
assert dist is not None, "Distribution already installed"
232226
self._check_metadata_consistency(dist)
233-
self._dist = dist
234-
235-
@property
236-
def dist(self):
237-
# type: () -> Distribution
238-
if self._dist is None:
239-
self._prepare()
240-
return self._dist
227+
return dist
241228

242229
def _get_requires_python_dependency(self):
243230
# type: () -> Optional[Requirement]
@@ -261,7 +248,6 @@ def iter_dependencies(self, with_requires):
261248

262249
def get_install_requirement(self):
263250
# type: () -> Optional[InstallRequirement]
264-
self._prepare()
265251
return self._ireq
266252

267253

Diff for: src/pip/_internal/resolution/resolvelib/factory.py

+33-8
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
from pip._internal.exceptions import (
66
DistributionNotFound,
77
InstallationError,
8+
InstallationSubprocessError,
9+
MetadataInconsistent,
810
UnsupportedPythonVersion,
911
UnsupportedWheel,
1012
)
@@ -33,6 +35,7 @@
3335
ExplicitRequirement,
3436
RequiresPythonRequirement,
3537
SpecifierRequirement,
38+
UnsatisfiableRequirement,
3639
)
3740

3841
if MYPY_CHECK_RUNNING:
@@ -96,6 +99,7 @@ def __init__(
9699

97100
self._link_candidate_cache = {} # type: Cache[LinkCandidate]
98101
self._editable_candidate_cache = {} # type: Cache[EditableCandidate]
102+
self._build_failures = {} # type: Cache[InstallationError]
99103

100104
if not ignore_installed:
101105
self._installed_dists = {
@@ -130,20 +134,34 @@ def _make_candidate_from_link(
130134
name, # type: Optional[str]
131135
version, # type: Optional[_BaseVersion]
132136
):
133-
# type: (...) -> Candidate
137+
# type: (...) -> Optional[Candidate]
134138
# TODO: Check already installed candidate, and use it if the link and
135139
# editable flag match.
140+
if link in self._build_failures:
141+
return None
136142
if template.editable:
137143
if link not in self._editable_candidate_cache:
138-
self._editable_candidate_cache[link] = EditableCandidate(
139-
link, template, factory=self, name=name, version=version,
140-
)
144+
try:
145+
self._editable_candidate_cache[link] = EditableCandidate(
146+
link, template, factory=self,
147+
name=name, version=version,
148+
)
149+
except (InstallationSubprocessError, MetadataInconsistent) as e:
150+
logger.warning("Discarding %s. %s", link, e)
151+
self._build_failures[link] = e
152+
return None
141153
base = self._editable_candidate_cache[link] # type: BaseCandidate
142154
else:
143155
if link not in self._link_candidate_cache:
144-
self._link_candidate_cache[link] = LinkCandidate(
145-
link, template, factory=self, name=name, version=version,
146-
)
156+
try:
157+
self._link_candidate_cache[link] = LinkCandidate(
158+
link, template, factory=self,
159+
name=name, version=version,
160+
)
161+
except (InstallationSubprocessError, MetadataInconsistent) as e:
162+
logger.warning("Discarding %s. %s", link, e)
163+
self._build_failures[link] = e
164+
return None
147165
base = self._link_candidate_cache[link]
148166
if extras:
149167
return ExtrasCandidate(base, extras)
@@ -204,13 +222,16 @@ def iter_index_candidates():
204222
for ican in reversed(icans):
205223
if not all_yanked and ican.link.is_yanked:
206224
continue
207-
yield self._make_candidate_from_link(
225+
candidate = self._make_candidate_from_link(
208226
link=ican.link,
209227
extras=extras,
210228
template=template,
211229
name=name,
212230
version=ican.version,
213231
)
232+
if candidate is None:
233+
continue
234+
yield candidate
214235

215236
return FoundCandidates(
216237
iter_index_candidates,
@@ -274,6 +295,10 @@ def make_requirement_from_install_req(self, ireq, requested_extras):
274295
name=canonicalize_name(ireq.name) if ireq.name else None,
275296
version=None,
276297
)
298+
if cand is None:
299+
if not ireq.name:
300+
raise self._build_failures[ireq.link]
301+
return UnsatisfiableRequirement(canonicalize_name(ireq.name))
277302
return self.make_requirement_from_candidate(cand)
278303

279304
def make_requirement_from_candidate(self, candidate):

Diff for: src/pip/_internal/resolution/resolvelib/requirements.py

+41
Original file line numberDiff line numberDiff line change
@@ -158,3 +158,44 @@ def is_satisfied_by(self, candidate):
158158
# already implements the prerelease logic, and would have filtered out
159159
# prerelease candidates if the user does not expect them.
160160
return self.specifier.contains(candidate.version, prereleases=True)
161+
162+
163+
class UnsatisfiableRequirement(Requirement):
164+
"""A requirement that cannot be satisfied.
165+
"""
166+
def __init__(self, name):
167+
# type: (str) -> None
168+
self._name = name
169+
170+
def __str__(self):
171+
# type: () -> str
172+
return "{} (unavailable)".format(self._name)
173+
174+
def __repr__(self):
175+
# type: () -> str
176+
return "{class_name}({name!r})".format(
177+
class_name=self.__class__.__name__,
178+
name=str(self._name),
179+
)
180+
181+
@property
182+
def project_name(self):
183+
# type: () -> str
184+
return self._name
185+
186+
@property
187+
def name(self):
188+
# type: () -> str
189+
return self._name
190+
191+
def format_for_error(self):
192+
# type: () -> str
193+
return str(self)
194+
195+
def get_candidate_lookup(self):
196+
# type: () -> CandidateLookup
197+
return None, None
198+
199+
def is_satisfied_by(self, candidate):
200+
# type: (Candidate) -> bool
201+
return False

Diff for: src/pip/_internal/utils/subprocess.py

+2-6
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
from pip._vendor.six.moves import shlex_quote
88

99
from pip._internal.cli.spinners import SpinnerInterface, open_spinner
10-
from pip._internal.exceptions import InstallationError
10+
from pip._internal.exceptions import InstallationSubprocessError
1111
from pip._internal.utils.compat import console_to_str, str_to_display
1212
from pip._internal.utils.logging import subprocess_logger
1313
from pip._internal.utils.misc import HiddenText, path_to_display
@@ -233,11 +233,7 @@ def call_subprocess(
233233
exit_status=proc.returncode,
234234
)
235235
subprocess_logger.error(msg)
236-
exc_msg = (
237-
'Command errored out with exit status {}: {} '
238-
'Check the logs for full command output.'
239-
).format(proc.returncode, command_desc)
240-
raise InstallationError(exc_msg)
236+
raise InstallationSubprocessError(proc.returncode, command_desc)
241237
elif on_returncode == 'warn':
242238
subprocess_logger.warning(
243239
'Command "%s" had error code %s in %s',

Diff for: tests/functional/test_new_resolver.py

+19
Original file line numberDiff line numberDiff line change
@@ -1218,3 +1218,22 @@ def test_new_resolver_does_not_reinstall_when_from_a_local_index(script):
12181218
assert "Installing collected packages: simple" not in result.stdout, str(result)
12191219
assert "Requirement already satisfied: simple" in result.stdout, str(result)
12201220
assert_installed(script, simple="0.1.0")
1221+
1222+
1223+
def test_new_resolver_skip_inconsistent_metadata(script):
1224+
create_basic_wheel_for_package(script, "A", "1")
1225+
1226+
a_2 = create_basic_wheel_for_package(script, "A", "2")
1227+
a_2.rename(a_2.parent.joinpath("a-3-py2.py3-none-any.whl"))
1228+
1229+
result = script.pip(
1230+
"install",
1231+
"--no-cache-dir", "--no-index",
1232+
"--find-links", script.scratch_path,
1233+
"--verbose",
1234+
"A",
1235+
allow_stderr_warning=True,
1236+
)
1237+
1238+
assert " different version in metadata: '2'" in result.stderr, str(result)
1239+
assert_installed(script, a="1")

0 commit comments

Comments
 (0)