From 37e3f9f41a44b172d68a5ef47fec89e990665724 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Randy=20D=C3=B6ring?= <30527984+radoering@users.noreply.github.com> Date: Sun, 13 Aug 2023 13:47:25 +0200 Subject: [PATCH 1/3] installer: respect source if the same version of a package has been locked from different sources Co-authored-by: David Hotham --- src/poetry/installation/installer.py | 17 +-- src/poetry/utils/env/mock_env.py | 8 ++ tests/console/commands/test_add.py | 23 ++-- tests/installation/test_installer.py | 167 ++++++++++++++++++++++++++- 4 files changed, 189 insertions(+), 26 deletions(-) diff --git a/src/poetry/installation/installer.py b/src/poetry/installation/installer.py index a5250750804..6661b5f49fe 100644 --- a/src/poetry/installation/installer.py +++ b/src/poetry/installation/installer.py @@ -286,17 +286,20 @@ def _do_install(self) -> int: ) # We resolve again by only using the lock file - pool = RepositoryPool(ignore_repository_names=True, config=self._config) + pool = RepositoryPool(config=self._config) - # Making a new repo containing the packages - # newly resolved and the ones from the current lock file - repo = Repository("poetry-repo") + # Make a new collection of repositories containing the packages newly resolved + # and the ones from the current lock file. for package in lockfile_repo.packages + locked_repository.packages: - if not package.is_direct_origin() and not repo.has_package(package): + if not package.is_direct_origin(): + repo_name = package.source_reference or "PyPI" + try: + repo = pool.repository(repo_name) + except IndexError: + repo = Repository(repo_name) + pool.add_repository(repo) repo.add_package(package) - pool.add_repository(repo) - solver = Solver( root, pool, diff --git a/src/poetry/utils/env/mock_env.py b/src/poetry/utils/env/mock_env.py index 35ea1eb07e8..225b88f69be 100644 --- a/src/poetry/utils/env/mock_env.py +++ b/src/poetry/utils/env/mock_env.py @@ -16,8 +16,10 @@ class MockEnv(NullEnv): def __init__( self, version_info: tuple[int, int, int] = (3, 7, 0), + *, python_implementation: str = "CPython", platform: str = "darwin", + platform_machine: str = "amd64", os_name: str = "posix", is_venv: bool = False, pip_version: str = "19.1", @@ -31,6 +33,7 @@ def __init__( self._version_info = version_info self._python_implementation = python_implementation self._platform = platform + self._platform_machine = platform_machine self._os_name = os_name self._is_venv = is_venv self._pip_version: Version = Version.parse(pip_version) @@ -42,6 +45,10 @@ def __init__( def platform(self) -> str: return self._platform + @property + def platform_machine(self) -> str: + return self._platform_machine + @property def os(self) -> str: return self._os_name @@ -67,6 +74,7 @@ def get_marker_env(self) -> dict[str, Any]: marker_env["python_version"] = ".".join(str(v) for v in self._version_info[:2]) marker_env["python_full_version"] = ".".join(str(v) for v in self._version_info) marker_env["sys_platform"] = self._platform + marker_env["platform_machine"] = self._platform_machine marker_env["interpreter_name"] = self._python_implementation.lower() marker_env["interpreter_version"] = "cp" + "".join( str(v) for v in self._version_info[:2] diff --git a/tests/console/commands/test_add.py b/tests/console/commands/test_add.py index 33d7848b482..c8fd87a5766 100644 --- a/tests/console/commands/test_add.py +++ b/tests/console/commands/test_add.py @@ -902,21 +902,16 @@ def test_add_constraint_with_source( mocker: MockerFixture, ) -> None: repo = LegacyRepository(name="my-index", url="https://my-index.fake") - repo.add_package(get_package("cachy", "0.2.0")) - mocker.patch.object( - repo, - "_find_packages", - wraps=lambda _, name: [ - Package( - "cachy", - Version.parse("0.2.0"), - source_type="legacy", - source_reference=repo.name, - source_url=repo._url, - yanked=False, - ) - ], + package = Package( + "cachy", + Version.parse("0.2.0"), + source_type="legacy", + source_reference=repo.name, + source_url=repo._url, + yanked=False, ) + mocker.patch.object(repo, "package", return_value=package) + mocker.patch.object(repo, "_find_packages", wraps=lambda _, name: [package]) poetry.pool.add_repository(repo) diff --git a/tests/installation/test_installer.py b/tests/installation/test_installer.py index d3017464754..0d4ec364778 100644 --- a/tests/installation/test_installer.py +++ b/tests/installation/test_installer.py @@ -2558,9 +2558,8 @@ def test_installer_should_use_the_locked_version_of_git_dependencies_without_ref ) -# https://github.com/python-poetry/poetry/issues/6710 @pytest.mark.parametrize("env_platform", ["darwin", "linux"]) -def test_installer_distinguishes_locked_packages_by_source( +def test_installer_distinguishes_locked_packages_with_local_version_by_source( pool: RepositoryPool, locker: Locker, installed: CustomInstalledRepository, @@ -2569,6 +2568,7 @@ def test_installer_distinguishes_locked_packages_by_source( package: ProjectPackage, env_platform: str, ) -> None: + """https://github.com/python-poetry/poetry/issues/6710""" # Require 1.11.0+cpu from pytorch for most platforms, but specify 1.11.0 and pypi on # darwin. package.add_dependency( @@ -2661,6 +2661,110 @@ def test_installer_distinguishes_locked_packages_by_source( ) +@pytest.mark.parametrize("env_platform_machine", ["aarch64", "amd64"]) +def test_installer_distinguishes_locked_packages_with_same_version_by_source( + pool: RepositoryPool, + locker: Locker, + installed: CustomInstalledRepository, + config: Config, + repo: Repository, + package: ProjectPackage, + env_platform_machine: str, +) -> None: + """https://github.com/python-poetry/poetry/issues/8303""" + package.add_dependency( + Factory.create_dependency( + "kivy", + { + "version": "2.2.1", + "markers": "platform_machine == 'aarch64'", + "source": "pywheels", + }, + ) + ) + package.add_dependency( + Factory.create_dependency( + "kivy", + { + "version": "2.2.1", + "markers": "platform_machine != 'aarch64'", + "source": "PyPI", + }, + ) + ) + + # Locking finds both the pypi and the pyhweels packages. + locker.locked(True) + locker.mock_lock_data( + { + "package": [ + { + "name": "kivy", + "version": "2.2.1", + "optional": False, + "files": [], + "python-versions": "*", + }, + { + "name": "kivy", + "version": "2.2.1", + "optional": False, + "files": [], + "python-versions": "*", + "source": { + "type": "legacy", + "url": "https://www.piwheels.org/simple", + "reference": "pywheels", + }, + }, + ], + "metadata": { + "python-versions": "*", + "platform": "*", + "content-hash": "123456789", + }, + } + ) + installer = Installer( + NullIO(), + MockEnv(platform_machine=env_platform_machine), + package, + locker, + pool, + config, + installed=installed, + executor=Executor( + MockEnv(platform_machine=env_platform_machine), + pool, + config, + NullIO(), + ), + ) + result = installer.run() + assert result == 0 + + # Results of installation are consistent with the platform requirements. + version = "2.2.1" + if env_platform_machine == "aarch64": + source_type = "legacy" + source_url = "https://www.piwheels.org/simple" + source_reference = "pywheels" + else: + source_type = None + source_url = None + source_reference = None + + assert isinstance(installer.executor, Executor) + assert len(installer.executor.installations) == 1 + assert installer.executor.installations[0] == Package( + "kivy", + version, + source_type=source_type, + source_url=source_url, + source_reference=source_reference, + ) + + @pytest.mark.parametrize("env_platform", ["darwin", "linux"]) def test_explicit_source_dependency_with_direct_origin_dependency( pool: RepositoryPool, @@ -2675,12 +2779,13 @@ def test_explicit_source_dependency_with_direct_origin_dependency( A dependency with explicit source should not be satisfied by a direct origin dependency even if there is a version match. """ + demo_url = "https://python-poetry.org/distributions/demo-0.1.0-py2.py3-none-any.whl" package.add_dependency( Factory.create_dependency( "demo", { "markers": "sys_platform != 'darwin'", - "url": "https://python-poetry.org/distributions/demo-0.1.0-py2.py3-none-any.whl", + "url": demo_url, }, ) ) @@ -2698,6 +2803,50 @@ def test_explicit_source_dependency_with_direct_origin_dependency( repo.add_package(get_package("pendulum", "1.4.4")) repo.add_package(get_package("demo", "0.1.0")) + # Locking finds both the direct origin and the explicit source packages. + locker.locked(True) + locker.mock_lock_data( + { + "package": [ + { + "name": "demo", + "version": "0.1.0", + "optional": False, + "files": [], + "python-versions": "*", + "dependencies": {"pendulum": ">=1.4.4"}, + "source": { + "type": "url", + "url": demo_url, + }, + }, + { + "name": "demo", + "version": "0.1.0", + "optional": False, + "files": [], + "python-versions": "*", + "source": { + "type": "legacy", + "url": "https://www.demo.org/simple", + "reference": "repo", + }, + }, + { + "name": "pendulum", + "version": "1.4.4", + "optional": False, + "files": [], + "python-versions": "*", + }, + ], + "metadata": { + "python-versions": "*", + "platform": "*", + "content-hash": "123456789", + }, + } + ) installer = Installer( NullIO(), MockEnv(platform=env_platform), @@ -2725,8 +2874,16 @@ def test_explicit_source_dependency_with_direct_origin_dependency( "demo", "0.1.0", source_type="url", - source_url="https://python-poetry.org/distributions/demo-0.1.0-py2.py3-none-any.whl", + source_url=demo_url, ), ] else: - assert installer.executor.installations == [Package("demo", "0.1.0")] + assert installer.executor.installations == [ + Package( + "demo", + "0.1.0", + source_type="legacy", + source_url="https://www.demo.org/simple", + source_reference="repo", + ) + ] From a0c85aab3014ac556a3dbead0b89228a4efc1cb6 Mon Sep 17 00:00:00 2001 From: David Hotham Date: Sun, 13 Aug 2023 13:17:58 +0100 Subject: [PATCH 2/3] refactor --- src/poetry/console/commands/show.py | 3 +-- src/poetry/installation/installer.py | 15 ++---------- src/poetry/repositories/repository_pool.py | 28 +++++++++++++++++----- 3 files changed, 25 insertions(+), 21 deletions(-) diff --git a/src/poetry/console/commands/show.py b/src/poetry/console/commands/show.py index b5ae58bdb07..8b22e089b8b 100644 --- a/src/poetry/console/commands/show.py +++ b/src/poetry/console/commands/show.py @@ -211,8 +211,7 @@ def _display_packages_information( from poetry.utils.helpers import get_package_version_display_string locked_packages = locked_repository.packages - pool = RepositoryPool(ignore_repository_names=True, config=self.poetry.config) - pool.add_repository(locked_repository) + pool = RepositoryPool.from_packages(locked_packages, self.poetry.config) solver = Solver( root, pool=pool, diff --git a/src/poetry/installation/installer.py b/src/poetry/installation/installer.py index 6661b5f49fe..c30dd1b45ea 100644 --- a/src/poetry/installation/installer.py +++ b/src/poetry/installation/installer.py @@ -286,19 +286,8 @@ def _do_install(self) -> int: ) # We resolve again by only using the lock file - pool = RepositoryPool(config=self._config) - - # Make a new collection of repositories containing the packages newly resolved - # and the ones from the current lock file. - for package in lockfile_repo.packages + locked_repository.packages: - if not package.is_direct_origin(): - repo_name = package.source_reference or "PyPI" - try: - repo = pool.repository(repo_name) - except IndexError: - repo = Repository(repo_name) - pool.add_repository(repo) - repo.add_package(package) + packages = lockfile_repo.packages + locked_repository.packages + pool = RepositoryPool.from_packages(packages, self._config) solver = Solver( root, diff --git a/src/poetry/repositories/repository_pool.py b/src/poetry/repositories/repository_pool.py index 0d8a861951a..c1dd7ccb9a4 100644 --- a/src/poetry/repositories/repository_pool.py +++ b/src/poetry/repositories/repository_pool.py @@ -11,6 +11,7 @@ from poetry.config.config import Config from poetry.repositories.abstract_repository import AbstractRepository from poetry.repositories.exceptions import PackageNotFound +from poetry.repositories.repository import Repository from poetry.utils.cache import ArtifactCache @@ -19,8 +20,6 @@ from poetry.core.packages.dependency import Dependency from poetry.core.packages.package import Package - from poetry.repositories.repository import Repository - class Priority(IntEnum): # The order of the members below dictates the actual priority. The first member has @@ -42,13 +41,11 @@ class RepositoryPool(AbstractRepository): def __init__( self, repositories: list[Repository] | None = None, - ignore_repository_names: bool = False, *, config: Config | None = None, ) -> None: super().__init__("poetry-repository-pool") self._repositories: OrderedDict[str, PrioritizedRepository] = OrderedDict() - self._ignore_repository_names = ignore_repository_names if repositories is None: repositories = [] @@ -59,6 +56,25 @@ def __init__( cache_dir=(config or Config.create()).artifacts_cache_directory ) + @staticmethod + def from_packages(packages: list[Package], config: Config | None) -> RepositoryPool: + pool = RepositoryPool(config=config) + for package in packages: + if package.is_direct_origin(): + continue + + repo_name = package.source_reference or "PyPI" + try: + repo = pool.repository(repo_name) + except IndexError: + repo = Repository(repo_name) + pool.add_repository(repo) + + if not repo.has_package(package): + repo.add_package(package) + + return pool + @property def repositories(self) -> list[Repository]: """ @@ -166,7 +182,7 @@ def package( extras: list[str] | None = None, repository_name: str | None = None, ) -> Package: - if repository_name and not self._ignore_repository_names: + if repository_name: return self.repository(repository_name).package( name, version, extras=extras ) @@ -180,7 +196,7 @@ def package( def find_packages(self, dependency: Dependency) -> list[Package]: repository_name = dependency.source_name - if repository_name and not self._ignore_repository_names: + if repository_name: return self.repository(repository_name).find_packages(dependency) packages: list[Package] = [] From 38001dd8028b655738b496bb679a1e389976942b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Randy=20D=C3=B6ring?= <30527984+radoering@users.noreply.github.com> Date: Sun, 13 Aug 2023 17:52:30 +0200 Subject: [PATCH 3/3] be nice to plugin authors --- src/poetry/repositories/repository_pool.py | 12 ++++++++++++ tests/repositories/test_repository_pool.py | 11 +++++++++++ 2 files changed, 23 insertions(+) diff --git a/src/poetry/repositories/repository_pool.py b/src/poetry/repositories/repository_pool.py index c1dd7ccb9a4..9d6338f4ff2 100644 --- a/src/poetry/repositories/repository_pool.py +++ b/src/poetry/repositories/repository_pool.py @@ -20,6 +20,8 @@ from poetry.core.packages.dependency import Dependency from poetry.core.packages.package import Package +_SENTINEL = object() + class Priority(IntEnum): # The order of the members below dictates the actual priority. The first member has @@ -41,6 +43,7 @@ class RepositoryPool(AbstractRepository): def __init__( self, repositories: list[Repository] | None = None, + ignore_repository_names: object = _SENTINEL, *, config: Config | None = None, ) -> None: @@ -56,6 +59,15 @@ def __init__( cache_dir=(config or Config.create()).artifacts_cache_directory ) + if ignore_repository_names is not _SENTINEL: + warnings.warn( + "The 'ignore_repository_names' argument to 'RepositoryPool.__init__' is" + " deprecated. It has no effect anymore and will be removed in a future" + " version.", + DeprecationWarning, + stacklevel=2, + ) + @staticmethod def from_packages(packages: list[Package], config: Config | None) -> RepositoryPool: pool = RepositoryPool(config=config) diff --git a/tests/repositories/test_repository_pool.py b/tests/repositories/test_repository_pool.py index 9ec5f68401e..2a62f360815 100644 --- a/tests/repositories/test_repository_pool.py +++ b/tests/repositories/test_repository_pool.py @@ -38,6 +38,17 @@ def test_repository_no_repository() -> None: pool.repository("foo") +def test_repository_deprecated_ignore_repository_names() -> None: + with pytest.warns(DeprecationWarning): + RepositoryPool(ignore_repository_names=True) + with pytest.warns(DeprecationWarning): + RepositoryPool(ignore_repository_names=False) + with pytest.warns(DeprecationWarning): + RepositoryPool(None, True) + with pytest.warns(DeprecationWarning): + RepositoryPool(None, False) + + def test_adding_repositories_with_same_name_twice_raises_value_error() -> None: repo1 = Repository("repo") repo2 = Repository("repo")