Skip to content

Commit ae8d0e0

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 ae8d0e0

File tree

7 files changed

+116
-31
lines changed

7 files changed

+116
-31
lines changed

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.

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

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

src/pip/_internal/resolution/resolvelib/factory.py

+34-8
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,10 @@
44

55
from pip._internal.exceptions import (
66
DistributionNotFound,
7+
HashError,
78
InstallationError,
9+
InstallationSubprocessError,
10+
MetadataInconsistent,
811
UnsupportedPythonVersion,
912
UnsupportedWheel,
1013
)
@@ -33,6 +36,7 @@
3336
ExplicitRequirement,
3437
RequiresPythonRequirement,
3538
SpecifierRequirement,
39+
UnsatisfiableRequirement,
3640
)
3741

3842
if MYPY_CHECK_RUNNING:
@@ -96,6 +100,7 @@ def __init__(
96100

97101
self._link_candidate_cache = {} # type: Cache[LinkCandidate]
98102
self._editable_candidate_cache = {} # type: Cache[EditableCandidate]
103+
self._build_failures = {} # type: Cache[InstallationError]
99104

100105
if not ignore_installed:
101106
self._installed_dists = {
@@ -130,20 +135,34 @@ def _make_candidate_from_link(
130135
name, # type: Optional[str]
131136
version, # type: Optional[_BaseVersion]
132137
):
133-
# type: (...) -> Candidate
138+
# type: (...) -> Optional[Candidate]
134139
# TODO: Check already installed candidate, and use it if the link and
135140
# editable flag match.
141+
if link in self._build_failures:
142+
return None
136143
if template.editable:
137144
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-
)
145+
try:
146+
self._editable_candidate_cache[link] = EditableCandidate(
147+
link, template, factory=self,
148+
name=name, version=version,
149+
)
150+
except (InstallationSubprocessError, MetadataInconsistent) as e:
151+
logger.warning("Discarding %s. %s", link, e)
152+
self._build_failures[link] = e
153+
return None
141154
base = self._editable_candidate_cache[link] # type: BaseCandidate
142155
else:
143156
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-
)
157+
try:
158+
self._link_candidate_cache[link] = LinkCandidate(
159+
link, template, factory=self,
160+
name=name, version=version,
161+
)
162+
except (InstallationSubprocessError, MetadataInconsistent) as e:
163+
logger.warning("Discarding %s. %s", link, e)
164+
self._build_failures[link] = e
165+
return None
147166
base = self._link_candidate_cache[link]
148167
if extras:
149168
return ExtrasCandidate(base, extras)
@@ -204,13 +223,16 @@ def iter_index_candidates():
204223
for ican in reversed(icans):
205224
if not all_yanked and ican.link.is_yanked:
206225
continue
207-
yield self._make_candidate_from_link(
226+
candidate = self._make_candidate_from_link(
208227
link=ican.link,
209228
extras=extras,
210229
template=template,
211230
name=name,
212231
version=ican.version,
213232
)
233+
if candidate is None:
234+
continue
235+
yield candidate
214236

215237
return FoundCandidates(
216238
iter_index_candidates,
@@ -274,6 +296,10 @@ def make_requirement_from_install_req(self, ireq, requested_extras):
274296
name=canonicalize_name(ireq.name) if ireq.name else None,
275297
version=None,
276298
)
299+
if cand is None:
300+
if not ireq.name:
301+
raise self._build_failures[ireq.link]
302+
return UnsatisfiableRequirement(canonicalize_name(ireq.name))
277303
return self.make_requirement_from_candidate(cand)
278304

279305
def make_requirement_from_candidate(self, candidate):

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

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',

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)