Skip to content

Commit

Permalink
Implement review suggestions to also make add the hash for cached non…
Browse files Browse the repository at this point in the history
… wheel archives
  • Loading branch information
quantum-byte committed Mar 5, 2023
1 parent 6143b4e commit 87181dc
Show file tree
Hide file tree
Showing 4 changed files with 102 additions and 15 deletions.
9 changes: 8 additions & 1 deletion src/poetry/installation/chef.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
16 changes: 8 additions & 8 deletions src/poetry/installation/executor.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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 = (
Expand All @@ -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

Expand Down
68 changes: 64 additions & 4 deletions tests/installation/test_chef.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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

Expand Down
24 changes: 22 additions & 2 deletions tests/installation/test_executor.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down

0 comments on commit 87181dc

Please sign in to comment.