From 87181dcc585e9d493361fcf8796b27d0f910e9ac Mon Sep 17 00:00:00 2001 From: quantum-byte Date: Sun, 5 Mar 2023 23:26:24 +0100 Subject: [PATCH] Implement review suggestions to also make add the hash for cached non wheel archives --- src/poetry/installation/chef.py | 9 +++- src/poetry/installation/executor.py | 16 +++---- tests/installation/test_chef.py | 68 +++++++++++++++++++++++++++-- tests/installation/test_executor.py | 24 +++++++++- 4 files changed, 102 insertions(+), 15 deletions(-) diff --git a/src/poetry/installation/chef.py b/src/poetry/installation/chef.py index 7ba51b1e64a..f866dd3946a 100644 --- a/src/poetry/installation/chef.py +++ b/src/poetry/installation/chef.py @@ -199,13 +199,20 @@ 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 + else: + continue if archive.suffix != ".whl": candidates.append((float("inf"), archive)) continue diff --git a/src/poetry/installation/executor.py b/src/poetry/installation/executor.py index e69a376ce1f..2972ec72a5a 100644 --- a/src/poetry/installation/executor.py +++ b/src/poetry/installation/executor.py @@ -698,11 +698,11 @@ 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: + original_archive = self._chef.get_cached_archive_for_link(link, strict=True) + if original_archive is None: # No cached 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) @@ -713,10 +713,8 @@ def _download_link(self, operation: Install | Update, link: Link) -> Path: raise - # validate the original (downloaded or cached) archive only if we can. e.g. if - # we use the cached wheel or the actual downloaded archive - if package.files and link.filename == archive.name: - self._populate_hashes_dict(archive, package) + # get potential previously built and cached wheel + archive = self._chef.get_cached_archive_for_link(link, strict=False) if archive.suffix != ".whl": message = ( @@ -725,7 +723,9 @@ def _download_link(self, operation: Install | Update, link: Link) -> Path: ) self._write(operation, message) - archive = self._chef.prepare(archive, output_dir=output_dir) + archive = self._chef.prepare(original_archive, output_dir=output_dir) + + self._populate_hashes_dict(original_archive, package) return archive diff --git a/tests/installation/test_chef.py b/tests/installation/test_chef.py index e9046c5433f..bda5102f469 100644 --- a/tests/installation/test_chef.py +++ b/tests/installation/test_chef.py @@ -38,20 +38,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 None is archive + + +@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, @@ -77,7 +137,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 diff --git a/tests/installation/test_executor.py b/tests/installation/test_executor.py index fe0ce46cb44..21f698ffe26 100644 --- a/tests/installation/test_executor.py +++ b/tests/installation/test_executor.py @@ -538,7 +538,7 @@ def test_executor_should_delete_incomplete_downloads( ) mocker.patch( "poetry.installation.chef.Chef.get_cached_archive_for_link", - side_effect=lambda link: None, + side_effect=lambda link, strict: None, ) mocker.patch( "poetry.installation.chef.Chef.get_cache_directory_for_link", @@ -740,13 +740,23 @@ def test_executor_should_write_pep610_url_references_for_editable_directories( ) +@pytest.mark.parametrize("cached", [False, True]) def test_executor_should_write_pep610_url_references_for_wheel_urls( tmp_venv: VirtualEnv, pool: RepositoryPool, config: Config, io: BufferedIO, mock_file_downloads: None, + mocker: MockerFixture, + fixture_dir: FixtureDirGetter, + cached: bool, ): + if 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", + return_value=link_cached, + ) package = Package( "demo", "0.1.0", @@ -776,13 +786,23 @@ def test_executor_should_write_pep610_url_references_for_wheel_urls( verify_installed_distribution(tmp_venv, package, expected_url_reference) -def test_executor_should_not_write_pep610_url_references_for_non_wheel_urls( +@pytest.mark.parametrize("cached", [False, True]) +def test_executor_should_write_pep610_url_references_for_non_wheel_urls( tmp_venv: VirtualEnv, pool: RepositoryPool, config: Config, io: BufferedIO, mock_file_downloads: None, + mocker: MockerFixture, + fixture_dir: FixtureDirGetter, + cached: bool, ): + if cached: + link_cached = fixture_dir("distributions") / "demo-0.1.0.tar.gz" + mocker.patch( + "poetry.installation.chef.Chef.get_cached_archive_for_link", + return_value=link_cached, + ) package = Package( "demo", "0.1.0",