diff --git a/src/poetry/installation/chef.py b/src/poetry/installation/chef.py index ec75b0478e3..975a966158c 100644 --- a/src/poetry/installation/chef.py +++ b/src/poetry/installation/chef.py @@ -94,18 +94,22 @@ def __init__(self, config: Config, env: Env) -> None: ) self._lock = threading.Lock() - def prepare(self, archive: Path, output_dir: Path | None = None) -> Path: + def prepare( + self, archive: Path, output_dir: Path | None = None, *, editable: bool = False + ) -> Path: if not self.should_prepare(archive): return archive if archive.is_dir(): tmp_dir = tempfile.mkdtemp(prefix="poetry-chef-") - return self._prepare(archive, Path(tmp_dir)) + return self._prepare(archive, Path(tmp_dir), editable=editable) return self._prepare_sdist(archive, destination=output_dir) - def _prepare(self, directory: Path, destination: Path) -> Path: + def _prepare( + self, directory: Path, destination: Path, *, editable: bool = False + ) -> Path: from poetry.utils.env import EnvManager from poetry.utils.env import VirtualEnv @@ -129,7 +133,7 @@ def _prepare(self, directory: Path, destination: Path) -> Path: try: return Path( builder.build( - "wheel", + "wheel" if not editable else "editable", destination.as_posix(), ) ) diff --git a/src/poetry/installation/executor.py b/src/poetry/installation/executor.py index 905294d9671..afd9cfad10d 100644 --- a/src/poetry/installation/executor.py +++ b/src/poetry/installation/executor.py @@ -468,17 +468,18 @@ def _execute_uninstall(self, operation: Uninstall) -> int: def _install(self, operation: Install | Update) -> int: package = operation.package - if package.source_type == "directory": - if not self._use_wheel_installer: - return self._install_directory_without_wheel_installer(operation) - - return self._install_directory(operation) + if package.source_type == "directory" and not self._use_wheel_installer: + return self._install_directory_without_wheel_installer(operation) + cleanup_archive: bool = False if package.source_type == "git": - return self._install_git(operation) - - if package.source_type == "file": + archive = self._prepare_git_archive(operation) + cleanup_archive = True + elif package.source_type == "file": archive = self._prepare_archive(operation) + elif package.source_type == "directory": + archive = self._prepare_directory_archive(operation) + cleanup_archive = True elif package.source_type == "url": assert package.source_url is not None archive = self._download_link(operation, Link(package.source_url)) @@ -495,7 +496,17 @@ def _install(self, operation: Install | Update) -> int: if not self._use_wheel_installer: return self.pip_install(archive, upgrade=operation.job_type == "update") - self._wheel_installer.install(archive) + try: + 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(cast(Update, operation).initial_package) + + self._wheel_installer.install(archive) + finally: + if cleanup_archive: + archive.unlink() return 0 @@ -532,9 +543,9 @@ def _prepare_archive(self, operation: Install | Update) -> Path: if not Path(package.source_url).is_absolute() and package.root_dir: archive = package.root_dir / archive - return self._chef.prepare(archive) + return self._chef.prepare(archive, editable=package.develop) - def _install_directory(self, operation: Install | Update) -> int: + def _prepare_directory_archive(self, operation: Install | Update) -> Path: package = operation.package operation_message = self.get_operation_message(operation) @@ -553,26 +564,35 @@ def _install_directory(self, operation: Install | Update) -> int: if package.source_subdirectory: req /= package.source_subdirectory - if package.develop: - # Editable installations are currently not supported - # for PEP-517 build systems so we defer to pip. - # TODO: Remove this workaround once either PEP-660 or PEP-662 is accepted - return self.pip_install(req, editable=True) + return self._prepare_archive(operation) - archive = self._prepare_archive(operation) + def _prepare_git_archive(self, operation: Install | Update) -> Path: + from poetry.vcs.git import Git - try: - 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(cast(Update, operation).initial_package) + package = operation.package + operation_message = self.get_operation_message(operation) - self._wheel_installer.install(archive) - finally: - archive.unlink() + message = ( + f" • {operation_message}: Cloning..." + ) + self._write(operation, message) - return 0 + assert package.source_url is not None + source = Git.clone( + url=package.source_url, + source_root=self._env.path / "src", + revision=package.source_resolved_reference or package.source_reference, + ) + + # Now we just need to install from the source directory + original_url = package.source_url + package._source_url = str(source.path) + + archive = self._prepare_directory_archive(operation) + + package._source_url = original_url + + return archive def _install_directory_without_wheel_installer( self, operation: Install | Update @@ -640,34 +660,6 @@ def _install_directory_without_wheel_installer( return self.pip_install(req, upgrade=True, editable=package.develop) - def _install_git(self, operation: Install | Update) -> int: - from poetry.vcs.git import Git - - package = operation.package - operation_message = self.get_operation_message(operation) - - message = ( - f" • {operation_message}: Cloning..." - ) - self._write(operation, message) - - assert package.source_url is not None - source = Git.clone( - url=package.source_url, - source_root=self._env.path / "src", - revision=package.source_resolved_reference or package.source_reference, - ) - - # Now we just need to install from the source directory - original_url = package.source_url - package._source_url = str(source.path) - - status_code = self._install_directory(operation) - - package._source_url = original_url - - return status_code - def _download(self, operation: Install | Update) -> Path: link = self._chooser.choose_for(operation.package) diff --git a/tests/installation/test_chef.py b/tests/installation/test_chef.py index 2eebceee4cc..14274e3bc95 100644 --- a/tests/installation/test_chef.py +++ b/tests/installation/test_chef.py @@ -2,6 +2,7 @@ from pathlib import Path from typing import TYPE_CHECKING +from zipfile import ZipFile import pytest @@ -165,3 +166,16 @@ def test_prepare_directory_with_extensions( wheel = chef.prepare(archive) assert wheel.name == f"extended-0.1-{env.supported_tags[0]}.whl" + + +def test_prepare_directory_editable(config: Config, config_cache_dir: Path): + chef = Chef(config, EnvManager.get_system_env()) + + archive = Path(__file__).parent.parent.joinpath("fixtures/simple_project").resolve() + + wheel = chef.prepare(archive, editable=True) + + assert wheel.name == "simple_project-1.2.3-py2.py3-none-any.whl" + + with ZipFile(wheel) as z: + assert "simple_project.pth" in z.namelist() diff --git a/tests/installation/test_executor.py b/tests/installation/test_executor.py index fd38b5415a3..3ab3bfbf68c 100644 --- a/tests/installation/test_executor.py +++ b/tests/installation/test_executor.py @@ -4,16 +4,19 @@ import json import re import shutil +import tempfile from pathlib import Path from typing import TYPE_CHECKING from typing import Any +from typing import Callable from urllib.parse import urlparse import pytest from cleo.formatters.style import Style from cleo.io.buffered_io import BufferedIO +from cleo.io.outputs.output import Verbosity from poetry.core.packages.package import Package from poetry.core.packages.utils.link import Link @@ -41,26 +44,40 @@ class Chef(BaseChef): - _directory_wheel = None - _sdist_wheel = None + _directory_wheels: list[Path] | None = None + _sdist_wheels: list[Path] | None = None - def set_directory_wheel(self, wheel: Path) -> None: - self._directory_wheel = wheel + def set_directory_wheel(self, wheels: Path | list[Path]) -> None: + if not isinstance(wheels, list): + wheels = [wheels] - def set_sdist_wheel(self, wheel: Path) -> None: - self._sdist_wheel = wheel + self._directory_wheels = wheels + + def set_sdist_wheel(self, wheels: Path | list[Path]) -> None: + if not isinstance(wheels, list): + wheels = [wheels] + + self._sdist_wheels = wheels def _prepare_sdist(self, archive: Path, destination: Path | None = None) -> Path: - if self._sdist_wheel is not None: - return self._sdist_wheel + if self._sdist_wheels is not None: + wheel = self._sdist_wheels.pop(0) + self._sdist_wheels.append(wheel) + + return wheel return super()._prepare_sdist(archive) - def _prepare(self, directory: Path, destination: Path) -> Path: - if self._directory_wheel is not None: - return self._directory_wheel + def _prepare( + self, directory: Path, destination: Path, *, editable: bool = False + ) -> Path: + if self._directory_wheels is not None: + wheel = self._directory_wheels.pop(0) + self._directory_wheels.append(wheel) - return super()._prepare(directory, destination) + return wheel + + return super()._prepare(directory, destination, editable=editable) @pytest.fixture @@ -132,17 +149,38 @@ def callback( @pytest.fixture() -def wheel(tmp_dir: Path) -> Path: - shutil.copyfile( - Path(__file__) - .parent.parent.joinpath( - "fixtures/distributions/demo-0.1.2-py2.py3-none-any.whl" +def copy_wheel(tmp_dir: Path) -> 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(), ) - .as_posix(), - Path(tmp_dir).joinpath("demo-0.1.2-py2.py3-none-any.whl").as_posix(), - ) - return Path(tmp_dir).joinpath("demo-0.1.2-py2.py3-none-any.whl") + return ( + Path(tmp_dir).joinpath(tmp_name).joinpath("demo-0.1.2-py2.py3-none-any.whl") + ) + + return _copy_wheel + + +@pytest.fixture() +def wheel(copy_wheel: Callable[[], Path]) -> Path: + archive = copy_wheel() + + yield archive + + if archive.exists(): + archive.unlink() def test_execute_executes_a_batch_of_operations( @@ -153,15 +191,18 @@ def test_execute_executes_a_batch_of_operations( tmp_dir: str, mock_file_downloads: None, env: MockEnv, - wheel: Path, + copy_wheel: Callable[[], Path], ): wheel_install = mocker.patch.object(WheelInstaller, "install") config.merge({"cache-dir": tmp_dir}) + prepare_spy = mocker.spy(Chef, "_prepare") chef = Chef(config, env) - chef.set_directory_wheel(wheel) - chef.set_sdist_wheel(wheel) + chef.set_directory_wheel([copy_wheel(), copy_wheel()]) + chef.set_sdist_wheel(copy_wheel()) + + io.set_verbosity(Verbosity.VERY_VERBOSE) executor = Executor(env, pool, config, io) executor.set_chef(chef) @@ -223,11 +264,17 @@ def test_execute_executes_a_batch_of_operations( expected = set(expected.splitlines()) output = set(io.fetch_output().splitlines()) assert output == expected - assert wheel_install.call_count == 4 - # One pip uninstall and one pip editable install + assert wheel_install.call_count == 5 + # Two pip uninstalls: one for the remove operation one for the update operation assert len(env.executed) == 2 assert return_code == 0 + assert prepare_spy.call_count == 2 + assert prepare_spy.call_args_list == [ + mocker.call(chef, mocker.ANY, mocker.ANY, editable=False), + mocker.call(chef, mocker.ANY, mocker.ANY, editable=True), + ] + @pytest.mark.parametrize( "operations, has_warning", @@ -586,7 +633,7 @@ def test_executor_should_write_pep610_url_references_for_directories( def test_executor_should_write_pep610_url_references_for_editable_directories( - tmp_venv: VirtualEnv, pool: Pool, config: Config, io: BufferedIO + tmp_venv: VirtualEnv, pool: Pool, config: Config, io: BufferedIO, wheel: Path ): url = ( Path(__file__) @@ -605,7 +652,7 @@ def test_executor_should_write_pep610_url_references_for_editable_directories( chef.set_directory_wheel(wheel) executor = Executor(tmp_venv, pool, config, io) - executor.set_chef(wheel) + executor.set_chef(chef) executor.execute([Install(package)]) verify_installed_distribution( tmp_venv, package, {"dir_info": {"editable": True}, "url": url.as_uri()}