Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[1.4] Fix certain hash miss matches in case of non weel installations #7619

Merged
merged 1 commit into from
Mar 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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)