From 610abf7f018c77d7e23954b516685c985b62b5e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Bompard?= Date: Thu, 24 Aug 2023 11:48:31 +0200 Subject: [PATCH 1/6] Consider system packages as installed if the venv includes them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This changesets adds a getter to `env.virtual_env.VirtualEnv` to check whether it has access to system packages, and overrides the `is_path_relative_to_lib` method to take it into account. This will prevent Poetry from reinstalling system packages in the venv when they are already installed with a compatible version. Fixes: #6035 Signed-off-by: Aurélien Bompard --- src/poetry/utils/env/virtual_env.py | 13 +++++++++++++ tests/utils/test_env.py | 13 +++++++++++++ 2 files changed, 26 insertions(+) diff --git a/src/poetry/utils/env/virtual_env.py b/src/poetry/utils/env/virtual_env.py index 4487683fc56..e213394cb7c 100644 --- a/src/poetry/utils/env/virtual_env.py +++ b/src/poetry/utils/env/virtual_env.py @@ -3,6 +3,7 @@ import json import os import re +import sys from contextlib import contextmanager from copy import deepcopy @@ -20,6 +21,7 @@ from poetry.utils.env.script_strings import GET_PYTHON_VERSION from poetry.utils.env.script_strings import GET_SYS_PATH from poetry.utils.env.script_strings import GET_SYS_TAGS +from poetry.utils.env.system_env import SystemEnv if TYPE_CHECKING: @@ -133,3 +135,14 @@ def temp_environ(self) -> Iterator[None]: def _updated_path(self) -> str: return os.pathsep.join([str(self._bin_dir), os.environ.get("PATH", "")]) + + def includes_system_site_packages(self) -> bool: + pyvenv_cfg = self._path / "pyvenv.cfg" + return "include-system-site-packages = true" in pyvenv_cfg.read_text() + + def is_path_relative_to_lib(self, path: Path) -> bool: + system_env = SystemEnv(Path(sys.prefix)) + return ( + self.includes_system_site_packages() + and system_env.is_path_relative_to_lib(path) + ) or super().is_path_relative_to_lib(path) diff --git a/tests/utils/test_env.py b/tests/utils/test_env.py index 269287dea5e..1ca5893e816 100644 --- a/tests/utils/test_env.py +++ b/tests/utils/test_env.py @@ -18,6 +18,7 @@ from poetry.repositories.installed_repository import InstalledRepository from poetry.toml.file import TOMLFile from poetry.utils._compat import WINDOWS +from poetry.utils._compat import metadata from poetry.utils.env import GET_BASE_PREFIX from poetry.utils.env import GET_PYTHON_VERSION_ONELINER from poetry.utils.env import EnvCommandError @@ -1462,8 +1463,20 @@ def test_env_system_packages(tmp_path: Path, poetry: Poetry) -> None: pyvenv_cfg = venv_path / "pyvenv.cfg" EnvManager(poetry).build_venv(path=venv_path, flags={"system-site-packages": True}) + env = VirtualEnv(venv_path) assert "include-system-site-packages = true" in pyvenv_cfg.read_text() + assert env.includes_system_site_packages() + + +def test_env_system_packages_are_relative_to_lib( + tmp_path: Path, poetry: Poetry +) -> None: + venv_path = tmp_path / "venv" + EnvManager(poetry).build_venv(path=venv_path, flags={"system-site-packages": True}) + env = VirtualEnv(venv_path) + pytest_dist = metadata.distribution("pytest") + assert env.is_path_relative_to_lib(pytest_dist._path) # type: ignore[attr-defined] @pytest.mark.parametrize( From 113120b16af3bb5a216b667b9e4b467f5323c92e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Bompard?= Date: Thu, 24 Aug 2023 15:24:00 +0200 Subject: [PATCH 2/6] The system env can have packages in more than purelib and platlib MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Aurélien Bompard --- src/poetry/utils/env/base_env.py | 5 ++++- src/poetry/utils/env/system_env.py | 4 ++++ tests/utils/test_env.py | 11 +++++++++-- 3 files changed, 17 insertions(+), 3 deletions(-) diff --git a/src/poetry/utils/env/base_env.py b/src/poetry/utils/env/base_env.py index 487a0aa2276..eb8dac6cf97 100644 --- a/src/poetry/utils/env/base_env.py +++ b/src/poetry/utils/env/base_env.py @@ -219,8 +219,11 @@ def platlib(self) -> Path: return self._platlib + def _get_lib_dirs(self) -> list[Path]: + return [self.purelib, self.platlib] + def is_path_relative_to_lib(self, path: Path) -> bool: - for lib_path in [self.purelib, self.platlib]: + for lib_path in self._get_lib_dirs(): with contextlib.suppress(ValueError): path.relative_to(lib_path) return True diff --git a/src/poetry/utils/env/system_env.py b/src/poetry/utils/env/system_env.py index 81ec7400d67..e088e684480 100644 --- a/src/poetry/utils/env/system_env.py +++ b/src/poetry/utils/env/system_env.py @@ -2,6 +2,7 @@ import os import platform +import site import sys import sysconfig @@ -87,3 +88,6 @@ def get_pip_version(self) -> Version: def is_venv(self) -> bool: return self._path != self._base + + def _get_lib_dirs(self) -> list[Path]: + return super()._get_lib_dirs() + [Path(d) for d in site.getsitepackages()] diff --git a/tests/utils/test_env.py b/tests/utils/test_env.py index 1ca5893e816..d7d64a832b1 100644 --- a/tests/utils/test_env.py +++ b/tests/utils/test_env.py @@ -1,6 +1,8 @@ from __future__ import annotations +import contextlib import os +import site import subprocess import sys @@ -1475,8 +1477,13 @@ def test_env_system_packages_are_relative_to_lib( venv_path = tmp_path / "venv" EnvManager(poetry).build_venv(path=venv_path, flags={"system-site-packages": True}) env = VirtualEnv(venv_path) - pytest_dist = metadata.distribution("pytest") - assert env.is_path_relative_to_lib(pytest_dist._path) # type: ignore[attr-defined] + site_dir = Path(site.getsitepackages()[-1]) + for dist in metadata.distributions(): + # Emulate is_relative_to, only available in 3.9+ + with contextlib.suppress(ValueError): + dist._path.relative_to(site_dir) # type: ignore[attr-defined] + break + assert env.is_path_relative_to_lib(dist._path) # type: ignore[attr-defined] @pytest.mark.parametrize( From 40bd40949926245b9a597360560632cb0531de1b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Bompard?= Date: Fri, 25 Aug 2023 18:37:11 +0200 Subject: [PATCH 3/6] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Randy Döring <30527984+radoering@users.noreply.github.com> --- src/poetry/utils/env/virtual_env.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/poetry/utils/env/virtual_env.py b/src/poetry/utils/env/virtual_env.py index e213394cb7c..264b9c378d6 100644 --- a/src/poetry/utils/env/virtual_env.py +++ b/src/poetry/utils/env/virtual_env.py @@ -141,8 +141,7 @@ def includes_system_site_packages(self) -> bool: return "include-system-site-packages = true" in pyvenv_cfg.read_text() def is_path_relative_to_lib(self, path: Path) -> bool: - system_env = SystemEnv(Path(sys.prefix)) - return ( + return super().is_path_relative_to_lib(path) or ( self.includes_system_site_packages() - and system_env.is_path_relative_to_lib(path) - ) or super().is_path_relative_to_lib(path) + and SystemEnv(Path(sys.prefix)).is_path_relative_to_lib(path) + ) From 3c1b911a2743dcfca63a258cc0b9527856b715f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Bompard?= Date: Fri, 25 Aug 2023 19:10:28 +0200 Subject: [PATCH 4/6] Cache the includes_system_site_packages property MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Aurélien Bompard --- src/poetry/utils/env/virtual_env.py | 4 +++- tests/utils/test_env.py | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/poetry/utils/env/virtual_env.py b/src/poetry/utils/env/virtual_env.py index 264b9c378d6..ced6293153d 100644 --- a/src/poetry/utils/env/virtual_env.py +++ b/src/poetry/utils/env/virtual_env.py @@ -7,6 +7,7 @@ from contextlib import contextmanager from copy import deepcopy +from functools import cached_property from pathlib import Path from typing import TYPE_CHECKING from typing import Any @@ -136,12 +137,13 @@ def temp_environ(self) -> Iterator[None]: def _updated_path(self) -> str: return os.pathsep.join([str(self._bin_dir), os.environ.get("PATH", "")]) + @cached_property def includes_system_site_packages(self) -> bool: pyvenv_cfg = self._path / "pyvenv.cfg" return "include-system-site-packages = true" in pyvenv_cfg.read_text() def is_path_relative_to_lib(self, path: Path) -> bool: return super().is_path_relative_to_lib(path) or ( - self.includes_system_site_packages() + self.includes_system_site_packages and SystemEnv(Path(sys.prefix)).is_path_relative_to_lib(path) ) diff --git a/tests/utils/test_env.py b/tests/utils/test_env.py index d7d64a832b1..3845a7e1177 100644 --- a/tests/utils/test_env.py +++ b/tests/utils/test_env.py @@ -1468,7 +1468,7 @@ def test_env_system_packages(tmp_path: Path, poetry: Poetry) -> None: env = VirtualEnv(venv_path) assert "include-system-site-packages = true" in pyvenv_cfg.read_text() - assert env.includes_system_site_packages() + assert env.includes_system_site_packages def test_env_system_packages_are_relative_to_lib( From 2268cbdd88733d3c48eb890397a71470b51c43fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Randy=20D=C3=B6ring?= <30527984+radoering@users.noreply.github.com> Date: Sat, 26 Aug 2023 12:46:40 +0200 Subject: [PATCH 5/6] Add test for original issue --- .../repositories/test_installed_repository.py | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/tests/repositories/test_installed_repository.py b/tests/repositories/test_installed_repository.py index b69df38b6f4..2dbf01410b3 100644 --- a/tests/repositories/test_installed_repository.py +++ b/tests/repositories/test_installed_repository.py @@ -1,5 +1,6 @@ from __future__ import annotations +import shutil import zipfile from pathlib import Path @@ -10,7 +11,9 @@ from poetry.repositories.installed_repository import InstalledRepository from poetry.utils._compat import metadata +from poetry.utils.env import EnvManager from poetry.utils.env import MockEnv as BaseMockEnv +from poetry.utils.env import VirtualEnv if TYPE_CHECKING: @@ -18,6 +21,10 @@ from poetry.core.packages.package import Package from pytest_mock.plugin import MockerFixture + from poetry.poetry import Poetry + from tests.types import FixtureDirGetter + from tests.types import ProjectFactory + FIXTURES_DIR = Path(__file__).parent / "fixtures" ENV_DIR = (FIXTURES_DIR / "installed").resolve() SITE_PURELIB = ENV_DIR / "lib" / "python3.7" / "site-packages" @@ -104,6 +111,11 @@ def get_package_from_repository( return None +@pytest.fixture +def poetry(project_factory: ProjectFactory, fixture_dir: FixtureDirGetter) -> Poetry: + return project_factory("simple", source=fixture_dir("simple_project")) + + def test_load_successful(repository: InstalledRepository) -> None: assert len(repository.packages) == len(INSTALLED_RESULTS) @@ -301,3 +313,27 @@ def test_load_pep_610_compliant_editable_directory_packages( assert package.source_type == "directory" assert package.source_url == "/path/to/distributions/directory-pep-610" assert package.develop + + +def test_system_site_packages_source_type( + tmp_path: Path, mocker: MockerFixture, poetry: Poetry +) -> None: + """ + The source type of system site packages + must not be falsely identified as "directory". + """ + venv_path = tmp_path / "venv" + site_path = tmp_path / "site" + 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)]) + + EnvManager(poetry).build_venv(path=venv_path, flags={"system-site-packages": True}) + env = VirtualEnv(venv_path) + installed_repository = InstalledRepository.load(env) + + source_types = { + package.name: package.source_type for package in installed_repository.packages + } + assert source_types == {"cleo": None, "directory-pep-610": "directory"} From 3fe197b89bf96454bc1cb59a323f4ba7e5530c06 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Bompard?= Date: Mon, 28 Aug 2023 06:29:57 +0200 Subject: [PATCH 6/6] Make the detection of system-site-packages more robust MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit References: - https://github.com/python/cpython/blob/042aa88bcc6541cb8b312f1119452f7a58a5b4df/Lib/site.py#L515-L522 - https://peps.python.org/pep-0405/#specification Signed-off-by: Aurélien Bompard --- src/poetry/utils/env/virtual_env.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/poetry/utils/env/virtual_env.py b/src/poetry/utils/env/virtual_env.py index ced6293153d..c8a81153f66 100644 --- a/src/poetry/utils/env/virtual_env.py +++ b/src/poetry/utils/env/virtual_env.py @@ -140,7 +140,14 @@ def _updated_path(self) -> str: @cached_property def includes_system_site_packages(self) -> bool: pyvenv_cfg = self._path / "pyvenv.cfg" - return "include-system-site-packages = true" in pyvenv_cfg.read_text() + return ( + re.search( + r"^\s*include-system-site-packages\s*=\s*true\s*$", + pyvenv_cfg.read_text(), + re.IGNORECASE | re.MULTILINE, + ) + is not None + ) def is_path_relative_to_lib(self, path: Path) -> bool: return super().is_path_relative_to_lib(path) or (