Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

install: make --sync compatible with virtualenvs.options.system-site-packages = true #9863

Merged
merged 1 commit into from
Nov 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion src/poetry/installation/installer.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ def __init__(
locker: Locker,
pool: RepositoryPool,
config: Config,
installed: Repository | None = None,
installed: InstalledRepository | None = None,
executor: Executor | None = None,
disable_cache: bool = False,
) -> None:
Expand Down Expand Up @@ -332,6 +332,9 @@ def _do_install(self) -> int:
synchronize=self._requires_synchronization,
skip_directory=self._skip_directory,
extras=set(self._extras),
system_site_packages={
p.name for p in self._installed_repository.system_site_packages
},
)

# Validate the dependencies
Expand Down
3 changes: 1 addition & 2 deletions src/poetry/plugins/plugin_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
from poetry.packages import Locker
from poetry.plugins.application_plugin import ApplicationPlugin
from poetry.plugins.plugin import Plugin
from poetry.repositories import Repository
from poetry.repositories.installed_repository import InstalledRepository
from poetry.toml import TOMLFile
from poetry.utils._compat import metadata
Expand Down Expand Up @@ -288,7 +287,7 @@ def _install(
self._poetry.config,
# Build installed repository from locked packages so that plugins
# that may be overwritten are not included.
Repository("poetry-repo", locked_packages),
InstalledRepository(locked_packages),
)
installer.update(True)

Expand Down
15 changes: 11 additions & 4 deletions src/poetry/puzzle/transaction.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,16 +42,20 @@ def get_solved_packages(self) -> dict[Package, TransitivePackageInfo]:

def calculate_operations(
self,
*,
with_uninstalls: bool = True,
synchronize: bool = False,
*,
skip_directory: bool = False,
extras: set[NormalizedName] | None = None,
system_site_packages: set[NormalizedName] | None = None,
) -> list[Operation]:
from poetry.installation.operations import Install
from poetry.installation.operations import Uninstall
from poetry.installation.operations import Update

if not system_site_packages:
system_site_packages = set()

operations: list[Operation] = []

extra_packages: set[NormalizedName] = set()
Expand Down Expand Up @@ -105,7 +109,8 @@ def calculate_operations(
# Extras that were not requested are always uninstalled.
if is_unsolicited_extra:
uninstalls.add(installed_package.name)
operations.append(Uninstall(installed_package))
if installed_package.name not in system_site_packages:
operations.append(Uninstall(installed_package))

# We have to perform an update if the version or another
# attribute of the package has changed (source type, url, ref, ...).
Expand Down Expand Up @@ -156,7 +161,8 @@ def calculate_operations(
for installed_package in self._installed_packages:
if installed_package.name == current_package.name:
uninstalls.add(installed_package.name)
operations.append(Uninstall(installed_package))
if installed_package.name not in system_site_packages:
operations.append(Uninstall(installed_package))

if synchronize:
# We preserve pip when not managed by poetry, this is done to avoid
Expand All @@ -178,7 +184,8 @@ def calculate_operations(

if installed_package.name not in relevant_result_packages:
uninstalls.add(installed_package.name)
operations.append(Uninstall(installed_package))
if installed_package.name not in system_site_packages:
operations.append(Uninstall(installed_package))

return sorted(
operations,
Expand Down
30 changes: 26 additions & 4 deletions src/poetry/repositories/installed_repository.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,26 @@
from poetry.repositories.repository import Repository
from poetry.utils._compat import getencoding
from poetry.utils._compat import metadata
from poetry.utils.env import VirtualEnv


if TYPE_CHECKING:
from poetry.utils.env import Env
from collections.abc import Sequence

from poetry.utils.env import Env

logger = logging.getLogger(__name__)


class InstalledRepository(Repository):
def __init__(self) -> None:
super().__init__("poetry-installed")
def __init__(self, packages: Sequence[Package] | None = None) -> None:
super().__init__("poetry-installed", packages)
self.system_site_packages: list[Package] = []

def add_package(self, package: Package, *, is_system_site: bool = False) -> None:
super().add_package(package)
if is_system_site:
self.system_site_packages.append(package)

@classmethod
def get_package_paths(cls, env: Env, name: str) -> set[Path]:
Expand Down Expand Up @@ -242,6 +250,12 @@ def load(cls, env: Env, with_dependencies: bool = False) -> InstalledRepository:
seen = set()
skipped = set()

base_env = (
env.parent_env
if isinstance(env, VirtualEnv) and env.includes_system_site_packages
else None
)

for entry in env.sys_path:
if not entry.strip():
logger.debug(
Expand Down Expand Up @@ -283,6 +297,14 @@ def load(cls, env: Env, with_dependencies: bool = False) -> InstalledRepository:
package.add_dependency(dep)

seen.add(package.name)
repo.add_package(package)
repo.add_package(
package,
is_system_site=bool(
base_env
and base_env.is_path_relative_to_lib(
Path(str(distribution._path)) # type: ignore[attr-defined]
)
),
)

return repo
8 changes: 4 additions & 4 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@
from poetry.factory import Factory
from poetry.layouts import layout
from poetry.packages.direct_origin import _get_package_from_git
from poetry.repositories import Repository
from poetry.repositories import RepositoryPool
from poetry.repositories.installed_repository import InstalledRepository
from poetry.utils.cache import ArtifactCache
from poetry.utils.env import EnvManager
from poetry.utils.env import SystemEnv
Expand Down Expand Up @@ -379,8 +379,8 @@ def tmp_venv(tmp_path: Path) -> Iterator[VirtualEnv]:


@pytest.fixture
def installed() -> Repository:
return Repository("installed")
def installed() -> InstalledRepository:
return InstalledRepository()


@pytest.fixture(scope="session")
Expand Down Expand Up @@ -412,7 +412,7 @@ def project_factory(
tmp_path: Path,
config: Config,
repo: TestRepository,
installed: Repository,
installed: InstalledRepository,
default_python: str,
load_required_fixtures: None,
) -> ProjectFactory:
Expand Down
65 changes: 61 additions & 4 deletions tests/puzzle/test_transaction.py
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,59 @@ def test_it_should_remove_installed_packages_if_required() -> None:
)


def test_it_should_not_remove_system_site_packages() -> None:
"""
Different types of uninstalls:
- c: tracked but not required
- e: not tracked
- f: root extra that is not requested
"""
extra_name = canonicalize_name("foo")
package = ProjectPackage("root", "1.0")
dep_f = Dependency("f", "1", optional=True)
dep_f._in_extras = [extra_name]
package.add_dependency(dep_f)
package.extras = {extra_name: [dep_f]}
opt_f = Package("f", "6.0.0")
opt_f.optional = True
transaction = Transaction(
[Package("a", "1.0.0"), Package("b", "2.0.0"), Package("c", "3.0.0")],
{
Package("a", "1.0.0"): get_transitive_info(1),
Package("b", "2.1.0"): get_transitive_info(2),
Package("d", "4.0.0"): get_transitive_info(0),
opt_f: get_transitive_info(0),
},
installed_packages=[
Package("a", "1.0.0"),
Package("b", "2.0.0"),
Package("c", "3.0.0"),
Package("e", "5.0.0"),
Package("f", "6.0.0"),
],
root_package=package,
)

check_operations(
transaction.calculate_operations(
synchronize=True,
extras=set(),
system_site_packages={
canonicalize_name(name) for name in ("a", "b", "c", "e", "f")
},
),
[
{
"job": "update",
"from": Package("b", "2.0.0"),
"to": Package("b", "2.1.0"),
},
{"job": "install", "package": Package("a", "1.0.0"), "skipped": True},
{"job": "install", "package": Package("d", "4.0.0")},
],
)


def test_it_should_not_remove_installed_packages_that_are_in_result() -> None:
transaction = Transaction(
[],
Expand Down Expand Up @@ -236,7 +289,9 @@ def test_calculate_operations_with_groups(
for name in sorted({"a", "b", "c"}.difference(expected), reverse=True):
expected_ops.insert(0, {"job": "remove", "package": Package(name, "1")})

check_operations(transaction.calculate_operations(sync), expected_ops)
check_operations(
transaction.calculate_operations(with_uninstalls=sync), expected_ops
)


@pytest.mark.parametrize(
Expand Down Expand Up @@ -273,7 +328,9 @@ def test_calculate_operations_with_markers(
for name in sorted({"a", "b"}.difference(expected), reverse=True):
expected_ops.insert(0, {"job": "remove", "package": Package(name, "1")})

check_operations(transaction.calculate_operations(sync), expected_ops)
check_operations(
transaction.calculate_operations(with_uninstalls=sync), expected_ops
)


@pytest.mark.parametrize(
Expand Down Expand Up @@ -366,8 +423,8 @@ def test_calculate_operations_extras(

check_operations(
transaction.calculate_operations(
with_uninstalls,
sync,
with_uninstalls=with_uninstalls,
synchronize=sync,
extras={extra_name} if extras else set(),
),
ops,
Expand Down
51 changes: 50 additions & 1 deletion tests/repositories/test_installed_repository.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from __future__ import annotations

import os
import platform
import shutil
import zipfile

Expand Down Expand Up @@ -415,6 +416,48 @@ def test_load_pep_610_compliant_editable_directory_packages(
assert package.develop


@pytest.mark.parametrize("with_system_site_packages", [False, True])
def test_system_site_packages(
tmp_path: Path,
mocker: MockerFixture,
poetry: Poetry,
site_purelib: Path,
with_system_site_packages: bool,
) -> None:
venv_path = tmp_path / "venv"
site_path = tmp_path / "site"
cleo_dist_info = "cleo-0.7.6.dist-info"
shutil.copytree(site_purelib / cleo_dist_info, site_path / cleo_dist_info)

EnvManager(poetry).build_venv(
path=venv_path, flags={"system-site-packages": with_system_site_packages}
)
env = VirtualEnv(venv_path)
standard_dist_info = "standard-1.2.3.dist-info"
shutil.copytree(site_purelib / standard_dist_info, env.purelib / standard_dist_info)
orig_sys_path = env.sys_path
if with_system_site_packages:
mocker.patch(
"poetry.utils.env.virtual_env.VirtualEnv.sys_path",
orig_sys_path[:-1] + [str(site_path)],
)
mocker.patch(
"poetry.utils.env.generic_env.GenericEnv.get_paths",
return_value={"purelib": str(site_path)},
)

installed_repository = InstalledRepository.load(env)

expected_system_site_packages = {"cleo"} if with_system_site_packages else set()
expected_packages = {"standard"} | expected_system_site_packages
if platform.system() == "FreeBSD":
expected_packages.add("sqlite3")
assert {p.name for p in installed_repository.packages} == expected_packages
assert {
p.name for p in installed_repository.system_site_packages
} == expected_system_site_packages


def test_system_site_packages_source_type(
tmp_path: Path, mocker: MockerFixture, poetry: Poetry, site_purelib: Path
) -> None:
Expand All @@ -427,12 +470,17 @@ def test_system_site_packages_source_type(
for dist_info in {"cleo-0.7.6.dist-info", "directory_pep_610-1.2.3.dist-info"}:
shutil.copytree(site_purelib / dist_info, site_path / dist_info)
mocker.patch("poetry.utils.env.virtual_env.VirtualEnv.sys_path", [str(site_path)])
mocker.patch("site.getsitepackages", return_value=[str(site_path)])
mocker.patch(
"poetry.utils.env.generic_env.GenericEnv.get_paths",
return_value={"purelib": str(site_path)},
)

EnvManager(poetry).build_venv(path=venv_path, flags={"system-site-packages": True})
env = VirtualEnv(venv_path)

installed_repository = InstalledRepository.load(env)

assert installed_repository.packages == installed_repository.system_site_packages
source_types = {
package.name: package.source_type for package in installed_repository.packages
}
Expand All @@ -458,6 +506,7 @@ def test_pipx_shared_lib_site_packages(
installed_repository = InstalledRepository.load(env)

assert len(installed_repository.packages) == 1
assert installed_repository.system_site_packages == []
cleo_package = installed_repository.packages[0]
cleo_package.to_dependency()
# There must not be a warning
Expand Down