Skip to content

Commit

Permalink
Fix and improve wheel installation
Browse files Browse the repository at this point in the history
  • Loading branch information
sdispater committed Jul 30, 2021
1 parent 37a8fd0 commit 9be3b0f
Show file tree
Hide file tree
Showing 7 changed files with 104 additions and 64 deletions.
12 changes: 6 additions & 6 deletions poetry/console/commands/source/add.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
from poetry.config.source import Source
from poetry.console.commands.command import Command
from poetry.factory import Factory
from poetry.repositories import Pool


class SourceAddCommand(Command):
Expand Down Expand Up @@ -89,13 +88,14 @@ def handle(self) -> Optional[int]:
self.line(f"Adding source with name <c1>{name}</c1>.")
sources.append(self.source_to_table(new_source))

self.poetry.config.merge(
{"sources": {source["name"]: source for source in sources}}
)

# ensure new source is valid. eg: invalid name etc.
self.poetry._pool = Pool()
try:
Factory.configure_sources(
self.poetry, sources, self.poetry.config, NullIO()
)
self.poetry.pool.repository(name)
pool = Factory.create_pool(self.poetry.config, NullIO())
pool.repository(name)
except ValueError as e:
self.line_error(
f"<error>Failed to validate addition of <c1>{name}</c1>: {e}</error>"
Expand Down
48 changes: 27 additions & 21 deletions poetry/installation/chef.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import json
import tarfile
import tempfile
import threading
import zipfile

from contextlib import redirect_stdout
Expand Down Expand Up @@ -78,6 +79,7 @@ def __init__(self, config: "Config", env: "Env") -> None:
self._cache_dir = (
Path(config.get("cache-dir")).expanduser().joinpath("artifacts")
)
self._lock = threading.Lock()

def prepare(self, archive: Path, output_dir: Optional[Path] = None) -> Path:
if not self.should_prepare(archive):
Expand Down Expand Up @@ -127,29 +129,33 @@ def _prepare(self, directory: Path, destination: Path) -> Path:
from poetry.utils.env import EnvManager
from poetry.utils.env import VirtualEnv

with temporary_directory() as tmp_dir:
EnvManager.build_venv(tmp_dir, executable=self._env.python, with_pip=True)
venv = VirtualEnv(Path(tmp_dir))
env = IsolatedEnv(venv, self._config)
builder = ProjectBuilder(
directory,
python_executable=env.executable,
scripts_dir=env.scripts_dir,
runner=quiet_subprocess_runner,
)
env.install(builder.build_system_requires)
env.install(
builder.build_system_requires | builder.get_requires_for_build("wheel")
)
with self._lock:
with temporary_directory() as tmp_dir:
EnvManager.build_venv(
tmp_dir, executable=self._env.python, with_pip=True
)
venv = VirtualEnv(Path(tmp_dir))
env = IsolatedEnv(venv, self._config)
builder = ProjectBuilder(
directory,
python_executable=env.executable,
scripts_dir=env.scripts_dir,
runner=quiet_subprocess_runner,
)
env.install(builder.build_system_requires)
env.install(
builder.build_system_requires
| builder.get_requires_for_build("wheel")
)

stdout = StringIO()
with redirect_stdout(stdout):
return Path(
builder.build(
"wheel",
destination.as_posix(),
stdout = StringIO()
with redirect_stdout(stdout):
return Path(
builder.build(
"wheel",
destination.as_posix(),
)
)
)

def should_prepare(self, archive: Path) -> bool:
return archive.is_dir() or not self.is_wheel(archive)
Expand Down
15 changes: 8 additions & 7 deletions poetry/installation/executor.py
Original file line number Diff line number Diff line change
Expand Up @@ -488,7 +488,7 @@ def _execute_uninstall(self, operation: Uninstall) -> int:
)
self._write(operation, message)

return self._remove(operation)
return self._remove(operation.package)

def _install(self, operation: Union[Install, Update]) -> int:
package = operation.package
Expand Down Expand Up @@ -520,9 +520,7 @@ def _install(self, operation: Union[Install, Update]) -> int:
def _update(self, operation: Union[Install, Update]) -> int:
return self._install(operation)

def _remove(self, operation: Uninstall) -> int:
package = operation.package

def _remove(self, package: "Package") -> int:
# If we have a VCS package, remove its source directory
if package.source_type == "git":
src_dir = self._env.path / "src" / package.name
Expand Down Expand Up @@ -597,9 +595,12 @@ def _install_directory(self, operation: Union[Install, Update]) -> int:
archive = self._prepare_archive(operation)

try:
return self.pip_install(
str(archive), upgrade=operation.job_type == "update"
)
if operation.job_type == "update":
# Uninstall first
# TODO: Make an uninstaller and find a way to rollback in case the new package can't be installed
self._remove(operation.initial_package)

self._wheel_installer.install(archive)
finally:
archive.unlink()

Expand Down
50 changes: 41 additions & 9 deletions poetry/installation/wheel_installer.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,14 @@

from pathlib import Path
from typing import TYPE_CHECKING
from typing import Iterator
from typing import Tuple
from typing import Union
from typing import cast

from installer.destinations import SchemeDictionaryDestination as BaseDestination
from installer.records import parse_record_file
from installer.sources import WheelFile as BaseWheelFile

from poetry import __version__
from poetry.utils._compat import WINDOWS
Expand All @@ -21,6 +26,29 @@
from poetry.utils.env import Env


class WheelFile(BaseWheelFile):
def get_contents(
self,
) -> Iterator[Tuple[Tuple[Union[Path, str], str, str], "BinaryIO"]]:
record_lines = self.read_dist_info("RECORD").splitlines()
records = parse_record_file(record_lines)
record_mapping = {record[0]: record for record in records}

for item in self._zipfile.infolist():
if item.is_dir():
continue

record = record_mapping.pop(item.filename)
assert record is not None, "In {}, {} is not mentioned in RECORD".format(
self._zipfile.filename,
item.filename,
) # should not happen for valid wheels

with self._zipfile.open(item) as stream:
stream_casted = cast("BinaryIO", stream)
yield record, stream_casted


class WheelDestination(BaseDestination):
def write_to_fs(
self, scheme: "Scheme", path: Union[Path, str], stream: "BinaryIO"
Expand Down Expand Up @@ -63,14 +91,18 @@ def __init__(self, env: "Env") -> None:

def install(self, wheel: Path) -> None:
from installer import install
from installer.sources import WheelFile

with WheelFile.open(wheel.as_posix()) as source:
install(
source=source,
destination=self._destination,
# Additional metadata that is generated by the installation tool.
additional_metadata={
"INSTALLER": f"Poetry {__version__}".encode(),
},
)
try:
install(
source=source,
destination=self._destination,
# Additional metadata that is generated by the installation tool.
additional_metadata={
"INSTALLER": f"Poetry {__version__}".encode(),
},
)
except Exception as e:
print(e)

raise
Binary file modified tests/fixtures/distributions/demo-0.1.2-py2.py3-none-any.whl
Binary file not shown.
43 changes: 22 additions & 21 deletions tests/installation/test_executor.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import shutil

from pathlib import Path
from urllib.parse import urlparse

import pytest

Expand All @@ -20,6 +21,7 @@
from poetry.installation.operations import Install
from poetry.installation.operations import Uninstall
from poetry.installation.operations import Update
from poetry.installation.wheel_installer import WheelInstaller
from poetry.repositories.pool import Pool
from poetry.utils.env import MockEnv
from tests.repositories.test_pypi_repository import MockRepository
Expand Down Expand Up @@ -95,9 +97,15 @@ def pool():
@pytest.fixture()
def mock_file_downloads(http):
def callback(request, uri, headers):
name = Path(urlparse(uri).path).name

fixture = Path(__file__).parent.parent.joinpath(
"fixtures/distributions/demo-0.1.0-py2.py3-none-any.whl"
"repositories/fixtures/pypi.org/dists/" + name
)
if not fixture.exists():
fixture = Path(__file__).parent.parent.joinpath(
"fixtures/distributions/demo-0.1.0-py2.py3-none-any.whl"
)

with fixture.open("rb") as f:
return [200, headers, f.read()]
Expand Down Expand Up @@ -129,6 +137,7 @@ def test_execute_executes_a_batch_of_operations(
pip_editable_install = mocker.patch(
"poetry.installation.executor.pip_editable_install", unsafe=not PY36
)
wheel_install = mocker.patch.object(WheelInstaller, "install")

config = Config()
config.merge({"cache-dir": tmp_dir})
Expand Down Expand Up @@ -199,9 +208,11 @@ def test_execute_executes_a_batch_of_operations(
expected = set(expected.splitlines())
output = set(io.fetch_output().splitlines())
assert expected == output
assert 5 == len(env.executed)
# One pip uninstall command
assert 1 == len(env.executed)
assert 0 == return_code
pip_editable_install.assert_called_once()
assert wheel_install.call_count == 4


def test_execute_shows_skipped_operations_if_verbose(
Expand Down Expand Up @@ -248,30 +259,25 @@ def test_execute_should_show_errors(config, mocker, io, env):


def test_execute_works_with_ansi_output(
mocker, config, pool, io_decorated, tmp_dir, mock_file_downloads, env
config, pool, io_decorated, tmp_dir, mock_file_downloads, env, wheel
):
config = Config()
config.merge({"cache-dir": tmp_dir})

executor = Executor(env, pool, config, io_decorated)

install_output = (
"some string that does not contain a keyb0ard !nterrupt or cance11ed by u$er"
)
mocker.patch.object(env, "_run", return_value=install_output)
return_code = executor.execute(
[
Install(Package("pytest", "3.5.2")),
Install(Package("clikit", "0.2.4")),
]
)
env._run.assert_called_once()

expected = [
"\x1b[39;1mPackage operations\x1b[39;22m: \x1b[34m1\x1b[39m install, \x1b[34m0\x1b[39m updates, \x1b[34m0\x1b[39m removals",
"\x1b[34;1m•\x1b[39;22m \x1b[39mInstalling \x1b[39m\x1b[36mpytest\x1b[39m\x1b[39m (\x1b[39m\x1b[39;1m3.5.2\x1b[39;22m\x1b[39m)\x1b[39m: \x1b[34mPending...\x1b[39m",
"\x1b[34;1m•\x1b[39;22m \x1b[39mInstalling \x1b[39m\x1b[36mpytest\x1b[39m\x1b[39m (\x1b[39m\x1b[39;1m3.5.2\x1b[39;22m\x1b[39m)\x1b[39m: \x1b[34mDownloading...\x1b[39m",
"\x1b[34;1m•\x1b[39;22m \x1b[39mInstalling \x1b[39m\x1b[36mpytest\x1b[39m\x1b[39m (\x1b[39m\x1b[39;1m3.5.2\x1b[39;22m\x1b[39m)\x1b[39m: \x1b[34mInstalling...\x1b[39m",
"\x1b[32;1m•\x1b[39;22m \x1b[39mInstalling \x1b[39m\x1b[36mpytest\x1b[39m\x1b[39m (\x1b[39m\x1b[32m3.5.2\x1b[39m\x1b[39m)\x1b[39m", # finished
"\x1b[34;1m•\x1b[39;22m \x1b[39mInstalling \x1b[39m\x1b[36mclikit\x1b[39m\x1b[39m (\x1b[39m\x1b[39;1m0.2.4\x1b[39;22m\x1b[39m)\x1b[39m: \x1b[34mPending...\x1b[39m",
"\x1b[34;1m•\x1b[39;22m \x1b[39mInstalling \x1b[39m\x1b[36mclikit\x1b[39m\x1b[39m (\x1b[39m\x1b[39;1m0.2.4\x1b[39;22m\x1b[39m)\x1b[39m: \x1b[34mDownloading...\x1b[39m",
"\x1b[34;1m•\x1b[39;22m \x1b[39mInstalling \x1b[39m\x1b[36mclikit\x1b[39m\x1b[39m (\x1b[39m\x1b[39;1m0.2.4\x1b[39;22m\x1b[39m)\x1b[39m: \x1b[34mInstalling...\x1b[39m",
"\x1b[32;1m•\x1b[39;22m \x1b[39mInstalling \x1b[39m\x1b[36mclikit\x1b[39m\x1b[39m (\x1b[39m\x1b[32m0.2.4\x1b[39m\x1b[39m)\x1b[39m", # finished
]
output = io_decorated.fetch_output()
# hint: use print(repr(output)) if you need to debug this
Expand All @@ -282,28 +288,23 @@ def test_execute_works_with_ansi_output(


def test_execute_works_with_no_ansi_output(
mocker, config, pool, io_not_decorated, tmp_dir, mock_file_downloads, env
config, pool, io_not_decorated, tmp_dir, mock_file_downloads, env
):
config = Config()
config.merge({"cache-dir": tmp_dir})

executor = Executor(env, pool, config, io_not_decorated)

install_output = (
"some string that does not contain a keyb0ard !nterrupt or cance11ed by u$er"
)
mocker.patch.object(env, "_run", return_value=install_output)
return_code = executor.execute(
[
Install(Package("pytest", "3.5.2")),
Install(Package("clikit", "0.2.4")),
]
)
env._run.assert_called_once()

expected = """
Package operations: 1 install, 0 updates, 0 removals
• Installing pytest (3.5.2)
• Installing clikit (0.2.4)
"""
expected = set(expected.splitlines())
output = set(io_not_decorated.fetch_output().splitlines())
Expand Down
Binary file not shown.

0 comments on commit 9be3b0f

Please sign in to comment.