Skip to content

Commit

Permalink
Drop pip from critical packages in project venv
Browse files Browse the repository at this point in the history
Use embedded pip wheel from virtualenv package. This avoids the need
for pip to be a critical package in the project's virtual environment.
  • Loading branch information
abn committed Sep 28, 2020
1 parent 5b94aa8 commit 235c796
Show file tree
Hide file tree
Showing 8 changed files with 53 additions and 40 deletions.
7 changes: 2 additions & 5 deletions poetry/inspection/info.py
Original file line number Diff line number Diff line change
Expand Up @@ -444,17 +444,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",
Expand Down
2 changes: 1 addition & 1 deletion poetry/puzzle/provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ def _formatter_elapsed(self):

class Provider:

UNSAFE_PACKAGES = {"setuptools", "distribute", "pip", "wheel"}
UNSAFE_PACKAGES = {"setuptools"}

def __init__(
self, package, pool, io, env=None
Expand Down
48 changes: 32 additions & 16 deletions poetry/utils/env.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,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 = """\
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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__)
Expand Down Expand Up @@ -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():
Expand Down Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions tests/inspection/test_info.py
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ def test_info_setup_simple(mocker, demo_setup):
def test_info_setup_simple_py2(mocker, demo_setup):
spy = mocker.spy(VirtualEnv, "run")
info = PackageInfo.from_directory(demo_setup)
assert spy.call_count == 2
assert spy.call_count == 1
demo_check_info(info, requires_dist={"package"})


Expand Down Expand Up @@ -234,7 +234,7 @@ def test_info_setup_missing_mandatory_should_trigger_pep517(
except PackageInfoError:
assert spy.call_count == 3
else:
assert spy.call_count == 2
assert spy.call_count == 1


def test_info_prefer_poetry_config_over_egg_info():
Expand Down
10 changes: 7 additions & 3 deletions tests/installation/test_installer.py
Original file line number Diff line number Diff line change
Expand Up @@ -370,15 +370,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(Factory.create_dependency("A", "~1.0"))
Expand All @@ -388,8 +392,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):
Expand Down
8 changes: 6 additions & 2 deletions tests/installation/test_installer_old.py
Original file line number Diff line number Diff line change
Expand Up @@ -330,15 +330,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(Factory.create_dependency("A", "~1.0"))
Expand All @@ -353,7 +357,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):
Expand Down
12 changes: 2 additions & 10 deletions tests/masonry/builders/test_editable_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down
2 changes: 1 addition & 1 deletion tests/puzzle/test_solver.py
Original file line number Diff line number Diff line change
Expand Up @@ -2192,7 +2192,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()
Expand Down

0 comments on commit 235c796

Please sign in to comment.