Skip to content

Commit

Permalink
Refactor yanked warning out of package finder
Browse files Browse the repository at this point in the history 
The previous implementation assumes that when a best candidate is found,
it will be downloaded/installed. This will no longer be the case for the
new resolver, since it may decide to discard the candidate after
inspection and fetch a new one.

By moving it to populate_link(), we can be sure that the warning is only
set when the yanked link is actually populated for installation.
  • Loading branch information
@uranusjr
uranusjr committed Feb 27, 2020
1 parent 9e884a4 commit e878aac
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 52 deletions.
21 changes: 3 additions & 18 deletions src/pip/_internal/index/package_finder.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -558,21 +558,6 @@ def sort_best_candidate(
return None

best_candidate = max(candidates, key=self._sort_key)

# Log a warning per PEP 592 if necessary before returning.
link = best_candidate.link
if link.is_yanked:
reason = link.yanked_reason or '<none given>'
msg = (
# Mark this as a unicode string to prevent
# "UnicodeEncodeError: 'ascii' codec can't encode character"
# in Python 2 when the reason contains non-ascii characters.
u'The candidate selected for download or install is a '
'yanked version: {candidate}\n'
'Reason for being yanked: {reason}'
).format(candidate=best_candidate, reason=reason)
logger.warning(msg)

return best_candidate

def compute_best_candidate(
Expand Down Expand Up @@ -889,8 +874,8 @@ def find_best_candidate(
return candidate_evaluator.compute_best_candidate(candidates)

def find_requirement(self, req, upgrade):
# type: (InstallRequirement, bool) -> Optional[Link]
"""Try to find a Link matching req
# type: (InstallRequirement, bool) -> Optional[InstallationCandidate]
"""Try to find a candidate matching req
Expects req, an InstallRequirement and upgrade, a boolean
Returns a Link if found,
Expand Down Expand Up @@ -967,7 +952,7 @@ def _format_versions(cand_iter):
best_candidate.version,
_format_versions(best_candidate_result.iter_applicable()),
)
return best_candidate.link
return best_candidate


def _find_name_version_sep(fragment, canonical_name):
Expand Down
25 changes: 24 additions & 1 deletion src/pip/_internal/req/req_install.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,28 @@ def format_debug(self):
state=", ".join(state),
)

def _find_link(self, finder, upgrade):
# type: (PackageFinder, bool) -> None
best_candidate = finder.find_requirement(self, upgrade)
if not best_candidate:
return None
link = best_candidate.link

# Log a warning per PEP 592 if necessary.
if link.is_yanked:
reason = link.yanked_reason or '<none given>'
# Mark this as a unicode string to prevent
# "UnicodeEncodeError: 'ascii' codec can't encode character"
# in Python 2 when the reason contains non-ascii characters.
msg = (
u'The candidate selected for download or install is a '
'yanked version: {candidate}\n'
'Reason for being yanked: {reason}'
).format(candidate=best_candidate, reason=reason)
logger.warning(msg)

return link

def populate_link(self, finder, upgrade, require_hashes):
# type: (PackageFinder, bool, bool) -> None
"""Ensure that if a link can be found for this, that it is found.
Expand All @@ -255,7 +277,8 @@ def populate_link(self, finder, upgrade, require_hashes):
to file modification times.
"""
if self.link is None:
self.link = finder.find_requirement(self, upgrade)
self.link = self._find_link(finder, upgrade)

if self._wheel_cache is not None and not require_hashes:
old_link = self.link
supported_tags = pep425tags.get_supported()
Expand Down
68 changes: 35 additions & 33 deletions tests/unit/test_finder.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ def test_no_mpkg(data):
req = install_req_from_line("pkgwithmpkg")
found = finder.find_requirement(req, False)

assert found.url.endswith("pkgwithmpkg-1.0.tar.gz"), found
assert found.link.url.endswith("pkgwithmpkg-1.0.tar.gz"), found


def test_no_partial_name_match(data):
Expand All @@ -57,7 +57,7 @@ def test_no_partial_name_match(data):
req = install_req_from_line("gmpy")
found = finder.find_requirement(req, False)

assert found.url.endswith("gmpy-1.15.tar.gz"), found
assert found.link.url.endswith("gmpy-1.15.tar.gz"), found


def test_tilde():
Expand All @@ -79,23 +79,23 @@ def test_duplicates_sort_ok(data):
req = install_req_from_line("duplicate")
found = finder.find_requirement(req, False)

assert found.url.endswith("duplicate-1.0.tar.gz"), found
assert found.link.url.endswith("duplicate-1.0.tar.gz"), found


def test_finder_detects_latest_find_links(data):
"""Test PackageFinder detects latest using find-links"""
req = install_req_from_line('simple', None)
finder = make_test_finder(find_links=[data.find_links])
link = finder.find_requirement(req, False)
assert link.url.endswith("simple-3.0.tar.gz")
candidate = finder.find_requirement(req, False)
assert candidate.link.url.endswith("simple-3.0.tar.gz")


def test_incorrect_case_file_index(data):
"""Test PackageFinder detects latest using wrong case"""
req = install_req_from_line('dinner', None)
finder = make_test_finder(index_urls=[data.find_links3])
link = finder.find_requirement(req, False)
assert link.url.endswith("Dinner-2.0.tar.gz")
candidate = finder.find_requirement(req, False)
assert candidate.link.url.endswith("Dinner-2.0.tar.gz")


@pytest.mark.network
Expand Down Expand Up @@ -180,7 +180,7 @@ def test_find_wheel_supported(self, data, monkeypatch):
finder = make_test_finder(find_links=[data.find_links])
found = finder.find_requirement(req, True)
assert (
found.url.endswith("simple.dist-0.1-py2.py3-none-any.whl")
found.link.url.endswith("simple.dist-0.1-py2.py3-none-any.whl")
), found

def test_wheel_over_sdist_priority(self, data):
Expand All @@ -191,7 +191,9 @@ def test_wheel_over_sdist_priority(self, data):
req = install_req_from_line("priority")
finder = make_test_finder(find_links=[data.find_links])
found = finder.find_requirement(req, True)
assert found.url.endswith("priority-1.0-py2.py3-none-any.whl"), found
assert (
found.link.url.endswith("priority-1.0-py2.py3-none-any.whl")
), found

def test_existing_over_wheel_priority(self, data):
"""
Expand Down Expand Up @@ -292,8 +294,8 @@ def test_finder_priority_file_over_page(data):
assert all(version.link.scheme == 'https'
for version in all_versions[1:]), all_versions

link = finder.find_requirement(req, False)
assert link.url.startswith("file://")
candidate = finder.find_requirement(req, False)
assert candidate.link.url.startswith("file://")


def test_finder_priority_nonegg_over_eggfragments():
Expand All @@ -306,19 +308,19 @@ def test_finder_priority_nonegg_over_eggfragments():
assert all_versions[0].link.url.endswith('tar.gz')
assert all_versions[1].link.url.endswith('#egg=bar-1.0')

link = finder.find_requirement(req, False)
candidate = finder.find_requirement(req, False)

assert link.url.endswith('tar.gz')
assert candidate.link.url.endswith('tar.gz')

links.reverse()

finder = make_no_network_finder(links)
all_versions = finder.find_all_candidates(req.name)
assert all_versions[0].link.url.endswith('tar.gz')
assert all_versions[1].link.url.endswith('#egg=bar-1.0')
link = finder.find_requirement(req, False)
candidate = finder.find_requirement(req, False)

assert link.url.endswith('tar.gz')
assert candidate.link.url.endswith('tar.gz')


def test_finder_only_installs_stable_releases(data):
Expand All @@ -330,21 +332,21 @@ def test_finder_only_installs_stable_releases(data):

# using a local index (that has pre & dev releases)
finder = make_test_finder(index_urls=[data.index_url("pre")])
link = finder.find_requirement(req, False)
assert link.url.endswith("bar-1.0.tar.gz"), link.url
candidate = finder.find_requirement(req, False)
assert candidate.link.url.endswith("bar-1.0.tar.gz"), candidate.link.url

# using find-links
links = ["https://foo/bar-1.0.tar.gz", "https://foo/bar-2.0b1.tar.gz"]

finder = make_no_network_finder(links)
link = finder.find_requirement(req, False)
assert link.url == "https://foo/bar-1.0.tar.gz"
candidate = finder.find_requirement(req, False)
assert candidate.link.url == "https://foo/bar-1.0.tar.gz"

links.reverse()

finder = make_no_network_finder(links)
link = finder.find_requirement(req, False)
assert link.url == "https://foo/bar-1.0.tar.gz"
candidate = finder.find_requirement(req, False)
assert candidate.link.url == "https://foo/bar-1.0.tar.gz"


def test_finder_only_installs_data_require(data):
Expand Down Expand Up @@ -383,21 +385,21 @@ def test_finder_installs_pre_releases(data):
index_urls=[data.index_url("pre")],
allow_all_prereleases=True,
)
link = finder.find_requirement(req, False)
assert link.url.endswith("bar-2.0b1.tar.gz"), link.url
candidate = finder.find_requirement(req, False)
assert candidate.link.url.endswith("bar-2.0b1.tar.gz"), candidate.link.url

# using find-links
links = ["https://foo/bar-1.0.tar.gz", "https://foo/bar-2.0b1.tar.gz"]

finder = make_no_network_finder(links, allow_all_prereleases=True)
link = finder.find_requirement(req, False)
assert link.url == "https://foo/bar-2.0b1.tar.gz"
candidate = finder.find_requirement(req, False)
assert candidate.link.url == "https://foo/bar-2.0b1.tar.gz"

links.reverse()

finder = make_no_network_finder(links, allow_all_prereleases=True)
link = finder.find_requirement(req, False)
assert link.url == "https://foo/bar-2.0b1.tar.gz"
candidate = finder.find_requirement(req, False)
assert candidate.link.url == "https://foo/bar-2.0b1.tar.gz"


def test_finder_installs_dev_releases(data):
Expand All @@ -412,8 +414,8 @@ def test_finder_installs_dev_releases(data):
index_urls=[data.index_url("dev")],
allow_all_prereleases=True,
)
link = finder.find_requirement(req, False)
assert link.url.endswith("bar-2.0.dev1.tar.gz"), link.url
found = finder.find_requirement(req, False)
assert found.link.url.endswith("bar-2.0.dev1.tar.gz"), found.link.url


def test_finder_installs_pre_releases_with_version_spec():
Expand All @@ -424,14 +426,14 @@ def test_finder_installs_pre_releases_with_version_spec():
links = ["https://foo/bar-1.0.tar.gz", "https://foo/bar-2.0b1.tar.gz"]

finder = make_no_network_finder(links)
link = finder.find_requirement(req, False)
assert link.url == "https://foo/bar-2.0b1.tar.gz"
candidate = finder.find_requirement(req, False)
assert candidate.link.url == "https://foo/bar-2.0b1.tar.gz"

links.reverse()

finder = make_no_network_finder(links)
link = finder.find_requirement(req, False)
assert link.url == "https://foo/bar-2.0b1.tar.gz"
candidate = finder.find_requirement(req, False)
assert candidate.link.url == "https://foo/bar-2.0b1.tar.gz"


class TestLinkEvaluator(object):
Expand Down

0 comments on commit e878aac

Please sign in to comment.