Skip to content

Commit

Permalink
installer: fix handling of sdist hashes (#7594)
Browse files Browse the repository at this point in the history
* fix hash mismatch during installation with `no-binary` option for packages that provide suitable wheels
* do not skip hash check for sdists
* write hash for sdists in direct_url.json

(cherry picked from commit 26fbfd6)
  • Loading branch information
quantum-byte authored and github-actions[bot] committed Mar 7, 2023
1 parent c45c94e commit cda04a4
Show file tree
Hide file tree
Showing 4 changed files with 262 additions and 51 deletions.
9 changes: 7 additions & 2 deletions src/poetry/installation/chef.py
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,6 @@ def prepare(

if archive.is_dir():
tmp_dir = tempfile.mkdtemp(prefix="poetry-chef-")

return self._prepare(archive, Path(tmp_dir), editable=editable)

return self._prepare_sdist(archive, destination=output_dir)
Expand Down Expand Up @@ -198,13 +197,19 @@ def _should_prepare(self, archive: Path) -> bool:
def _is_wheel(cls, archive: Path) -> bool:
return archive.suffix == ".whl"

def get_cached_archive_for_link(self, link: Link) -> Path | None:
def get_cached_archive_for_link(self, link: Link, *, strict: bool) -> Path | None:
archives = self.get_cached_archives_for_link(link)
if not archives:
return None

candidates: list[tuple[float | None, Path]] = []
for archive in archives:
if strict:
# in strict mode return the original cached archive instead of the
# prioritized archive type.
if link.filename == archive.name:
return archive
continue
if archive.suffix != ".whl":
candidates.append((float("inf"), archive))
continue
Expand Down
21 changes: 15 additions & 6 deletions src/poetry/installation/executor.py
Original file line number Diff line number Diff line change
Expand Up @@ -693,11 +693,12 @@ def _download_link(self, operation: Install | Update, link: Link) -> Path:
package = operation.package

output_dir = self._chef.get_cache_directory_for_link(link)
archive = self._chef.get_cached_archive_for_link(link)
if archive is None:
# No cached distributions was found, so we download and prepare it
# Try to get cached original package for the link provided
original_archive = self._chef.get_cached_archive_for_link(link, strict=True)
if original_archive is None:
# No cached original distributions was found, so we download and prepare it
try:
archive = self._download_archive(operation, link)
original_archive = self._download_archive(operation, link)
except BaseException:
cache_directory = self._chef.get_cache_directory_for_link(link)
cached_file = cache_directory.joinpath(link.filename)
Expand All @@ -708,6 +709,13 @@ def _download_link(self, operation: Install | Update, link: Link) -> Path:

raise

# Get potential higher prioritized cached archive, otherwise it will fall back
# to the original archive.
archive = self._chef.get_cached_archive_for_link(link, strict=False)
# 'archive' can at this point never be None. Since we previously downloaded
# an archive, we now should have something cached that we can use here
assert archive is not None

if archive.suffix != ".whl":
message = (
f" <fg=blue;options=bold>•</> {self.get_operation_message(operation)}:"
Expand All @@ -717,7 +725,8 @@ def _download_link(self, operation: Install | Update, link: Link) -> Path:

archive = self._chef.prepare(archive, output_dir=output_dir)

self._populate_hashes_dict(archive, package)
# Use the original archive to provide the correct hash.
self._populate_hashes_dict(original_archive, package)

return archive

Expand All @@ -729,7 +738,7 @@ def _populate_hashes_dict(self, archive: Path, package: Package) -> None:
@staticmethod
def _validate_archive_hash(archive: Path, package: Package) -> str:
archive_hash: str = "sha256:" + get_file_hash(archive)
known_hashes = {f["hash"] for f in package.files}
known_hashes = {f["hash"] for f in package.files if f["file"] == archive.name}

if archive_hash not in known_hashes:
raise RuntimeError(
Expand Down
83 changes: 79 additions & 4 deletions tests/installation/test_chef.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
from __future__ import annotations

import os
import tempfile

from pathlib import Path
from typing import TYPE_CHECKING
from zipfile import ZipFile
Expand Down Expand Up @@ -38,20 +41,80 @@ def setup(mocker: MockerFixture, pool: RepositoryPool) -> None:


@pytest.mark.parametrize(
("link", "cached"),
("link", "strict", "available_packages"),
[
(
"https://files.python-poetry.org/demo-0.1.0.tar.gz",
True,
[
Path("/cache/demo-0.1.0-py2.py3-none-any"),
Path("/cache/demo-0.1.0-cp38-cp38-macosx_10_15_x86_64.whl"),
Path("/cache/demo-0.1.0-cp37-cp37-macosx_10_15_x86_64.whl"),
],
),
(
"https://example.com/demo-0.1.0-cp38-cp38-macosx_10_15_x86_64.whl",
False,
[],
),
],
)
def test_get_not_found_cached_archive_for_link(
config: Config,
mocker: MockerFixture,
link: str,
strict: bool,
available_packages: list[Path],
):
chef = Chef(
config,
MockEnv(
version_info=(3, 8, 3),
marker_env={"interpreter_name": "cpython", "interpreter_version": "3.8.3"},
supported_tags=[
Tag("cp38", "cp38", "macosx_10_15_x86_64"),
Tag("py3", "none", "any"),
],
),
Factory.create_pool(config),
)

mocker.patch.object(
chef, "get_cached_archives_for_link", return_value=available_packages
)

archive = chef.get_cached_archive_for_link(Link(link), strict=strict)

assert archive is None


@pytest.mark.parametrize(
("link", "cached", "strict"),
[
(
"https://files.python-poetry.org/demo-0.1.0.tar.gz",
"/cache/demo-0.1.0-cp38-cp38-macosx_10_15_x86_64.whl",
False,
),
(
"https://example.com/demo-0.1.0-cp38-cp38-macosx_10_15_x86_64.whl",
"/cache/demo-0.1.0-cp38-cp38-macosx_10_15_x86_64.whl",
False,
),
(
"https://files.python-poetry.org/demo-0.1.0.tar.gz",
"/cache/demo-0.1.0.tar.gz",
True,
),
(
"https://example.com/demo-0.1.0-cp38-cp38-macosx_10_15_x86_64.whl",
"/cache/demo-0.1.0-cp38-cp38-macosx_10_15_x86_64.whl",
True,
),
],
)
def test_get_cached_archive_for_link(
config: Config, mocker: MockerFixture, link: str, cached: str
def test_get_found_cached_archive_for_link(
config: Config, mocker: MockerFixture, link: str, cached: str, strict: bool
):
chef = Chef(
config,
Expand All @@ -77,7 +140,7 @@ def test_get_cached_archive_for_link(
],
)

archive = chef.get_cached_archive_for_link(Link(link))
archive = chef.get_cached_archive_for_link(Link(link), strict=strict)

assert Path(cached) == archive

Expand Down Expand Up @@ -153,6 +216,10 @@ def test_prepare_directory(config: Config, config_cache_dir: Path):

assert wheel.name == "simple_project-1.2.3-py2.py3-none-any.whl"

assert wheel.parent.parent == Path(tempfile.gettempdir())
# cleanup generated tmp dir artifact
os.unlink(wheel)


def test_prepare_directory_with_extensions(
config: Config, config_cache_dir: Path
Expand All @@ -168,8 +235,12 @@ def test_prepare_directory_with_extensions(

wheel = chef.prepare(archive)

assert wheel.parent.parent == Path(tempfile.gettempdir())
assert wheel.name == f"extended-0.1-{env.supported_tags[0]}.whl"

# cleanup generated tmp dir artifact
os.unlink(wheel)


def test_prepare_directory_editable(config: Config, config_cache_dir: Path):
chef = Chef(config, EnvManager.get_system_env(), Factory.create_pool(config))
Expand All @@ -178,7 +249,11 @@ def test_prepare_directory_editable(config: Config, config_cache_dir: Path):

wheel = chef.prepare(archive, editable=True)

assert wheel.parent.parent == Path(tempfile.gettempdir())
assert wheel.name == "simple_project-1.2.3-py2.py3-none-any.whl"

with ZipFile(wheel) as z:
assert "simple_project.pth" in z.namelist()

# cleanup generated tmp dir artifact
os.unlink(wheel)

0 comments on commit cda04a4

Please sign in to comment.