From 9949aa45ef78cfbac30ccaf0634a31908a9b7a36 Mon Sep 17 00:00:00 2001 From: ralbertazzi Date: Tue, 7 Mar 2023 17:51:23 +0100 Subject: [PATCH 1/7] refactor: extract cache utilities --- src/poetry/config/config.py | 4 + src/poetry/installation/chef.py | 84 +-------------- src/poetry/installation/executor.py | 61 ++++++++++- src/poetry/utils/cache.py | 37 +++++++ tests/installation/test_chef.py | 155 +--------------------------- tests/installation/test_executor.py | 119 +++++++++++++++++++-- tests/utils/test_cache.py | 33 +++++- 7 files changed, 248 insertions(+), 245 deletions(-) diff --git a/src/poetry/config/config.py b/src/poetry/config/config.py index 6eda9699870..1ea84b9c34c 100644 --- a/src/poetry/config/config.py +++ b/src/poetry/config/config.py @@ -212,6 +212,10 @@ def _get_environment_repositories() -> dict[str, dict[str, str]]: def repository_cache_directory(self) -> Path: return Path(self.get("cache-dir")) / "cache" / "repositories" + @property + def artifacts_cache_directory(self) -> Path: + return Path(self.get("cache-dir")) / "artifacts" + @property def virtualenvs_path(self) -> Path: path = self.get("virtualenvs.path") diff --git a/src/poetry/installation/chef.py b/src/poetry/installation/chef.py index 6eb1ae3f21b..b58d6de460b 100644 --- a/src/poetry/installation/chef.py +++ b/src/poetry/installation/chef.py @@ -1,7 +1,5 @@ from __future__ import annotations -import hashlib -import json import tarfile import tempfile import zipfile @@ -19,16 +17,13 @@ from poetry.core.utils.helpers import temporary_directory from pyproject_hooks import quiet_subprocess_runner # type: ignore[import] -from poetry.installation.chooser import InvalidWheelName -from poetry.installation.chooser import Wheel +from poetry.utils.cache import get_cache_directory_for_link from poetry.utils.env import ephemeral_environment if TYPE_CHECKING: from contextlib import AbstractContextManager - from poetry.core.packages.utils.link import Link - from poetry.config.config import Config from poetry.repositories import RepositoryPool from poetry.utils.env import Env @@ -89,9 +84,7 @@ class Chef: def __init__(self, config: Config, env: Env, pool: RepositoryPool) -> None: self._env = env self._pool = pool - self._cache_dir = ( - Path(config.get("cache-dir")).expanduser().joinpath("artifacts") - ) + self._cache_dir = config.artifacts_cache_directory def prepare( self, archive: Path, output_dir: Path | None = None, *, editable: bool = False @@ -181,7 +174,9 @@ def _prepare_sdist(self, archive: Path, destination: Path | None = None) -> Path sdist_dir = archive_dir if destination is None: - destination = self.get_cache_directory_for_link(Link(archive.as_uri())) + destination = get_cache_directory_for_link( + self._cache_dir, Link(archive.as_uri()) + ) destination.mkdir(parents=True, exist_ok=True) @@ -196,72 +191,3 @@ def _should_prepare(self, archive: Path) -> bool: @classmethod def _is_wheel(cls, archive: Path) -> bool: return archive.suffix == ".whl" - - 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 - - try: - wheel = Wheel(archive.name) - except InvalidWheelName: - continue - - if not wheel.is_supported_by_environment(self._env): - continue - - candidates.append( - (wheel.get_minimum_supported_index(self._env.supported_tags), archive), - ) - - if not candidates: - return None - - return min(candidates)[1] - - def get_cached_archives_for_link(self, link: Link) -> list[Path]: - cache_dir = self.get_cache_directory_for_link(link) - - archive_types = ["whl", "tar.gz", "tar.bz2", "bz2", "zip"] - paths = [] - for archive_type in archive_types: - for archive in cache_dir.glob(f"*.{archive_type}"): - paths.append(Path(archive)) - - return paths - - def get_cache_directory_for_link(self, link: Link) -> Path: - key_parts = {"url": link.url_without_fragment} - - if link.hash_name is not None and link.hash is not None: - key_parts[link.hash_name] = link.hash - - if link.subdirectory_fragment: - key_parts["subdirectory"] = link.subdirectory_fragment - - key_parts["interpreter_name"] = self._env.marker_env["interpreter_name"] - key_parts["interpreter_version"] = "".join( - self._env.marker_env["interpreter_version"].split(".")[:2] - ) - - key = hashlib.sha256( - json.dumps( - key_parts, sort_keys=True, separators=(",", ":"), ensure_ascii=True - ).encode("ascii") - ).hexdigest() - - split_key = [key[:2], key[2:4], key[4:6], key[6:]] - - return self._cache_dir.joinpath(*split_key) diff --git a/src/poetry/installation/executor.py b/src/poetry/installation/executor.py index 15ec0c0c47e..127c7aa69f5 100644 --- a/src/poetry/installation/executor.py +++ b/src/poetry/installation/executor.py @@ -20,6 +20,8 @@ from poetry.installation.chef import Chef from poetry.installation.chef import ChefBuildError from poetry.installation.chooser import Chooser +from poetry.installation.chooser import InvalidWheelName +from poetry.installation.chooser import Wheel from poetry.installation.operations import Install from poetry.installation.operations import Uninstall from poetry.installation.operations import Update @@ -27,6 +29,8 @@ from poetry.puzzle.exceptions import SolverProblemError from poetry.utils._compat import decode from poetry.utils.authenticator import Authenticator +from poetry.utils.cache import get_cache_directory_for_link +from poetry.utils.cache import get_cached_archives_for_link from poetry.utils.env import EnvCommandError from poetry.utils.helpers import atomic_open from poetry.utils.helpers import get_file_hash @@ -82,6 +86,7 @@ def __init__( ) self._chef = Chef(config, self._env, pool) self._chooser = Chooser(pool, self._env, config) + self._artifacts_cache_dir = config.artifacts_cache_directory self._executor = ThreadPoolExecutor(max_workers=self._max_workers) self._total_operations = 0 @@ -709,15 +714,19 @@ def _download(self, operation: Install | Update) -> Path: def _download_link(self, operation: Install | Update, link: Link) -> Path: package = operation.package - output_dir = self._chef.get_cache_directory_for_link(link) + output_dir = get_cache_directory_for_link(self._artifacts_cache_dir, link) # Try to get cached original package for the link provided - original_archive = self._chef.get_cached_archive_for_link(link, strict=True) + original_archive = self._get_cached_archive_for_link( + self._env, self._artifacts_cache_dir, link, strict=True + ) if original_archive is None: # No cached original distributions was found, so we download and prepare it try: original_archive = self._download_archive(operation, link) except BaseException: - cache_directory = self._chef.get_cache_directory_for_link(link) + cache_directory = get_cache_directory_for_link( + self._artifacts_cache_dir, link + ) cached_file = cache_directory.joinpath(link.filename) # We can't use unlink(missing_ok=True) because it's not available # prior to Python 3.8 @@ -728,7 +737,9 @@ def _download_link(self, operation: Install | Update, link: Link) -> Path: # 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 = self._get_cached_archive_for_link( + self._env, self._artifacts_cache_dir, 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 @@ -792,7 +803,10 @@ def _download_archive(self, operation: Install | Update, link: Link) -> Path: progress.start() done = 0 - archive = self._chef.get_cache_directory_for_link(link) / link.filename + archive = ( + get_cache_directory_for_link(self._artifacts_cache_dir, link) + / link.filename + ) archive.parent.mkdir(parents=True, exist_ok=True) with atomic_open(archive) as f: for chunk in response.iter_content(chunk_size=4096): @@ -926,3 +940,40 @@ def _get_archive_info(self, package: Package) -> dict[str, Any]: archive_info["hashes"] = {algorithm: value} return archive_info + + @staticmethod + def _get_cached_archive_for_link( + env: Env, cache_dir: Path, link: Link, *, strict: bool + ) -> Path | None: + archives = get_cached_archives_for_link(cache_dir, 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 + + try: + wheel = Wheel(archive.name) + except InvalidWheelName: + continue + + if not wheel.is_supported_by_environment(env): + continue + + candidates.append( + (wheel.get_minimum_supported_index(env.supported_tags), archive), + ) + + if not candidates: + return None + + return min(candidates)[1] diff --git a/src/poetry/utils/cache.py b/src/poetry/utils/cache.py index ba88a077055..e4530108c02 100644 --- a/src/poetry/utils/cache.py +++ b/src/poetry/utils/cache.py @@ -8,12 +8,17 @@ import time from pathlib import Path +from typing import TYPE_CHECKING from typing import Any from typing import Callable from typing import Generic from typing import TypeVar +if TYPE_CHECKING: + from poetry.core.packages.utils.link import Link + + # Used by Cachy for items that do not expire. MAX_DATE = 9999999999 T = TypeVar("T") @@ -196,3 +201,35 @@ def _deserialize(self, data_raw: bytes) -> CacheItem[T]: data = json.loads(data_str[10:]) expires = int(data_str[:10]) return CacheItem(data, expires) + + +def get_cached_archives_for_link(cache_dir: Path, link: Link) -> list[Path]: + cache_dir = get_cache_directory_for_link(cache_dir, link) + + archive_types = ["whl", "tar.gz", "tar.bz2", "bz2", "zip"] + paths = [] + for archive_type in archive_types: + for archive in cache_dir.glob(f"*.{archive_type}"): + paths.append(Path(archive)) + + return paths + + +def get_cache_directory_for_link(cache_dir: Path, link: Link) -> Path: + key_parts = {"url": link.url_without_fragment} + + if link.hash_name is not None and link.hash is not None: + key_parts[link.hash_name] = link.hash + + if link.subdirectory_fragment: + key_parts["subdirectory"] = link.subdirectory_fragment + + key = hashlib.sha256( + json.dumps( + key_parts, sort_keys=True, separators=(",", ":"), ensure_ascii=True + ).encode("ascii") + ).hexdigest() + + split_key = [key[:2], key[2:4], key[4:6], key[6:]] + + return cache_dir.joinpath(*split_key) diff --git a/tests/installation/test_chef.py b/tests/installation/test_chef.py index 19283cdeaaa..2de504c10b9 100644 --- a/tests/installation/test_chef.py +++ b/tests/installation/test_chef.py @@ -9,14 +9,13 @@ import pytest -from packaging.tags import Tag from poetry.core.packages.utils.link import Link from poetry.factory import Factory from poetry.installation.chef import Chef from poetry.repositories import RepositoryPool +from poetry.utils.cache import get_cache_directory_for_link from poetry.utils.env import EnvManager -from poetry.utils.env import MockEnv from tests.repositories.test_pypi_repository import MockRepository @@ -40,156 +39,6 @@ def setup(mocker: MockerFixture, pool: RepositoryPool) -> None: mocker.patch.object(Factory, "create_pool", return_value=pool) -@pytest.mark.parametrize( - ("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_found_cached_archive_for_link( - config: Config, mocker: MockerFixture, link: str, cached: str, strict: bool -): - 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=[ - Path("/cache/demo-0.1.0-py2.py3-none-any"), - Path("/cache/demo-0.1.0.tar.gz"), - 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"), - ], - ) - - archive = chef.get_cached_archive_for_link(Link(link), strict=strict) - - assert Path(cached) == archive - - -def test_get_cached_archives_for_link(config: Config, mocker: MockerFixture): - chef = Chef( - config, - MockEnv( - marker_env={"interpreter_name": "cpython", "interpreter_version": "3.8.3"} - ), - Factory.create_pool(config), - ) - - distributions = Path(__file__).parent.parent.joinpath("fixtures/distributions") - mocker.patch.object( - chef, - "get_cache_directory_for_link", - return_value=distributions, - ) - - archives = chef.get_cached_archives_for_link( - Link("https://files.python-poetry.org/demo-0.1.0.tar.gz") - ) - - assert archives - assert set(archives) == set(distributions.glob("demo-0.1.*")) - - -def test_get_cache_directory_for_link(config: Config, config_cache_dir: Path): - chef = Chef( - config, - MockEnv( - marker_env={"interpreter_name": "cpython", "interpreter_version": "3.8.3"} - ), - Factory.create_pool(config), - ) - - directory = chef.get_cache_directory_for_link( - Link("https://files.python-poetry.org/poetry-1.1.0.tar.gz") - ) - - expected = Path( - f"{config_cache_dir.as_posix()}/artifacts/ba/63/13/" - "283a3b3b7f95f05e9e6f84182d276f7bb0951d5b0cc24422b33f7a4648" - ) - - assert directory == expected - - def test_prepare_sdist(config: Config, config_cache_dir: Path) -> None: chef = Chef(config, EnvManager.get_system_env(), Factory.create_pool(config)) @@ -199,7 +48,7 @@ def test_prepare_sdist(config: Config, config_cache_dir: Path) -> None: .resolve() ) - destination = chef.get_cache_directory_for_link(Link(archive.as_uri())) + destination = get_cache_directory_for_link(chef._cache_dir, Link(archive.as_uri())) wheel = chef.prepare(archive) diff --git a/tests/installation/test_executor.py b/tests/installation/test_executor.py index c25c331d1ef..e809192fcd9 100644 --- a/tests/installation/test_executor.py +++ b/tests/installation/test_executor.py @@ -20,6 +20,7 @@ from cleo.formatters.style import Style from cleo.io.buffered_io import BufferedIO from cleo.io.outputs.output import Verbosity +from packaging.tags import Tag from poetry.core.packages.package import Package from poetry.core.packages.utils.link import Link from poetry.core.packages.utils.utils import path_to_url @@ -44,6 +45,7 @@ from poetry.config.config import Config from poetry.installation.operations.operation import Operation + from poetry.utils.env import Env from poetry.utils.env import VirtualEnv from tests.types import FixtureDirGetter @@ -538,11 +540,11 @@ def test_executor_should_delete_incomplete_downloads( side_effect=Exception("Download error"), ) mocker.patch( - "poetry.installation.chef.Chef.get_cached_archive_for_link", - side_effect=lambda link, strict: None, + "poetry.installation.executor.Executor._get_cached_archive_for_link", + return_value=None, ) mocker.patch( - "poetry.installation.chef.Chef.get_cache_directory_for_link", + "poetry.installation.executor.get_cache_directory_for_link", return_value=Path(tmp_dir), ) @@ -761,7 +763,7 @@ def test_executor_should_write_pep610_url_references_for_wheel_urls( if is_artifact_cached: link_cached = fixture_dir("distributions") / "demo-0.1.0-py2.py3-none-any.whl" mocker.patch( - "poetry.installation.chef.Chef.get_cached_archive_for_link", + "poetry.installation.executor.Executor._get_cached_archive_for_link", return_value=link_cached, ) download_spy = mocker.spy(Executor, "_download_archive") @@ -840,7 +842,9 @@ def test_executor_should_write_pep610_url_references_for_non_wheel_urls( cached_sdist = fixture_dir("distributions") / "demo-0.1.0.tar.gz" cached_wheel = fixture_dir("distributions") / "demo-0.1.0-py2.py3-none-any.whl" - def mock_get_cached_archive_for_link_func(_: Link, strict: bool): + def mock_get_cached_archive_for_link_func( + _: Env, __: Path, ___: Link, strict: bool + ): if is_wheel_cached and not strict: return cached_wheel if is_sdist_cached: @@ -848,7 +852,7 @@ def mock_get_cached_archive_for_link_func(_: Link, strict: bool): return None mocker.patch( - "poetry.installation.chef.Chef.get_cached_archive_for_link", + "poetry.installation.executor.Executor._get_cached_archive_for_link", side_effect=mock_get_cached_archive_for_link_func, ) @@ -1218,3 +1222,106 @@ def test_build_system_requires_not_available( output = io.fetch_output().strip() assert output.startswith(expected_start) assert output.endswith(expected_end) + + +@pytest.mark.parametrize( + ("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( + mocker: MockerFixture, + link: str, + strict: bool, + available_packages: list[Path], +) -> None: + env = 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"), + ], + ) + + mocker.patch( + "poetry.installation.executor.get_cached_archives_for_link", + return_value=available_packages, + ) + + archive = Executor._get_cached_archive_for_link( + env, Path(), 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_found_cached_archive_for_link( + mocker: MockerFixture, + link: str, + cached: str, + strict: bool, +) -> None: + env = 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"), + ], + ) + + mocker.patch( + "poetry.installation.executor.get_cached_archives_for_link", + return_value=[ + Path("/cache/demo-0.1.0-py2.py3-none-any"), + Path("/cache/demo-0.1.0.tar.gz"), + 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"), + ], + ) + + archive = Executor._get_cached_archive_for_link( + env, Path(), Link(link), strict=strict + ) + + assert Path(cached) == archive diff --git a/tests/utils/test_cache.py b/tests/utils/test_cache.py index c1bbae5071a..8cb6b5df068 100644 --- a/tests/utils/test_cache.py +++ b/tests/utils/test_cache.py @@ -1,5 +1,6 @@ from __future__ import annotations +from pathlib import Path from typing import TYPE_CHECKING from typing import Any from typing import TypeVar @@ -9,13 +10,14 @@ import pytest from cachy import CacheManager +from poetry.core.packages.utils.link import Link from poetry.utils.cache import FileCache +from poetry.utils.cache import get_cache_directory_for_link +from poetry.utils.cache import get_cached_archives_for_link if TYPE_CHECKING: - from pathlib import Path - from _pytest.monkeypatch import MonkeyPatch from pytest import FixtureRequest from pytest_mock import MockerFixture @@ -192,3 +194,30 @@ def test_cachy_compatibility( assert cachy_file_cache.get("key3") == test_str assert cachy_file_cache.get("key4") == test_obj + + +def test_get_cached_archives_for_link(mocker: MockerFixture) -> None: + distributions = Path(__file__).parent.parent.joinpath("fixtures/distributions") + mocker.patch( + "poetry.utils.cache.get_cache_directory_for_link", + return_value=distributions, + ) + archives = get_cached_archives_for_link( + Path(), Link("https://files.python-poetry.org/demo-0.1.0.tar.gz") + ) + + assert archives + assert set(archives) == set(distributions.glob("demo-0.1.*")) + + +def test_get_cache_directory_for_link(tmp_path: Path) -> None: + directory = get_cache_directory_for_link( + tmp_path, Link("https://files.python-poetry.org/poetry-1.1.0.tar.gz") + ) + + expected = Path( + f"{tmp_path.as_posix()}/11/4f/a8/" + "1c89d75547e4967082d30a28360401c82c83b964ddacee292201bf85f2" + ) + + assert directory == expected From fd14f7b0b3355eb40045156bfcbf565fd7b236fc Mon Sep 17 00:00:00 2001 From: ralbertazzi Date: Mon, 13 Mar 2023 16:55:16 +0100 Subject: [PATCH 2/7] refactor: move get_cached_archive_for_link and Wheel to utils --- src/poetry/installation/chooser.py | 35 +-------- src/poetry/installation/executor.py | 45 +----------- src/poetry/utils/cache.py | 42 +++++++++++ src/poetry/utils/wheel.py | 47 ++++++++++++ tests/installation/test_executor.py | 110 +--------------------------- tests/utils/test_cache.py | 102 ++++++++++++++++++++++++++ 6 files changed, 198 insertions(+), 183 deletions(-) create mode 100644 src/poetry/utils/wheel.py diff --git a/src/poetry/installation/chooser.py b/src/poetry/installation/chooser.py index 821c09b38c8..b484504ed9a 100644 --- a/src/poetry/installation/chooser.py +++ b/src/poetry/installation/chooser.py @@ -6,11 +6,9 @@ from typing import TYPE_CHECKING from typing import Any -from packaging.tags import Tag - from poetry.config.config import Config from poetry.config.config import PackageFilterPolicy -from poetry.utils.patterns import wheel_file_re +from poetry.utils.wheel import Wheel if TYPE_CHECKING: @@ -25,37 +23,6 @@ logger = logging.getLogger(__name__) -class InvalidWheelName(Exception): - pass - - -class Wheel: - def __init__(self, filename: str) -> None: - wheel_info = wheel_file_re.match(filename) - if not wheel_info: - raise InvalidWheelName(f"{filename} is not a valid wheel filename.") - - self.filename = filename - self.name = wheel_info.group("name").replace("_", "-") - self.version = wheel_info.group("ver").replace("_", "-") - self.build_tag = wheel_info.group("build") - self.pyversions = wheel_info.group("pyver").split(".") - self.abis = wheel_info.group("abi").split(".") - self.plats = wheel_info.group("plat").split(".") - - self.tags = { - Tag(x, y, z) for x in self.pyversions for y in self.abis for z in self.plats - } - - def get_minimum_supported_index(self, tags: list[Tag]) -> int | None: - indexes = [tags.index(t) for t in self.tags if t in tags] - - return min(indexes) if indexes else None - - def is_supported_by_environment(self, env: Env) -> bool: - return bool(set(env.supported_tags).intersection(self.tags)) - - class Chooser: """ A Chooser chooses an appropriate release archive for packages. diff --git a/src/poetry/installation/executor.py b/src/poetry/installation/executor.py index 127c7aa69f5..c25065ef770 100644 --- a/src/poetry/installation/executor.py +++ b/src/poetry/installation/executor.py @@ -20,8 +20,6 @@ from poetry.installation.chef import Chef from poetry.installation.chef import ChefBuildError from poetry.installation.chooser import Chooser -from poetry.installation.chooser import InvalidWheelName -from poetry.installation.chooser import Wheel from poetry.installation.operations import Install from poetry.installation.operations import Uninstall from poetry.installation.operations import Update @@ -30,7 +28,7 @@ from poetry.utils._compat import decode from poetry.utils.authenticator import Authenticator from poetry.utils.cache import get_cache_directory_for_link -from poetry.utils.cache import get_cached_archives_for_link +from poetry.utils.cache import get_cached_archive_for_link from poetry.utils.env import EnvCommandError from poetry.utils.helpers import atomic_open from poetry.utils.helpers import get_file_hash @@ -716,7 +714,7 @@ def _download_link(self, operation: Install | Update, link: Link) -> Path: output_dir = get_cache_directory_for_link(self._artifacts_cache_dir, link) # Try to get cached original package for the link provided - original_archive = self._get_cached_archive_for_link( + original_archive = get_cached_archive_for_link( self._env, self._artifacts_cache_dir, link, strict=True ) if original_archive is None: @@ -737,7 +735,7 @@ def _download_link(self, operation: Install | Update, link: Link) -> Path: # Get potential higher prioritized cached archive, otherwise it will fall back # to the original archive. - archive = self._get_cached_archive_for_link( + archive = get_cached_archive_for_link( self._env, self._artifacts_cache_dir, link, strict=False ) # 'archive' can at this point never be None. Since we previously downloaded @@ -940,40 +938,3 @@ def _get_archive_info(self, package: Package) -> dict[str, Any]: archive_info["hashes"] = {algorithm: value} return archive_info - - @staticmethod - def _get_cached_archive_for_link( - env: Env, cache_dir: Path, link: Link, *, strict: bool - ) -> Path | None: - archives = get_cached_archives_for_link(cache_dir, 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 - - try: - wheel = Wheel(archive.name) - except InvalidWheelName: - continue - - if not wheel.is_supported_by_environment(env): - continue - - candidates.append( - (wheel.get_minimum_supported_index(env.supported_tags), archive), - ) - - if not candidates: - return None - - return min(candidates)[1] diff --git a/src/poetry/utils/cache.py b/src/poetry/utils/cache.py index e4530108c02..c6ae6919435 100644 --- a/src/poetry/utils/cache.py +++ b/src/poetry/utils/cache.py @@ -14,10 +14,15 @@ from typing import Generic from typing import TypeVar +from poetry.utils.wheel import InvalidWheelName +from poetry.utils.wheel import Wheel + if TYPE_CHECKING: from poetry.core.packages.utils.link import Link + from poetry.utils.env import Env + # Used by Cachy for items that do not expire. MAX_DATE = 9999999999 @@ -215,6 +220,43 @@ def get_cached_archives_for_link(cache_dir: Path, link: Link) -> list[Path]: return paths +def get_cached_archive_for_link( + env: Env, cache_dir: Path, link: Link, *, strict: bool +) -> Path | None: + archives = get_cached_archives_for_link(cache_dir, 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 + + try: + wheel = Wheel(archive.name) + except InvalidWheelName: + continue + + if not wheel.is_supported_by_environment(env): + continue + + candidates.append( + (wheel.get_minimum_supported_index(env.supported_tags), archive), + ) + + if not candidates: + return None + + return min(candidates)[1] + + def get_cache_directory_for_link(cache_dir: Path, link: Link) -> Path: key_parts = {"url": link.url_without_fragment} diff --git a/src/poetry/utils/wheel.py b/src/poetry/utils/wheel.py new file mode 100644 index 00000000000..f45c50b3b35 --- /dev/null +++ b/src/poetry/utils/wheel.py @@ -0,0 +1,47 @@ +from __future__ import annotations + +import logging + +from typing import TYPE_CHECKING + +from packaging.tags import Tag + +from poetry.utils.patterns import wheel_file_re + + +if TYPE_CHECKING: + from poetry.utils.env import Env + + +logger = logging.getLogger(__name__) + + +class InvalidWheelName(Exception): + pass + + +class Wheel: + def __init__(self, filename: str) -> None: + wheel_info = wheel_file_re.match(filename) + if not wheel_info: + raise InvalidWheelName(f"{filename} is not a valid wheel filename.") + + self.filename = filename + self.name = wheel_info.group("name").replace("_", "-") + self.version = wheel_info.group("ver").replace("_", "-") + self.build_tag = wheel_info.group("build") + self.pyversions = wheel_info.group("pyver").split(".") + self.abis = wheel_info.group("abi").split(".") + self.plats = wheel_info.group("plat").split(".") + + self.tags = { + Tag(x, y, z) for x in self.pyversions for y in self.abis for z in self.plats + } + + def get_minimum_supported_index(self, tags: list[Tag]) -> int | None: + indexes = [tags.index(t) for t in self.tags if t in tags] + + return min(indexes) if indexes else None + + def is_supported_by_environment(self, env: Env) -> bool: + return bool(set(env.supported_tags).intersection(self.tags)) diff --git a/tests/installation/test_executor.py b/tests/installation/test_executor.py index e809192fcd9..937507b33b4 100644 --- a/tests/installation/test_executor.py +++ b/tests/installation/test_executor.py @@ -20,7 +20,6 @@ from cleo.formatters.style import Style from cleo.io.buffered_io import BufferedIO from cleo.io.outputs.output import Verbosity -from packaging.tags import Tag from poetry.core.packages.package import Package from poetry.core.packages.utils.link import Link from poetry.core.packages.utils.utils import path_to_url @@ -540,7 +539,7 @@ def test_executor_should_delete_incomplete_downloads( side_effect=Exception("Download error"), ) mocker.patch( - "poetry.installation.executor.Executor._get_cached_archive_for_link", + "poetry.installation.executor.get_cached_archive_for_link", return_value=None, ) mocker.patch( @@ -763,7 +762,7 @@ def test_executor_should_write_pep610_url_references_for_wheel_urls( if is_artifact_cached: link_cached = fixture_dir("distributions") / "demo-0.1.0-py2.py3-none-any.whl" mocker.patch( - "poetry.installation.executor.Executor._get_cached_archive_for_link", + "poetry.installation.executor.get_cached_archive_for_link", return_value=link_cached, ) download_spy = mocker.spy(Executor, "_download_archive") @@ -852,7 +851,7 @@ def mock_get_cached_archive_for_link_func( return None mocker.patch( - "poetry.installation.executor.Executor._get_cached_archive_for_link", + "poetry.installation.executor.get_cached_archive_for_link", side_effect=mock_get_cached_archive_for_link_func, ) @@ -1222,106 +1221,3 @@ def test_build_system_requires_not_available( output = io.fetch_output().strip() assert output.startswith(expected_start) assert output.endswith(expected_end) - - -@pytest.mark.parametrize( - ("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( - mocker: MockerFixture, - link: str, - strict: bool, - available_packages: list[Path], -) -> None: - env = 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"), - ], - ) - - mocker.patch( - "poetry.installation.executor.get_cached_archives_for_link", - return_value=available_packages, - ) - - archive = Executor._get_cached_archive_for_link( - env, Path(), 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_found_cached_archive_for_link( - mocker: MockerFixture, - link: str, - cached: str, - strict: bool, -) -> None: - env = 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"), - ], - ) - - mocker.patch( - "poetry.installation.executor.get_cached_archives_for_link", - return_value=[ - Path("/cache/demo-0.1.0-py2.py3-none-any"), - Path("/cache/demo-0.1.0.tar.gz"), - 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"), - ], - ) - - archive = Executor._get_cached_archive_for_link( - env, Path(), Link(link), strict=strict - ) - - assert Path(cached) == archive diff --git a/tests/utils/test_cache.py b/tests/utils/test_cache.py index 8cb6b5df068..7b276f58415 100644 --- a/tests/utils/test_cache.py +++ b/tests/utils/test_cache.py @@ -10,11 +10,14 @@ import pytest from cachy import CacheManager +from packaging.tags import Tag from poetry.core.packages.utils.link import Link from poetry.utils.cache import FileCache from poetry.utils.cache import get_cache_directory_for_link +from poetry.utils.cache import get_cached_archive_for_link from poetry.utils.cache import get_cached_archives_for_link +from poetry.utils.env import MockEnv if TYPE_CHECKING: @@ -210,6 +213,105 @@ def test_get_cached_archives_for_link(mocker: MockerFixture) -> None: assert set(archives) == set(distributions.glob("demo-0.1.*")) +@pytest.mark.parametrize( + ("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( + mocker: MockerFixture, + link: str, + strict: bool, + available_packages: list[Path], +) -> None: + env = 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"), + ], + ) + + mocker.patch( + "poetry.utils.cache.get_cached_archives_for_link", + return_value=available_packages, + ) + + archive = get_cached_archive_for_link(env, Path(), 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_found_cached_archive_for_link( + mocker: MockerFixture, + link: str, + cached: str, + strict: bool, +) -> None: + env = 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"), + ], + ) + + mocker.patch( + "poetry.utils.cache.get_cached_archives_for_link", + return_value=[ + Path("/cache/demo-0.1.0-py2.py3-none-any"), + Path("/cache/demo-0.1.0.tar.gz"), + 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"), + ], + ) + + archive = get_cached_archive_for_link(env, Path(), Link(link), strict=strict) + + assert Path(cached) == archive + + def test_get_cache_directory_for_link(tmp_path: Path) -> None: directory = get_cache_directory_for_link( tmp_path, Link("https://files.python-poetry.org/poetry-1.1.0.tar.gz") From a05021a2361eb031186657fb1565b8fc72b4ff19 Mon Sep 17 00:00:00 2001 From: ralbertazzi Date: Mon, 13 Mar 2023 17:00:58 +0100 Subject: [PATCH 3/7] refactor: add expanduser --- src/poetry/config/config.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/poetry/config/config.py b/src/poetry/config/config.py index 1ea84b9c34c..49b4631fbf4 100644 --- a/src/poetry/config/config.py +++ b/src/poetry/config/config.py @@ -210,11 +210,11 @@ def _get_environment_repositories() -> dict[str, dict[str, str]]: @property def repository_cache_directory(self) -> Path: - return Path(self.get("cache-dir")) / "cache" / "repositories" + return Path(self.get("cache-dir")).expanduser() / "cache" / "repositories" @property def artifacts_cache_directory(self) -> Path: - return Path(self.get("cache-dir")) / "artifacts" + return Path(self.get("cache-dir")).expanduser() / "artifacts" @property def virtualenvs_path(self) -> Path: From dd64d54552b520d904e49e64db2d38123102fa6d Mon Sep 17 00:00:00 2001 From: ralbertazzi Date: Mon, 13 Mar 2023 18:02:26 +0100 Subject: [PATCH 4/7] refactor: create ArtifactCache class --- src/poetry/installation/chef.py | 13 +-- src/poetry/installation/executor.py | 26 +++--- src/poetry/utils/cache.py | 127 +++++++++++++++------------- tests/installation/test_chef.py | 37 +++++--- tests/installation/test_executor.py | 13 ++- tests/utils/test_cache.py | 36 ++++---- 6 files changed, 142 insertions(+), 110 deletions(-) diff --git a/src/poetry/installation/chef.py b/src/poetry/installation/chef.py index b58d6de460b..5f57ba3e27b 100644 --- a/src/poetry/installation/chef.py +++ b/src/poetry/installation/chef.py @@ -17,15 +17,14 @@ from poetry.core.utils.helpers import temporary_directory from pyproject_hooks import quiet_subprocess_runner # type: ignore[import] -from poetry.utils.cache import get_cache_directory_for_link from poetry.utils.env import ephemeral_environment if TYPE_CHECKING: from contextlib import AbstractContextManager - from poetry.config.config import Config from poetry.repositories import RepositoryPool + from poetry.utils.cache import ArtifactCache from poetry.utils.env import Env @@ -81,10 +80,12 @@ def install(self, requirements: Collection[str]) -> None: class Chef: - def __init__(self, config: Config, env: Env, pool: RepositoryPool) -> None: + def __init__( + self, artifact_cache: ArtifactCache, env: Env, pool: RepositoryPool + ) -> None: self._env = env self._pool = pool - self._cache_dir = config.artifacts_cache_directory + self._artifact_cache = artifact_cache def prepare( self, archive: Path, output_dir: Path | None = None, *, editable: bool = False @@ -174,8 +175,8 @@ def _prepare_sdist(self, archive: Path, destination: Path | None = None) -> Path sdist_dir = archive_dir if destination is None: - destination = get_cache_directory_for_link( - self._cache_dir, Link(archive.as_uri()) + destination = self._artifact_cache.get_cache_directory_for_link( + Link(archive.as_uri()) ) destination.mkdir(parents=True, exist_ok=True) diff --git a/src/poetry/installation/executor.py b/src/poetry/installation/executor.py index c25065ef770..ebbba0a4d6c 100644 --- a/src/poetry/installation/executor.py +++ b/src/poetry/installation/executor.py @@ -27,8 +27,7 @@ from poetry.puzzle.exceptions import SolverProblemError from poetry.utils._compat import decode from poetry.utils.authenticator import Authenticator -from poetry.utils.cache import get_cache_directory_for_link -from poetry.utils.cache import get_cached_archive_for_link +from poetry.utils.cache import ArtifactCache from poetry.utils.env import EnvCommandError from poetry.utils.helpers import atomic_open from poetry.utils.helpers import get_file_hash @@ -79,12 +78,12 @@ def __init__( else: self._max_workers = 1 + self._artifact_cache = ArtifactCache(cache_dir=config.artifacts_cache_directory) self._authenticator = Authenticator( config, self._io, disable_cache=disable_cache, pool_size=self._max_workers ) - self._chef = Chef(config, self._env, pool) + self._chef = Chef(self._artifact_cache, self._env, pool) self._chooser = Chooser(pool, self._env, config) - self._artifacts_cache_dir = config.artifacts_cache_directory self._executor = ThreadPoolExecutor(max_workers=self._max_workers) self._total_operations = 0 @@ -712,18 +711,18 @@ def _download(self, operation: Install | Update) -> Path: def _download_link(self, operation: Install | Update, link: Link) -> Path: package = operation.package - output_dir = get_cache_directory_for_link(self._artifacts_cache_dir, link) + output_dir = self._artifact_cache.get_cache_directory_for_link(link) # Try to get cached original package for the link provided - original_archive = get_cached_archive_for_link( - self._env, self._artifacts_cache_dir, link, strict=True + original_archive = self._artifact_cache.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: original_archive = self._download_archive(operation, link) except BaseException: - cache_directory = get_cache_directory_for_link( - self._artifacts_cache_dir, link + cache_directory = self._artifact_cache.get_cache_directory_for_link( + link ) cached_file = cache_directory.joinpath(link.filename) # We can't use unlink(missing_ok=True) because it's not available @@ -735,8 +734,10 @@ def _download_link(self, operation: Install | Update, link: Link) -> Path: # Get potential higher prioritized cached archive, otherwise it will fall back # to the original archive. - archive = get_cached_archive_for_link( - self._env, self._artifacts_cache_dir, link, strict=False + archive = self._artifact_cache.get_cached_archive_for_link( + link, + strict=False, + env=self._env, ) # '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 @@ -802,8 +803,7 @@ def _download_archive(self, operation: Install | Update, link: Link) -> Path: done = 0 archive = ( - get_cache_directory_for_link(self._artifacts_cache_dir, link) - / link.filename + self._artifact_cache.get_cache_directory_for_link(link) / link.filename ) archive.parent.mkdir(parents=True, exist_ok=True) with atomic_open(archive) as f: diff --git a/src/poetry/utils/cache.py b/src/poetry/utils/cache.py index c6ae6919435..65554e122e2 100644 --- a/src/poetry/utils/cache.py +++ b/src/poetry/utils/cache.py @@ -208,70 +208,81 @@ def _deserialize(self, data_raw: bytes) -> CacheItem[T]: return CacheItem(data, expires) -def get_cached_archives_for_link(cache_dir: Path, link: Link) -> list[Path]: - cache_dir = get_cache_directory_for_link(cache_dir, link) - - archive_types = ["whl", "tar.gz", "tar.bz2", "bz2", "zip"] - paths = [] - for archive_type in archive_types: - for archive in cache_dir.glob(f"*.{archive_type}"): - paths.append(Path(archive)) - - return paths - - -def get_cached_archive_for_link( - env: Env, cache_dir: Path, link: Link, *, strict: bool -) -> Path | None: - archives = get_cached_archives_for_link(cache_dir, 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 - - try: - wheel = Wheel(archive.name) - except InvalidWheelName: - continue - - if not wheel.is_supported_by_environment(env): - continue - - candidates.append( - (wheel.get_minimum_supported_index(env.supported_tags), archive), - ) +class ArtifactCache: + def __init__(self, *, cache_dir: Path) -> None: + self._cache_dir = cache_dir + + def _get_cached_archives_for_link(self, link: Link) -> list[Path]: + cache_dir = self.get_cache_directory_for_link(link) + + archive_types = ["whl", "tar.gz", "tar.bz2", "bz2", "zip"] + paths = [] + for archive_type in archive_types: + for archive in cache_dir.glob(f"*.{archive_type}"): + paths.append(Path(archive)) + + return paths + + def get_cached_archive_for_link( + self, + link: Link, + *, + strict: bool, + env: Env | None = None, + ) -> Path | None: + assert strict or env is not 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 + + assert env is not None + + if archive.suffix != ".whl": + candidates.append((float("inf"), archive)) + continue - if not candidates: - return None + try: + wheel = Wheel(archive.name) + except InvalidWheelName: + continue - return min(candidates)[1] + if not wheel.is_supported_by_environment(env): + continue + + candidates.append( + (wheel.get_minimum_supported_index(env.supported_tags), archive), + ) + + if not candidates: + return None + return min(candidates)[1] -def get_cache_directory_for_link(cache_dir: Path, link: Link) -> Path: - key_parts = {"url": link.url_without_fragment} + def get_cache_directory_for_link(self, link: Link) -> Path: + key_parts = {"url": link.url_without_fragment} - if link.hash_name is not None and link.hash is not None: - key_parts[link.hash_name] = link.hash + if link.hash_name is not None and link.hash is not None: + key_parts[link.hash_name] = link.hash - if link.subdirectory_fragment: - key_parts["subdirectory"] = link.subdirectory_fragment + if link.subdirectory_fragment: + key_parts["subdirectory"] = link.subdirectory_fragment - key = hashlib.sha256( - json.dumps( - key_parts, sort_keys=True, separators=(",", ":"), ensure_ascii=True - ).encode("ascii") - ).hexdigest() + key = hashlib.sha256( + json.dumps( + key_parts, sort_keys=True, separators=(",", ":"), ensure_ascii=True + ).encode("ascii") + ).hexdigest() - split_key = [key[:2], key[2:4], key[4:6], key[6:]] + split_key = [key[:2], key[2:4], key[4:6], key[6:]] - return cache_dir.joinpath(*split_key) + return self._cache_dir.joinpath(*split_key) diff --git a/tests/installation/test_chef.py b/tests/installation/test_chef.py index 2de504c10b9..2c660905d92 100644 --- a/tests/installation/test_chef.py +++ b/tests/installation/test_chef.py @@ -14,7 +14,7 @@ from poetry.factory import Factory from poetry.installation.chef import Chef from poetry.repositories import RepositoryPool -from poetry.utils.cache import get_cache_directory_for_link +from poetry.utils.cache import ArtifactCache from poetry.utils.env import EnvManager from tests.repositories.test_pypi_repository import MockRepository @@ -39,8 +39,17 @@ def setup(mocker: MockerFixture, pool: RepositoryPool) -> None: mocker.patch.object(Factory, "create_pool", return_value=pool) -def test_prepare_sdist(config: Config, config_cache_dir: Path) -> None: - chef = Chef(config, EnvManager.get_system_env(), Factory.create_pool(config)) +@pytest.fixture() +def artifact_cache(config: Config) -> ArtifactCache: + return ArtifactCache(cache_dir=config.artifacts_cache_directory) + + +def test_prepare_sdist( + config: Config, config_cache_dir: Path, artifact_cache: ArtifactCache +) -> None: + chef = Chef( + artifact_cache, EnvManager.get_system_env(), Factory.create_pool(config) + ) archive = ( Path(__file__) @@ -48,7 +57,7 @@ def test_prepare_sdist(config: Config, config_cache_dir: Path) -> None: .resolve() ) - destination = get_cache_directory_for_link(chef._cache_dir, Link(archive.as_uri())) + destination = artifact_cache.get_cache_directory_for_link(Link(archive.as_uri())) wheel = chef.prepare(archive) @@ -56,8 +65,12 @@ def test_prepare_sdist(config: Config, config_cache_dir: Path) -> None: assert wheel.name == "demo-0.1.0-py3-none-any.whl" -def test_prepare_directory(config: Config, config_cache_dir: Path): - chef = Chef(config, EnvManager.get_system_env(), Factory.create_pool(config)) +def test_prepare_directory( + config: Config, config_cache_dir: Path, artifact_cache: ArtifactCache +): + chef = Chef( + artifact_cache, EnvManager.get_system_env(), Factory.create_pool(config) + ) archive = Path(__file__).parent.parent.joinpath("fixtures/simple_project").resolve() @@ -71,10 +84,10 @@ def test_prepare_directory(config: Config, config_cache_dir: Path): def test_prepare_directory_with_extensions( - config: Config, config_cache_dir: Path + config: Config, config_cache_dir: Path, artifact_cache: ArtifactCache ) -> None: env = EnvManager.get_system_env() - chef = Chef(config, env, Factory.create_pool(config)) + chef = Chef(artifact_cache, env, Factory.create_pool(config)) archive = ( Path(__file__) @@ -91,8 +104,12 @@ def test_prepare_directory_with_extensions( 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)) +def test_prepare_directory_editable( + config: Config, config_cache_dir: Path, artifact_cache: ArtifactCache +): + chef = Chef( + artifact_cache, EnvManager.get_system_env(), Factory.create_pool(config) + ) archive = Path(__file__).parent.parent.joinpath("fixtures/simple_project").resolve() diff --git a/tests/installation/test_executor.py b/tests/installation/test_executor.py index 937507b33b4..20e12578aab 100644 --- a/tests/installation/test_executor.py +++ b/tests/installation/test_executor.py @@ -44,7 +44,6 @@ from poetry.config.config import Config from poetry.installation.operations.operation import Operation - from poetry.utils.env import Env from poetry.utils.env import VirtualEnv from tests.types import FixtureDirGetter @@ -539,11 +538,11 @@ def test_executor_should_delete_incomplete_downloads( side_effect=Exception("Download error"), ) mocker.patch( - "poetry.installation.executor.get_cached_archive_for_link", + "poetry.installation.executor.ArtifactCache.get_cached_archive_for_link", return_value=None, ) mocker.patch( - "poetry.installation.executor.get_cache_directory_for_link", + "poetry.installation.executor.ArtifactCache.get_cache_directory_for_link", return_value=Path(tmp_dir), ) @@ -762,7 +761,7 @@ def test_executor_should_write_pep610_url_references_for_wheel_urls( if is_artifact_cached: link_cached = fixture_dir("distributions") / "demo-0.1.0-py2.py3-none-any.whl" mocker.patch( - "poetry.installation.executor.get_cached_archive_for_link", + "poetry.installation.executor.ArtifactCache.get_cached_archive_for_link", return_value=link_cached, ) download_spy = mocker.spy(Executor, "_download_archive") @@ -841,9 +840,7 @@ def test_executor_should_write_pep610_url_references_for_non_wheel_urls( cached_sdist = fixture_dir("distributions") / "demo-0.1.0.tar.gz" cached_wheel = fixture_dir("distributions") / "demo-0.1.0-py2.py3-none-any.whl" - def mock_get_cached_archive_for_link_func( - _: Env, __: Path, ___: Link, strict: bool - ): + def mock_get_cached_archive_for_link_func(_: Link, *, strict: bool, **__: Any): if is_wheel_cached and not strict: return cached_wheel if is_sdist_cached: @@ -851,7 +848,7 @@ def mock_get_cached_archive_for_link_func( return None mocker.patch( - "poetry.installation.executor.get_cached_archive_for_link", + "poetry.installation.executor.ArtifactCache.get_cached_archive_for_link", side_effect=mock_get_cached_archive_for_link_func, ) diff --git a/tests/utils/test_cache.py b/tests/utils/test_cache.py index 7b276f58415..ab94c66303d 100644 --- a/tests/utils/test_cache.py +++ b/tests/utils/test_cache.py @@ -13,10 +13,8 @@ from packaging.tags import Tag from poetry.core.packages.utils.link import Link +from poetry.utils.cache import ArtifactCache from poetry.utils.cache import FileCache -from poetry.utils.cache import get_cache_directory_for_link -from poetry.utils.cache import get_cached_archive_for_link -from poetry.utils.cache import get_cached_archives_for_link from poetry.utils.env import MockEnv @@ -201,12 +199,15 @@ def test_cachy_compatibility( def test_get_cached_archives_for_link(mocker: MockerFixture) -> None: distributions = Path(__file__).parent.parent.joinpath("fixtures/distributions") - mocker.patch( - "poetry.utils.cache.get_cache_directory_for_link", + cache = ArtifactCache(cache_dir=Path()) + + mocker.patch.object( + cache, + "get_cache_directory_for_link", return_value=distributions, ) - archives = get_cached_archives_for_link( - Path(), Link("https://files.python-poetry.org/demo-0.1.0.tar.gz") + archives = cache._get_cached_archives_for_link( + Link("https://files.python-poetry.org/demo-0.1.0.tar.gz") ) assert archives @@ -246,13 +247,15 @@ def test_get_not_found_cached_archive_for_link( Tag("py3", "none", "any"), ], ) + cache = ArtifactCache(cache_dir=Path()) - mocker.patch( - "poetry.utils.cache.get_cached_archives_for_link", + mocker.patch.object( + cache, + "_get_cached_archives_for_link", return_value=available_packages, ) - archive = get_cached_archive_for_link(env, Path(), Link(link), strict=strict) + archive = cache.get_cached_archive_for_link(Link(link), strict=strict, env=env) assert archive is None @@ -296,9 +299,11 @@ def test_get_found_cached_archive_for_link( Tag("py3", "none", "any"), ], ) + cache = ArtifactCache(cache_dir=Path()) - mocker.patch( - "poetry.utils.cache.get_cached_archives_for_link", + mocker.patch.object( + cache, + "_get_cached_archives_for_link", return_value=[ Path("/cache/demo-0.1.0-py2.py3-none-any"), Path("/cache/demo-0.1.0.tar.gz"), @@ -307,14 +312,15 @@ def test_get_found_cached_archive_for_link( ], ) - archive = get_cached_archive_for_link(env, Path(), Link(link), strict=strict) + archive = cache.get_cached_archive_for_link(Link(link), strict=strict, env=env) assert Path(cached) == archive def test_get_cache_directory_for_link(tmp_path: Path) -> None: - directory = get_cache_directory_for_link( - tmp_path, Link("https://files.python-poetry.org/poetry-1.1.0.tar.gz") + cache = ArtifactCache(cache_dir=tmp_path) + directory = cache.get_cache_directory_for_link( + Link("https://files.python-poetry.org/poetry-1.1.0.tar.gz") ) expected = Path( From 683d6a52eecdc6064b5f9744f8578947187dd0f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Randy=20D=C3=B6ring?= <30527984+radoering@users.noreply.github.com> Date: Sun, 19 Mar 2023 13:32:18 +0100 Subject: [PATCH 5/7] refactor: change order of methods --- src/poetry/utils/cache.py | 48 +++++++++++++++++++-------------------- tests/utils/test_cache.py | 28 +++++++++++------------ 2 files changed, 38 insertions(+), 38 deletions(-) diff --git a/src/poetry/utils/cache.py b/src/poetry/utils/cache.py index 65554e122e2..99bd5b40cee 100644 --- a/src/poetry/utils/cache.py +++ b/src/poetry/utils/cache.py @@ -212,16 +212,24 @@ class ArtifactCache: def __init__(self, *, cache_dir: Path) -> None: self._cache_dir = cache_dir - def _get_cached_archives_for_link(self, link: Link) -> list[Path]: - cache_dir = self.get_cache_directory_for_link(link) + def get_cache_directory_for_link(self, link: Link) -> Path: + key_parts = {"url": link.url_without_fragment} - archive_types = ["whl", "tar.gz", "tar.bz2", "bz2", "zip"] - paths = [] - for archive_type in archive_types: - for archive in cache_dir.glob(f"*.{archive_type}"): - paths.append(Path(archive)) + if link.hash_name is not None and link.hash is not None: + key_parts[link.hash_name] = link.hash - return paths + if link.subdirectory_fragment: + key_parts["subdirectory"] = link.subdirectory_fragment + + key = hashlib.sha256( + json.dumps( + key_parts, sort_keys=True, separators=(",", ":"), ensure_ascii=True + ).encode("ascii") + ).hexdigest() + + split_key = [key[:2], key[2:4], key[4:6], key[6:]] + + return self._cache_dir.joinpath(*split_key) def get_cached_archive_for_link( self, @@ -268,21 +276,13 @@ def get_cached_archive_for_link( return min(candidates)[1] - def get_cache_directory_for_link(self, link: Link) -> Path: - key_parts = {"url": link.url_without_fragment} - - if link.hash_name is not None and link.hash is not None: - key_parts[link.hash_name] = link.hash - - if link.subdirectory_fragment: - key_parts["subdirectory"] = link.subdirectory_fragment - - key = hashlib.sha256( - json.dumps( - key_parts, sort_keys=True, separators=(",", ":"), ensure_ascii=True - ).encode("ascii") - ).hexdigest() + def _get_cached_archives_for_link(self, link: Link) -> list[Path]: + cache_dir = self.get_cache_directory_for_link(link) - split_key = [key[:2], key[2:4], key[4:6], key[6:]] + archive_types = ["whl", "tar.gz", "tar.bz2", "bz2", "zip"] + paths = [] + for archive_type in archive_types: + for archive in cache_dir.glob(f"*.{archive_type}"): + paths.append(Path(archive)) - return self._cache_dir.joinpath(*split_key) + return paths diff --git a/tests/utils/test_cache.py b/tests/utils/test_cache.py index ab94c66303d..57320fb1363 100644 --- a/tests/utils/test_cache.py +++ b/tests/utils/test_cache.py @@ -197,6 +197,20 @@ def test_cachy_compatibility( assert cachy_file_cache.get("key4") == test_obj +def test_get_cache_directory_for_link(tmp_path: Path) -> None: + cache = ArtifactCache(cache_dir=tmp_path) + directory = cache.get_cache_directory_for_link( + Link("https://files.python-poetry.org/poetry-1.1.0.tar.gz") + ) + + expected = Path( + f"{tmp_path.as_posix()}/11/4f/a8/" + "1c89d75547e4967082d30a28360401c82c83b964ddacee292201bf85f2" + ) + + assert directory == expected + + def test_get_cached_archives_for_link(mocker: MockerFixture) -> None: distributions = Path(__file__).parent.parent.joinpath("fixtures/distributions") cache = ArtifactCache(cache_dir=Path()) @@ -315,17 +329,3 @@ def test_get_found_cached_archive_for_link( archive = cache.get_cached_archive_for_link(Link(link), strict=strict, env=env) assert Path(cached) == archive - - -def test_get_cache_directory_for_link(tmp_path: Path) -> None: - cache = ArtifactCache(cache_dir=tmp_path) - directory = cache.get_cache_directory_for_link( - Link("https://files.python-poetry.org/poetry-1.1.0.tar.gz") - ) - - expected = Path( - f"{tmp_path.as_posix()}/11/4f/a8/" - "1c89d75547e4967082d30a28360401c82c83b964ddacee292201bf85f2" - ) - - assert directory == expected From 414d23aba34f796d93892afde802bdd414309e11 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Randy=20D=C3=B6ring?= <30527984+radoering@users.noreply.github.com> Date: Sun, 19 Mar 2023 13:55:11 +0100 Subject: [PATCH 6/7] refactor: use fixture_dir --- tests/installation/test_chef.py | 42 ++++++------ tests/installation/test_executor.py | 99 +++++++++++------------------ tests/utils/test_cache.py | 7 +- 3 files changed, 63 insertions(+), 85 deletions(-) diff --git a/tests/installation/test_chef.py b/tests/installation/test_chef.py index 2c660905d92..6d69109aece 100644 --- a/tests/installation/test_chef.py +++ b/tests/installation/test_chef.py @@ -23,6 +23,7 @@ from pytest_mock import MockerFixture from tests.conftest import Config + from tests.types import FixtureDirGetter @pytest.fixture() @@ -45,18 +46,15 @@ def artifact_cache(config: Config) -> ArtifactCache: def test_prepare_sdist( - config: Config, config_cache_dir: Path, artifact_cache: ArtifactCache + config: Config, + config_cache_dir: Path, + artifact_cache: ArtifactCache, + fixture_dir: FixtureDirGetter, ) -> None: chef = Chef( artifact_cache, EnvManager.get_system_env(), Factory.create_pool(config) ) - - archive = ( - Path(__file__) - .parent.parent.joinpath("fixtures/distributions/demo-0.1.0.tar.gz") - .resolve() - ) - + archive = (fixture_dir("distributions") / "demo-0.1.0.tar.gz").resolve() destination = artifact_cache.get_cache_directory_for_link(Link(archive.as_uri())) wheel = chef.prepare(archive) @@ -66,13 +64,15 @@ def test_prepare_sdist( def test_prepare_directory( - config: Config, config_cache_dir: Path, artifact_cache: ArtifactCache + config: Config, + config_cache_dir: Path, + artifact_cache: ArtifactCache, + fixture_dir: FixtureDirGetter, ): chef = Chef( artifact_cache, EnvManager.get_system_env(), Factory.create_pool(config) ) - - archive = Path(__file__).parent.parent.joinpath("fixtures/simple_project").resolve() + archive = fixture_dir("simple_project").resolve() wheel = chef.prepare(archive) @@ -84,16 +84,14 @@ def test_prepare_directory( def test_prepare_directory_with_extensions( - config: Config, config_cache_dir: Path, artifact_cache: ArtifactCache + config: Config, + config_cache_dir: Path, + artifact_cache: ArtifactCache, + fixture_dir: FixtureDirGetter, ) -> None: env = EnvManager.get_system_env() chef = Chef(artifact_cache, env, Factory.create_pool(config)) - - archive = ( - Path(__file__) - .parent.parent.joinpath("fixtures/extended_with_no_setup") - .resolve() - ) + archive = fixture_dir("extended_with_no_setup").resolve() wheel = chef.prepare(archive) @@ -105,13 +103,15 @@ def test_prepare_directory_with_extensions( def test_prepare_directory_editable( - config: Config, config_cache_dir: Path, artifact_cache: ArtifactCache + config: Config, + config_cache_dir: Path, + artifact_cache: ArtifactCache, + fixture_dir: FixtureDirGetter, ): chef = Chef( artifact_cache, EnvManager.get_system_env(), Factory.create_pool(config) ) - - archive = Path(__file__).parent.parent.joinpath("fixtures/simple_project").resolve() + archive = fixture_dir("simple_project").resolve() wheel = chef.prepare(archive, editable=True) diff --git a/tests/installation/test_executor.py b/tests/installation/test_executor.py index 20e12578aab..959cf4f9b65 100644 --- a/tests/installation/test_executor.py +++ b/tests/installation/test_executor.py @@ -129,7 +129,9 @@ def pool() -> RepositoryPool: @pytest.fixture() -def mock_file_downloads(http: type[httpretty.httpretty]) -> None: +def mock_file_downloads( + http: type[httpretty.httpretty], fixture_dir: FixtureDirGetter +) -> None: def callback( request: HTTPrettyRequest, uri: str, headers: dict[str, Any] ) -> list[int | dict[str, Any] | str]: @@ -141,12 +143,10 @@ def callback( if not fixture.exists(): if name == "demo-0.1.0.tar.gz": - fixture = Path(__file__).parent.parent.joinpath( - "fixtures/distributions/demo-0.1.0.tar.gz" - ) + fixture = fixture_dir("distributions") / "demo-0.1.0.tar.gz" else: - fixture = Path(__file__).parent.parent.joinpath( - "fixtures/distributions/demo-0.1.0-py2.py3-none-any.whl" + fixture = ( + fixture_dir("distributions") / "demo-0.1.0-py2.py3-none-any.whl" ) return [200, headers, fixture.read_bytes()] @@ -159,26 +159,19 @@ def callback( @pytest.fixture() -def copy_wheel(tmp_dir: Path) -> Callable[[], Path]: +def copy_wheel(tmp_dir: Path, fixture_dir: FixtureDirGetter) -> Callable[[], Path]: def _copy_wheel() -> Path: tmp_name = tempfile.mktemp() Path(tmp_dir).joinpath(tmp_name).mkdir() shutil.copyfile( - Path(__file__) - .parent.parent.joinpath( - "fixtures/distributions/demo-0.1.2-py2.py3-none-any.whl" - ) - .as_posix(), - Path(tmp_dir) - .joinpath(tmp_name) - .joinpath("demo-0.1.2-py2.py3-none-any.whl") - .as_posix(), + ( + fixture_dir("distributions") / "demo-0.1.2-py2.py3-none-any.whl" + ).as_posix(), + (Path(tmp_dir) / tmp_name / "demo-0.1.2-py2.py3-none-any.whl").as_posix(), ) - return ( - Path(tmp_dir).joinpath(tmp_name).joinpath("demo-0.1.2-py2.py3-none-any.whl") - ) + return Path(tmp_dir) / tmp_name / "demo-0.1.2-py2.py3-none-any.whl" return _copy_wheel @@ -202,6 +195,7 @@ def test_execute_executes_a_batch_of_operations( mock_file_downloads: None, env: MockEnv, copy_wheel: Callable[[], Path], + fixture_dir: FixtureDirGetter, ): wheel_install = mocker.patch.object(WheelInstaller, "install") @@ -221,10 +215,7 @@ def test_execute_executes_a_batch_of_operations( "demo", "0.1.0", source_type="file", - source_url=Path(__file__) - .parent.parent.joinpath( - "fixtures/distributions/demo-0.1.0-py2.py3-none-any.whl" - ) + source_url=(fixture_dir("distributions") / "demo-0.1.0-py2.py3-none-any.whl") .resolve() .as_posix(), ) @@ -233,10 +224,7 @@ def test_execute_executes_a_batch_of_operations( "simple-project", "1.2.3", source_type="directory", - source_url=Path(__file__) - .parent.parent.joinpath("fixtures/simple_project") - .resolve() - .as_posix(), + source_url=fixture_dir("simple_project").resolve().as_posix(), ) git_package = Package( @@ -527,10 +515,9 @@ def test_executor_should_delete_incomplete_downloads( pool: RepositoryPool, mock_file_downloads: None, env: MockEnv, + fixture_dir: FixtureDirGetter, ): - fixture = Path(__file__).parent.parent.joinpath( - "fixtures/distributions/demo-0.1.0-py2.py3-none-any.whl" - ) + fixture = fixture_dir("distributions") / "demo-0.1.0-py2.py3-none-any.whl" destination_fixture = Path(tmp_dir) / "tomlkit-0.5.3-py2.py3-none-any.whl" shutil.copyfile(str(fixture), str(destination_fixture)) mocker.patch( @@ -624,15 +611,13 @@ def test_executor_should_not_write_pep610_url_references_for_cached_package( def test_executor_should_write_pep610_url_references_for_wheel_files( - tmp_venv: VirtualEnv, pool: RepositoryPool, config: Config, io: BufferedIO + tmp_venv: VirtualEnv, + pool: RepositoryPool, + config: Config, + io: BufferedIO, + fixture_dir: FixtureDirGetter, ): - url = ( - Path(__file__) - .parent.parent.joinpath( - "fixtures/distributions/demo-0.1.0-py2.py3-none-any.whl" - ) - .resolve() - ) + url = (fixture_dir("distributions") / "demo-0.1.0-py2.py3-none-any.whl").resolve() package = Package("demo", "0.1.0", source_type="file", source_url=url.as_posix()) # Set package.files so the executor will attempt to hash the package package.files = [ @@ -658,13 +643,13 @@ def test_executor_should_write_pep610_url_references_for_wheel_files( def test_executor_should_write_pep610_url_references_for_non_wheel_files( - tmp_venv: VirtualEnv, pool: RepositoryPool, config: Config, io: BufferedIO + tmp_venv: VirtualEnv, + pool: RepositoryPool, + config: Config, + io: BufferedIO, + fixture_dir: FixtureDirGetter, ): - url = ( - Path(__file__) - .parent.parent.joinpath("fixtures/distributions/demo-0.1.0.tar.gz") - .resolve() - ) + url = (fixture_dir("distributions") / "demo-0.1.0.tar.gz").resolve() package = Package("demo", "0.1.0", source_type="file", source_url=url.as_posix()) # Set package.files so the executor will attempt to hash the package package.files = [ @@ -695,12 +680,9 @@ def test_executor_should_write_pep610_url_references_for_directories( config: Config, io: BufferedIO, wheel: Path, + fixture_dir: FixtureDirGetter, ): - url = ( - Path(__file__) - .parent.parent.joinpath("fixtures/git/github.com/demo/demo") - .resolve() - ) + url = (fixture_dir("git") / "github.com" / "demo" / "demo").resolve() package = Package( "demo", "0.1.2", source_type="directory", source_url=url.as_posix() ) @@ -722,12 +704,9 @@ def test_executor_should_write_pep610_url_references_for_editable_directories( config: Config, io: BufferedIO, wheel: Path, + fixture_dir: FixtureDirGetter, ): - url = ( - Path(__file__) - .parent.parent.joinpath("fixtures/git/github.com/demo/demo") - .resolve() - ) + url = (fixture_dir("git") / "github.com" / "demo" / "demo").resolve() package = Package( "demo", "0.1.2", @@ -1040,6 +1019,7 @@ def test_executor_fallback_on_poetry_create_error_without_wheel_installer( tmp_dir: str, mock_file_downloads: None, env: MockEnv, + fixture_dir: FixtureDirGetter, ): mock_pip_install = mocker.patch("poetry.installation.executor.pip_install") mock_sdist_builder = mocker.patch("poetry.core.masonry.builders.sdist.SdistBuilder") @@ -1063,10 +1043,7 @@ def test_executor_fallback_on_poetry_create_error_without_wheel_installer( "simple-project", "1.2.3", source_type="directory", - source_url=Path(__file__) - .parent.parent.joinpath("fixtures/simple_project") - .resolve() - .as_posix(), + source_url=fixture_dir("simple_project").resolve().as_posix(), ) return_code = executor.execute( @@ -1105,6 +1082,7 @@ def test_build_backend_errors_are_reported_correctly_if_caused_by_subprocess( tmp_dir: str, mock_file_downloads: None, env: MockEnv, + fixture_dir: FixtureDirGetter, ) -> None: error = BuildBackendException( CalledProcessError(1, ["pip"], output=b"Error on stdout") @@ -1120,10 +1098,7 @@ def test_build_backend_errors_are_reported_correctly_if_caused_by_subprocess( package_name, package_version, source_type="directory", - source_url=Path(__file__) - .parent.parent.joinpath("fixtures/simple_project") - .resolve() - .as_posix(), + source_url=fixture_dir("simple_project").resolve().as_posix(), develop=editable, ) # must not be included in the error message diff --git a/tests/utils/test_cache.py b/tests/utils/test_cache.py index 57320fb1363..8cdbc93a284 100644 --- a/tests/utils/test_cache.py +++ b/tests/utils/test_cache.py @@ -24,6 +24,7 @@ from pytest_mock import MockerFixture from tests.conftest import Config + from tests.types import FixtureDirGetter FILE_CACHE = Union[FileCache, CacheManager] @@ -211,8 +212,10 @@ def test_get_cache_directory_for_link(tmp_path: Path) -> None: assert directory == expected -def test_get_cached_archives_for_link(mocker: MockerFixture) -> None: - distributions = Path(__file__).parent.parent.joinpath("fixtures/distributions") +def test_get_cached_archives_for_link( + fixture_dir: FixtureDirGetter, mocker: MockerFixture +) -> None: + distributions = fixture_dir("distributions") cache = ArtifactCache(cache_dir=Path()) mocker.patch.object( From abff1a289aec8a06236223c6c1261ec417f58a37 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Randy=20D=C3=B6ring?= <30527984+radoering@users.noreply.github.com> Date: Sun, 19 Mar 2023 14:08:26 +0100 Subject: [PATCH 7/7] fix: pass ArtifactCache instead of Config to Chef --- tests/installation/test_chef.py | 2 +- tests/installation/test_executor.py | 38 +++++++++++++++++++---------- 2 files changed, 26 insertions(+), 14 deletions(-) diff --git a/tests/installation/test_chef.py b/tests/installation/test_chef.py index 6d69109aece..c03dfd07c4d 100644 --- a/tests/installation/test_chef.py +++ b/tests/installation/test_chef.py @@ -40,7 +40,7 @@ def setup(mocker: MockerFixture, pool: RepositoryPool) -> None: mocker.patch.object(Factory, "create_pool", return_value=pool) -@pytest.fixture() +@pytest.fixture def artifact_cache(config: Config) -> ArtifactCache: return ArtifactCache(cache_dir=config.artifacts_cache_directory) diff --git a/tests/installation/test_executor.py b/tests/installation/test_executor.py index 959cf4f9b65..474aafab500 100644 --- a/tests/installation/test_executor.py +++ b/tests/installation/test_executor.py @@ -32,6 +32,7 @@ from poetry.installation.operations import Update from poetry.installation.wheel_installer import WheelInstaller from poetry.repositories.repository_pool import RepositoryPool +from poetry.utils.cache import ArtifactCache from poetry.utils.env import MockEnv from tests.repositories.test_pypi_repository import MockRepository @@ -93,7 +94,7 @@ def env(tmp_dir: str) -> MockEnv: return MockEnv(path=path, is_venv=True) -@pytest.fixture() +@pytest.fixture def io() -> BufferedIO: io = BufferedIO() io.output.formatter.set_style("c1_dark", Style("cyan", options=["dark"])) @@ -104,7 +105,7 @@ def io() -> BufferedIO: return io -@pytest.fixture() +@pytest.fixture def io_decorated() -> BufferedIO: io = BufferedIO(decorated=True) io.output.formatter.set_style("c1", Style("cyan")) @@ -113,14 +114,14 @@ def io_decorated() -> BufferedIO: return io -@pytest.fixture() +@pytest.fixture def io_not_decorated() -> BufferedIO: io = BufferedIO(decorated=False) return io -@pytest.fixture() +@pytest.fixture def pool() -> RepositoryPool: pool = RepositoryPool() pool.add_repository(MockRepository()) @@ -128,7 +129,12 @@ def pool() -> RepositoryPool: return pool -@pytest.fixture() +@pytest.fixture +def artifact_cache(config: Config) -> ArtifactCache: + return ArtifactCache(cache_dir=config.artifacts_cache_directory) + + +@pytest.fixture def mock_file_downloads( http: type[httpretty.httpretty], fixture_dir: FixtureDirGetter ) -> None: @@ -158,7 +164,7 @@ def callback( ) -@pytest.fixture() +@pytest.fixture def copy_wheel(tmp_dir: Path, fixture_dir: FixtureDirGetter) -> Callable[[], Path]: def _copy_wheel() -> Path: tmp_name = tempfile.mktemp() @@ -176,7 +182,7 @@ def _copy_wheel() -> Path: return _copy_wheel -@pytest.fixture() +@pytest.fixture def wheel(copy_wheel: Callable[[], Path]) -> Path: archive = copy_wheel() @@ -200,9 +206,10 @@ def test_execute_executes_a_batch_of_operations( wheel_install = mocker.patch.object(WheelInstaller, "install") config.merge({"cache-dir": tmp_dir}) + artifact_cache = ArtifactCache(cache_dir=config.artifacts_cache_directory) prepare_spy = mocker.spy(Chef, "_prepare") - chef = Chef(config, env, Factory.create_pool(config)) + chef = Chef(artifact_cache, env, Factory.create_pool(config)) chef.set_directory_wheel([copy_wheel(), copy_wheel()]) chef.set_sdist_wheel(copy_wheel()) @@ -678,6 +685,7 @@ def test_executor_should_write_pep610_url_references_for_directories( tmp_venv: VirtualEnv, pool: RepositoryPool, config: Config, + artifact_cache: ArtifactCache, io: BufferedIO, wheel: Path, fixture_dir: FixtureDirGetter, @@ -687,7 +695,7 @@ def test_executor_should_write_pep610_url_references_for_directories( "demo", "0.1.2", source_type="directory", source_url=url.as_posix() ) - chef = Chef(config, tmp_venv, Factory.create_pool(config)) + chef = Chef(artifact_cache, tmp_venv, Factory.create_pool(config)) chef.set_directory_wheel(wheel) executor = Executor(tmp_venv, pool, config, io) @@ -702,6 +710,7 @@ def test_executor_should_write_pep610_url_references_for_editable_directories( tmp_venv: VirtualEnv, pool: RepositoryPool, config: Config, + artifact_cache: ArtifactCache, io: BufferedIO, wheel: Path, fixture_dir: FixtureDirGetter, @@ -715,7 +724,7 @@ def test_executor_should_write_pep610_url_references_for_editable_directories( develop=True, ) - chef = Chef(config, tmp_venv, Factory.create_pool(config)) + chef = Chef(artifact_cache, tmp_venv, Factory.create_pool(config)) chef.set_directory_wheel(wheel) executor = Executor(tmp_venv, pool, config, io) @@ -877,6 +886,7 @@ def test_executor_should_write_pep610_url_references_for_git( tmp_venv: VirtualEnv, pool: RepositoryPool, config: Config, + artifact_cache: ArtifactCache, io: BufferedIO, mock_file_downloads: None, wheel: Path, @@ -890,7 +900,7 @@ def test_executor_should_write_pep610_url_references_for_git( source_url="https://github.com/demo/demo.git", ) - chef = Chef(config, tmp_venv, Factory.create_pool(config)) + chef = Chef(artifact_cache, tmp_venv, Factory.create_pool(config)) chef.set_directory_wheel(wheel) executor = Executor(tmp_venv, pool, config, io) @@ -915,6 +925,7 @@ def test_executor_should_append_subdirectory_for_git( tmp_venv: VirtualEnv, pool: RepositoryPool, config: Config, + artifact_cache: ArtifactCache, io: BufferedIO, mock_file_downloads: None, wheel: Path, @@ -929,7 +940,7 @@ def test_executor_should_append_subdirectory_for_git( source_subdirectory="two", ) - chef = Chef(config, tmp_venv, Factory.create_pool(config)) + chef = Chef(artifact_cache, tmp_venv, Factory.create_pool(config)) chef.set_directory_wheel(wheel) spy = mocker.spy(chef, "prepare") @@ -945,6 +956,7 @@ def test_executor_should_write_pep610_url_references_for_git_with_subdirectories tmp_venv: VirtualEnv, pool: RepositoryPool, config: Config, + artifact_cache: ArtifactCache, io: BufferedIO, mock_file_downloads: None, wheel: Path, @@ -959,7 +971,7 @@ def test_executor_should_write_pep610_url_references_for_git_with_subdirectories source_subdirectory="two", ) - chef = Chef(config, tmp_venv, Factory.create_pool(config)) + chef = Chef(artifact_cache, tmp_venv, Factory.create_pool(config)) chef.set_directory_wheel(wheel) executor = Executor(tmp_venv, pool, config, io)