Merge pull request #7324 from pradyunsg/refactor/require_hashes

Refactor handling of require_hashes in the Resolver+RequirementPreparer
This commit is contained in:
Christopher Hunt 2019-11-10 10:35:05 +08:00 committed by GitHub
commit d38cead031
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 68 additions and 84 deletions

View File

@ -173,6 +173,7 @@ class RequirementCommand(IndexGroupCommand):
req_tracker=req_tracker,
session=session,
finder=finder,
require_hashes=options.require_hashes,
)
@staticmethod
@ -210,7 +211,6 @@ class RequirementCommand(IndexGroupCommand):
force_reinstall=force_reinstall,
upgrade_strategy=upgrade_strategy,
py_version_info=py_version_info,
require_hashes=options.require_hashes,
)
def populate_requirement_set(
@ -226,9 +226,6 @@ class RequirementCommand(IndexGroupCommand):
"""
Marshal cmd line args into a requirement set.
"""
# NOTE: As a side-effect, options.require_hashes and
# requirement_set.require_hashes may be updated
for filename in options.constraints:
for req_to_add in parse_requirements(
filename,
@ -256,6 +253,7 @@ class RequirementCommand(IndexGroupCommand):
req_to_add.is_direct = True
requirement_set.add_requirement(req_to_add)
# NOTE: options.require_hashes may be set if --require-hashes is True
for filename in options.requirements:
for req_to_add in parse_requirements(
filename,
@ -265,6 +263,14 @@ class RequirementCommand(IndexGroupCommand):
req_to_add.is_direct = True
requirement_set.add_requirement(req_to_add)
# If any requirement has hash options, enable hash checking.
requirements = (
requirement_set.unnamed_requirements +
list(requirement_set.requirements.values())
)
if any(req.has_hash_options for req in requirements):
options.require_hashes = True
if not (args or options.editables or options.requirements):
opts = {'name': self.name}
if options.find_links:

View File

@ -124,7 +124,6 @@ class Resolver(object):
force_reinstall, # type: bool
upgrade_strategy, # type: str
py_version_info=None, # type: Optional[Tuple[int, ...]]
require_hashes=False, # type: bool
):
# type: (...) -> None
super(Resolver, self).__init__()
@ -140,8 +139,6 @@ class Resolver(object):
self.preparer = preparer
self.finder = finder
self.require_hashes_option = require_hashes
self.upgrade_strategy = upgrade_strategy
self.force_reinstall = force_reinstall
self.ignore_dependencies = ignore_dependencies
@ -176,11 +173,6 @@ class Resolver(object):
list(requirement_set.requirements.values())
)
require_hashes = (
self.require_hashes_option or
any(req.has_hash_options for req in root_reqs)
)
# Actually prepare the files, and collect any exceptions. Most hash
# exceptions cannot be checked ahead of time, because
# req.populate_link() needs to be called before we can make decisions
@ -189,9 +181,7 @@ class Resolver(object):
hash_errors = HashErrors()
for req in chain(root_reqs, discovered_reqs):
try:
discovered_reqs.extend(
self._resolve_one(requirement_set, req, require_hashes)
)
discovered_reqs.extend(self._resolve_one(requirement_set, req))
except HashError as exc:
exc.req = req
hash_errors.append(exc)
@ -273,14 +263,14 @@ class Resolver(object):
self._set_req_to_reinstall(req_to_install)
return None
def _get_abstract_dist_for(self, req, require_hashes):
# type: (InstallRequirement, bool) -> AbstractDistribution
def _get_abstract_dist_for(self, req):
# type: (InstallRequirement) -> AbstractDistribution
"""Takes a InstallRequirement and returns a single AbstractDist \
representing a prepared variant of the same.
"""
if req.editable:
return self.preparer.prepare_editable_requirement(
req, require_hashes, self.use_user_site,
req, self.use_user_site
)
# satisfied_by is only evaluated by calling _check_skip_installed,
@ -290,16 +280,15 @@ class Resolver(object):
if req.satisfied_by:
return self.preparer.prepare_installed_requirement(
req, require_hashes, skip_reason
req, skip_reason
)
upgrade_allowed = self._is_upgrade_allowed(req)
# We eagerly populate the link, since that's our "legacy" behavior.
require_hashes = self.preparer.require_hashes
req.populate_link(self.finder, upgrade_allowed, require_hashes)
abstract_dist = self.preparer.prepare_linked_requirement(
req, require_hashes
)
abstract_dist = self.preparer.prepare_linked_requirement(req)
# NOTE
# The following portion is for determining if a certain package is
@ -333,7 +322,6 @@ class Resolver(object):
self,
requirement_set, # type: RequirementSet
req_to_install, # type: InstallRequirement
require_hashes, # type: bool
):
# type: (...) -> List[InstallRequirement]
"""Prepare a single requirements file.
@ -351,9 +339,7 @@ class Resolver(object):
# register tmp src for cleanup in case something goes wrong
requirement_set.reqs_to_cleanup.append(req_to_install)
abstract_dist = self._get_abstract_dist_for(
req_to_install, require_hashes
)
abstract_dist = self._get_abstract_dist_for(req_to_install)
# Parse and return dependencies
dist = abstract_dist.get_pkg_resources_distribution()

View File

@ -510,6 +510,7 @@ class RequirementPreparer(object):
req_tracker, # type: RequirementTracker
session, # type: PipSession
finder, # type: PackageFinder
require_hashes, # type: bool
):
# type: (...) -> None
super(RequirementPreparer, self).__init__()
@ -543,6 +544,9 @@ class RequirementPreparer(object):
# Is build isolation allowed?
self.build_isolation = build_isolation
# Should hash-checking be required?
self.require_hashes = require_hashes
@property
def _download_should_save(self):
# type: () -> bool
@ -560,7 +564,6 @@ class RequirementPreparer(object):
def prepare_linked_requirement(
self,
req, # type: InstallRequirement
require_hashes, # type: bool
):
# type: (...) -> AbstractDistribution
"""Prepare a requirement that would be obtained from req.link
@ -600,7 +603,7 @@ class RequirementPreparer(object):
# requirements we have and raise some more informative errors
# than otherwise. (For example, we can raise VcsHashUnsupported
# for a VCS URL rather than HashMissing.)
if require_hashes:
if self.require_hashes:
# We could check these first 2 conditions inside
# unpack_url and save repetition of conditions, but then
# we would report less-useful error messages for
@ -620,8 +623,8 @@ class RequirementPreparer(object):
# about them not being pinned.
raise HashUnpinned()
hashes = req.hashes(trust_internet=not require_hashes)
if require_hashes and not hashes:
hashes = req.hashes(trust_internet=not self.require_hashes)
if self.require_hashes and not hashes:
# Known-good hashes are missing for this requirement, so
# shim it with a facade object that will provoke hash
# computation and then raise a HashMissing exception
@ -679,7 +682,6 @@ class RequirementPreparer(object):
def prepare_editable_requirement(
self,
req, # type: InstallRequirement
require_hashes, # type: bool
use_user_site, # type: bool
):
# type: (...) -> AbstractDistribution
@ -690,7 +692,7 @@ class RequirementPreparer(object):
logger.info('Obtaining %s', req)
with indent_log():
if require_hashes:
if self.require_hashes:
raise InstallationError(
'The editable requirement {} cannot be installed when '
'requiring hashes, because there is no single file to '
@ -712,7 +714,6 @@ class RequirementPreparer(object):
def prepare_installed_requirement(
self,
req, # type: InstallRequirement
require_hashes, # type: bool
skip_reason # type: str
):
# type: (...) -> AbstractDistribution
@ -728,7 +729,7 @@ class RequirementPreparer(object):
skip_reason, req, req.satisfied_by.version
)
with indent_log():
if require_hashes:
if self.require_hashes:
logger.debug(
'Since it is already installed, we are trusting this '
'package without checking its hash. To ensure a '

View File

@ -1,6 +1,7 @@
import distutils
import glob
import os
import re
import shutil
import ssl
import sys
@ -498,6 +499,41 @@ def test_hashed_install_failure(script, tmpdir):
assert len(result.files_created) == 0
def assert_re_match(pattern, text):
assert re.search(pattern, text), (
"Could not find {!r} in {!r}".format(pattern, text)
)
@pytest.mark.network
def test_hashed_install_failure_later_flag(script, tmpdir):
with requirements_file(
"blessings==1.0\n"
"tracefront==0.1 --hash=sha256:somehash\n"
"https://files.pythonhosted.org/packages/source/m/more-itertools/"
"more-itertools-1.0.tar.gz#md5=b21850c3cfa7efbb70fd662ab5413bdd\n"
"https://files.pythonhosted.org/"
"packages/source/p/peep/peep-3.1.1.tar.gz\n",
tmpdir,
) as reqs_file:
result = script.pip(
"install", "-r", reqs_file.resolve(), expect_error=True
)
assert_re_match(
r'Hashes are required in --require-hashes mode, but they are '
r'missing .*\n'
r' https://files\.pythonhosted\.org/packages/source/p/peep/peep'
r'-3\.1\.1\.tar\.gz --hash=sha256:[0-9a-f]+\n'
r' blessings==1.0 --hash=sha256:[0-9a-f]+\n'
r'THESE PACKAGES DO NOT MATCH THE HASHES.*\n'
r' tracefront==0.1 .*:\n'
r' Expected sha256 somehash\n'
r' Got [0-9a-f]+',
result.stderr,
)
def test_install_from_local_directory_with_symlinks_to_directories(
script, data):
"""

View File

@ -72,6 +72,7 @@ class TestRequirementSet(object):
req_tracker=RequirementTracker(),
session=PipSession(),
finder=finder,
require_hashes=require_hashes,
)
make_install_req = partial(
install_req_from_req_string,
@ -86,7 +87,6 @@ class TestRequirementSet(object):
use_user_site=False, upgrade_strategy="to-satisfy-only",
ignore_dependencies=False, ignore_installed=False,
ignore_requires_python=False, force_reinstall=False,
require_hashes=require_hashes,
)
def test_no_reuse_existing_build_dir(self, data):
@ -131,51 +131,6 @@ class TestRequirementSet(object):
else:
assert not reqset.has_requirement('simple')
@pytest.mark.network
def test_missing_hash_checking(self):
"""Make sure prepare_files() raises an error when a requirement has no
hash in implicit hash-checking mode.
"""
reqset = RequirementSet()
# No flags here. This tests that detection of later flags nonetheless
# requires earlier packages to have hashes:
reqset.add_requirement(get_processed_req_from_line(
'blessings==1.0', lineno=1
))
# This flag activates --require-hashes mode:
reqset.add_requirement(get_processed_req_from_line(
'tracefront==0.1 --hash=sha256:somehash', lineno=2,
))
# This hash should be accepted because it came from the reqs file, not
# from the internet:
reqset.add_requirement(get_processed_req_from_line(
'https://files.pythonhosted.org/packages/source/m/more-itertools/'
'more-itertools-1.0.tar.gz#md5=b21850c3cfa7efbb70fd662ab5413bdd',
lineno=3,
))
# The error text should list this as a URL and not `peep==3.1.1`:
reqset.add_requirement(get_processed_req_from_line(
'https://files.pythonhosted.org/'
'packages/source/p/peep/peep-3.1.1.tar.gz',
lineno=4,
))
finder = make_test_finder(index_urls=['https://pypi.org/simple/'])
resolver = self._basic_resolver(finder)
assert_raises_regexp(
HashErrors,
r'Hashes are required in --require-hashes mode, but they are '
r'missing .*\n'
r' https://files\.pythonhosted\.org/packages/source/p/peep/peep'
r'-3\.1\.1\.tar\.gz --hash=sha256:[0-9a-f]+\n'
r' blessings==1.0 --hash=sha256:[0-9a-f]+\n'
r'THESE PACKAGES DO NOT MATCH THE HASHES.*\n'
r' tracefront==0.1 .*:\n'
r' Expected sha256 somehash\n'
r' Got [0-9a-f]+$',
resolver.resolve,
reqset
)
def test_missing_hash_with_require_hashes(self, data):
"""Setting --require-hashes explicitly should raise errors if hashes
are missing.
@ -232,7 +187,7 @@ class TestRequirementSet(object):
lineno=2,
))
finder = make_test_finder(find_links=[data.find_links])
resolver = self._basic_resolver(finder)
resolver = self._basic_resolver(finder, require_hashes=True)
sep = os.path.sep
if sep == '\\':
sep = '\\\\' # This needs to be escaped for the regex
@ -266,7 +221,7 @@ class TestRequirementSet(object):
lineno=2,
))
finder = make_test_finder(find_links=[data.find_links])
resolver = self._basic_resolver(finder)
resolver = self._basic_resolver(finder, require_hashes=True)
assert_raises_regexp(
HashErrors,
# Make sure all failing requirements are listed:
@ -285,7 +240,7 @@ class TestRequirementSet(object):
'%s --hash=sha256:badbad' % file_url, lineno=1,
))
finder = make_test_finder(find_links=[data.find_links])
resolver = self._basic_resolver(finder)
resolver = self._basic_resolver(finder, require_hashes=True)
assert_raises_regexp(
HashErrors,
r'THESE PACKAGES DO NOT MATCH THE HASHES.*\n'
@ -301,7 +256,7 @@ class TestRequirementSet(object):
dependencies get complained about when --require-hashes is on."""
reqset = RequirementSet()
finder = make_test_finder(find_links=[data.find_links])
resolver = self._basic_resolver(finder)
resolver = self._basic_resolver(finder, require_hashes=True)
reqset.add_requirement(get_processed_req_from_line(
'TopoRequires2==0.0.1 ' # requires TopoRequires
'--hash=sha256:eaf9a01242c9f2f42cf2bd82a6a848cd'