diff --git a/poetry/puzzle/solver.py b/poetry/puzzle/solver.py index b5a413f15e0..2e864bc9b20 100644 --- a/poetry/puzzle/solver.py +++ b/poetry/puzzle/solver.py @@ -63,10 +63,31 @@ def __init__( self._overrides = [] self._remove_untracked = remove_untracked + self._preserved_package_names = None + @property def provider(self) -> Provider: return self._provider + @property + def preserved_package_names(self): + if self._preserved_package_names is None: + self._preserved_package_names = { + self._package.name, + *Provider.UNSAFE_PACKAGES, + } + + deps = {package.name for package in self._locked.packages} + + # preserve pip/setuptools/wheel when not managed by poetry, this is so + # to avoid externally managed virtual environments causing unnecessary + # removals. + for name in {"pip", "wheel", "setuptools"}: + if name not in deps: + self._preserved_package_names.add(name) + + return self._preserved_package_names + @contextmanager def use_environment(self, env: Env) -> None: with self.provider.use_environment(env): @@ -190,11 +211,9 @@ def solve(self, use_latest: List[str] = None) -> List["OperationTypes"]: locked_names = {locked.name for locked in self._locked.packages} for installed in self._installed.packages: - if installed.name == self._package.name: - continue - if installed.name in Provider.UNSAFE_PACKAGES: - # Never remove pip, setuptools etc. + if installed.name in self.preserved_package_names: continue + if installed.name not in locked_names: operations.append(Uninstall(installed)) diff --git a/poetry/utils/env.py b/poetry/utils/env.py index 8d5fa861c1b..195d08162de 100644 --- a/poetry/utils/env.py +++ b/poetry/utils/env.py @@ -877,13 +877,8 @@ def create_venv( io.write_line( "Creating virtualenv {} in {}".format(name, str(venv_path)) ) - - self.build_venv( - venv, - executable=executable, - flags=self._poetry.config.get("virtualenvs.options"), - ) else: + create_venv = False if force: if not env.is_sane(): io.write_line( @@ -895,14 +890,23 @@ def create_venv( "Recreating virtualenv {} in {}".format(name, str(venv)) ) self.remove_venv(venv) - self.build_venv( - venv, - executable=executable, - flags=self._poetry.config.get("virtualenvs.options"), - ) + create_venv = True elif io.is_very_verbose(): io.write_line(f"Virtualenv {name} already exists.") + if create_venv: + self.build_venv( + venv, + executable=executable, + flags=self._poetry.config.get("virtualenvs.options"), + # TODO: in a future version switch remove pip/setuptools/wheel + # poetry does not need them these exists today to not break developer + # environment assumptions + with_pip=True, + with_setuptools=True, + with_wheel=True, + ) + # venv detection: # stdlib venv may symlink sys.executable, so we can't use realpath. # but others can symlink *to* the venv Python, @@ -927,12 +931,29 @@ def build_venv( path: Union[Path, str], executable: Optional[Union[str, Path]] = None, flags: Dict[str, bool] = None, - with_pip: bool = False, + with_pip: Optional[bool] = None, with_wheel: Optional[bool] = None, with_setuptools: Optional[bool] = None, ) -> virtualenv.run.session.Session: flags = flags or {} + flags["no-pip"] = ( + not with_pip if with_pip is not None else flags.pop("no-pip", True) + ) + + flags["no-setuptools"] = ( + not with_setuptools + if with_setuptools is not None + else flags.pop("no-setuptools", True) + ) + + # we want wheels to be enabled when pip is required and it has not been explicitly disabled + flags["no-wheel"] = ( + not with_wheel + if with_wheel is not None + else flags.pop("no-wheel", flags["no-pip"]) + ) + if isinstance(executable, Path): executable = executable.resolve().as_posix() @@ -943,20 +964,6 @@ def build_venv( executable or sys.executable, ] - if not with_pip: - args.append("--no-pip") - else: - if with_wheel is None: - # we want wheels to be enabled when pip is required and it has - # not been explicitly disabled - with_wheel = True - - if with_wheel is None or not with_wheel: - args.append("--no-wheel") - - if with_setuptools is None or not with_setuptools: - args.append("--no-setuptools") - for flag, value in flags.items(): if value is True: args.append(f"--{flag}") @@ -1039,6 +1046,8 @@ def __init__(self, path: Path, base: Optional[Path] = None) -> None: self._platlib = None self._script_dirs = None + self._embedded_pip_path = None + @property def path(self) -> Path: return self._path @@ -1074,6 +1083,12 @@ def get_embedded_wheel(self, distribution): distribution, "{}.{}".format(self.version_info[0], self.version_info[1]) ).path + @property + def pip_embedded(self) -> str: + if self._embedded_pip_path is None: + self._embedded_pip_path = str(self.get_embedded_wheel("pip") / "pip") + return self._embedded_pip_path + @property def pip(self) -> str: """ @@ -1082,7 +1097,7 @@ def pip(self) -> str: # we do not use as_posix() here due to issues with windows pathlib2 implementation path = self._bin("pip") if not Path(path).exists(): - return str(self.get_embedded_wheel("pip") / "pip") + return str(self.pip_embedded) return path @property @@ -1187,7 +1202,7 @@ def get_python_implementation(self) -> str: def get_marker_env(self) -> Dict[str, Any]: raise NotImplementedError() - def get_pip_command(self) -> List[str]: + def get_pip_command(self, embedded: bool = False) -> List[str]: raise NotImplementedError() def get_supported_tags(self) -> List[Tag]: @@ -1208,16 +1223,20 @@ def is_sane(self) -> bool: """ return True - def run(self, bin: str, *args: str, **kwargs: Any) -> Union[str, int]: + def get_command_from_bin(self, bin: str) -> List[str]: if bin == "pip": - return self.run_pip(*args, **kwargs) + # when pip is required we need to ensure that we fallback to + # embedded pip when pip is not available in the environment + return self.get_pip_command() + + return [self._bin(bin)] - bin = self._bin(bin) - cmd = [bin] + list(args) + def run(self, bin: str, *args: str, **kwargs: Any) -> Union[str, int]: + cmd = self.get_command_from_bin(bin) + list(args) return self._run(cmd, **kwargs) def run_pip(self, *args: str, **kwargs: Any) -> Union[int, str]: - pip = self.get_pip_command() + pip = self.get_pip_command(embedded=True) cmd = pip + list(args) return self._run(cmd, **kwargs) @@ -1260,17 +1279,13 @@ def _run(self, cmd: List[str], **kwargs: Any) -> Union[int, str]: return decode(output) def execute(self, bin: str, *args: str, **kwargs: Any) -> Optional[int]: - if bin == "pip": - return self.run_pip(*args, **kwargs) - - bin = self._bin(bin) + command = self.get_command_from_bin(bin) + list(args) env = kwargs.pop("env", {k: v for k, v in os.environ.items()}) if not self._is_windows: - args = [bin] + list(args) - return os.execvpe(bin, args, env=env) + return os.execvpe(command[0], command, env=env) else: - exe = subprocess.Popen([bin] + list(args), env=env, **kwargs) + exe = subprocess.Popen([command[0]] + command[1:], env=env, **kwargs) exe.communicate() return exe.returncode @@ -1338,10 +1353,10 @@ def get_version_info(self) -> Tuple[int]: def get_python_implementation(self) -> str: return platform.python_implementation() - def get_pip_command(self) -> List[str]: + def get_pip_command(self, embedded: bool = False) -> List[str]: # If we're not in a venv, assume the interpreter we're running on # has a pip and use that - return [sys.executable, self.pip] + return [sys.executable, self.pip_embedded if embedded else self.pip] def get_paths(self) -> Dict[str, str]: # We can't use sysconfig.get_paths() because @@ -1445,10 +1460,10 @@ def get_version_info(self) -> Tuple[int]: def get_python_implementation(self) -> str: return self.marker_env["platform_python_implementation"] - def get_pip_command(self) -> List[str]: + def get_pip_command(self, embedded: bool = False) -> List[str]: # We're in a virtualenv that is known to be sane, # so assume that we have a functional pip - return [self._bin("python"), self.pip] + return [self._bin("python"), self.pip_embedded if embedded else self.pip] def get_supported_tags(self) -> List[Tag]: file_path = Path(packaging.tags.__file__) @@ -1560,8 +1575,8 @@ def __init__( self._execute = execute self.executed = [] - def get_pip_command(self) -> List[str]: - return [self._bin("python"), self.pip] + def get_pip_command(self, embedded: bool = False) -> List[str]: + return [self._bin("python"), self.pip_embedded if embedded else self.pip] def _run(self, cmd: List[str], **kwargs: Any) -> int: self.executed.append(cmd) diff --git a/poetry/utils/pip.py b/poetry/utils/pip.py index 5267cf435af..416c56b97ae 100644 --- a/poetry/utils/pip.py +++ b/poetry/utils/pip.py @@ -53,7 +53,7 @@ def pip_install( executable=environment.python, with_pip=True, with_setuptools=True ) as env: return environment.run( - env._bin("pip"), + *env.get_pip_command(), *args, env={**os.environ, "PYTHONPATH": str(env.purelib)}, ) diff --git a/tests/console/commands/env/test_use.py b/tests/console/commands/env/test_use.py index 93e96c02523..8b37c8e726e 100644 --- a/tests/console/commands/env/test_use.py +++ b/tests/console/commands/env/test_use.py @@ -55,6 +55,9 @@ def test_activate_activates_non_existing_virtualenv_no_envs_file( venv_py37, executable="python3.7", flags={"always-copy": False, "system-site-packages": False}, + with_pip=True, + with_setuptools=True, + with_wheel=True, ) envs_file = TOMLFile(venv_cache / "envs.toml") diff --git a/tests/inspection/test_info.py b/tests/inspection/test_info.py index 215109d551f..3a15e605a6f 100644 --- a/tests/inspection/test_info.py +++ b/tests/inspection/test_info.py @@ -217,7 +217,7 @@ def test_info_setup_missing_mandatory_should_trigger_pep517( except PackageInfoError: assert spy.call_count == 3 else: - assert spy.call_count == 1 + assert spy.call_count == 2 def test_info_prefer_poetry_config_over_egg_info(): diff --git a/tests/installation/test_installer.py b/tests/installation/test_installer.py index e9bc0e4e1c9..00986905984 100644 --- a/tests/installation/test_installer.py +++ b/tests/installation/test_installer.py @@ -1,5 +1,6 @@ from __future__ import unicode_literals +import itertools import json import sys @@ -35,6 +36,9 @@ from tests.repositories.test_pypi_repository import MockRepository +RESERVED_PACKAGES = ("pip", "setuptools", "wheel") + + class Installer(BaseInstaller): def _get_installer(self): return NoopInstaller() @@ -367,59 +371,88 @@ def test_run_install_no_dev_and_dev_only(installer, locker, repo, package, insta assert 1 == installer.executor.removals_count -def test_run_install_remove_untracked(installer, locker, repo, package, installed): +@pytest.mark.parametrize( + "managed_reserved_package_names", + [ + i + for i in itertools.chain( + [tuple()], + itertools.permutations(RESERVED_PACKAGES, 1), + itertools.permutations(RESERVED_PACKAGES, 2), + [RESERVED_PACKAGES], + ) + ], +) +def test_run_install_remove_untracked( + managed_reserved_package_names, installer, locker, repo, package, installed +): + package_a = get_package("a", "1.0") + package_b = get_package("b", "1.1") + package_c = get_package("c", "1.2") + package_pip = get_package("pip", "20.0.0") + package_setuptools = get_package("setuptools", "20.0.0") + package_wheel = get_package("wheel", "20.0.0") + + all_packages = [ + package_a, + package_b, + package_c, + package_pip, + package_setuptools, + package_wheel, + ] + + managed_reserved_packages = [ + pkg for pkg in all_packages if pkg.name in managed_reserved_package_names + ] + locked_packages = [package_a, *managed_reserved_packages] + + for pkg in all_packages: + repo.add_package(pkg) + installed.add_package(pkg) + + installed.add_package(package) # Root package never removed. + + package.add_dependency(Factory.create_dependency(package_a.name, package_a.version)) + locker.locked(True) locker.mock_lock_data( { "package": [ { - "name": "a", - "version": "1.0", + "name": pkg.name, + "version": pkg.version, "category": "main", "optional": False, "platform": "*", "python-versions": "*", "checksum": [], } + for pkg in locked_packages ], "metadata": { "python-versions": "*", "platform": "*", "content-hash": "123456789", - "hashes": {"a": []}, + "hashes": {pkg.name: [] for pkg in locked_packages}, }, } ) - package_a = get_package("a", "1.0") - package_b = get_package("b", "1.1") - package_c = get_package("c", "1.2") - package_pip = get_package("pip", "20.0.0") - package_setuptools = get_package("setuptools", "20.0.0") - - repo.add_package(package_a) - repo.add_package(package_b) - repo.add_package(package_c) - repo.add_package(package_pip) - repo.add_package(package_setuptools) - - installed.add_package(package_a) - installed.add_package(package_b) - installed.add_package(package_c) - installed.add_package(package_pip) - installed.add_package(package_setuptools) - installed.add_package(package) # Root package never removed. - - package.add_dependency(Factory.create_dependency("A", "~1.0")) installer.dev_mode(True).remove_untracked(True) installer.run() assert 0 == installer.executor.installations_count assert 0 == installer.executor.updates_count - assert 4 == installer.executor.removals_count - assert {"b", "c", "pip", "setuptools"} == set( - r.name for r in installer.executor.removals - ) + assert 2 + len(managed_reserved_packages) == installer.executor.removals_count + + expected_removals = { + package_b.name, + package_c.name, + *managed_reserved_package_names, + } + + assert expected_removals == set(r.name for r in installer.executor.removals) def test_run_whitelist_add(installer, locker, repo, package): diff --git a/tests/installation/test_installer_old.py b/tests/installation/test_installer_old.py index a8e7a9c1877..20936fe5f09 100644 --- a/tests/installation/test_installer_old.py +++ b/tests/installation/test_installer_old.py @@ -1,5 +1,6 @@ from __future__ import unicode_literals +import itertools import sys from pathlib import Path @@ -28,6 +29,9 @@ from tests.repositories.test_pypi_repository import MockRepository +RESERVED_PACKAGES = ("pip", "setuptools", "wheel") + + class Installer(BaseInstaller): def _get_installer(self): return NoopInstaller() @@ -292,49 +296,73 @@ def test_run_install_no_dev(installer, locker, repo, package, installed): assert len(removals) == 1 -def test_run_install_remove_untracked(installer, locker, repo, package, installed): +@pytest.mark.parametrize( + "managed_reserved_package_names", + [ + i + for i in itertools.chain( + [tuple()], + itertools.permutations(RESERVED_PACKAGES, 1), + itertools.permutations(RESERVED_PACKAGES, 2), + [RESERVED_PACKAGES], + ) + ], +) +def test_run_install_remove_untracked( + managed_reserved_package_names, installer, locker, repo, package, installed +): + package_a = get_package("a", "1.0") + package_b = get_package("b", "1.1") + package_c = get_package("c", "1.2") + package_pip = get_package("pip", "20.0.0") + package_setuptools = get_package("setuptools", "20.0.0") + package_wheel = get_package("wheel", "20.0.0") + + all_packages = [ + package_a, + package_b, + package_c, + package_pip, + package_setuptools, + package_wheel, + ] + + managed_reserved_packages = [ + pkg for pkg in all_packages if pkg.name in managed_reserved_package_names + ] + locked_packages = [package_a, *managed_reserved_packages] + + for pkg in all_packages: + repo.add_package(pkg) + installed.add_package(pkg) + + installed.add_package(package) # Root package never removed. + + package.add_dependency(Factory.create_dependency(package_a.name, package_a.version)) + locker.locked(True) locker.mock_lock_data( { "package": [ { - "name": "a", - "version": "1.0", + "name": pkg.name, + "version": pkg.version, "category": "main", "optional": False, "platform": "*", "python-versions": "*", "checksum": [], } + for pkg in locked_packages ], "metadata": { "python-versions": "*", "platform": "*", "content-hash": "123456789", - "hashes": {"a": []}, + "hashes": {pkg.name: [] for pkg in locked_packages}, }, } ) - package_a = get_package("a", "1.0") - package_b = get_package("b", "1.1") - package_c = get_package("c", "1.2") - package_pip = get_package("pip", "20.0.0") - package_setuptools = get_package("setuptools", "20.0.0") - - repo.add_package(package_a) - repo.add_package(package_b) - repo.add_package(package_c) - repo.add_package(package_pip) - repo.add_package(package_setuptools) - - installed.add_package(package_a) - installed.add_package(package_b) - installed.add_package(package_c) - installed.add_package(package_pip) - installed.add_package(package_setuptools) - installed.add_package(package) # Root package never removed. - - package.add_dependency(Factory.create_dependency("A", "~1.0")) installer.dev_mode(True).remove_untracked(True) installer.run() @@ -346,7 +374,12 @@ def test_run_install_remove_untracked(installer, locker, repo, package, installe assert len(updates) == 0 removals = installer.installer.removals - assert set(r.name for r in removals) == {"b", "c", "pip", "setuptools"} + expected_removals = { + package_b.name, + package_c.name, + *managed_reserved_package_names, + } + assert set(r.name for r in removals) == expected_removals def test_run_whitelist_add(installer, locker, repo, package): diff --git a/tests/utils/test_env.py b/tests/utils/test_env.py index 1cfc9ad5099..d09012c821f 100644 --- a/tests/utils/test_env.py +++ b/tests/utils/test_env.py @@ -4,7 +4,7 @@ import sys from pathlib import Path -from typing import Optional +from typing import Any from typing import Union import pytest @@ -118,9 +118,7 @@ def test_env_get_venv_with_venv_folder_present( assert venv.path == in_project_venv_dir -def build_venv( - path: Union[Path, str], executable: Optional[str] = None, flags: bool = None -) -> (): +def build_venv(path: Union[Path, str], **__: Any) -> (): os.mkdir(str(path)) @@ -161,6 +159,9 @@ def test_activate_activates_non_existing_virtualenv_no_envs_file( Path(tmp_dir) / "{}-py3.7".format(venv_name), executable="python3.7", flags={"always-copy": False, "system-site-packages": False}, + with_pip=True, + with_setuptools=True, + with_wheel=True, ) envs_file = TOMLFile(Path(tmp_dir) / "envs.toml") @@ -281,6 +282,9 @@ def test_activate_activates_different_virtualenv_with_envs_file( Path(tmp_dir) / "{}-py3.6".format(venv_name), executable="python3.6", flags={"always-copy": False, "system-site-packages": False}, + with_pip=True, + with_setuptools=True, + with_wheel=True, ) assert envs_file.exists() @@ -335,6 +339,9 @@ def test_activate_activates_recreates_for_different_patch( Path(tmp_dir) / "{}-py3.7".format(venv_name), executable="python3.7", flags={"always-copy": False, "system-site-packages": False}, + with_pip=True, + with_setuptools=True, + with_wheel=True, ) remove_venv_m.assert_called_with(Path(tmp_dir) / "{}-py3.7".format(venv_name)) @@ -715,6 +722,9 @@ def test_create_venv_tries_to_find_a_compatible_python_executable_using_generic_ config_virtualenvs_path / "{}-py3.7".format(venv_name), executable="python3", flags={"always-copy": False, "system-site-packages": False}, + with_pip=True, + with_setuptools=True, + with_wheel=True, ) @@ -739,6 +749,9 @@ def test_create_venv_tries_to_find_a_compatible_python_executable_using_specific config_virtualenvs_path / "{}-py3.9".format(venv_name), executable="python3.9", flags={"always-copy": False, "system-site-packages": False}, + with_pip=True, + with_setuptools=True, + with_wheel=True, ) @@ -823,6 +836,9 @@ def test_create_venv_uses_patch_version_to_detect_compatibility( / "{}-py{}.{}".format(venv_name, version.major, version.minor), executable=None, flags={"always-copy": False, "system-site-packages": False}, + with_pip=True, + with_setuptools=True, + with_wheel=True, ) @@ -858,6 +874,9 @@ def test_create_venv_uses_patch_version_to_detect_compatibility_with_executable( / "{}-py{}.{}".format(venv_name, version.major, version.minor - 1), executable="python{}.{}".format(version.major, version.minor - 1), flags={"always-copy": False, "system-site-packages": False}, + with_pip=True, + with_setuptools=True, + with_wheel=True, ) @@ -892,6 +911,9 @@ def test_activate_with_in_project_setting_does_not_fail_if_no_venvs_dir( poetry.file.parent / ".venv", executable="python3.7", flags={"always-copy": False, "system-site-packages": False}, + with_pip=True, + with_setuptools=True, + with_wheel=True, ) envs_file = TOMLFile(Path(tmp_dir) / "virtualenvs" / "envs.toml")