Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Use locally cached wheels during install #5871

Merged
merged 2 commits into from
Jul 3, 2022
Merged

Use locally cached wheels during install #5871

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6da7a67
Use locally cached wheels during install
dimbleby Jun 18, 2022
d34eae4
Have the Chef return Paths instead of Links
dimbleby Jun 19, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 11 additions & 17 deletions src/poetry/installation/chef.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,12 @@
from pathlib import Path
from typing import TYPE_CHECKING

from poetry.core.packages.utils.link import Link

from poetry.installation.chooser import InvalidWheelName
from poetry.installation.chooser import Wheel


if TYPE_CHECKING:
from poetry.core.packages.utils.link import Link

from poetry.config.config import Config
from poetry.utils.env import Env
Expand All @@ -25,24 +24,19 @@ def __init__(self, config: Config, env: Env) -> None:
Path(config.get("cache-dir")).expanduser().joinpath("artifacts")
)

def get_cached_archive_for_link(self, link: Link) -> Link:
# If the archive is already a wheel, there is no need to cache it.
if link.is_wheel:
return link
Comment on lines -29 to -31
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Considering the comment, I can only imagine that might be a shortcut for local wheels? Maybe, you can examine if the change has some side effects on wheels as path dependencies. 🙏

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only place this is called is in the executor _download_link() code path, which means that it is never relevant for path dependencies.

I'm quite unsure about what this is supposed to be doing - Chef isn't a name that helps much. At first I'd thought it was going to be some general purpose artifact cache, but its extremely limited scope is puzzling to me.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to look at this when I have more time. But I suspect the original intent was to cache the environment specific when built. Useful when the link is an sdist.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#5871 (comment)


def get_cached_archive_for_link(self, link: Link) -> Path | None:
archives = self.get_cached_archives_for_link(link)

if not archives:
return link
return None

candidates: list[tuple[float | None, Link]] = []
candidates: list[tuple[float | None, Path]] = []
for archive in archives:
if not archive.is_wheel:
if archive.suffix != ".whl":
candidates.append((float("inf"), archive))
continue

try:
wheel = Wheel(archive.filename)
wheel = Wheel(archive.name)
except InvalidWheelName:
continue

Expand All @@ -54,20 +48,20 @@ def get_cached_archive_for_link(self, link: Link) -> Link:
)

if not candidates:
return link
return None

return min(candidates)[1]

def get_cached_archives_for_link(self, link: Link) -> list[Link]:
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"]
links = []
paths = []
for archive_type in archive_types:
for archive in cache_dir.glob(f"*.{archive_type}"):
links.append(Link(archive.as_uri()))
paths.append(Path(archive))

return links
return paths

def get_cache_directory_for_link(self, link: Link) -> Path:
key_parts = {"url": link.url_without_fragment}
Expand Down
25 changes: 9 additions & 16 deletions src/poetry/installation/executor.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
from cleo.io.null_io import NullIO
from poetry.core.packages.file_dependency import FileDependency
from poetry.core.packages.utils.link import Link
from poetry.core.packages.utils.utils import url_to_path
from poetry.core.pyproject.toml import PyProjectTOML

from poetry.installation.chef import Chef
Expand Down Expand Up @@ -114,7 +113,7 @@ def verbose(self, verbose: bool = True) -> Executor:
return self

def pip_install(
self, req: Path | Link, upgrade: bool = False, editable: bool = False
self, req: Path, upgrade: bool = False, editable: bool = False
) -> int:
try:
pip_install(req, self._env, upgrade=upgrade, editable=editable)
Expand Down Expand Up @@ -463,7 +462,7 @@ def _install(self, operation: Install | Update) -> int:
if package.source_type == "git":
return self._install_git(operation)

archive: Link | Path
archive: Path
if package.source_type == "file":
archive = self._prepare_file(operation)
elif package.source_type == "url":
Expand Down Expand Up @@ -606,17 +605,17 @@ def _install_git(self, operation: Install | Update) -> int:

return status_code

def _download(self, operation: Install | Update) -> Link | Path:
def _download(self, operation: Install | Update) -> Path:
link = self._chooser.choose_for(operation.package)

return self._download_link(operation, link)

def _download_link(self, operation: Install | Update, link: Link) -> Link | Path:
def _download_link(self, operation: Install | Update, link: Link) -> Path:
package = operation.package

archive: Link | Path
archive: Path | None
archive = self._chef.get_cached_archive_for_link(link)
if archive is link:
if archive is None:
# No cached distributions was found, so we download and prepare it
try:
archive = self._download_archive(operation, link)
Expand All @@ -638,20 +637,14 @@ def _download_link(self, operation: Install | Update, link: Link) -> Link | Path
return archive

@staticmethod
def _validate_archive_hash(archive: Path | Link, package: Package) -> str:
archive_path = (
url_to_path(archive.url) if isinstance(archive, Link) else archive
)
file_dep = FileDependency(
package.name,
archive_path,
)
def _validate_archive_hash(archive: Path, package: Package) -> str:
file_dep = FileDependency(package.name, archive)
archive_hash: str = "sha256:" + file_dep.hash()
known_hashes = {f["hash"] for f in package.files}

if archive_hash not in known_hashes:
raise RuntimeError(
f"Hash for {package} from archive {archive_path.name} not found in"
f"Hash for {package} from archive {archive.name} not found in"
f" known hashes (was: {archive_hash})"
)

Expand Down
6 changes: 1 addition & 5 deletions src/poetry/utils/pip.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,6 @@

from typing import TYPE_CHECKING

from poetry.core.packages.utils.link import Link
from poetry.core.packages.utils.utils import url_to_path

from poetry.exceptions import PoetryException
from poetry.utils.env import EnvCommandError

Expand All @@ -16,13 +13,12 @@


def pip_install(
path: Path | Link,
path: Path,
environment: Env,
editable: bool = False,
deps: bool = False,
upgrade: bool = False,
) -> int | str:
path = url_to_path(path.url) if isinstance(path, Link) else path
is_wheel = path.suffix == ".whl"

# We disable version check here as we are already pinning to version available in
Expand Down
37 changes: 25 additions & 12 deletions tests/installation/test_chef.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
from pathlib import Path
from typing import TYPE_CHECKING

import pytest

from packaging.tags import Tag
from poetry.core.packages.utils.link import Link

Expand All @@ -16,7 +18,22 @@
from tests.conftest import Config


def test_get_cached_archive_for_link(config: Config, mocker: MockerFixture):
@pytest.mark.parametrize(
("link", "cached"),
[
(
"https://files.python-poetry.org/demo-0.1.0.tar.gz",
"/cache/demo-0.1.0-cp38-cp38-macosx_10_15_x86_64.whl",
),
(
"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",
),
],
)
def test_get_cached_archive_for_link(
config: Config, mocker: MockerFixture, link: str, cached: str
):
chef = Chef(
config,
MockEnv(
Expand All @@ -33,18 +50,16 @@ def test_get_cached_archive_for_link(config: Config, mocker: MockerFixture):
chef,
"get_cached_archives_for_link",
return_value=[
Link("file:///foo/demo-0.1.0-py2.py3-none-any"),
Link("file:///foo/demo-0.1.0.tar.gz"),
Link("file:///foo/demo-0.1.0-cp38-cp38-macosx_10_15_x86_64.whl"),
Link("file:///foo/demo-0.1.0-cp37-cp37-macosx_10_15_x86_64.whl"),
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("https://files.python-poetry.org/demo-0.1.0.tar.gz")
)
archive = chef.get_cached_archive_for_link(Link(link))

assert Link("file:///foo/demo-0.1.0-cp38-cp38-macosx_10_15_x86_64.whl") == archive
assert Path(cached) == archive


def test_get_cached_archives_for_link(config: Config, mocker: MockerFixture):
Expand All @@ -67,9 +82,7 @@ def test_get_cached_archives_for_link(config: Config, mocker: MockerFixture):
)

assert archives
assert set(archives) == {
Link(path.as_uri()) for path in distributions.glob("demo-0.1.0*")
}
assert set(archives) == {Path(path) for path in distributions.glob("demo-0.1.0*")}


def test_get_cache_directory_for_link(config: Config, config_cache_dir: Path):
Expand Down
17 changes: 5 additions & 12 deletions tests/installation/test_executor.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,7 @@ def test_executor_should_delete_incomplete_downloads(
)
mocker.patch(
"poetry.installation.chef.Chef.get_cached_archive_for_link",
side_effect=lambda link: link,
side_effect=lambda link: None,
)
mocker.patch(
"poetry.installation.chef.Chef.get_cache_directory_for_link",
Expand Down Expand Up @@ -436,11 +436,8 @@ def test_executor_should_not_write_pep610_url_references_for_cached_package(
config: Config,
io: BufferedIO,
):
link_cached = Link(
fixture_dir("distributions")
.joinpath("demo-0.1.0-py2.py3-none-any.whl")
.as_uri()
)
link_cached = fixture_dir("distributions") / "demo-0.1.0-py2.py3-none-any.whl"

mocker.patch(
"poetry.installation.executor.Executor._download", return_value=link_cached
)
Expand Down Expand Up @@ -564,12 +561,8 @@ def test_executor_should_use_cached_link_and_hash(
mocker: MockerFixture,
fixture_dir: FixtureDirGetter,
):
# Produce a file:/// URI that is a valid link
link_cached = Link(
fixture_dir("distributions")
.joinpath("demo-0.1.0-py2.py3-none-any.whl")
.as_uri()
)
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,
Expand Down
14 changes: 0 additions & 14 deletions tests/utils/test_pip.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,6 @@

import pytest

from poetry.core.packages.utils.link import Link
from poetry.core.packages.utils.utils import path_to_url

from poetry.utils.pip import pip_install


Expand All @@ -28,17 +25,6 @@ def test_pip_install_successful(
assert "Successfully installed demo-0.1.0" in result


def test_pip_install_link(
tmp_dir: str, tmp_venv: VirtualEnv, fixture_dir: FixtureDirGetter
):
file_path = Link(
path_to_url(fixture_dir("distributions/demo-0.1.0-py2.py3-none-any.whl"))
)
result = pip_install(file_path, tmp_venv)

assert "Successfully installed demo-0.1.0" in result


def test_pip_install_with_keyboard_interrupt(
tmp_dir: str,
tmp_venv: VirtualEnv,
Expand Down