From 4ba86429be0a560f2f067b8f453631a81f94d3f6 Mon Sep 17 00:00:00 2001 From: Arun Babu Neelicattu Date: Thu, 19 May 2022 14:54:28 +0200 Subject: [PATCH 1/3] tests: ensure mock env signature matches base class --- tests/repositories/test_installed_repository.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/repositories/test_installed_repository.py b/tests/repositories/test_installed_repository.py index f6cb2ceb8a6..d76b2276f81 100644 --- a/tests/repositories/test_installed_repository.py +++ b/tests/repositories/test_installed_repository.py @@ -56,8 +56,8 @@ def paths(self) -> dict[str, Path]: } @property - def sys_path(self) -> list[Path]: - return [ENV_DIR, SITE_PLATLIB, SITE_PURELIB] + def sys_path(self) -> list[str]: + return [str(path) for path in [ENV_DIR, SITE_PLATLIB, SITE_PURELIB]] @pytest.fixture From 592ba2617a558f1b79b35646e7a5e13ae6f40868 Mon Sep 17 00:00:00 2001 From: Arun Babu Neelicattu Date: Thu, 19 May 2022 14:55:01 +0200 Subject: [PATCH 2/3] tests: cleanup installed repo mocking --- .../repositories/test_installed_repository.py | 20 +++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/tests/repositories/test_installed_repository.py b/tests/repositories/test_installed_repository.py index d76b2276f81..055533f24a5 100644 --- a/tests/repositories/test_installed_repository.py +++ b/tests/repositories/test_installed_repository.py @@ -65,12 +65,8 @@ def env() -> MockEnv: return MockEnv(path=ENV_DIR) -@pytest.fixture -def repository(mocker: MockerFixture, env: MockEnv) -> InstalledRepository: - mocker.patch( - "poetry.utils._compat.metadata.Distribution.discover", - return_value=INSTALLED_RESULTS, - ) +@pytest.fixture(autouse=True) +def mock_git_info(mocker: MockerFixture) -> None: mocker.patch( "poetry.vcs.git.Git.info", return_value=namedtuple("GitRepoLocalInfo", "origin revision")( @@ -78,7 +74,19 @@ def repository(mocker: MockerFixture, env: MockEnv) -> InstalledRepository: revision="bb058f6b78b2d28ef5d9a5e759cfa179a1a713d6", ), ) + + +@pytest.fixture(autouse=True) +def mock_installed_repository_vendors(mocker: MockerFixture) -> None: mocker.patch("poetry.repositories.installed_repository._VENDORS", str(VENDOR_DIR)) + + +@pytest.fixture +def repository(mocker: MockerFixture, env: MockEnv) -> InstalledRepository: + mocker.patch( + "poetry.utils._compat.metadata.Distribution.discover", + return_value=INSTALLED_RESULTS, + ) return InstalledRepository.load(env) From 5f68912cfd4a4f61b0134485eb7212531974de99 Mon Sep 17 00:00:00 2001 From: Arun Babu Neelicattu Date: Thu, 19 May 2022 14:57:43 +0200 Subject: [PATCH 3/3] repo: handle invalid distributions gracefully This change ensures that poetry does not bail out when encountering a bad distributions in `sys_path`. This behaviour is similar to how `pip` handles this today. In addition to ignoring these distributions, we also issue a warning log so users can choose to act on this. Further, this change also handles a scenario where an empty path is present in the `sys_path`. --- .../repositories/installed_repository.py | 31 +++++++++++++++++-- .../repositories/test_installed_repository.py | 22 +++++++++++++ 2 files changed, 50 insertions(+), 3 deletions(-) diff --git a/src/poetry/repositories/installed_repository.py b/src/poetry/repositories/installed_repository.py index d7a6d9abb6e..28839550f1b 100644 --- a/src/poetry/repositories/installed_repository.py +++ b/src/poetry/repositories/installed_repository.py @@ -2,6 +2,7 @@ import itertools import json +import logging from pathlib import Path from typing import TYPE_CHECKING @@ -28,6 +29,9 @@ FileNotFoundError = OSError +logger = logging.getLogger(__name__) + + class InstalledRepository(Repository): @classmethod def get_package_paths(cls, env: Env, name: str) -> set[Path]: @@ -233,20 +237,41 @@ def load(cls, env: Env, with_dependencies: bool = False) -> InstalledRepository: repo = cls() seen = set() + skipped = set() for entry in reversed(env.sys_path): + if not entry.strip(): + logger.debug( + "Project environment contains an empty path in sys_path," + " ignoring." + ) + continue + for distribution in sorted( metadata.distributions( # type: ignore[no-untyped-call] path=[entry], ), key=lambda d: str(d._path), # type: ignore[attr-defined] ): - name = canonicalize_name(distribution.metadata["name"]) + path = Path(str(distribution._path)) # type: ignore[attr-defined] - if name in seen: + if path in skipped: continue - path = Path(str(distribution._path)) # type: ignore[attr-defined] + try: + name = canonicalize_name(distribution.metadata["name"]) + except TypeError: + logger.warning( + "Project environment contains an invalid distribution" + " (%s). Consider removing it manually or recreate the" + " environment.", + path, + ) + skipped.add(path) + continue + + if name in seen: + continue try: path.relative_to(_VENDORS) diff --git a/tests/repositories/test_installed_repository.py b/tests/repositories/test_installed_repository.py index 055533f24a5..e6c36349ec3 100644 --- a/tests/repositories/test_installed_repository.py +++ b/tests/repositories/test_installed_repository.py @@ -13,6 +13,7 @@ if TYPE_CHECKING: + from _pytest.logging import LogCaptureFixture from poetry.core.packages.package import Package from pytest_mock.plugin import MockerFixture @@ -103,6 +104,27 @@ def test_load_successful(repository: InstalledRepository): assert len(repository.packages) == len(INSTALLED_RESULTS) - 1 +def test_load_successful_with_invalid_distribution( + caplog: LogCaptureFixture, mocker: MockerFixture, env: MockEnv, tmp_dir: str +) -> None: + invalid_dist_info = Path(tmp_dir) / "site-packages" / "invalid-0.1.0.dist-info" + invalid_dist_info.mkdir(parents=True) + mocker.patch( + "poetry.utils._compat.metadata.Distribution.discover", + return_value=INSTALLED_RESULTS + [metadata.PathDistribution(invalid_dist_info)], + ) + repository_with_invalid_distribution = InstalledRepository.load(env) + + assert ( + len(repository_with_invalid_distribution.packages) == len(INSTALLED_RESULTS) - 1 + ) + assert len(caplog.messages) == 1 + + message = caplog.messages[0] + assert message.startswith("Project environment contains an invalid distribution") + assert str(invalid_dist_info) in message + + def test_load_ensure_isolation(repository: InstalledRepository): package = get_package_from_repository("attrs", repository) assert package is None