From 76d47e76e4c374b23009c7c813b365fa405a4e75 Mon Sep 17 00:00:00 2001 From: Arun Babu Neelicattu Date: Tue, 18 Aug 2020 01:04:02 +0200 Subject: [PATCH] Drop pip from critical packages in project venv An initial attempt at using the embedded pip wheel from virtualenv package directly. This avoids the need for pip to be a critical package in the project's virtual environment. Setuptools is still maintained as this is required form edgeases where we still do `setuptools` editable installs (git, path dependencies). --- poetry/inspection/info.py | 7 +-- poetry/puzzle/provider.py | 2 +- poetry/utils/env.py | 48 ++++++++++++------- tests/installation/test_installer.py | 10 ++-- tests/installation/test_installer_old.py | 8 +++- .../masonry/builders/test_editable_builder.py | 12 +---- tests/puzzle/test_solver.py | 2 +- 7 files changed, 51 insertions(+), 38 deletions(-) diff --git a/poetry/inspection/info.py b/poetry/inspection/info.py index 6585575daf3..eb6de554ad8 100644 --- a/poetry/inspection/info.py +++ b/poetry/inspection/info.py @@ -435,17 +435,14 @@ def _pep517_metadata(cls, path): # type (Path) -> PackageInfo with temporary_directory() as tmp_dir: # TODO: cache PEP 517 build environment corresponding to each project venv venv_dir = Path(tmp_dir) / ".venv" - EnvManager.build_venv(venv_dir.as_posix()) + EnvManager.build_venv(venv_dir.as_posix(), with_pip=True) venv = VirtualEnv(venv_dir, venv_dir) dest_dir = Path(tmp_dir) / "dist" dest_dir.mkdir() try: - venv.run( - "python", - "-m", - "pip", + venv.run_pip( "install", "--disable-pip-version-check", "--ignore-installed", diff --git a/poetry/puzzle/provider.py b/poetry/puzzle/provider.py index d785377d8c8..981dec17b87 100644 --- a/poetry/puzzle/provider.py +++ b/poetry/puzzle/provider.py @@ -52,7 +52,7 @@ def _formatter_elapsed(self): class Provider: - UNSAFE_PACKAGES = {"setuptools", "distribute", "pip"} + UNSAFE_PACKAGES = {"setuptools"} def __init__( self, package, pool, io, env=None diff --git a/poetry/utils/env.py b/poetry/utils/env.py index 2c07b0b764e..307c1ad6fa1 100644 --- a/poetry/utils/env.py +++ b/poetry/utils/env.py @@ -40,6 +40,7 @@ from poetry.utils._compat import list_to_shell_command from poetry.utils._compat import subprocess from poetry.utils.toml_file import TomlFile +from virtualenv.seed.wheels.embed import get_embed_wheel GET_ENVIRONMENT_INFO = """\ @@ -679,19 +680,25 @@ def create_venv( @classmethod def build_venv( - cls, path, executable=None - ): # type: (Union[Path,str], Optional[Union[str, Path]]) -> virtualenv.run.session.Session + cls, path, executable=None, with_pip=False + ): # type: (Union[Path,str], Optional[Union[str, Path]], bool) -> virtualenv.run.session.Session if isinstance(executable, Path): executable = executable.resolve().as_posix() - return virtualenv.cli_run( - [ - "--no-download", - "--no-periodic-update", - "--python", - executable or sys.executable, - str(path), - ] - ) + + opts = [ + "--no-download", + "--no-periodic-update", + "--python", + executable or sys.executable, + ] + + if not with_pip: + # we cannot drop setuptools yet because we do editable installs (git, path) in project envs + opts.extend(["--no-pip", "--no-wheel"]) + + opts.append(str(path)) + + return virtualenv.cli_run(opts) @classmethod def remove_venv(cls, path): # type: (Union[Path,str]) -> None @@ -787,12 +794,21 @@ def marker_env(self): return self._marker_env + def get_embedded_wheel(self, distribution): + return get_embed_wheel( + distribution, "{}.{}".format(self.version_info[0], self.version_info[1]) + ).path + @property def pip(self): # type: () -> str """ Path to current pip executable """ - return self._bin("pip") + # 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").joinpath("pip")) + return path @property def platform(self): # type: () -> str @@ -1010,7 +1026,7 @@ def get_python_implementation(self): # type: () -> str def get_pip_command(self): # type: () -> 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, "-m", "pip"] + return [sys.executable, self.pip] def get_paths(self): # type: () -> Dict[str, str] # We can't use sysconfig.get_paths() because @@ -1112,7 +1128,7 @@ def get_python_implementation(self): # type: () -> str def get_pip_command(self): # type: () -> List[str] # We're in a virtualenv that is known to be sane, # so assume that we have a functional pip - return [self._bin("pip")] + return [self._bin("python"), self.pip] def get_supported_tags(self): # type: () -> List[Tag] file_path = Path(packaging.tags.__file__) @@ -1167,7 +1183,7 @@ def is_venv(self): # type: () -> bool def is_sane(self): # A virtualenv is considered sane if both "python" and "pip" exist. - return os.path.exists(self._bin("python")) and os.path.exists(self._bin("pip")) + return os.path.exists(self._bin("python")) def _run(self, cmd, **kwargs): with self.temp_environ(): @@ -1217,7 +1233,7 @@ def __init__(self, path=None, base=None, execute=False): self.executed = [] def get_pip_command(self): # type: () -> List[str] - return [self._bin("python"), "-m", "pip"] + return [self._bin("python"), self.pip] def _run(self, cmd, **kwargs): self.executed.append(cmd) diff --git a/tests/installation/test_installer.py b/tests/installation/test_installer.py index 88103e7c323..89ba638504f 100644 --- a/tests/installation/test_installer.py +++ b/tests/installation/test_installer.py @@ -362,15 +362,19 @@ def test_run_install_remove_untracked(installer, locker, repo, package, installe 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) # Always required and never removed. + installed.add_package(package_pip) + installed.add_package(package_setuptools) # Always required and never removed. installed.add_package(package) # Root package never removed. package.add_dependency("A", "~1.0") @@ -380,8 +384,8 @@ def test_run_install_remove_untracked(installer, locker, repo, package, installe assert 0 == installer.executor.installations_count assert 0 == installer.executor.updates_count - assert 2 == installer.executor.removals_count - assert {"b", "c"} == set(r.name for r in installer.executor.removals) + assert 3 == installer.executor.removals_count + assert {"b", "c", "pip"} == 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 3aab6831526..89cbded0b46 100644 --- a/tests/installation/test_installer_old.py +++ b/tests/installation/test_installer_old.py @@ -323,15 +323,19 @@ def test_run_install_remove_untracked(installer, locker, repo, package, installe 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) # Always required and never removed. + installed.add_package(package_pip) + installed.add_package(package_setuptools) # Always required and never removed. installed.add_package(package) # Root package never removed. package.add_dependency("A", "~1.0") @@ -346,7 +350,7 @@ 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"} + assert set(r.name for r in removals) == {"b", "c", "pip"} def test_run_whitelist_add(installer, locker, repo, package): diff --git a/tests/masonry/builders/test_editable_builder.py b/tests/masonry/builders/test_editable_builder.py index e2aac5beb41..ab9cca50075 100644 --- a/tests/masonry/builders/test_editable_builder.py +++ b/tests/masonry/builders/test_editable_builder.py @@ -170,17 +170,9 @@ def test_builder_falls_back_on_setup_and_pip_for_packages_with_build_scripts( builder = EditableBuilder(extended_poetry, env, NullIO()) builder.build() - assert [ - [ - "python", - "-m", - "pip", - "install", - "-e", - str(extended_poetry.file.parent), - "--no-deps", - ] + env.get_pip_command() + + ["install", "-e", str(extended_poetry.file.parent), "--no-deps"] ] == env.executed diff --git a/tests/puzzle/test_solver.py b/tests/puzzle/test_solver.py index c3540a36591..67f819563a3 100644 --- a/tests/puzzle/test_solver.py +++ b/tests/puzzle/test_solver.py @@ -1894,7 +1894,7 @@ def test_solver_remove_untracked_keeps_critical_package( package, pool, installed, locked, io ): solver = Solver(package, pool, installed, locked, io, remove_untracked=True) - package_pip = get_package("pip", "1.0") + package_pip = get_package("setuptools", "1.0") installed.add_package(package_pip) ops = solver.solve()