summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorStéphane Bidoul <stephane.bidoul@gmail.com>2023-04-15 11:38:38 +0200
committerGitHub <noreply@github.com>2023-04-15 11:38:38 +0200
commitdbf4e6842c9603792f6d3944a5c9cec17bd0a92a (patch)
treea9e4630d04fa03dea335d5a93a6041cf736c0bde
parentf7787f8798712e475ebbf71f5487f92158f043a9 (diff)
parentefe2d27451d50b165df78093bf5885da713fbdf8 (diff)
downloadpip-dbf4e6842c9603792f6d3944a5c9cec17bd0a92a.tar.gz
Merge pull request #11897 from sbidoul/cache-hash-checking-sbi
Support wheel cache when using --require-hashes
-rw-r--r--news/5037.feature.rst1
-rw-r--r--src/pip/_internal/operations/prepare.py59
-rw-r--r--src/pip/_internal/req/req_install.py12
-rw-r--r--src/pip/_internal/resolution/legacy/resolver.py2
-rw-r--r--src/pip/_internal/resolution/resolvelib/candidates.py6
-rw-r--r--src/pip/_internal/resolution/resolvelib/factory.py2
-rw-r--r--src/pip/_internal/utils/hashes.py7
-rw-r--r--tests/functional/test_install.py42
-rw-r--r--tests/unit/test_req.py6
-rw-r--r--tests/unit/test_utils.py8
10 files changed, 129 insertions, 16 deletions
diff --git a/news/5037.feature.rst b/news/5037.feature.rst
new file mode 100644
index 000000000..fe4637b6c
--- /dev/null
+++ b/news/5037.feature.rst
@@ -0,0 +1 @@
+Support wheel cache when using ``--require-hashes``.
diff --git a/src/pip/_internal/operations/prepare.py b/src/pip/_internal/operations/prepare.py
index dda92d29b..227331523 100644
--- a/src/pip/_internal/operations/prepare.py
+++ b/src/pip/_internal/operations/prepare.py
@@ -179,7 +179,10 @@ def unpack_url(
def _check_download_dir(
- link: Link, download_dir: str, hashes: Optional[Hashes]
+ link: Link,
+ download_dir: str,
+ hashes: Optional[Hashes],
+ warn_on_hash_mismatch: bool = True,
) -> Optional[str]:
"""Check download_dir for previously downloaded file with correct hash
If a correct file is found return its path else None
@@ -195,10 +198,11 @@ def _check_download_dir(
try:
hashes.check_against_path(download_path)
except HashMismatch:
- logger.warning(
- "Previously-downloaded file %s has bad hash. Re-downloading.",
- download_path,
- )
+ if warn_on_hash_mismatch:
+ logger.warning(
+ "Previously-downloaded file %s has bad hash. Re-downloading.",
+ download_path,
+ )
os.unlink(download_path)
return None
return download_path
@@ -263,7 +267,7 @@ class RequirementPreparer:
def _log_preparing_link(self, req: InstallRequirement) -> None:
"""Provide context for the requirement being prepared."""
- if req.link.is_file and not req.original_link_is_in_wheel_cache:
+ if req.link.is_file and not req.is_wheel_from_cache:
message = "Processing %s"
information = str(display_path(req.link.file_path))
else:
@@ -284,7 +288,7 @@ class RequirementPreparer:
self._previous_requirement_header = (message, information)
logger.info(message, information)
- if req.original_link_is_in_wheel_cache:
+ if req.is_wheel_from_cache:
with indent_log():
logger.info("Using cached %s", req.link.filename)
@@ -485,7 +489,18 @@ class RequirementPreparer:
file_path = None
if self.download_dir is not None and req.link.is_wheel:
hashes = self._get_linked_req_hashes(req)
- file_path = _check_download_dir(req.link, self.download_dir, hashes)
+ file_path = _check_download_dir(
+ req.link,
+ self.download_dir,
+ hashes,
+ # When a locally built wheel has been found in cache, we don't warn
+ # about re-downloading when the already downloaded wheel hash does
+ # not match. This is because the hash must be checked against the
+ # original link, not the cached link. It that case the already
+ # downloaded file will be removed and re-fetched from cache (which
+ # implies a hash check against the cache entry's origin.json).
+ warn_on_hash_mismatch=not req.is_wheel_from_cache,
+ )
if file_path is not None:
# The file is already available, so mark it as downloaded
@@ -536,9 +551,35 @@ class RequirementPreparer:
assert req.link
link = req.link
- self._ensure_link_req_src_dir(req, parallel_builds)
hashes = self._get_linked_req_hashes(req)
+ if hashes and req.is_wheel_from_cache:
+ assert req.download_info is not None
+ assert link.is_wheel
+ assert link.is_file
+ # We need to verify hashes, and we have found the requirement in the cache
+ # of locally built wheels.
+ if (
+ isinstance(req.download_info.info, ArchiveInfo)
+ and req.download_info.info.hashes
+ and hashes.has_one_of(req.download_info.info.hashes)
+ ):
+ # At this point we know the requirement was built from a hashable source
+ # artifact, and we verified that the cache entry's hash of the original
+ # artifact matches one of the hashes we expect. We don't verify hashes
+ # against the cached wheel, because the wheel is not the original.
+ hashes = None
+ else:
+ logger.warning(
+ "The hashes of the source archive found in cache entry "
+ "don't match, ignoring cached built wheel "
+ "and re-downloading source."
+ )
+ req.link = req.cached_wheel_source_link
+ link = req.link
+
+ self._ensure_link_req_src_dir(req, parallel_builds)
+
if link.is_existing_dir():
local_file = None
elif link.url not in self._downloaded:
diff --git a/src/pip/_internal/req/req_install.py b/src/pip/_internal/req/req_install.py
index e2353f032..a0ea58bd1 100644
--- a/src/pip/_internal/req/req_install.py
+++ b/src/pip/_internal/req/req_install.py
@@ -108,7 +108,11 @@ class InstallRequirement:
# PEP 508 URL requirement
link = Link(req.url)
self.link = self.original_link = link
- self.original_link_is_in_wheel_cache = False
+
+ # When this InstallRequirement is a wheel obtained from the cache of locally
+ # built wheels, this is the source link corresponding to the cache entry, which
+ # was used to download and build the cached wheel.
+ self.cached_wheel_source_link: Optional[Link] = None
# Information about the location of the artifact that was downloaded . This
# property is guaranteed to be set in resolver results.
@@ -437,6 +441,12 @@ class InstallRequirement:
return False
return self.link.is_wheel
+ @property
+ def is_wheel_from_cache(self) -> bool:
+ # When True, it means that this InstallRequirement is a local wheel file in the
+ # cache of locally built wheels.
+ return self.cached_wheel_source_link is not None
+
# Things valid for sdists
@property
def unpacked_source_directory(self) -> str:
diff --git a/src/pip/_internal/resolution/legacy/resolver.py b/src/pip/_internal/resolution/legacy/resolver.py
index 3a561e6db..b17b7e453 100644
--- a/src/pip/_internal/resolution/legacy/resolver.py
+++ b/src/pip/_internal/resolution/legacy/resolver.py
@@ -431,7 +431,7 @@ class Resolver(BaseResolver):
if cache_entry is not None:
logger.debug("Using cached wheel link: %s", cache_entry.link)
if req.link is req.original_link and cache_entry.persistent:
- req.original_link_is_in_wheel_cache = True
+ req.cached_wheel_source_link = req.link
if cache_entry.origin is not None:
req.download_info = cache_entry.origin
else:
diff --git a/src/pip/_internal/resolution/resolvelib/candidates.py b/src/pip/_internal/resolution/resolvelib/candidates.py
index e5e9d1fd7..31020e27a 100644
--- a/src/pip/_internal/resolution/resolvelib/candidates.py
+++ b/src/pip/_internal/resolution/resolvelib/candidates.py
@@ -259,7 +259,7 @@ class LinkCandidate(_InstallRequirementBackedCandidate):
version: Optional[CandidateVersion] = None,
) -> None:
source_link = link
- cache_entry = factory.get_wheel_cache_entry(link, name)
+ cache_entry = factory.get_wheel_cache_entry(source_link, name)
if cache_entry is not None:
logger.debug("Using cached wheel link: %s", cache_entry.link)
link = cache_entry.link
@@ -277,8 +277,10 @@ class LinkCandidate(_InstallRequirementBackedCandidate):
)
if cache_entry is not None:
+ assert ireq.link.is_wheel
+ assert ireq.link.is_file
if cache_entry.persistent and template.link is template.original_link:
- ireq.original_link_is_in_wheel_cache = True
+ ireq.cached_wheel_source_link = source_link
if cache_entry.origin is not None:
ireq.download_info = cache_entry.origin
else:
diff --git a/src/pip/_internal/resolution/resolvelib/factory.py b/src/pip/_internal/resolution/resolvelib/factory.py
index 0ad4641b1..0331297b8 100644
--- a/src/pip/_internal/resolution/resolvelib/factory.py
+++ b/src/pip/_internal/resolution/resolvelib/factory.py
@@ -535,7 +535,7 @@ class Factory:
hash mismatches. Furthermore, cached wheels at present have
nondeterministic contents due to file modification times.
"""
- if self._wheel_cache is None or self.preparer.require_hashes:
+ if self._wheel_cache is None:
return None
return self._wheel_cache.get_cache_entry(
link=link,
diff --git a/src/pip/_internal/utils/hashes.py b/src/pip/_internal/utils/hashes.py
index 76727306a..843cffc6b 100644
--- a/src/pip/_internal/utils/hashes.py
+++ b/src/pip/_internal/utils/hashes.py
@@ -105,6 +105,13 @@ class Hashes:
with open(path, "rb") as file:
return self.check_against_file(file)
+ def has_one_of(self, hashes: Dict[str, str]) -> bool:
+ """Return whether any of the given hashes are allowed."""
+ for hash_name, hex_digest in hashes.items():
+ if self.is_hash_allowed(hash_name, hex_digest):
+ return True
+ return False
+
def __bool__(self) -> bool:
"""Return whether I know any known-good hashes."""
return bool(self._allowed)
diff --git a/tests/functional/test_install.py b/tests/functional/test_install.py
index e50779688..637128274 100644
--- a/tests/functional/test_install.py
+++ b/tests/functional/test_install.py
@@ -729,6 +729,48 @@ def test_bad_link_hash_in_dep_install_failure(
assert "THESE PACKAGES DO NOT MATCH THE HASHES" in result.stderr, result.stderr
+def test_hashed_install_from_cache(
+ script: PipTestEnvironment, data: TestData, tmpdir: Path
+) -> None:
+ """
+ Test that installing from a cached built wheel works and that the hash is verified
+ against the hash of the original source archived stored in the cache entry.
+ """
+ with requirements_file(
+ "simple2==1.0 --hash=sha256:"
+ "9336af72ca661e6336eb87bc7de3e8844d853e3848c2b9bbd2e8bf01db88c2c7\n",
+ tmpdir,
+ ) as reqs_file:
+ result = script.pip_install_local(
+ "--use-pep517", "--no-build-isolation", "-r", reqs_file.resolve()
+ )
+ assert "Created wheel for simple2" in result.stdout
+ script.pip("uninstall", "simple2", "-y")
+ result = script.pip_install_local(
+ "--use-pep517", "--no-build-isolation", "-r", reqs_file.resolve()
+ )
+ assert "Using cached simple2" in result.stdout
+ # now try with an invalid hash
+ with requirements_file(
+ "simple2==1.0 --hash=sha256:invalid\n",
+ tmpdir,
+ ) as reqs_file:
+ script.pip("uninstall", "simple2", "-y")
+ result = script.pip_install_local(
+ "--use-pep517",
+ "--no-build-isolation",
+ "-r",
+ reqs_file.resolve(),
+ expect_error=True,
+ )
+ assert (
+ "WARNING: The hashes of the source archive found in cache entry "
+ "don't match, ignoring cached built wheel and re-downloading source."
+ ) in result.stderr
+ assert "Using cached simple2" in result.stdout
+ assert "ERROR: THESE PACKAGES DO NOT MATCH THE HASHES" in result.stderr
+
+
def assert_re_match(pattern: str, text: str) -> None:
assert re.search(pattern, text), f"Could not find {pattern!r} in {text!r}"
diff --git a/tests/unit/test_req.py b/tests/unit/test_req.py
index a5286c13a..c9742812b 100644
--- a/tests/unit/test_req.py
+++ b/tests/unit/test_req.py
@@ -411,7 +411,8 @@ class TestRequirementSet:
reqset = resolver.resolve([ireq], True)
assert len(reqset.all_requirements) == 1
req = reqset.all_requirements[0]
- assert req.original_link_is_in_wheel_cache
+ assert req.is_wheel_from_cache
+ assert req.cached_wheel_source_link
assert req.download_info
assert req.download_info.url == url
assert isinstance(req.download_info.info, ArchiveInfo)
@@ -437,7 +438,8 @@ class TestRequirementSet:
reqset = resolver.resolve([ireq], True)
assert len(reqset.all_requirements) == 1
req = reqset.all_requirements[0]
- assert req.original_link_is_in_wheel_cache
+ assert req.is_wheel_from_cache
+ assert req.cached_wheel_source_link
assert req.download_info
assert req.download_info.url == url
assert isinstance(req.download_info.info, ArchiveInfo)
diff --git a/tests/unit/test_utils.py b/tests/unit/test_utils.py
index a67a7c110..450081cfd 100644
--- a/tests/unit/test_utils.py
+++ b/tests/unit/test_utils.py
@@ -425,6 +425,14 @@ class TestHashes:
cache[Hashes({"sha256": ["ab", "cd"]})] = 42
assert cache[Hashes({"sha256": ["ab", "cd"]})] == 42
+ def test_has_one_of(self) -> None:
+ hashes = Hashes({"sha256": ["abcd", "efgh"], "sha384": ["ijkl"]})
+ assert hashes.has_one_of({"sha256": "abcd"})
+ assert hashes.has_one_of({"sha256": "efgh"})
+ assert not hashes.has_one_of({"sha256": "xyzt"})
+ empty_hashes = Hashes()
+ assert not empty_hashes.has_one_of({"sha256": "xyzt"})
+
class TestEncoding:
"""Tests for pip._internal.utils.encoding"""