Skip to content

Commit

Permalink
Remove RequirementSet.require_hashes (#7068)
Browse files Browse the repository at this point in the history
  • Loading branch information
pradyunsg authored Oct 20, 2019
2 parents 2472903 + b8fb97a commit 9ab9040
Show file tree
Hide file tree
Showing 7 changed files with 31 additions and 38 deletions.
6 changes: 2 additions & 4 deletions src/pip/_internal/cli/req_command.py
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,8 @@ def make_resolver(
ignore_requires_python=ignore_requires_python,
force_reinstall=force_reinstall,
upgrade_strategy=upgrade_strategy,
py_version_info=py_version_info
py_version_info=py_version_info,
require_hashes=options.require_hashes,
)

def populate_requirement_set(
Expand Down Expand Up @@ -258,9 +259,6 @@ def populate_requirement_set(
use_pep517=options.use_pep517):
req_to_add.is_direct = True
requirement_set.add_requirement(req_to_add)
# If --require-hashes was a line in a requirements file, tell
# RequirementSet about it:
requirement_set.require_hashes = options.require_hashes

if not (args or options.editables or options.requirements):
opts = {'name': self.name}
Expand Down
4 changes: 1 addition & 3 deletions src/pip/_internal/commands/download.py
Original file line number Diff line number Diff line change
Expand Up @@ -115,9 +115,7 @@ def run(self, options, args):
options.build_dir, delete=build_delete, kind="download"
) as directory:

requirement_set = RequirementSet(
require_hashes=options.require_hashes,
)
requirement_set = RequirementSet()
self.populate_requirement_set(
requirement_set,
args,
Expand Down
1 change: 0 additions & 1 deletion src/pip/_internal/commands/install.py
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,6 @@ def run(self, options, args):
options.build_dir, delete=build_delete, kind="install"
) as directory:
requirement_set = RequirementSet(
require_hashes=options.require_hashes,
check_supported_wheels=not options.target_dir,
)

Expand Down
4 changes: 1 addition & 3 deletions src/pip/_internal/commands/wheel.py
Original file line number Diff line number Diff line change
Expand Up @@ -161,9 +161,7 @@ def run(self, options, args):
options.build_dir, delete=build_delete, kind="wheel"
) as directory:

requirement_set = RequirementSet(
require_hashes=options.require_hashes,
)
requirement_set = RequirementSet()

try:
self.populate_requirement_set(
Expand Down
34 changes: 17 additions & 17 deletions src/pip/_internal/legacy_resolve.py
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ def __init__(
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__()
Expand All @@ -142,8 +143,7 @@ def __init__(
self.finder = finder
self.session = session

# This is set in resolve
self.require_hashes = None # type: Optional[bool]
self.require_hashes_option = require_hashes

self.upgrade_strategy = upgrade_strategy
self.force_reinstall = force_reinstall
Expand Down Expand Up @@ -178,8 +178,9 @@ def resolve(self, requirement_set):
requirement_set.unnamed_requirements +
list(requirement_set.requirements.values())
)
self.require_hashes = (
requirement_set.require_hashes or

require_hashes = (
self.require_hashes_option or
any(req.has_hash_options for req in root_reqs)
)

Expand All @@ -198,7 +199,7 @@ def resolve(self, requirement_set):
for req in chain(root_reqs, discovered_reqs):
try:
discovered_reqs.extend(
self._resolve_one(requirement_set, req)
self._resolve_one(requirement_set, req, require_hashes)
)
except HashError as exc:
exc.req = req
Expand Down Expand Up @@ -281,18 +282,14 @@ def _check_skip_installed(self, req_to_install):
self._set_req_to_reinstall(req_to_install)
return None

def _get_abstract_dist_for(self, req):
# type: (InstallRequirement) -> AbstractDistribution
def _get_abstract_dist_for(self, req, require_hashes):
# type: (InstallRequirement, bool) -> AbstractDistribution
"""Takes a InstallRequirement and returns a single AbstractDist \
representing a prepared variant of the same.
"""
assert self.require_hashes is not None, (
"require_hashes should have been set in Resolver.resolve()"
)

if req.editable:
return self.preparer.prepare_editable_requirement(
req, self.require_hashes, self.use_user_site, self.finder,
req, require_hashes, self.use_user_site, self.finder,
)

# satisfied_by is only evaluated by calling _check_skip_installed,
Expand All @@ -302,15 +299,15 @@ def _get_abstract_dist_for(self, req):

if req.satisfied_by:
return self.preparer.prepare_installed_requirement(
req, self.require_hashes, skip_reason
req, require_hashes, skip_reason
)

upgrade_allowed = self._is_upgrade_allowed(req)

# We eagerly populate the link, since that's our "legacy" behavior.
req.populate_link(self.finder, upgrade_allowed, self.require_hashes)
req.populate_link(self.finder, upgrade_allowed, require_hashes)
abstract_dist = self.preparer.prepare_linked_requirement(
req, self.session, self.finder, self.require_hashes
req, self.session, self.finder, require_hashes
)

# NOTE
Expand Down Expand Up @@ -344,7 +341,8 @@ def _get_abstract_dist_for(self, req):
def _resolve_one(
self,
requirement_set, # type: RequirementSet
req_to_install # type: InstallRequirement
req_to_install, # type: InstallRequirement
require_hashes, # type: bool

This comment has been minimized.

Copy link
@techalchemy

techalchemy Nov 9, 2019

Member

@pradyunsg any reason this can't have a default value? It breaks compatibility (more) -- can this not rely on self.require_hashes?

This comment has been minimized.

Copy link
@chrahunt

chrahunt Nov 9, 2019

Member

require_hashes is going to be factored out of Resolver altogether eventually - the only reason it is still being taken as an argument is because we haven't moved its calculation from Resolver.resolve to some other place that is also shared among the commands invoking it.

):
# type: (...) -> List[InstallRequirement]
"""Prepare a single requirements file.
Expand All @@ -362,7 +360,9 @@ def _resolve_one(
# 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)
abstract_dist = self._get_abstract_dist_for(
req_to_install, require_hashes
)

# Parse and return dependencies
dist = abstract_dist.get_pkg_resources_distribution()
Expand Down
5 changes: 2 additions & 3 deletions src/pip/_internal/req/req_set.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,12 @@

class RequirementSet(object):

def __init__(self, require_hashes=False, check_supported_wheels=True):
# type: (bool, bool) -> None
def __init__(self, check_supported_wheels=True):
# type: (bool) -> None
"""Create a RequirementSet.
"""

self.requirements = OrderedDict() # type: Dict[str, InstallRequirement] # noqa: E501
self.require_hashes = require_hashes
self.check_supported_wheels = check_supported_wheels

self.unnamed_requirements = [] # type: List[InstallRequirement]
Expand Down
15 changes: 8 additions & 7 deletions tests/unit/test_req.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ def setup(self):
def teardown(self):
shutil.rmtree(self.tempdir, ignore_errors=True)

def _basic_resolver(self, finder):
def _basic_resolver(self, finder, require_hashes=False):
preparer = RequirementPreparer(
build_dir=os.path.join(self.tempdir, 'build'),
src_dir=os.path.join(self.tempdir, 'src'),
Expand All @@ -78,6 +78,7 @@ def _basic_resolver(self, finder):
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):
Expand Down Expand Up @@ -171,13 +172,13 @@ def test_missing_hash_with_require_hashes(self, data):
"""Setting --require-hashes explicitly should raise errors if hashes
are missing.
"""
reqset = RequirementSet(require_hashes=True)
reqset = RequirementSet()
reqset.add_requirement(get_processed_req_from_line(
'simple==1.0', 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,
Expand All @@ -193,7 +194,7 @@ def test_missing_hash_with_require_hashes_in_reqs_file(self, data, tmpdir):
"""--require-hashes in a requirements file should make its way to the
RequirementSet.
"""
req_set = RequirementSet(require_hashes=False)
req_set = RequirementSet()
finder = make_test_finder(find_links=[data.find_links])
session = finder._link_collector.session
command = create_command('install')
Expand All @@ -202,7 +203,7 @@ def test_missing_hash_with_require_hashes_in_reqs_file(self, data, tmpdir):
command.populate_requirement_set(
req_set, args, options, finder, session, wheel_cache=None,
)
assert req_set.require_hashes
assert options.require_hashes

def test_unsupported_hashes(self, data):
"""VCS and dir links should raise errors when --require-hashes is
Expand All @@ -212,7 +213,7 @@ def test_unsupported_hashes(self, data):
should trump the presence or absence of a hash.
"""
reqset = RequirementSet(require_hashes=True)
reqset = RequirementSet()
reqset.add_requirement(get_processed_req_from_line(
'git+git://github.com/pypa/pip-test-package --hash=sha256:123',
lineno=1,
Expand Down Expand Up @@ -271,7 +272,7 @@ def test_hash_mismatch(self, data):
"""A hash mismatch should raise an error."""
file_url = path_to_url(
(data.packages / 'simple-1.0.tar.gz').resolve())
reqset = RequirementSet(require_hashes=True)
reqset = RequirementSet()
reqset.add_requirement(get_processed_req_from_line(
'%s --hash=sha256:badbad' % file_url, lineno=1,
))
Expand Down

0 comments on commit 9ab9040

Please sign in to comment.