From 301b28805330edf3bb684279f5ca8439d361c5ee Mon Sep 17 00:00:00 2001 From: Reese Hyde Date: Sat, 13 Jul 2024 20:12:05 -0400 Subject: [PATCH 01/39] add breaking unit test for conflicting pytorch cpu/cuda extras --- tests/installation/test_installer.py | 118 ++++++++++++++++++++++++++- 1 file changed, 117 insertions(+), 1 deletion(-) diff --git a/tests/installation/test_installer.py b/tests/installation/test_installer.py index 8776482aa4d..760cf261bd2 100644 --- a/tests/installation/test_installer.py +++ b/tests/installation/test_installer.py @@ -5,7 +5,7 @@ import shutil from pathlib import Path -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, IO from typing import Any import pytest @@ -2536,6 +2536,122 @@ def test_installer_distinguishes_locked_packages_with_local_version_by_source( ) +@pytest.mark.parametrize("extra", [None, "cpu", "cuda"]) +def test_installer_distinguishes_locked_packages_with_local_version_by_extra( + pool: RepositoryPool, + locker: Locker, + installed: CustomInstalledRepository, + config: Config, + repo: Repository, + package: ProjectPackage, + extra: str | None, +) -> None: + """https://github.com/python-poetry/poetry/issues/6409; https://github.com/python-poetry/poetry/issues/6419""" + # Require 1.11.0+cpu from pytorch for when extra is 'cpu', or 1.11.0+cuda when extra is 'cuda' + cpu_dep = Factory.create_dependency( + "torch", + { + "version": "1.11.0+cpu", + "markers": "extra == 'cpu' and extra != 'cuda'", + "source": "pytorch-cpu", + }, + ) + cuda_dep = Factory.create_dependency( + "torch", + { + "version": "1.11.0+cuda", + "markers": "extra != 'cpu' and extra == 'cuda'", + "source": "pytorch-cuda", + }, + ) + package.add_dependency(cpu_dep) + package.add_dependency(cuda_dep) + # We don't want to cheat by only including the correct dependency in the 'extra' mapping + package.extras = { + "cpu": [cpu_dep, cuda_dep], + "cuda": [cpu_dep, cuda_dep], + } + + # Locking finds packages from each pytorch repository + locker.locked(True) + locker.mock_lock_data( + { + "package": [ + { + "name": "torch", + "version": "1.11.0+cpu", + "optional": True, + "files": [], + "python-versions": "*", + "source": { + "type": "legacy", + "url": "https://download.pytorch.org/whl/cpu", + "reference": "pytorch-cpu", + }, + }, + { + "name": "torch", + "version": "1.11.0+cuda", + "optional": True, + "files": [], + "python-versions": "*", + "source": { + "type": "legacy", + "url": "https://download.pytorch.org/whl/cuda", + "reference": "pytorch-cuda", + }, + }, + ], + "metadata": { + "python-versions": "*", + "platform": "*", + "content-hash": "123456789", + }, + "extras": { + "cpu": ["torch"], + "cuda": ["torch"], + } + } + ) + + installer = Installer( + NullIO(), + MockEnv(), + package, + locker, + pool, + config, + installed=installed, + executor=Executor( + MockEnv(), + pool, + config, + NullIO(), + ), + ) + if extra is not None: + installer.extras([extra]) + result = installer.run() + assert result == 0 + + # Results of installation are consistent with the 'extra' input + assert isinstance(installer.executor, Executor) + if extra is None: + assert len(installer.executor.installations) == 0 + else: + assert len(installer.executor.installations) == 1 + version = f"1.11.0+{extra}" + source_url = f"https://download.pytorch.org/whl/{extra}" + source_reference = f"pytorch-{extra}" + assert installer.executor.installations[0] == Package( + "torch", + version, + source_type="legacy", + source_url=source_url, + source_reference=source_reference, + ) + + @pytest.mark.parametrize("env_platform_machine", ["aarch64", "amd64"]) def test_installer_distinguishes_locked_packages_with_same_version_by_source( pool: RepositoryPool, From b018d44e0459b06d3fb0f3346b4523a0799bdda6 Mon Sep 17 00:00:00 2001 From: Reese Hyde Date: Fri, 12 Jul 2024 14:41:05 -0400 Subject: [PATCH 02/39] support project extras in markers: populate 'extras' marker including with active root extras --- src/poetry/installation/installer.py | 2 +- src/poetry/puzzle/provider.py | 32 +++++++++++++++++++++++----- src/poetry/puzzle/solver.py | 4 +++- 3 files changed, 31 insertions(+), 7 deletions(-) diff --git a/src/poetry/installation/installer.py b/src/poetry/installation/installer.py index 5dc1606863f..91d73ed6e24 100644 --- a/src/poetry/installation/installer.py +++ b/src/poetry/installation/installer.py @@ -20,7 +20,6 @@ from cleo.io.io import IO from packaging.utils import NormalizedName - from poetry.core.packages.path_dependency import PathDependency from poetry.core.packages.project_package import ProjectPackage from poetry.config.config import Config @@ -287,6 +286,7 @@ def _do_install(self) -> int: self._installed_repository.packages, locked_repository.packages, NullIO(), + active_root_extras=self._extras, ) # Everything is resolved at this point, so we no longer need # to load deferred dependencies (i.e. VCS, URL and path dependencies) diff --git a/src/poetry/puzzle/provider.py b/src/poetry/puzzle/provider.py index 590afc8c7b1..a0788749e94 100644 --- a/src/poetry/puzzle/provider.py +++ b/src/poetry/puzzle/provider.py @@ -7,7 +7,7 @@ from collections import defaultdict from contextlib import contextmanager -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, Any from typing import ClassVar from typing import cast @@ -117,6 +117,7 @@ def __init__( *, installed: list[Package] | None = None, locked: list[Package] | None = None, + active_root_extras: Collection[NormalizedName] | None = None, ) -> None: self._package = package self._pool = pool @@ -133,6 +134,7 @@ def __init__( self._direct_origin_packages: dict[str, Package] = {} self._locked: dict[NormalizedName, list[DependencyPackage]] = defaultdict(list) self._use_latest: Collection[NormalizedName] = [] + self._active_root_extras = frozenset(active_root_extras) if active_root_extras is not None else frozenset() self._explicit_sources: dict[str, str] = {} for package in locked or []: @@ -457,7 +459,12 @@ def incompatibilities_for( for dep in dependencies if dep.name not in self.UNSAFE_PACKAGES and self._python_constraint.allows_any(dep.python_constraint) - and (not self._env or dep.marker.validate(self._env.marker_env)) + and (not self._env or dep.marker.validate( + self._marker_values( + self._active_root_extras if dependency_package.package.is_root() + else dependency_package.dependency.extras) + ) + ) ] dependencies = self._get_dependencies_with_overrides(_dependencies, package) @@ -541,7 +548,8 @@ def complete_package( if dep.name in self.UNSAFE_PACKAGES: continue - if self._env and not dep.marker.validate(self._env.marker_env): + active_extras = self._active_root_extras if package.is_root() else dependency.extras + if self._env and not dep.marker.validate(self._marker_values(active_extras)): continue if not package.is_root() and ( @@ -601,7 +609,7 @@ def complete_package( # For dependency resolution, markers of duplicate dependencies must be # mutually exclusive. - active_extras = None if package.is_root() else dependency.extras + active_extras = self._active_root_extras if package.is_root() else dependency.extras deps = self._resolve_overlapping_markers(package, deps, active_extras) if len(deps) == 1: @@ -860,7 +868,7 @@ def _is_relevant_marker( get_python_constraint_from_marker(marker) ) and (active_extras is None or marker.validate({"extra": active_extras})) - and (not self._env or marker.validate(self._env.marker_env)) + and (not self._env or marker.validate(self._marker_values(active_extras))) ) def _resolve_overlapping_markers( @@ -953,3 +961,17 @@ def _resolve_overlapping_markers( # dependencies by constraint again. After overlapping markers were # resolved, there might be new dependencies with the same constraint. return self._merge_dependencies_by_constraint(new_dependencies) + + def _marker_values(self, extras: set[NormalizedName] | None = None) -> dict[str, Any]: + """ + Marker values, per: + 1. marker_env of `self._env` + 2. 'extras' will be added to the 'extra' marker if not already present + """ + result = self._env.marker_env.copy() + if extras is not None: + if 'extra' not in result.keys(): + result['extra'] = extras + else: + result['extra'] = set(result['extra']).union(extras) + return result diff --git a/src/poetry/puzzle/solver.py b/src/poetry/puzzle/solver.py index bf080b32970..0f510ed0a37 100644 --- a/src/poetry/puzzle/solver.py +++ b/src/poetry/puzzle/solver.py @@ -39,6 +39,7 @@ def __init__( installed: list[Package], locked: list[Package], io: IO, + active_root_extras: Collection[NormalizedName] | None = None ) -> None: self._package = package self._pool = pool @@ -47,7 +48,8 @@ def __init__( self._io = io self._provider = Provider( - self._package, self._pool, self._io, installed=installed, locked=locked + self._package, self._pool, self._io, installed=installed, locked=locked, + active_root_extras=active_root_extras ) self._overrides: list[dict[Package, dict[str, Dependency]]] = [] From e1adc539e46cc4092a36947c61a7e83d1b41e2ea Mon Sep 17 00:00:00 2001 From: Reese Hyde Date: Sun, 14 Jul 2024 12:21:39 -0400 Subject: [PATCH 03/39] add locking test --- .../fixtures/with-exclusive-extras.test | 20 ++++++++++++ tests/installation/test_installer.py | 32 +++++++++++++++++++ 2 files changed, 52 insertions(+) create mode 100644 tests/installation/fixtures/with-exclusive-extras.test diff --git a/tests/installation/fixtures/with-exclusive-extras.test b/tests/installation/fixtures/with-exclusive-extras.test new file mode 100644 index 00000000000..59d8fc499fe --- /dev/null +++ b/tests/installation/fixtures/with-exclusive-extras.test @@ -0,0 +1,20 @@ +[[package]] +name = "torch" +version = "1.11.0+cpu" +description = "" +optional = true +python-versions = "*" +files = [] + +[[package]] +name = "torch" +version = "1.11.0+cuda" +description = "" +optional = true +python-versions = "*" +files = [] + +[metadata] +python-versions = "*" +lock-version = "2.0" +content-hash = "123456789" diff --git a/tests/installation/test_installer.py b/tests/installation/test_installer.py index 760cf261bd2..edb4514bde3 100644 --- a/tests/installation/test_installer.py +++ b/tests/installation/test_installer.py @@ -1027,6 +1027,38 @@ def test_run_with_dependencies_nested_extras( assert locker.written_data == expected +def test_run_with_exclusive_extras( + installer: Installer, locker: Locker, repo: Repository, package: ProjectPackage +) -> None: + """https://github.com/python-poetry/poetry/issues/6409; https://github.com/python-poetry/poetry/issues/6419""" + torch_cpu = get_package("torch", "1.11.0+cpu") + torch_cuda = get_package("torch", "1.11.0+cuda") + + repo.add_package(torch_cpu) + repo.add_package(torch_cuda) + + package.add_dependency(Factory.create_dependency( + "torch", + { + "version": "1.11.0+cpu", + "markers": "extra == 'cpu' and extra != 'cuda'", + }) + ) + package.add_dependency(Factory.create_dependency( + "torch", + { + "version": "1.11.0+cuda", + "markers": "extra != 'cpu' and extra == 'cuda'", + }) + ) + + result = installer.run() + assert result == 0 + + expected = fixture("with-exclusive-extras") + assert locker.written_data == expected + + @pytest.mark.parametrize("is_locked", [False, True]) @pytest.mark.parametrize("is_installed", [False, True]) @pytest.mark.parametrize("with_extras", [False, True]) From ce435c7ca053ba1e20ec61083feddc1113fb73d0 Mon Sep 17 00:00:00 2001 From: Reese Hyde Date: Sun, 14 Jul 2024 12:22:10 -0400 Subject: [PATCH 04/39] fix default when active root extras not passed --- src/poetry/puzzle/provider.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/poetry/puzzle/provider.py b/src/poetry/puzzle/provider.py index a0788749e94..5f755e4ff18 100644 --- a/src/poetry/puzzle/provider.py +++ b/src/poetry/puzzle/provider.py @@ -134,7 +134,7 @@ def __init__( self._direct_origin_packages: dict[str, Package] = {} self._locked: dict[NormalizedName, list[DependencyPackage]] = defaultdict(list) self._use_latest: Collection[NormalizedName] = [] - self._active_root_extras = frozenset(active_root_extras) if active_root_extras is not None else frozenset() + self._active_root_extras = frozenset(active_root_extras) if active_root_extras is not None else None self._explicit_sources: dict[str, str] = {} for package in locked or []: From 84999c1c08398ef42ef9c967e2274ef1e18b68da Mon Sep 17 00:00:00 2001 From: Reese Hyde Date: Sun, 14 Jul 2024 13:39:29 -0400 Subject: [PATCH 05/39] add unit tests for #834 --- .../with-dependencies-differing-extras.test | 36 ++++++ tests/installation/test_installer.py | 113 ++++++++++++++++++ 2 files changed, 149 insertions(+) create mode 100644 tests/installation/fixtures/with-dependencies-differing-extras.test diff --git a/tests/installation/fixtures/with-dependencies-differing-extras.test b/tests/installation/fixtures/with-dependencies-differing-extras.test new file mode 100644 index 00000000000..e0689b5005b --- /dev/null +++ b/tests/installation/fixtures/with-dependencies-differing-extras.test @@ -0,0 +1,36 @@ +[[package]] +name = "demo" +version = "1.0.0" +description = "" +optional = true +python-versions = "*" +files = [] + +[package.extras] +demo-extra-one = ["transitive-dep-one"] +demo-extra-two = ["transitive-dep-two"] + +[package.dependencies] +transitive-dep-one = {version = "1.1.0", optional = true} +transitive-dep-two = {version = "1.2.0", optional = true} + +[[package]] +name = "transitive-dep-one" +version = "1.1.0" +description = "" +optional = true +python-versions = "*" +files = [] + +[[package]] +name = "transitive-dep-two" +version = "1.2.0" +description = "" +optional = true +python-versions = "*" +files = [] + +[metadata] +python-versions = "*" +lock-version = "2.0" +content-hash = "123456789" diff --git a/tests/installation/test_installer.py b/tests/installation/test_installer.py index edb4514bde3..b7a194d6b2d 100644 --- a/tests/installation/test_installer.py +++ b/tests/installation/test_installer.py @@ -1059,6 +1059,51 @@ def test_run_with_exclusive_extras( assert locker.written_data == expected +def test_run_with_dependencies_different_extras( + installer: Installer, locker: Locker, repo: Repository, package: ProjectPackage +) -> None: + demo = get_package("demo", "1.0.0") + dep_one = get_package("transitive-dep-one", "1.1.0") + dep_two = get_package("transitive-dep-two", "1.2.0") + demo.extras = { + canonicalize_name("demo-extra-one"): [get_dependency("transitive-dep-one")], + canonicalize_name("demo-extra-two"): [get_dependency("transitive-dep-two")], + } + demo.add_dependency( + Factory.create_dependency("transitive-dep-one", {"version": "1.1.0", "optional": True}) + ) + demo.add_dependency( + Factory.create_dependency("transitive-dep-two", {"version": "1.2.0", "optional": True}) + ) + + repo.add_package(demo) + repo.add_package(dep_one) + repo.add_package(dep_two) + + package.add_dependency(Factory.create_dependency( + "demo", + { + "version": "1.0.0", + "markers": "extra == 'extra-one' and extra != 'extra-two'", + "extras": ["demo-extra-one"], + }) + ) + package.add_dependency(Factory.create_dependency( + "demo", + { + "version": "1.0.0", + "markers": "extra != 'extra-one' and extra == 'extra-two'", + "extras": ["demo-extra-two"], + }) + ) + + result = installer.run() + assert result == 0 + + expected = fixture("with-dependencies-differing-extras") + assert locker.written_data == expected + + @pytest.mark.parametrize("is_locked", [False, True]) @pytest.mark.parametrize("is_installed", [False, True]) @pytest.mark.parametrize("with_extras", [False, True]) @@ -2788,6 +2833,74 @@ def test_installer_distinguishes_locked_packages_with_same_version_by_source( ) +@pytest.mark.parametrize("extra", [None, "extra-one", "extra-two"]) +def test_installer_distinguishes_locked_packages_with_local_version_by_extra( + pool: RepositoryPool, + locker: Locker, + installed: CustomInstalledRepository, + config: Config, + repo: Repository, + package: ProjectPackage, + extra: str | None, +) -> None: + """https://github.com/python-poetry/poetry/issues/834""" + # 'demo' with extra 'one' when package has 'extra-one' extra and with extra 'two' when 'extra-two' + extra_one_dep = Factory.create_dependency( + "demo", + { + "version": "1.0.0", + "markers": "extra == 'extra-one' and extra != 'extra-two'", + "extras": ["demo-extra-one"] + }, + ) + extra_two_dep = Factory.create_dependency( + "demo", + { + "version": "1.0.0", + "markers": "extra != 'extra-one' and extra == 'extra-two'", + "extras": ["demo-extra-two"] + }, + ) + package.add_dependency(extra_one_dep) + package.add_dependency(extra_two_dep) + # We don't want to cheat by only including the correct dependency in the 'extra' mapping + package.extras = { + "extra-one": [extra_one_dep, extra_two_dep], + "extra-two": [extra_one_dep, extra_two_dep], + } + + # Locking finds packages from extra deps + locker.locked(True) + locker.mock_lock_data(dict(fixture("with-dependencies-differing-extras"))) + + installer = Installer( + NullIO(), + MockEnv(), + package, + locker, + pool, + config, + installed=installed, + executor=Executor( + MockEnv(), + pool, + config, + NullIO(), + ), + ) + if extra is not None: + installer.extras([extra]) + result = installer.run() + assert result == 0 + + # Results of installation are consistent with the 'extra' input + assert isinstance(installer.executor, Executor) + if extra is None: + assert len(installer.executor.installations) == 0 + else: + assert len(installer.executor.installations) == 2 + + @pytest.mark.parametrize("env_platform", ["darwin", "linux"]) def test_explicit_source_dependency_with_direct_origin_dependency( pool: RepositoryPool, From 4ae7b5f865c064f0617504e211a01f872cdc9725 Mon Sep 17 00:00:00 2001 From: Reese Hyde Date: Sun, 14 Jul 2024 20:19:36 -0400 Subject: [PATCH 06/39] consolidate tests --- .../with-dependencies-differing-extras.test | 4 + .../fixtures/with-exclusive-extras.test | 14 + tests/installation/test_installer.py | 376 ++++++++---------- 3 files changed, 176 insertions(+), 218 deletions(-) diff --git a/tests/installation/fixtures/with-dependencies-differing-extras.test b/tests/installation/fixtures/with-dependencies-differing-extras.test index e0689b5005b..d4c6d85909d 100644 --- a/tests/installation/fixtures/with-dependencies-differing-extras.test +++ b/tests/installation/fixtures/with-dependencies-differing-extras.test @@ -30,6 +30,10 @@ optional = true python-versions = "*" files = [] +[extras] +extra-one = ["demo", "demo"] +extra-two = ["demo", "demo"] + [metadata] python-versions = "*" lock-version = "2.0" diff --git a/tests/installation/fixtures/with-exclusive-extras.test b/tests/installation/fixtures/with-exclusive-extras.test index 59d8fc499fe..4d05775b13c 100644 --- a/tests/installation/fixtures/with-exclusive-extras.test +++ b/tests/installation/fixtures/with-exclusive-extras.test @@ -6,6 +6,11 @@ optional = true python-versions = "*" files = [] +[package.source] +reference = "pytorch-cpu" +type = "legacy" +url = "https://download.pytorch.org/whl/cpu" + [[package]] name = "torch" version = "1.11.0+cuda" @@ -14,6 +19,15 @@ optional = true python-versions = "*" files = [] +[package.source] +reference = "pytorch-cuda" +type = "legacy" +url = "https://download.pytorch.org/whl/cuda" + +[extras] +cpu = ["torch", "torch"] +cuda = ["torch", "torch"] + [metadata] python-versions = "*" lock-version = "2.0" diff --git a/tests/installation/test_installer.py b/tests/installation/test_installer.py index b7a194d6b2d..b75af6f6660 100644 --- a/tests/installation/test_installer.py +++ b/tests/installation/test_installer.py @@ -1027,82 +1027,206 @@ def test_run_with_dependencies_nested_extras( assert locker.written_data == expected -def test_run_with_exclusive_extras( - installer: Installer, locker: Locker, repo: Repository, package: ProjectPackage +@pytest.mark.parametrize("locked", [True, False]) +@pytest.mark.parametrize("extra", [None, "cpu", "cuda"]) +def test_run_with_exclusive_extras_different_sources( + installer: Installer, + locker: Locker, + installed: CustomInstalledRepository, + config: Config, + package: ProjectPackage, + extra: str | None, + locked: bool, ) -> None: - """https://github.com/python-poetry/poetry/issues/6409; https://github.com/python-poetry/poetry/issues/6419""" - torch_cpu = get_package("torch", "1.11.0+cpu") - torch_cuda = get_package("torch", "1.11.0+cuda") - - repo.add_package(torch_cpu) - repo.add_package(torch_cuda) + """ + - https://github.com/python-poetry/poetry/issues/6409 + - https://github.com/python-poetry/poetry/issues/6419 + - https://github.com/python-poetry/poetry/issues/7748 + """ + # Setup repo for each of our sources + cpu_repo = Repository("pytorch-cpu") + cuda_repo = Repository("pytorch-cuda") + pool = RepositoryPool() + pool.add_repository(cpu_repo) + pool.add_repository(cuda_repo) + config.config['repositories'] = { + "pytorch-cpu": {"url": "https://download.pytorch.org/whl/cpu"}, + "pytorch-cuda": {"url": "https://download.pytorch.org/whl/cuda"}, + } - package.add_dependency(Factory.create_dependency( + # Configure packages that read from each of the different sources + torch_cpu_pkg = get_package("torch", "1.11.0+cpu") + torch_cpu_pkg._source_reference = "pytorch-cpu" + torch_cpu_pkg._source_type = "legacy" + torch_cpu_pkg._source_url = "https://download.pytorch.org/whl/cpu" + torch_cuda_pkg = get_package("torch", "1.11.0+cuda") + torch_cuda_pkg._source_reference = "pytorch-cuda" + torch_cuda_pkg._source_type = "legacy" + torch_cuda_pkg._source_url = "https://download.pytorch.org/whl/cuda" + cpu_repo.add_package(torch_cpu_pkg) + cuda_repo.add_package(torch_cuda_pkg) + + # Depend on each package based on exclusive extras + torch_cpu_dep = Factory.create_dependency( "torch", { "version": "1.11.0+cpu", "markers": "extra == 'cpu' and extra != 'cuda'", + "source": "pytorch-cpu", }) - ) - package.add_dependency(Factory.create_dependency( + torch_cuda_dep = Factory.create_dependency( "torch", { "version": "1.11.0+cuda", "markers": "extra != 'cpu' and extra == 'cuda'", + "source": "pytorch-cuda", }) - ) + package.add_dependency(torch_cpu_dep) + package.add_dependency(torch_cuda_dep) + # We don't want to cheat by only including the correct dependency in the 'extra' mapping + package.extras = { + canonicalize_name("cpu"): [torch_cpu_dep, torch_cuda_dep], + canonicalize_name("cuda"): [torch_cpu_dep, torch_cuda_dep], + } + # Set locker state + locker.locked(locked) + if locked: + locker.mock_lock_data(dict(fixture("with-exclusive-extras"))) + + # Perform install + installer = Installer( + NullIO(), + MockEnv(), + package, + locker, + pool, + config, + installed=installed, + executor=Executor( + MockEnv(), + pool, + config, + NullIO(), + ), + ) + if extra is not None: + installer.extras([extra]) result = installer.run() assert result == 0 - expected = fixture("with-exclusive-extras") - assert locker.written_data == expected + # Results of locking are expected and installation are consistent with the 'extra' input + if not locked: + expected = fixture("with-exclusive-extras") + assert locker.written_data == expected + assert isinstance(installer.executor, Executor) + if extra is None: + assert len(installer.executor.installations) == 0 + else: + assert len(installer.executor.installations) == 1 + version = f"1.11.0+{extra}" + source_url = f"https://download.pytorch.org/whl/{extra}" + source_reference = f"pytorch-{extra}" + assert installer.executor.installations[0] == Package( + "torch", + version, + source_type="legacy", + source_url=source_url, + source_reference=source_reference, + ) -def test_run_with_dependencies_different_extras( - installer: Installer, locker: Locker, repo: Repository, package: ProjectPackage +@pytest.mark.parametrize("locked", [True, False]) +@pytest.mark.parametrize("extra", [None, "extra-one", "extra-two"]) +def test_run_with_different_dependency_extras( + installer: Installer, + pool: RepositoryPool, + locker: Locker, + installed: CustomInstalledRepository, + repo: Repository, + config: Config, + package: ProjectPackage, + extra: str | None, + locked: bool ) -> None: - demo = get_package("demo", "1.0.0") - dep_one = get_package("transitive-dep-one", "1.1.0") - dep_two = get_package("transitive-dep-two", "1.2.0") - demo.extras = { + """https://github.com/python-poetry/poetry/issues/834""" + # Demo package with two optional transitive dependencies, one for each extra + demo_pkg = get_package("demo", "1.0.0") + transitive_dep_one = get_package("transitive-dep-one", "1.1.0") + transitive_dep_two = get_package("transitive-dep-two", "1.2.0") + demo_pkg.extras = { canonicalize_name("demo-extra-one"): [get_dependency("transitive-dep-one")], canonicalize_name("demo-extra-two"): [get_dependency("transitive-dep-two")], } - demo.add_dependency( + demo_pkg.add_dependency( Factory.create_dependency("transitive-dep-one", {"version": "1.1.0", "optional": True}) ) - demo.add_dependency( + demo_pkg.add_dependency( Factory.create_dependency("transitive-dep-two", {"version": "1.2.0", "optional": True}) ) + repo.add_package(demo_pkg) + repo.add_package(transitive_dep_one) + repo.add_package(transitive_dep_two) - repo.add_package(demo) - repo.add_package(dep_one) - repo.add_package(dep_two) - - package.add_dependency(Factory.create_dependency( + # 'demo' with extra 'one' when package has 'extra-one' extra and with extra 'two' when 'extra-two' + extra_one_dep = Factory.create_dependency( "demo", { "version": "1.0.0", "markers": "extra == 'extra-one' and extra != 'extra-two'", - "extras": ["demo-extra-one"], - }) + "extras": ["demo-extra-one"] + }, ) - package.add_dependency(Factory.create_dependency( + extra_two_dep = Factory.create_dependency( "demo", { "version": "1.0.0", "markers": "extra != 'extra-one' and extra == 'extra-two'", - "extras": ["demo-extra-two"], - }) + "extras": ["demo-extra-two"] + }, ) + package.add_dependency(extra_one_dep) + package.add_dependency(extra_two_dep) + # We don't want to cheat by only including the correct dependency in the 'extra' mapping + package.extras = { + "extra-one": [extra_one_dep, extra_two_dep], + "extra-two": [extra_one_dep, extra_two_dep], + } + + locker.locked(locked) + if locked: + locker.mock_lock_data(dict(fixture("with-dependencies-differing-extras"))) + installer = Installer( + NullIO(), + MockEnv(), + package, + locker, + pool, + config, + installed=installed, + executor=Executor( + MockEnv(), + pool, + config, + NullIO(), + ), + ) + if extra is not None: + installer.extras([extra]) result = installer.run() assert result == 0 - expected = fixture("with-dependencies-differing-extras") - assert locker.written_data == expected + if not locked: + expected = fixture("with-dependencies-differing-extras") + assert locker.written_data == expected + # Results of installation are consistent with the 'extra' input + assert isinstance(installer.executor, Executor) + if extra is None: + assert len(installer.executor.installations) == 0 + else: + assert len(installer.executor.installations) == 2 @pytest.mark.parametrize("is_locked", [False, True]) @pytest.mark.parametrize("is_installed", [False, True]) @@ -2613,122 +2737,6 @@ def test_installer_distinguishes_locked_packages_with_local_version_by_source( ) -@pytest.mark.parametrize("extra", [None, "cpu", "cuda"]) -def test_installer_distinguishes_locked_packages_with_local_version_by_extra( - pool: RepositoryPool, - locker: Locker, - installed: CustomInstalledRepository, - config: Config, - repo: Repository, - package: ProjectPackage, - extra: str | None, -) -> None: - """https://github.com/python-poetry/poetry/issues/6409; https://github.com/python-poetry/poetry/issues/6419""" - # Require 1.11.0+cpu from pytorch for when extra is 'cpu', or 1.11.0+cuda when extra is 'cuda' - cpu_dep = Factory.create_dependency( - "torch", - { - "version": "1.11.0+cpu", - "markers": "extra == 'cpu' and extra != 'cuda'", - "source": "pytorch-cpu", - }, - ) - cuda_dep = Factory.create_dependency( - "torch", - { - "version": "1.11.0+cuda", - "markers": "extra != 'cpu' and extra == 'cuda'", - "source": "pytorch-cuda", - }, - ) - package.add_dependency(cpu_dep) - package.add_dependency(cuda_dep) - # We don't want to cheat by only including the correct dependency in the 'extra' mapping - package.extras = { - "cpu": [cpu_dep, cuda_dep], - "cuda": [cpu_dep, cuda_dep], - } - - # Locking finds packages from each pytorch repository - locker.locked(True) - locker.mock_lock_data( - { - "package": [ - { - "name": "torch", - "version": "1.11.0+cpu", - "optional": True, - "files": [], - "python-versions": "*", - "source": { - "type": "legacy", - "url": "https://download.pytorch.org/whl/cpu", - "reference": "pytorch-cpu", - }, - }, - { - "name": "torch", - "version": "1.11.0+cuda", - "optional": True, - "files": [], - "python-versions": "*", - "source": { - "type": "legacy", - "url": "https://download.pytorch.org/whl/cuda", - "reference": "pytorch-cuda", - }, - }, - ], - "metadata": { - "python-versions": "*", - "platform": "*", - "content-hash": "123456789", - }, - "extras": { - "cpu": ["torch"], - "cuda": ["torch"], - } - } - ) - - installer = Installer( - NullIO(), - MockEnv(), - package, - locker, - pool, - config, - installed=installed, - executor=Executor( - MockEnv(), - pool, - config, - NullIO(), - ), - ) - if extra is not None: - installer.extras([extra]) - result = installer.run() - assert result == 0 - - # Results of installation are consistent with the 'extra' input - assert isinstance(installer.executor, Executor) - if extra is None: - assert len(installer.executor.installations) == 0 - else: - assert len(installer.executor.installations) == 1 - version = f"1.11.0+{extra}" - source_url = f"https://download.pytorch.org/whl/{extra}" - source_reference = f"pytorch-{extra}" - assert installer.executor.installations[0] == Package( - "torch", - version, - source_type="legacy", - source_url=source_url, - source_reference=source_reference, - ) - - @pytest.mark.parametrize("env_platform_machine", ["aarch64", "amd64"]) def test_installer_distinguishes_locked_packages_with_same_version_by_source( pool: RepositoryPool, @@ -2833,74 +2841,6 @@ def test_installer_distinguishes_locked_packages_with_same_version_by_source( ) -@pytest.mark.parametrize("extra", [None, "extra-one", "extra-two"]) -def test_installer_distinguishes_locked_packages_with_local_version_by_extra( - pool: RepositoryPool, - locker: Locker, - installed: CustomInstalledRepository, - config: Config, - repo: Repository, - package: ProjectPackage, - extra: str | None, -) -> None: - """https://github.com/python-poetry/poetry/issues/834""" - # 'demo' with extra 'one' when package has 'extra-one' extra and with extra 'two' when 'extra-two' - extra_one_dep = Factory.create_dependency( - "demo", - { - "version": "1.0.0", - "markers": "extra == 'extra-one' and extra != 'extra-two'", - "extras": ["demo-extra-one"] - }, - ) - extra_two_dep = Factory.create_dependency( - "demo", - { - "version": "1.0.0", - "markers": "extra != 'extra-one' and extra == 'extra-two'", - "extras": ["demo-extra-two"] - }, - ) - package.add_dependency(extra_one_dep) - package.add_dependency(extra_two_dep) - # We don't want to cheat by only including the correct dependency in the 'extra' mapping - package.extras = { - "extra-one": [extra_one_dep, extra_two_dep], - "extra-two": [extra_one_dep, extra_two_dep], - } - - # Locking finds packages from extra deps - locker.locked(True) - locker.mock_lock_data(dict(fixture("with-dependencies-differing-extras"))) - - installer = Installer( - NullIO(), - MockEnv(), - package, - locker, - pool, - config, - installed=installed, - executor=Executor( - MockEnv(), - pool, - config, - NullIO(), - ), - ) - if extra is not None: - installer.extras([extra]) - result = installer.run() - assert result == 0 - - # Results of installation are consistent with the 'extra' input - assert isinstance(installer.executor, Executor) - if extra is None: - assert len(installer.executor.installations) == 0 - else: - assert len(installer.executor.installations) == 2 - - @pytest.mark.parametrize("env_platform", ["darwin", "linux"]) def test_explicit_source_dependency_with_direct_origin_dependency( pool: RepositoryPool, From bc1202c67aa4ffcc4c821c17880dd53058210716 Mon Sep 17 00:00:00 2001 From: Reese Hyde Date: Mon, 15 Jul 2024 11:52:26 -0400 Subject: [PATCH 07/39] add documentation --- docs/dependency-specification.md | 45 ++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/docs/dependency-specification.md b/docs/dependency-specification.md index adc4c000a66..ec47fdbe5a9 100644 --- a/docs/dependency-specification.md +++ b/docs/dependency-specification.md @@ -471,6 +471,51 @@ dependencies = [ ] ``` +### `extra` environment marker + +Poetry populates the `extra` marker with each of the selected extras for the parent declaring the dependency. For +example, consider the following dependency in your root package: +```toml +[tool.poetry.dependencies] +pathlib2 = { version = "^2.2", markers = "extra == 'paths' and sys_platform == 'win32'", optional = true} +``` +`pathlib2` will be installed when you install your package with `--extras paths` on a `win32` machine. +You'll also need to [define the `paths` extra in your project](./pyproject.md#extras). + +#### Exclusive extras + +Keep in mind that all combinations of extras available in your project need to be compatible with each other. This +means that in order to use differing or incompatible versions across different extras, you need to make your extra +markers *exclusive*. For example, the following installs PyTorch from one source repository with CPU versions when +the `cpu` extra is specified, while the other installs from another repository with a separate version set for GPUs +when the `cuda` extra is specified: + +```toml +[tool.poetry.dependencies] +torch = [ + { markers = "extra == 'cpu' and extra != 'cuda'", version = "2.3.1+cpu", source = "pytorch-cpu", optional = true}, + { markers = "extra != 'cpu' and extra == 'cuda'", version = "2.3.1+cu118", source = "pytorch-cu118", optional = true}, + ] + +[tool.poetry.extras] +cpu = ["torch"] +cuda = ["torch"] + +[[tool.poetry.source]] +name = "pytorch-cpu" +url = "https://download.pytorch.org/whl/cpu" +priority = "explicit" + +[[tool.poetry.source]] +name = "pytorch-cu118" +url = "https://download.pytorch.org/whl/cu118" +priority = "explicit" +``` + +For the `cpu` extra marker, we have to specify `"extra == 'cpu' and extra != 'cuda'"` and vice-versa for the `cuda` +extra marker. There is no version that satisfies both constraints, so installing with `--extras cpu --extras gpu` +would match no cases and not install `torch`. + ## Multiple constraints dependencies Sometimes, one of your dependency may have different version ranges depending From fc05bf71c17b52eae43b17bff87b9aa73e59596a Mon Sep 17 00:00:00 2001 From: Reese Hyde Date: Mon, 15 Jul 2024 11:54:42 -0400 Subject: [PATCH 08/39] fix types --- src/poetry/installation/installer.py | 1 + src/poetry/puzzle/provider.py | 6 +++--- tests/installation/test_installer.py | 4 ++-- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/poetry/installation/installer.py b/src/poetry/installation/installer.py index 91d73ed6e24..d4ddc7a043c 100644 --- a/src/poetry/installation/installer.py +++ b/src/poetry/installation/installer.py @@ -20,6 +20,7 @@ from cleo.io.io import IO from packaging.utils import NormalizedName + from poetry.core.packages.path_dependency import PathDependency from poetry.core.packages.project_package import ProjectPackage from poetry.config.config import Config diff --git a/src/poetry/puzzle/provider.py b/src/poetry/puzzle/provider.py index 5f755e4ff18..72fc81ba289 100644 --- a/src/poetry/puzzle/provider.py +++ b/src/poetry/puzzle/provider.py @@ -962,16 +962,16 @@ def _resolve_overlapping_markers( # resolved, there might be new dependencies with the same constraint. return self._merge_dependencies_by_constraint(new_dependencies) - def _marker_values(self, extras: set[NormalizedName] | None = None) -> dict[str, Any]: + def _marker_values(self, extras: Collection[NormalizedName] | None = None) -> dict[str, Any]: """ Marker values, per: 1. marker_env of `self._env` 2. 'extras' will be added to the 'extra' marker if not already present """ - result = self._env.marker_env.copy() + result = self._env.marker_env.copy() if self._env is not None else {} if extras is not None: if 'extra' not in result.keys(): - result['extra'] = extras + result['extra'] = set(extras) else: result['extra'] = set(result['extra']).union(extras) return result diff --git a/tests/installation/test_installer.py b/tests/installation/test_installer.py index b75af6f6660..36dd43c280b 100644 --- a/tests/installation/test_installer.py +++ b/tests/installation/test_installer.py @@ -1189,8 +1189,8 @@ def test_run_with_different_dependency_extras( package.add_dependency(extra_two_dep) # We don't want to cheat by only including the correct dependency in the 'extra' mapping package.extras = { - "extra-one": [extra_one_dep, extra_two_dep], - "extra-two": [extra_one_dep, extra_two_dep], + canonicalize_name("extra-one"): [extra_one_dep, extra_two_dep], + canonicalize_name("extra-two"): [extra_one_dep, extra_two_dep], } locker.locked(locked) From eeb990f09332d8feda63df68f30b67e554721854 Mon Sep 17 00:00:00 2001 From: Reese Hyde Date: Mon, 15 Jul 2024 13:03:25 -0400 Subject: [PATCH 09/39] fix formatting --- src/poetry/puzzle/solver.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/poetry/puzzle/solver.py b/src/poetry/puzzle/solver.py index 0f510ed0a37..e72c07204b4 100644 --- a/src/poetry/puzzle/solver.py +++ b/src/poetry/puzzle/solver.py @@ -48,8 +48,12 @@ def __init__( self._io = io self._provider = Provider( - self._package, self._pool, self._io, installed=installed, locked=locked, - active_root_extras=active_root_extras + self._package, + self._pool, + self._io, + installed=installed, + locked=locked, + active_root_extras=active_root_extras, ) self._overrides: list[dict[Package, dict[str, Dependency]]] = [] From a0fcab7fda51e79f286112391ea3b3a7cb3ec7a4 Mon Sep 17 00:00:00 2001 From: Reese Hyde Date: Mon, 15 Jul 2024 13:16:46 -0400 Subject: [PATCH 10/39] simplify exclusive extras example --- docs/dependency-specification.md | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/docs/dependency-specification.md b/docs/dependency-specification.md index ec47fdbe5a9..37eb67dc5e5 100644 --- a/docs/dependency-specification.md +++ b/docs/dependency-specification.md @@ -484,21 +484,20 @@ You'll also need to [define the `paths` extra in your project](./pyproject.md#ex #### Exclusive extras -Keep in mind that all combinations of extras available in your project need to be compatible with each other. This -means that in order to use differing or incompatible versions across different extras, you need to make your extra -markers *exclusive*. For example, the following installs PyTorch from one source repository with CPU versions when -the `cpu` extra is specified, while the other installs from another repository with a separate version set for GPUs -when the `cuda` extra is specified: +Keep in mind that all combinations of possible extras available in your project need to be compatible with each other. +This means that in order to use differing or incompatible versions across different combinations, you need to make your +extra markers *exclusive*. For example, the following installs PyTorch from one source repository with CPU versions +when the `cuda` extra is *not* specified, while the other installs from another repository with a separate version set +for GPUs when the `cuda` extra *is* specified: ```toml [tool.poetry.dependencies] torch = [ - { markers = "extra == 'cpu' and extra != 'cuda'", version = "2.3.1+cpu", source = "pytorch-cpu", optional = true}, - { markers = "extra != 'cpu' and extra == 'cuda'", version = "2.3.1+cu118", source = "pytorch-cu118", optional = true}, + { markers = "extra != 'cuda'", version = "2.3.1+cpu", source = "pytorch-cpu", optional = true}, + { markers = "extra == 'cuda'", version = "2.3.1+cu118", source = "pytorch-cu118", optional = true}, ] [tool.poetry.extras] -cpu = ["torch"] cuda = ["torch"] [[tool.poetry.source]] @@ -512,9 +511,12 @@ url = "https://download.pytorch.org/whl/cu118" priority = "explicit" ``` -For the `cpu` extra marker, we have to specify `"extra == 'cpu' and extra != 'cuda'"` and vice-versa for the `cuda` -extra marker. There is no version that satisfies both constraints, so installing with `--extras cpu --extras gpu` -would match no cases and not install `torch`. +For the CPU case, we have to specify `"extra != 'cuda'"` because the version specified is not compatible with the +GPU (`cuda`) version. + +This same logic applies when you want either-or extras. If a dependency for `extra-one` and +`extra-two` conflict, they will need to be restricted using `markers = "extra == 'extra-one' and extra != 'extra-two'"` +and vice versa. ## Multiple constraints dependencies From 63b2798d7aaf27748cdcdadea2491dde4a345392 Mon Sep 17 00:00:00 2001 From: Reese Hyde Date: Mon, 15 Jul 2024 13:31:52 -0400 Subject: [PATCH 11/39] run ruff formatter --- src/poetry/puzzle/provider.py | 41 +++++++++++++++++++--------- src/poetry/puzzle/solver.py | 2 +- tests/installation/test_installer.py | 25 +++++++++++------ 3 files changed, 45 insertions(+), 23 deletions(-) diff --git a/src/poetry/puzzle/provider.py b/src/poetry/puzzle/provider.py index 72fc81ba289..aaf1fcd186d 100644 --- a/src/poetry/puzzle/provider.py +++ b/src/poetry/puzzle/provider.py @@ -7,7 +7,8 @@ from collections import defaultdict from contextlib import contextmanager -from typing import TYPE_CHECKING, Any +from typing import TYPE_CHECKING +from typing import Any from typing import ClassVar from typing import cast @@ -134,7 +135,9 @@ def __init__( self._direct_origin_packages: dict[str, Package] = {} self._locked: dict[NormalizedName, list[DependencyPackage]] = defaultdict(list) self._use_latest: Collection[NormalizedName] = [] - self._active_root_extras = frozenset(active_root_extras) if active_root_extras is not None else None + self._active_root_extras = ( + frozenset(active_root_extras) if active_root_extras is not None else None + ) self._explicit_sources: dict[str, str] = {} for package in locked or []: @@ -459,10 +462,14 @@ def incompatibilities_for( for dep in dependencies if dep.name not in self.UNSAFE_PACKAGES and self._python_constraint.allows_any(dep.python_constraint) - and (not self._env or dep.marker.validate( - self._marker_values( - self._active_root_extras if dependency_package.package.is_root() - else dependency_package.dependency.extras) + and ( + not self._env + or dep.marker.validate( + self._marker_values( + self._active_root_extras + if dependency_package.package.is_root() + else dependency_package.dependency.extras + ) ) ) ] @@ -548,8 +555,12 @@ def complete_package( if dep.name in self.UNSAFE_PACKAGES: continue - active_extras = self._active_root_extras if package.is_root() else dependency.extras - if self._env and not dep.marker.validate(self._marker_values(active_extras)): + active_extras = ( + self._active_root_extras if package.is_root() else dependency.extras + ) + if self._env and not dep.marker.validate( + self._marker_values(active_extras) + ): continue if not package.is_root() and ( @@ -609,7 +620,9 @@ def complete_package( # For dependency resolution, markers of duplicate dependencies must be # mutually exclusive. - active_extras = self._active_root_extras if package.is_root() else dependency.extras + active_extras = ( + self._active_root_extras if package.is_root() else dependency.extras + ) deps = self._resolve_overlapping_markers(package, deps, active_extras) if len(deps) == 1: @@ -962,7 +975,9 @@ def _resolve_overlapping_markers( # resolved, there might be new dependencies with the same constraint. return self._merge_dependencies_by_constraint(new_dependencies) - def _marker_values(self, extras: Collection[NormalizedName] | None = None) -> dict[str, Any]: + def _marker_values( + self, extras: Collection[NormalizedName] | None = None + ) -> dict[str, Any]: """ Marker values, per: 1. marker_env of `self._env` @@ -970,8 +985,8 @@ def _marker_values(self, extras: Collection[NormalizedName] | None = None) -> di """ result = self._env.marker_env.copy() if self._env is not None else {} if extras is not None: - if 'extra' not in result.keys(): - result['extra'] = set(extras) + if "extra" not in result.keys(): + result["extra"] = set(extras) else: - result['extra'] = set(result['extra']).union(extras) + result["extra"] = set(result["extra"]).union(extras) return result diff --git a/src/poetry/puzzle/solver.py b/src/poetry/puzzle/solver.py index e72c07204b4..fdd68e08ce3 100644 --- a/src/poetry/puzzle/solver.py +++ b/src/poetry/puzzle/solver.py @@ -39,7 +39,7 @@ def __init__( installed: list[Package], locked: list[Package], io: IO, - active_root_extras: Collection[NormalizedName] | None = None + active_root_extras: Collection[NormalizedName] | None = None, ) -> None: self._package = package self._pool = pool diff --git a/tests/installation/test_installer.py b/tests/installation/test_installer.py index 36dd43c280b..373b6a81dd7 100644 --- a/tests/installation/test_installer.py +++ b/tests/installation/test_installer.py @@ -5,7 +5,7 @@ import shutil from pathlib import Path -from typing import TYPE_CHECKING, IO +from typing import TYPE_CHECKING from typing import Any import pytest @@ -1049,7 +1049,7 @@ def test_run_with_exclusive_extras_different_sources( pool = RepositoryPool() pool.add_repository(cpu_repo) pool.add_repository(cuda_repo) - config.config['repositories'] = { + config.config["repositories"] = { "pytorch-cpu": {"url": "https://download.pytorch.org/whl/cpu"}, "pytorch-cuda": {"url": "https://download.pytorch.org/whl/cuda"}, } @@ -1073,14 +1073,16 @@ def test_run_with_exclusive_extras_different_sources( "version": "1.11.0+cpu", "markers": "extra == 'cpu' and extra != 'cuda'", "source": "pytorch-cpu", - }) + }, + ) torch_cuda_dep = Factory.create_dependency( "torch", { "version": "1.11.0+cuda", "markers": "extra != 'cpu' and extra == 'cuda'", "source": "pytorch-cuda", - }) + }, + ) package.add_dependency(torch_cpu_dep) package.add_dependency(torch_cuda_dep) # We don't want to cheat by only including the correct dependency in the 'extra' mapping @@ -1147,7 +1149,7 @@ def test_run_with_different_dependency_extras( config: Config, package: ProjectPackage, extra: str | None, - locked: bool + locked: bool, ) -> None: """https://github.com/python-poetry/poetry/issues/834""" # Demo package with two optional transitive dependencies, one for each extra @@ -1159,10 +1161,14 @@ def test_run_with_different_dependency_extras( canonicalize_name("demo-extra-two"): [get_dependency("transitive-dep-two")], } demo_pkg.add_dependency( - Factory.create_dependency("transitive-dep-one", {"version": "1.1.0", "optional": True}) + Factory.create_dependency( + "transitive-dep-one", {"version": "1.1.0", "optional": True} + ) ) demo_pkg.add_dependency( - Factory.create_dependency("transitive-dep-two", {"version": "1.2.0", "optional": True}) + Factory.create_dependency( + "transitive-dep-two", {"version": "1.2.0", "optional": True} + ) ) repo.add_package(demo_pkg) repo.add_package(transitive_dep_one) @@ -1174,7 +1180,7 @@ def test_run_with_different_dependency_extras( { "version": "1.0.0", "markers": "extra == 'extra-one' and extra != 'extra-two'", - "extras": ["demo-extra-one"] + "extras": ["demo-extra-one"], }, ) extra_two_dep = Factory.create_dependency( @@ -1182,7 +1188,7 @@ def test_run_with_different_dependency_extras( { "version": "1.0.0", "markers": "extra != 'extra-one' and extra == 'extra-two'", - "extras": ["demo-extra-two"] + "extras": ["demo-extra-two"], }, ) package.add_dependency(extra_one_dep) @@ -1228,6 +1234,7 @@ def test_run_with_different_dependency_extras( else: assert len(installer.executor.installations) == 2 + @pytest.mark.parametrize("is_locked", [False, True]) @pytest.mark.parametrize("is_installed", [False, True]) @pytest.mark.parametrize("with_extras", [False, True]) From 7a5a3c917920038caa2bac32fbe69b60899d9c6a Mon Sep 17 00:00:00 2001 From: Reese Hyde Date: Mon, 15 Jul 2024 13:33:39 -0400 Subject: [PATCH 12/39] fix key not in dict format --- src/poetry/puzzle/provider.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/poetry/puzzle/provider.py b/src/poetry/puzzle/provider.py index aaf1fcd186d..dfebdbb265a 100644 --- a/src/poetry/puzzle/provider.py +++ b/src/poetry/puzzle/provider.py @@ -985,7 +985,7 @@ def _marker_values( """ result = self._env.marker_env.copy() if self._env is not None else {} if extras is not None: - if "extra" not in result.keys(): + if "extra" not in result: result["extra"] = set(extras) else: result["extra"] = set(result["extra"]).union(extras) From c1fe2677b27d39a931a3f0ddc1128d7692254213 Mon Sep 17 00:00:00 2001 From: Reese Hyde Date: Mon, 15 Jul 2024 13:52:48 -0400 Subject: [PATCH 13/39] fix pre-commit and full run --- docs/dependency-specification.md | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/docs/dependency-specification.md b/docs/dependency-specification.md index 37eb67dc5e5..e523c5cc90f 100644 --- a/docs/dependency-specification.md +++ b/docs/dependency-specification.md @@ -473,21 +473,21 @@ dependencies = [ ### `extra` environment marker -Poetry populates the `extra` marker with each of the selected extras for the parent declaring the dependency. For +Poetry populates the `extra` marker with each of the selected extras for the parent declaring the dependency. For example, consider the following dependency in your root package: ```toml [tool.poetry.dependencies] pathlib2 = { version = "^2.2", markers = "extra == 'paths' and sys_platform == 'win32'", optional = true} ``` -`pathlib2` will be installed when you install your package with `--extras paths` on a `win32` machine. +`pathlib2` will be installed when you install your package with `--extras paths` on a `win32` machine. You'll also need to [define the `paths` extra in your project](./pyproject.md#extras). #### Exclusive extras -Keep in mind that all combinations of possible extras available in your project need to be compatible with each other. -This means that in order to use differing or incompatible versions across different combinations, you need to make your -extra markers *exclusive*. For example, the following installs PyTorch from one source repository with CPU versions -when the `cuda` extra is *not* specified, while the other installs from another repository with a separate version set +Keep in mind that all combinations of possible extras available in your project need to be compatible with each other. +This means that in order to use differing or incompatible versions across different combinations, you need to make your +extra markers *exclusive*. For example, the following installs PyTorch from one source repository with CPU versions +when the `cuda` extra is *not* specified, while the other installs from another repository with a separate version set for GPUs when the `cuda` extra *is* specified: ```toml @@ -511,11 +511,11 @@ url = "https://download.pytorch.org/whl/cu118" priority = "explicit" ``` -For the CPU case, we have to specify `"extra != 'cuda'"` because the version specified is not compatible with the +For the CPU case, we have to specify `"extra != 'cuda'"` because the version specified is not compatible with the GPU (`cuda`) version. -This same logic applies when you want either-or extras. If a dependency for `extra-one` and -`extra-two` conflict, they will need to be restricted using `markers = "extra == 'extra-one' and extra != 'extra-two'"` +This same logic applies when you want either-or extras. If a dependency for `extra-one` and +`extra-two` conflict, they will need to be restricted using `markers = "extra == 'extra-one' and extra != 'extra-two'"` and vice versa. ## Multiple constraints dependencies From 2659365749e31781c6a70576ce9d8c4f22b48544 Mon Sep 17 00:00:00 2001 From: Reese Hyde Date: Fri, 11 Oct 2024 15:35:41 -0400 Subject: [PATCH 14/39] add markers assert and remove unnecessary check --- src/poetry/puzzle/provider.py | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/poetry/puzzle/provider.py b/src/poetry/puzzle/provider.py index dfebdbb265a..f5846e38d72 100644 --- a/src/poetry/puzzle/provider.py +++ b/src/poetry/puzzle/provider.py @@ -881,7 +881,7 @@ def _is_relevant_marker( get_python_constraint_from_marker(marker) ) and (active_extras is None or marker.validate({"extra": active_extras})) - and (not self._env or marker.validate(self._marker_values(active_extras))) + and (not self._env or marker.validate(self._env.marker_env)) ) def _resolve_overlapping_markers( @@ -979,14 +979,12 @@ def _marker_values( self, extras: Collection[NormalizedName] | None = None ) -> dict[str, Any]: """ - Marker values, per: - 1. marker_env of `self._env` - 2. 'extras' will be added to the 'extra' marker if not already present + Marker values, from `self._env` if present plus the supplied extras + + :param extras: the values to add to the 'extra' marker value """ result = self._env.marker_env.copy() if self._env is not None else {} if extras is not None: - if "extra" not in result: - result["extra"] = set(extras) - else: - result["extra"] = set(result["extra"]).union(extras) + assert "extra" not in result, "'extra' marker key is already present in environment" + result["extra"] = set(extras) return result From 36fd729f4e944ce0c78d8f184427cdfef1806305 Mon Sep 17 00:00:00 2001 From: Reese Hyde Date: Fri, 11 Oct 2024 16:15:01 -0400 Subject: [PATCH 15/39] add test for root extras --- ...-conflicting-dependencies-root-extras.test | 24 ++++++ tests/installation/test_installer.py | 76 +++++++++++++++++++ 2 files changed, 100 insertions(+) create mode 100644 tests/installation/fixtures/with-conflicting-dependencies-root-extras.test diff --git a/tests/installation/fixtures/with-conflicting-dependencies-root-extras.test b/tests/installation/fixtures/with-conflicting-dependencies-root-extras.test new file mode 100644 index 00000000000..0534be4a068 --- /dev/null +++ b/tests/installation/fixtures/with-conflicting-dependencies-root-extras.test @@ -0,0 +1,24 @@ +[[package]] +name = "dependency" +version = "1.1.0" +description = "" +optional = true +python-versions = "*" +files = [] + +[[package]] +name = "dependency" +version = "1.2.0" +description = "" +optional = true +python-versions = "*" +files = [] + +[extras] +extra-one = ["dependency"] +extra-two = ["dependency"] + +[metadata] +python-versions = "*" +lock-version = "2.0" +content-hash = "123456789" diff --git a/tests/installation/test_installer.py b/tests/installation/test_installer.py index 373b6a81dd7..77c82b29b7c 100644 --- a/tests/installation/test_installer.py +++ b/tests/installation/test_installer.py @@ -1138,6 +1138,82 @@ def test_run_with_exclusive_extras_different_sources( ) +@pytest.mark.parametrize("locked", [True, False]) +@pytest.mark.parametrize("extra", [None, "extra-one", "extra-two"]) +def test_run_with_conflicting_root_dependency_extras( + installer: Installer, + pool: RepositoryPool, + locker: Locker, + installed: CustomInstalledRepository, + repo: Repository, + config: Config, + package: ProjectPackage, + extra: str | None, + locked: bool, +) -> None: + """https://github.com/python-poetry/poetry/issues/834 + + Tests resolution of extras in both root ('extra-one', 'extra-two') and transitive + ('demo-extra-one', 'demo-extra-two') dependencies + """ + dep_one = get_package("dependency", "1.1.0") + dep_two = get_package("dependency", "1.2.0") + package.extras = { + canonicalize_name("extra-one"): [ + get_dependency("dependency", constraint={"version": "1.1.0", "optional": True}) + ], + canonicalize_name("extra-two"): [ + get_dependency("dependency", constraint={"version": "1.2.0", "optional": True}) + ], + } + package.add_dependency( + Factory.create_dependency( + "dependency", + { + "version": "1.1.0", + "markers": "extra == 'extra-one' and extra != 'extra-two'", + "optional": True, + } + ) + ) + package.add_dependency( + Factory.create_dependency( + "dependency", + { + "version": "1.2.0", + "markers": "extra != 'extra-one' and extra == 'extra-two'", + "optional": True, + } + ) + ) + repo.add_package(dep_one) + repo.add_package(dep_two) + + locker.locked(locked) + if locked: + locker.mock_lock_data(dict(fixture("with-conflicting-dependencies-root-extras"))) + + if extra is not None: + installer.extras([extra]) + result = installer.run() + assert result == 0 + + if not locked: + expected = fixture("with-conflicting-dependencies-root-extras") + assert locker.written_data == expected + + # Results of installation are consistent with the 'extra' input + assert isinstance(installer.executor, Executor) + if extra is None: + assert len(installer.executor.installations) == 0 + elif extra == "extra-one": + assert installer.executor.installations == [dep_one] + elif extra == "extra-two": + assert installer.executor.installations == [dep_two] + else: + raise ValueError(f"Unexpected extra value {extra}") + + @pytest.mark.parametrize("locked", [True, False]) @pytest.mark.parametrize("extra", [None, "extra-one", "extra-two"]) def test_run_with_different_dependency_extras( From 7646ed076763e75efaad0b746fb28688e8c6e07b Mon Sep 17 00:00:00 2001 From: Reese Hyde Date: Fri, 11 Oct 2024 15:59:29 -0400 Subject: [PATCH 16/39] add broken tests for conflicting dependency extras --- tests/installation/test_installer.py | 104 +++++++++++++++++++++++++++ tests/puzzle/test_solver.py | 41 +++++++++++ 2 files changed, 145 insertions(+) diff --git a/tests/installation/test_installer.py b/tests/installation/test_installer.py index 77c82b29b7c..dba859439b8 100644 --- a/tests/installation/test_installer.py +++ b/tests/installation/test_installer.py @@ -1027,6 +1027,110 @@ def test_run_with_dependencies_nested_extras( assert locker.written_data == expected +@pytest.mark.parametrize("locked", [False]) # TODO: lock data +@pytest.mark.parametrize("extra", [None, "extra-one", "extra-two"]) +def test_run_with_conflicting_dependency_extras( + installer: Installer, + pool: RepositoryPool, + locker: Locker, + installed: CustomInstalledRepository, + repo: Repository, + config: Config, + package: ProjectPackage, + extra: str | None, + locked: bool, +) -> None: + """https://github.com/python-poetry/poetry/issues/834 + + Tests resolution of extras in both root ('extra-one', 'extra-two') and transitive + ('demo-extra-one', 'demo-extra-two') dependencies + """ + # Demo package with two optional transitive dependencies, one for each extra + demo_pkg = get_package("demo", "1.0.0") + transitive_dep_one = get_package("transitive-dep", "1.1.0") + transitive_dep_two = get_package("transitive-dep", "1.2.0") + demo_pkg.extras = { + canonicalize_name("demo-extra-one"): [ + get_dependency("transitive-dep", constraint={"version": "1.1.0", "optional": True}) + ], + canonicalize_name("demo-extra-two"): [ + get_dependency("transitive-dep", constraint={"version": "1.2.0", "optional": True}) + ], + } + demo_pkg.add_dependency( + Factory.create_dependency( + "transitive-dep", + { + "version": "1.1.0", + "markers": "extra == 'demo-extra-one' and extra != 'demo-extra-two'", + "optional": True, + } + ) + ) + demo_pkg.add_dependency( + Factory.create_dependency( + "transitive-dep", + { + "version": "1.2.0", + "markers": "extra != 'demo-extra-one' and extra == 'demo-extra-two'", + "optional": True, + } + ) + ) + repo.add_package(demo_pkg) + repo.add_package(transitive_dep_one) + repo.add_package(transitive_dep_two) + + # 'demo' with extra 'demo-extra-one' when package has 'extra-one' extra + # and with extra 'demo-extra-two' when 'extra-two' + extra_one_dep = Factory.create_dependency( + "demo", + { + "version": "1.0.0", + "markers": "extra == 'extra-one' and extra != 'extra-two'", + "extras": ["demo-extra-one"], + "optional": True, + }, + ) + extra_two_dep = Factory.create_dependency( + "demo", + { + "version": "1.0.0", + "markers": "extra != 'extra-one' and extra == 'extra-two'", + "extras": ["demo-extra-two"], + "optional": True, + }, + ) + package.add_dependency(extra_one_dep) + package.add_dependency(extra_two_dep) + package.extras = { + canonicalize_name("extra-one"): [extra_one_dep], + canonicalize_name("extra-two"): [extra_two_dep], + } + + locker.locked(locked) + if locked: + raise ValueError("no lock data for this test yet") + locker.mock_lock_data(dict(fixture("TODO"))) + + if extra is not None: + installer.extras([extra]) + result = installer.run() + assert result == 0 + + if not locked: + raise ValueError("no lock data for this test yet") + expected = fixture("TODO") + assert locker.written_data == expected + + # Results of installation are consistent with the 'extra' input + assert isinstance(installer.executor, Executor) + if extra is None: + assert len(installer.executor.installations) == 0 + else: + assert len(installer.executor.installations) == 2 + + @pytest.mark.parametrize("locked", [True, False]) @pytest.mark.parametrize("extra", [None, "cpu", "cuda"]) def test_run_with_exclusive_extras_different_sources( diff --git a/tests/puzzle/test_solver.py b/tests/puzzle/test_solver.py index 5c49557962d..a20658a1000 100644 --- a/tests/puzzle/test_solver.py +++ b/tests/puzzle/test_solver.py @@ -4765,6 +4765,47 @@ def test_solver_resolves_duplicate_dependency_in_extra( ) +@pytest.mark.parametrize("with_extra", [False, True]) +def test_solver_resolves_duplicate_dependency_in_root_extra( + package: ProjectPackage, + pool: RepositoryPool, + repo: Repository, + io: NullIO, + with_extra: bool, +) -> None: + """ + Without extras, a newer version of A can be chosen than with root extras. + """ + constraint: dict[str, Any] = {"version": "*"} + if with_extra: + constraint["extras"] = ["foo"] + package_a1 = get_package("A", "1.0") + package_a2 = get_package("A", "2.0") + + dep = get_dependency("A", ">=1.0") + package.add_dependency(dep) + + dep_extra = get_dependency("A", "^1.0", optional=True) + dep_extra.marker = parse_marker("extra == 'foo'") + package.extras = {canonicalize_name("foo"): [dep_extra]} + package.add_dependency(dep_extra) + + repo.add_package(package_a1) + repo.add_package(package_a2) + + solver = Solver(package, pool, [], [], io) + transaction = solver.solve() + + check_solver_result( + transaction, + ( + [ + {"job": "install", "package": package_a1 if with_extra else package_a2}, + ] + ), + ) + + def test_solver_resolves_duplicate_dependencies_with_restricted_extras( package: ProjectPackage, pool: RepositoryPool, From ad85d8818f2db73aee4ba6c209e8b978e5cd112d Mon Sep 17 00:00:00 2001 From: Reese Hyde Date: Wed, 13 Nov 2024 16:29:02 -0500 Subject: [PATCH 17/39] pr feedback: remove unneeded non-root case and fix tests --- src/poetry/puzzle/provider.py | 14 +- .../with-dependencies-differing-extras.test | 30 ++-- tests/installation/test_installer.py | 162 ++++-------------- tests/puzzle/test_solver.py | 7 +- 4 files changed, 65 insertions(+), 148 deletions(-) diff --git a/src/poetry/puzzle/provider.py b/src/poetry/puzzle/provider.py index f5846e38d72..ecadc022e4b 100644 --- a/src/poetry/puzzle/provider.py +++ b/src/poetry/puzzle/provider.py @@ -555,13 +555,13 @@ def complete_package( if dep.name in self.UNSAFE_PACKAGES: continue - active_extras = ( - self._active_root_extras if package.is_root() else dependency.extras - ) - if self._env and not dep.marker.validate( - self._marker_values(active_extras) - ): - continue + if self._env: + marker_values = ( + self._marker_values(self._active_root_extras) if package.is_root() + else self._env.marker_env + ) + if not dep.marker.validate(marker_values): + continue if not package.is_root() and ( (dep.is_optional() and dep.name not in optional_dependencies) diff --git a/tests/installation/fixtures/with-dependencies-differing-extras.test b/tests/installation/fixtures/with-dependencies-differing-extras.test index d4c6d85909d..69827c4db28 100644 --- a/tests/installation/fixtures/with-dependencies-differing-extras.test +++ b/tests/installation/fixtures/with-dependencies-differing-extras.test @@ -4,15 +4,21 @@ version = "1.0.0" description = "" optional = true python-versions = "*" -files = [] +files = [ ] -[package.extras] -demo-extra-one = ["transitive-dep-one"] -demo-extra-two = ["transitive-dep-two"] +[package.dependencies.transitive-dep-one] +version = "1.1.0" +optional = true +markers = 'extra == "demo-extra-one" and extra != "demo-extra-two"' -[package.dependencies] -transitive-dep-one = {version = "1.1.0", optional = true} -transitive-dep-two = {version = "1.2.0", optional = true} +[package.dependencies.transitive-dep-two] +version = "1.2.0" +optional = true +markers = 'extra != "demo-extra-one" and extra == "demo-extra-two"' + + [package.extras] + demo-extra-one = [ "transitive-dep-one", "transitive-dep-two" ] + demo-extra-two = [ "transitive-dep-one", "transitive-dep-two" ] [[package]] name = "transitive-dep-one" @@ -20,7 +26,7 @@ version = "1.1.0" description = "" optional = true python-versions = "*" -files = [] +files = [ ] [[package]] name = "transitive-dep-two" @@ -28,13 +34,13 @@ version = "1.2.0" description = "" optional = true python-versions = "*" -files = [] +files = [ ] [extras] -extra-one = ["demo", "demo"] -extra-two = ["demo", "demo"] +extra-one = [ "demo", "demo" ] +extra-two = [ "demo", "demo" ] [metadata] -python-versions = "*" lock-version = "2.0" +python-versions = "*" content-hash = "123456789" diff --git a/tests/installation/test_installer.py b/tests/installation/test_installer.py index dba859439b8..c10000e4af1 100644 --- a/tests/installation/test_installer.py +++ b/tests/installation/test_installer.py @@ -1027,110 +1027,6 @@ def test_run_with_dependencies_nested_extras( assert locker.written_data == expected -@pytest.mark.parametrize("locked", [False]) # TODO: lock data -@pytest.mark.parametrize("extra", [None, "extra-one", "extra-two"]) -def test_run_with_conflicting_dependency_extras( - installer: Installer, - pool: RepositoryPool, - locker: Locker, - installed: CustomInstalledRepository, - repo: Repository, - config: Config, - package: ProjectPackage, - extra: str | None, - locked: bool, -) -> None: - """https://github.com/python-poetry/poetry/issues/834 - - Tests resolution of extras in both root ('extra-one', 'extra-two') and transitive - ('demo-extra-one', 'demo-extra-two') dependencies - """ - # Demo package with two optional transitive dependencies, one for each extra - demo_pkg = get_package("demo", "1.0.0") - transitive_dep_one = get_package("transitive-dep", "1.1.0") - transitive_dep_two = get_package("transitive-dep", "1.2.0") - demo_pkg.extras = { - canonicalize_name("demo-extra-one"): [ - get_dependency("transitive-dep", constraint={"version": "1.1.0", "optional": True}) - ], - canonicalize_name("demo-extra-two"): [ - get_dependency("transitive-dep", constraint={"version": "1.2.0", "optional": True}) - ], - } - demo_pkg.add_dependency( - Factory.create_dependency( - "transitive-dep", - { - "version": "1.1.0", - "markers": "extra == 'demo-extra-one' and extra != 'demo-extra-two'", - "optional": True, - } - ) - ) - demo_pkg.add_dependency( - Factory.create_dependency( - "transitive-dep", - { - "version": "1.2.0", - "markers": "extra != 'demo-extra-one' and extra == 'demo-extra-two'", - "optional": True, - } - ) - ) - repo.add_package(demo_pkg) - repo.add_package(transitive_dep_one) - repo.add_package(transitive_dep_two) - - # 'demo' with extra 'demo-extra-one' when package has 'extra-one' extra - # and with extra 'demo-extra-two' when 'extra-two' - extra_one_dep = Factory.create_dependency( - "demo", - { - "version": "1.0.0", - "markers": "extra == 'extra-one' and extra != 'extra-two'", - "extras": ["demo-extra-one"], - "optional": True, - }, - ) - extra_two_dep = Factory.create_dependency( - "demo", - { - "version": "1.0.0", - "markers": "extra != 'extra-one' and extra == 'extra-two'", - "extras": ["demo-extra-two"], - "optional": True, - }, - ) - package.add_dependency(extra_one_dep) - package.add_dependency(extra_two_dep) - package.extras = { - canonicalize_name("extra-one"): [extra_one_dep], - canonicalize_name("extra-two"): [extra_two_dep], - } - - locker.locked(locked) - if locked: - raise ValueError("no lock data for this test yet") - locker.mock_lock_data(dict(fixture("TODO"))) - - if extra is not None: - installer.extras([extra]) - result = installer.run() - assert result == 0 - - if not locked: - raise ValueError("no lock data for this test yet") - expected = fixture("TODO") - assert locker.written_data == expected - - # Results of installation are consistent with the 'extra' input - assert isinstance(installer.executor, Executor) - if extra is None: - assert len(installer.executor.installations) == 0 - else: - assert len(installer.executor.installations) == 2 - - @pytest.mark.parametrize("locked", [True, False]) @pytest.mark.parametrize("extra", [None, "cpu", "cuda"]) def test_run_with_exclusive_extras_different_sources( @@ -1331,30 +1227,42 @@ def test_run_with_different_dependency_extras( extra: str | None, locked: bool, ) -> None: - """https://github.com/python-poetry/poetry/issues/834""" - # Demo package with two optional transitive dependencies, one for each extra + """https://github.com/python-poetry/poetry/issues/834 + + This tests different sets of extras in a dependency of the root project. These different dependency extras are + themselves conditioned on extras in the root project. + """ + # Three packages in additon to root: demo (direct dependency) and two transitive dep packages demo_pkg = get_package("demo", "1.0.0") - transitive_dep_one = get_package("transitive-dep-one", "1.1.0") - transitive_dep_two = get_package("transitive-dep-two", "1.2.0") - demo_pkg.extras = { - canonicalize_name("demo-extra-one"): [get_dependency("transitive-dep-one")], - canonicalize_name("demo-extra-two"): [get_dependency("transitive-dep-two")], - } - demo_pkg.add_dependency( - Factory.create_dependency( - "transitive-dep-one", {"version": "1.1.0", "optional": True} - ) + transitive_one_pkg = get_package("transitive-dep-one", "1.1.0") + transitive_two_pkg = get_package("transitive-dep-two", "1.2.0") + + # Switch each transitive dependency based on extra markers in the 'demo' package + transitive_dep_one = Factory.create_dependency( + "transitive-dep-one", + { + "version": "1.1.0", + "markers": "extra == 'demo-extra-one' and extra != 'demo-extra-two'", + "optional": True, + } ) - demo_pkg.add_dependency( - Factory.create_dependency( - "transitive-dep-two", {"version": "1.2.0", "optional": True} - ) + transitive_dep_two = Factory.create_dependency( + "transitive-dep-two", + { + "version": "1.2.0", + "markers": "extra != 'demo-extra-one' and extra == 'demo-extra-two'", + "optional": True, + } ) - repo.add_package(demo_pkg) - repo.add_package(transitive_dep_one) - repo.add_package(transitive_dep_two) + # Include both packages in both demo extras, to validate that they're filtered out based on extra markers alone + demo_pkg.extras = { + canonicalize_name("demo-extra-one"): [get_dependency("transitive-dep-one"), get_dependency("transitive-dep-two")], + canonicalize_name("demo-extra-two"): [get_dependency("transitive-dep-one"), get_dependency("transitive-dep-two")], + } + demo_pkg.add_dependency(transitive_dep_one) + demo_pkg.add_dependency(transitive_dep_two) - # 'demo' with extra 'one' when package has 'extra-one' extra and with extra 'two' when 'extra-two' + # Now define the demo dependency, similarly switched on extra markers in the root package extra_one_dep = Factory.create_dependency( "demo", { @@ -1373,12 +1281,16 @@ def test_run_with_different_dependency_extras( ) package.add_dependency(extra_one_dep) package.add_dependency(extra_two_dep) - # We don't want to cheat by only including the correct dependency in the 'extra' mapping + # Again we don't want to cheat by only including the correct dependency in the 'extra' mapping package.extras = { canonicalize_name("extra-one"): [extra_one_dep, extra_two_dep], canonicalize_name("extra-two"): [extra_one_dep, extra_two_dep], } + repo.add_package(demo_pkg) + repo.add_package(transitive_one_pkg) + repo.add_package(transitive_two_pkg) + locker.locked(locked) if locked: locker.mock_lock_data(dict(fixture("with-dependencies-differing-extras"))) diff --git a/tests/puzzle/test_solver.py b/tests/puzzle/test_solver.py index a20658a1000..8b658f89752 100644 --- a/tests/puzzle/test_solver.py +++ b/tests/puzzle/test_solver.py @@ -4776,9 +4776,8 @@ def test_solver_resolves_duplicate_dependency_in_root_extra( """ Without extras, a newer version of A can be chosen than with root extras. """ - constraint: dict[str, Any] = {"version": "*"} - if with_extra: - constraint["extras"] = ["foo"] + extra = ["foo"] if with_extra else [] + package_a1 = get_package("A", "1.0") package_a2 = get_package("A", "2.0") @@ -4793,7 +4792,7 @@ def test_solver_resolves_duplicate_dependency_in_root_extra( repo.add_package(package_a1) repo.add_package(package_a2) - solver = Solver(package, pool, [], [], io) + solver = Solver(package, pool, [], [], io, active_root_extras=extra) transaction = solver.solve() check_solver_result( From 3a23622a64c485b4268ea28072b4f76ec4d12177 Mon Sep 17 00:00:00 2001 From: Reese Hyde Date: Thu, 14 Nov 2024 09:57:09 -0500 Subject: [PATCH 18/39] fix extra name type in unit test --- tests/puzzle/test_solver.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/puzzle/test_solver.py b/tests/puzzle/test_solver.py index 8b658f89752..05e3b7ce65a 100644 --- a/tests/puzzle/test_solver.py +++ b/tests/puzzle/test_solver.py @@ -4776,7 +4776,7 @@ def test_solver_resolves_duplicate_dependency_in_root_extra( """ Without extras, a newer version of A can be chosen than with root extras. """ - extra = ["foo"] if with_extra else [] + extra = [canonicalize_name("foo")] if with_extra else [] package_a1 = get_package("A", "1.0") package_a2 = get_package("A", "2.0") From 00649a04bc4ff0d7da58395a67f64b99da61f5bd Mon Sep 17 00:00:00 2001 From: Reese Hyde Date: Thu, 14 Nov 2024 16:04:44 -0500 Subject: [PATCH 19/39] better breaking test for conflicting dependencies in root and transient deps --- ...th-conflicting-dependency-extras-root.test | 24 + ...flicting-dependency-extras-transitive.test | 46 ++ tests/installation/test_installer.py | 617 +++++++++++------- 3 files changed, 436 insertions(+), 251 deletions(-) create mode 100644 tests/installation/fixtures/with-conflicting-dependency-extras-root.test create mode 100644 tests/installation/fixtures/with-conflicting-dependency-extras-transitive.test diff --git a/tests/installation/fixtures/with-conflicting-dependency-extras-root.test b/tests/installation/fixtures/with-conflicting-dependency-extras-root.test new file mode 100644 index 00000000000..1d8f0db385c --- /dev/null +++ b/tests/installation/fixtures/with-conflicting-dependency-extras-root.test @@ -0,0 +1,24 @@ +[[package]] +name = "conflicting-dep" +version = "1.1.0" +description = "" +optional = true +python-versions = "*" +files = [ ] + +[[package]] +name = "conflicting-dep" +version = "1.2.0" +description = "" +optional = true +python-versions = "*" +files = [ ] + +[extras] +extra-one = [ "conflicting-dep" ] +extra-two = [ "conflicting-dep" ] + +[metadata] +lock-version = "2.0" +python-versions = "*" +content-hash = "123456789" diff --git a/tests/installation/fixtures/with-conflicting-dependency-extras-transitive.test b/tests/installation/fixtures/with-conflicting-dependency-extras-transitive.test new file mode 100644 index 00000000000..2550d8dbc4f --- /dev/null +++ b/tests/installation/fixtures/with-conflicting-dependency-extras-transitive.test @@ -0,0 +1,46 @@ +[[package]] +name = "conflicting-dep" +version = "1.1.0" +description = "" +optional = true +python-versions = "*" +files = [ ] + +[[package]] +name = "conflicting-dep" +version = "1.2.0" +description = "" +optional = true +python-versions = "*" +files = [ ] + +[[package]] +name = "intermediate-dep" +version = "1.0.0" +description = "" +optional = true +python-versions = "*" +files = [ ] + +[[package.dependencies.conflicting-dep]] +version = "1.1.0" +optional = true +markers = 'extra == "extra-one" and extra != "extra-two"' + +[[package.dependencies.conflicting-dep]] +version = "1.2.0" +optional = true +markers = 'extra != "extra-one" and extra == "extra-two"' + + [package.extras] + extra-one = [ "conflicting-dep (==1.1.0)" ] + extra-two = [ "conflicting-dep (==1.2.0)" ] + +[extras] +root-extra-one = [ "intermediate-dep" ] +root-extra-two = [ "intermediate-dep" ] + +[metadata] +lock-version = "2.0" +python-versions = "*" +content-hash = "123456789" diff --git a/tests/installation/test_installer.py b/tests/installation/test_installer.py index c10000e4af1..560344696e2 100644 --- a/tests/installation/test_installer.py +++ b/tests/installation/test_installer.py @@ -34,7 +34,6 @@ from tests.helpers import get_dependency from tests.helpers import get_package - if TYPE_CHECKING: from pytest_mock import MockerFixture @@ -87,7 +86,7 @@ def _execute_uninstall(self, operation: Operation) -> int: class CustomInstalledRepository(InstalledRepository): @classmethod def load( - cls, env: Env, with_dependencies: bool = False + cls, env: Env, with_dependencies: bool = False ) -> CustomInstalledRepository: return cls() @@ -180,12 +179,12 @@ def env(tmp_path: Path) -> NullEnv: @pytest.fixture() def installer( - package: ProjectPackage, - pool: RepositoryPool, - locker: Locker, - env: NullEnv, - installed: CustomInstalledRepository, - config: Config, + package: ProjectPackage, + pool: RepositoryPool, + locker: Locker, + env: NullEnv, + installed: CustomInstalledRepository, + config: Config, ) -> Installer: return Installer( NullIO(), @@ -217,17 +216,17 @@ def test_run_no_dependencies(installer: Installer, locker: Locker) -> None: def test_not_fresh_lock(installer: Installer, locker: Locker) -> None: locker.locked().fresh(False) with pytest.raises( - ValueError, - match=re.escape( - "pyproject.toml changed significantly since poetry.lock was last generated. " - "Run `poetry lock [--no-update]` to fix the lock file." - ), + ValueError, + match=re.escape( + "pyproject.toml changed significantly since poetry.lock was last generated. " + "Run `poetry lock [--no-update]` to fix the lock file." + ), ): installer.run() def test_run_with_dependencies( - installer: Installer, locker: Locker, repo: Repository, package: ProjectPackage + installer: Installer, locker: Locker, repo: Repository, package: ProjectPackage ) -> None: package_a = get_package("A", "1.0") package_b = get_package("B", "1.1") @@ -245,11 +244,11 @@ def test_run_with_dependencies( def test_run_update_after_removing_dependencies( - installer: Installer, - locker: Locker, - repo: Repository, - package: ProjectPackage, - installed: CustomInstalledRepository, + installer: Installer, + locker: Locker, + repo: Repository, + package: ProjectPackage, + installed: CustomInstalledRepository, ) -> None: locker.locked(True) locker.mock_lock_data( @@ -315,12 +314,12 @@ def test_run_update_after_removing_dependencies( def _configure_run_install_dev( - locker: Locker, - repo: Repository, - package: ProjectPackage, - installed: CustomInstalledRepository, - with_optional_group: bool = False, - with_packages_installed: bool = False, + locker: Locker, + repo: Repository, + package: ProjectPackage, + installed: CustomInstalledRepository, + with_optional_group: bool = False, + with_packages_installed: bool = False, ) -> None: """ Perform common test setup for `test_run_install_*dev*()` methods. @@ -398,16 +397,16 @@ def _configure_run_install_dev( ], ) def test_run_install_with_dependency_groups( - groups: list[str] | None, - installs: int, - updates: int, - removals: int, - with_packages_installed: bool, - installer: Installer, - locker: Locker, - repo: Repository, - package: ProjectPackage, - installed: CustomInstalledRepository, + groups: list[str] | None, + installs: int, + updates: int, + removals: int, + with_packages_installed: bool, + installer: Installer, + locker: Locker, + repo: Repository, + package: ProjectPackage, + installed: CustomInstalledRepository, ) -> None: _configure_run_install_dev( locker, @@ -431,11 +430,11 @@ def test_run_install_with_dependency_groups( def test_run_install_does_not_remove_locked_packages_if_installed_but_not_required( - installer: Installer, - locker: Locker, - repo: Repository, - package: ProjectPackage, - installed: CustomInstalledRepository, + installer: Installer, + locker: Locker, + repo: Repository, + package: ProjectPackage, + installed: CustomInstalledRepository, ) -> None: package_a = get_package("a", "1.0") package_b = get_package("b", "1.1") @@ -501,11 +500,11 @@ def test_run_install_does_not_remove_locked_packages_if_installed_but_not_requir def test_run_install_removes_locked_packages_if_installed_and_synchronization_is_required( - installer: Installer, - locker: Locker, - repo: Repository, - package: ProjectPackage, - installed: CustomInstalledRepository, + installer: Installer, + locker: Locker, + repo: Repository, + package: ProjectPackage, + installed: CustomInstalledRepository, ) -> None: package_a = get_package("a", "1.0") package_b = get_package("b", "1.1") @@ -571,11 +570,11 @@ def test_run_install_removes_locked_packages_if_installed_and_synchronization_is def test_run_install_removes_no_longer_locked_packages_if_installed( - installer: Installer, - locker: Locker, - repo: Repository, - package: ProjectPackage, - installed: CustomInstalledRepository, + installer: Installer, + locker: Locker, + repo: Repository, + package: ProjectPackage, + installed: CustomInstalledRepository, ) -> None: package_a = get_package("a", "1.0") package_b = get_package("b", "1.1") @@ -646,12 +645,12 @@ def test_run_install_removes_no_longer_locked_packages_if_installed( [(), ("pip",)], ) def test_run_install_with_synchronization( - managed_reserved_package_names: tuple[str, ...], - installer: Installer, - locker: Locker, - repo: Repository, - package: ProjectPackage, - installed: CustomInstalledRepository, + managed_reserved_package_names: tuple[str, ...], + installer: Installer, + locker: Locker, + repo: Repository, + package: ProjectPackage, + installed: CustomInstalledRepository, ) -> None: package_a = get_package("a", "1.0") package_b = get_package("b", "1.1") @@ -722,7 +721,7 @@ def test_run_install_with_synchronization( def test_run_whitelist_add( - installer: Installer, locker: Locker, repo: Repository, package: ProjectPackage + installer: Installer, locker: Locker, repo: Repository, package: ProjectPackage ) -> None: locker.locked(True) locker.mock_lock_data( @@ -766,11 +765,11 @@ def test_run_whitelist_add( def test_run_whitelist_remove( - installer: Installer, - locker: Locker, - repo: Repository, - package: ProjectPackage, - installed: CustomInstalledRepository, + installer: Installer, + locker: Locker, + repo: Repository, + package: ProjectPackage, + installed: CustomInstalledRepository, ) -> None: locker.locked(True) locker.mock_lock_data( @@ -823,7 +822,7 @@ def test_run_whitelist_remove( def test_add_with_sub_dependencies( - installer: Installer, locker: Locker, repo: Repository, package: ProjectPackage + installer: Installer, locker: Locker, repo: Repository, package: ProjectPackage ) -> None: package_a = get_package("A", "1.0") package_b = get_package("B", "1.1") @@ -848,7 +847,7 @@ def test_add_with_sub_dependencies( def test_run_with_python_versions( - installer: Installer, locker: Locker, repo: Repository, package: ProjectPackage + installer: Installer, locker: Locker, repo: Repository, package: ProjectPackage ) -> None: package.python_versions = "~2.7 || ^3.4" @@ -876,7 +875,7 @@ def test_run_with_python_versions( def test_run_with_optional_and_python_restricted_dependencies( - installer: Installer, locker: Locker, repo: Repository, package: ProjectPackage + installer: Installer, locker: Locker, repo: Repository, package: ProjectPackage ) -> None: package.python_versions = "~2.7 || ^3.4" @@ -920,11 +919,11 @@ def test_run_with_optional_and_python_restricted_dependencies( def test_run_with_optional_and_platform_restricted_dependencies( - installer: Installer, - locker: Locker, - repo: Repository, - package: ProjectPackage, - mocker: MockerFixture, + installer: Installer, + locker: Locker, + repo: Repository, + package: ProjectPackage, + mocker: MockerFixture, ) -> None: mocker.patch("sys.platform", "darwin") @@ -968,7 +967,7 @@ def test_run_with_optional_and_platform_restricted_dependencies( def test_run_with_dependencies_extras( - installer: Installer, locker: Locker, repo: Repository, package: ProjectPackage + installer: Installer, locker: Locker, repo: Repository, package: ProjectPackage ) -> None: package_a = get_package("A", "1.0") package_b = get_package("B", "1.0") @@ -996,7 +995,7 @@ def test_run_with_dependencies_extras( def test_run_with_dependencies_nested_extras( - installer: Installer, locker: Locker, repo: Repository, package: ProjectPackage + installer: Installer, locker: Locker, repo: Repository, package: ProjectPackage ) -> None: package_a = get_package("A", "1.0") package_b = get_package("B", "1.0") @@ -1027,16 +1026,130 @@ def test_run_with_dependencies_nested_extras( assert locker.written_data == expected +@pytest.mark.parametrize("root", [True, False]) +@pytest.mark.parametrize("locked", [False, True]) +@pytest.mark.parametrize("extra", [None, "extra-one", "extra-two"]) +def test_run_with_conflicting_dependency_extras( + installer: Installer, + pool: RepositoryPool, + locker: Locker, + installed: CustomInstalledRepository, + repo: Repository, + config: Config, + package: ProjectPackage, + extra: str | None, + locked: bool, + root: bool, +) -> None: + """https://github.com/python-poetry/poetry/issues/834 + Tests resolution of extras in both root ('extra-one', 'extra-two') and transitive + ('demo-extra-one', 'demo-extra-two') dependencies + """ + # A package with two optional dependencies, one for each extra + # If root, this is the root package, otherwise an intermediate package + main_package = package if root else get_package("intermediate-dep", "1.0.0") + + # Two conflicting versions of a dependency, one in each extra + conflicting_dep_one_pkg = get_package("conflicting-dep", "1.1.0") + conflicting_dep_two_pkg = get_package("conflicting-dep", "1.2.0") + + conflicting_dep_one = Factory.create_dependency( + "conflicting-dep", + { + "version": "1.1.0", + "markers": "extra == 'extra-one' and extra != 'extra-two'", + "optional": True, + } + ) + conflicting_dep_two = Factory.create_dependency( + "conflicting-dep", + { + "version": "1.2.0", + "markers": "extra != 'extra-one' and extra == 'extra-two'", + "optional": True, + } + ) + + main_package.extras = { + canonicalize_name("extra-one"): [conflicting_dep_one], + canonicalize_name("extra-two"): [conflicting_dep_two], + } + main_package.add_dependency(conflicting_dep_one) + + main_package.add_dependency( + conflicting_dep_two + ) + repo.add_package(conflicting_dep_one_pkg) + repo.add_package(conflicting_dep_two_pkg) + if not root: + repo.add_package(main_package) + + # If we have an intermediate package, add extras to our root package + if not root: + extra_one_dep = Factory.create_dependency( + "intermediate-dep", + { + "version": "1.0.0", + "markers": "extra == 'root-extra-one' and extra != 'root-extra-two'", + "extras": ["extra-one"], + "optional": True, + }, + ) + extra_two_dep = Factory.create_dependency( + "intermediate-dep", + { + "version": "1.0.0", + "markers": "extra != 'root-extra-one' and extra == 'root-extra-two'", + "extras": ["extra-two"], + "optional": True, + }, + ) + package.add_dependency(extra_one_dep) + package.add_dependency(extra_two_dep) + package.extras = { + canonicalize_name("root-extra-one"): [extra_one_dep], + canonicalize_name("root-extra-two"): [extra_two_dep], + } + + fixture_name = "with-conflicting-dependency-extras-" + ("root" if root else "transitive") + locker.locked(locked) + if locked: + locker.mock_lock_data(dict(fixture(fixture_name))) + + if extra is not None: + extras = [f"root-{extra}"] if not root else [extra] + installer.extras(extras) + result = installer.run() + assert result == 0 + + if not locked: + expected = fixture(fixture_name) + assert locker.written_data == expected + + # Results of installation are consistent with the 'extra' input + assert isinstance(installer.executor, Executor) + + expected_installations = [] + if extra == "extra-one": + expected_installations.append(conflicting_dep_one_pkg) + elif extra == "extra-two": + expected_installations.append(conflicting_dep_two_pkg) + if not root and extra is not None: + expected_installations.append(get_package("intermediate-dep", "1.0.0")) + + assert installer.executor.installations == expected_installations + + @pytest.mark.parametrize("locked", [True, False]) @pytest.mark.parametrize("extra", [None, "cpu", "cuda"]) def test_run_with_exclusive_extras_different_sources( - installer: Installer, - locker: Locker, - installed: CustomInstalledRepository, - config: Config, - package: ProjectPackage, - extra: str | None, - locked: bool, + installer: Installer, + locker: Locker, + installed: CustomInstalledRepository, + config: Config, + package: ProjectPackage, + extra: str | None, + locked: bool, ) -> None: """ - https://github.com/python-poetry/poetry/issues/6409 @@ -1141,15 +1254,15 @@ def test_run_with_exclusive_extras_different_sources( @pytest.mark.parametrize("locked", [True, False]) @pytest.mark.parametrize("extra", [None, "extra-one", "extra-two"]) def test_run_with_conflicting_root_dependency_extras( - installer: Installer, - pool: RepositoryPool, - locker: Locker, - installed: CustomInstalledRepository, - repo: Repository, - config: Config, - package: ProjectPackage, - extra: str | None, - locked: bool, + installer: Installer, + pool: RepositoryPool, + locker: Locker, + installed: CustomInstalledRepository, + repo: Repository, + config: Config, + package: ProjectPackage, + extra: str | None, + locked: bool, ) -> None: """https://github.com/python-poetry/poetry/issues/834 @@ -1217,22 +1330,22 @@ def test_run_with_conflicting_root_dependency_extras( @pytest.mark.parametrize("locked", [True, False]) @pytest.mark.parametrize("extra", [None, "extra-one", "extra-two"]) def test_run_with_different_dependency_extras( - installer: Installer, - pool: RepositoryPool, - locker: Locker, - installed: CustomInstalledRepository, - repo: Repository, - config: Config, - package: ProjectPackage, - extra: str | None, - locked: bool, + installer: Installer, + pool: RepositoryPool, + locker: Locker, + installed: CustomInstalledRepository, + repo: Repository, + config: Config, + package: ProjectPackage, + extra: str | None, + locked: bool, ) -> None: """https://github.com/python-poetry/poetry/issues/834 This tests different sets of extras in a dependency of the root project. These different dependency extras are themselves conditioned on extras in the root project. """ - # Three packages in additon to root: demo (direct dependency) and two transitive dep packages + # Three packages in addition to root: demo (direct dependency) and two transitive dep packages demo_pkg = get_package("demo", "1.0.0") transitive_one_pkg = get_package("transitive-dep-one", "1.1.0") transitive_two_pkg = get_package("transitive-dep-two", "1.2.0") @@ -1256,8 +1369,10 @@ def test_run_with_different_dependency_extras( ) # Include both packages in both demo extras, to validate that they're filtered out based on extra markers alone demo_pkg.extras = { - canonicalize_name("demo-extra-one"): [get_dependency("transitive-dep-one"), get_dependency("transitive-dep-two")], - canonicalize_name("demo-extra-two"): [get_dependency("transitive-dep-one"), get_dependency("transitive-dep-two")], + canonicalize_name("demo-extra-one"): [get_dependency("transitive-dep-one"), + get_dependency("transitive-dep-two")], + canonicalize_name("demo-extra-two"): [get_dependency("transitive-dep-one"), + get_dependency("transitive-dep-two")], } demo_pkg.add_dependency(transitive_dep_one) demo_pkg.add_dependency(transitive_dep_two) @@ -1332,15 +1447,15 @@ def test_run_with_different_dependency_extras( @pytest.mark.parametrize("with_extras", [False, True]) @pytest.mark.parametrize("do_sync", [False, True]) def test_run_installs_extras_with_deps_if_requested( - installer: Installer, - locker: Locker, - repo: Repository, - installed: CustomInstalledRepository, - package: ProjectPackage, - is_locked: bool, - is_installed: bool, - with_extras: bool, - do_sync: bool, + installer: Installer, + locker: Locker, + repo: Repository, + installed: CustomInstalledRepository, + package: ProjectPackage, + is_locked: bool, + is_installed: bool, + with_extras: bool, + do_sync: bool, ) -> None: package.extras = {canonicalize_name("foo"): [get_dependency("C")]} package_a = get_package("A", "1.0") @@ -1396,12 +1511,12 @@ def test_run_installs_extras_with_deps_if_requested( def test_installer_with_pypi_repository( - package: ProjectPackage, - locker: Locker, - installed: CustomInstalledRepository, - config: Config, - env: NullEnv, - pypi_repository: PyPiRepository, + package: ProjectPackage, + locker: Locker, + installed: CustomInstalledRepository, + config: Config, + env: NullEnv, + pypi_repository: PyPiRepository, ) -> None: pool = RepositoryPool() pool.add_repository(pypi_repository) @@ -1420,11 +1535,11 @@ def test_installer_with_pypi_repository( def test_run_installs_with_local_file( - installer: Installer, - locker: Locker, - repo: Repository, - package: ProjectPackage, - fixture_dir: FixtureDirGetter, + installer: Installer, + locker: Locker, + repo: Repository, + package: ProjectPackage, + fixture_dir: FixtureDirGetter, ) -> None: root_dir = Path(__file__).parent.parent.parent package.root_dir = root_dir @@ -1448,11 +1563,11 @@ def test_run_installs_with_local_file( def test_run_installs_wheel_with_no_requires_dist( - installer: Installer, - locker: Locker, - repo: Repository, - package: ProjectPackage, - fixture_dir: FixtureDirGetter, + installer: Installer, + locker: Locker, + repo: Repository, + package: ProjectPackage, + fixture_dir: FixtureDirGetter, ) -> None: root_dir = Path(__file__).parent.parent.parent package.root_dir = root_dir @@ -1477,12 +1592,12 @@ def test_run_installs_wheel_with_no_requires_dist( def test_run_installs_with_local_poetry_directory_and_extras( - installer: Installer, - locker: Locker, - repo: Repository, - package: ProjectPackage, - tmpdir: Path, - fixture_dir: FixtureDirGetter, + installer: Installer, + locker: Locker, + repo: Repository, + package: ProjectPackage, + tmpdir: Path, + fixture_dir: FixtureDirGetter, ) -> None: root_dir = Path(__file__).parent.parent.parent package.root_dir = root_dir @@ -1509,12 +1624,12 @@ def test_run_installs_with_local_poetry_directory_and_extras( @pytest.mark.parametrize("skip_directory", [True, False]) def test_run_installs_with_local_poetry_directory_and_skip_directory_flag( - installer: Installer, - locker: Locker, - repo: Repository, - package: ProjectPackage, - fixture_dir: FixtureDirGetter, - skip_directory: bool, + installer: Installer, + locker: Locker, + repo: Repository, + package: ProjectPackage, + fixture_dir: FixtureDirGetter, + skip_directory: bool, ) -> None: """When we set Installer.skip_directory(True) no path dependencies should be installed (including transitive dependencies). @@ -1557,12 +1672,12 @@ def test_run_installs_with_local_poetry_directory_and_skip_directory_flag( def test_run_installs_with_local_poetry_file_transitive( - installer: Installer, - locker: Locker, - repo: Repository, - package: ProjectPackage, - tmpdir: str, - fixture_dir: FixtureDirGetter, + installer: Installer, + locker: Locker, + repo: Repository, + package: ProjectPackage, + tmpdir: str, + fixture_dir: FixtureDirGetter, ) -> None: root_dir = fixture_dir("directory") package.root_dir = root_dir @@ -1592,12 +1707,12 @@ def test_run_installs_with_local_poetry_file_transitive( def test_run_installs_with_local_setuptools_directory( - installer: Installer, - locker: Locker, - repo: Repository, - package: ProjectPackage, - tmp_path: Path, - fixture_dir: FixtureDirGetter, + installer: Installer, + locker: Locker, + repo: Repository, + package: ProjectPackage, + tmp_path: Path, + fixture_dir: FixtureDirGetter, ) -> None: root_dir = tmp_path / "root" package.root_dir = root_dir @@ -1624,7 +1739,7 @@ def test_run_installs_with_local_setuptools_directory( def test_run_with_prereleases( - installer: Installer, locker: Locker, repo: Repository, package: ProjectPackage + installer: Installer, locker: Locker, repo: Repository, package: ProjectPackage ) -> None: locker.locked(True) locker.mock_lock_data( @@ -1668,7 +1783,7 @@ def test_run_with_prereleases( def test_run_update_all_with_lock( - installer: Installer, locker: Locker, repo: Repository, package: ProjectPackage + installer: Installer, locker: Locker, repo: Repository, package: ProjectPackage ) -> None: locker.locked(True) locker.mock_lock_data( @@ -1707,7 +1822,7 @@ def test_run_update_all_with_lock( def test_run_update_with_locked_extras( - installer: Installer, locker: Locker, repo: Repository, package: ProjectPackage + installer: Installer, locker: Locker, repo: Repository, package: ProjectPackage ) -> None: locker.locked(True) locker.mock_lock_data( @@ -1778,7 +1893,7 @@ def test_run_update_with_locked_extras( def test_run_install_duplicate_dependencies_different_constraints( - installer: Installer, locker: Locker, repo: Repository, package: ProjectPackage + installer: Installer, locker: Locker, repo: Repository, package: ProjectPackage ) -> None: package.add_dependency(Factory.create_dependency("A", "*")) @@ -1823,7 +1938,7 @@ def test_run_install_duplicate_dependencies_different_constraints( def test_run_install_duplicate_dependencies_different_constraints_with_lock( - installer: Installer, locker: Locker, repo: Repository, package: ProjectPackage + installer: Installer, locker: Locker, repo: Repository, package: ProjectPackage ) -> None: locker.locked(True) locker.mock_lock_data( @@ -1926,11 +2041,11 @@ def test_run_install_duplicate_dependencies_different_constraints_with_lock( def test_run_update_uninstalls_after_removal_transitive_dependency( - installer: Installer, - locker: Locker, - repo: Repository, - package: ProjectPackage, - installed: CustomInstalledRepository, + installer: Installer, + locker: Locker, + repo: Repository, + package: ProjectPackage, + installed: CustomInstalledRepository, ) -> None: locker.locked(True) locker.mock_lock_data( @@ -1987,11 +2102,11 @@ def test_run_update_uninstalls_after_removal_transitive_dependency( def test_run_install_duplicate_dependencies_different_constraints_with_lock_update( - installer: Installer, - locker: Locker, - repo: Repository, - package: ProjectPackage, - installed: CustomInstalledRepository, + installer: Installer, + locker: Locker, + repo: Repository, + package: ProjectPackage, + installed: CustomInstalledRepository, ) -> None: locker.locked(True) locker.mock_lock_data( @@ -2092,11 +2207,11 @@ def test_run_install_duplicate_dependencies_different_constraints_with_lock_upda def test_installer_test_solver_finds_compatible_package_for_dependency_python_not_fully_compatible_with_package_python( - installer: Installer, - locker: Locker, - repo: Repository, - package: ProjectPackage, - installed: CustomInstalledRepository, + installer: Installer, + locker: Locker, + repo: Repository, + package: ProjectPackage, + installed: CustomInstalledRepository, ) -> None: package.python_versions = "~2.7 || ^3.4" package.add_dependency( @@ -2121,14 +2236,14 @@ def test_installer_test_solver_finds_compatible_package_for_dependency_python_no def test_installer_required_extras_should_not_be_removed_when_updating_single_dependency( - installer: Installer, - locker: Locker, - repo: Repository, - package: ProjectPackage, - installed: CustomInstalledRepository, - env: NullEnv, - pool: RepositoryPool, - config: Config, + installer: Installer, + locker: Locker, + repo: Repository, + package: ProjectPackage, + installed: CustomInstalledRepository, + env: NullEnv, + pool: RepositoryPool, + config: Config, ) -> None: package.add_dependency(Factory.create_dependency("A", {"version": "^1.0"})) @@ -2188,14 +2303,14 @@ def test_installer_required_extras_should_not_be_removed_when_updating_single_de def test_installer_required_extras_should_not_be_removed_when_updating_single_dependency_pypi_repository( - locker: Locker, - repo: Repository, - package: ProjectPackage, - installed: CustomInstalledRepository, - env: NullEnv, - mocker: MockerFixture, - config: Config, - pypi_repository: PyPiRepository, + locker: Locker, + repo: Repository, + package: ProjectPackage, + installed: CustomInstalledRepository, + env: NullEnv, + mocker: MockerFixture, + config: Config, + pypi_repository: PyPiRepository, ) -> None: mocker.patch("sys.platform", "darwin") @@ -2257,13 +2372,13 @@ def test_installer_required_extras_should_not_be_removed_when_updating_single_de def test_installer_required_extras_should_be_installed( - locker: Locker, - repo: Repository, - package: ProjectPackage, - installed: CustomInstalledRepository, - env: NullEnv, - config: Config, - pypi_repository: PyPiRepository, + locker: Locker, + repo: Repository, + package: ProjectPackage, + installed: CustomInstalledRepository, + env: NullEnv, + config: Config, + pypi_repository: PyPiRepository, ) -> None: pool = RepositoryPool() pool.add_repository(pypi_repository) @@ -2315,7 +2430,7 @@ def test_installer_required_extras_should_be_installed( def test_update_multiple_times_with_split_dependencies_is_idempotent( - installer: Installer, locker: Locker, repo: Repository, package: ProjectPackage + installer: Installer, locker: Locker, repo: Repository, package: ProjectPackage ) -> None: locker.locked(True) locker.mock_lock_data( @@ -2395,13 +2510,13 @@ def test_update_multiple_times_with_split_dependencies_is_idempotent( def test_installer_can_install_dependencies_from_forced_source( - locker: Locker, - package: ProjectPackage, - installed: CustomInstalledRepository, - env: NullEnv, - config: Config, - legacy_repository: LegacyRepository, - pypi_repository: PyPiRepository, + locker: Locker, + package: ProjectPackage, + installed: CustomInstalledRepository, + env: NullEnv, + config: Config, + legacy_repository: LegacyRepository, + pypi_repository: PyPiRepository, ) -> None: package.python_versions = "^3.7" package.add_dependency( @@ -2432,7 +2547,7 @@ def test_installer_can_install_dependencies_from_forced_source( def test_run_installs_with_url_file( - installer: Installer, locker: Locker, repo: Repository, package: ProjectPackage + installer: Installer, locker: Locker, repo: Repository, package: ProjectPackage ) -> None: url = "https://files.pythonhosted.org/distributions/demo-0.1.0-py2.py3-none-any.whl" package.add_dependency(Factory.create_dependency("demo", {"url": url})) @@ -2451,13 +2566,13 @@ def test_run_installs_with_url_file( @pytest.mark.parametrize("env_platform", ["linux", "win32"]) def test_run_installs_with_same_version_url_files( - pool: RepositoryPool, - locker: Locker, - installed: CustomInstalledRepository, - config: Config, - repo: Repository, - package: ProjectPackage, - env_platform: str, + pool: RepositoryPool, + locker: Locker, + installed: CustomInstalledRepository, + config: Config, + repo: Repository, + package: ProjectPackage, + env_platform: str, ) -> None: urls = { "linux": "https://files.pythonhosted.org/distributions/demo-0.1.0.tar.gz", @@ -2501,7 +2616,7 @@ def test_run_installs_with_same_version_url_files( def test_installer_uses_prereleases_if_they_are_compatible( - installer: Installer, locker: Locker, package: ProjectPackage, repo: Repository + installer: Installer, locker: Locker, package: ProjectPackage, repo: Repository ) -> None: package.python_versions = "~2.7 || ^3.4" package.add_dependency( @@ -2532,11 +2647,11 @@ def test_installer_uses_prereleases_if_they_are_compatible( def test_installer_does_not_write_lock_file_when_installation_fails( - installer: Installer, - locker: Locker, - repo: Repository, - package: ProjectPackage, - mocker: MockerFixture, + installer: Installer, + locker: Locker, + repo: Repository, + package: ProjectPackage, + mocker: MockerFixture, ) -> None: repo.add_package(get_package("A", "1.0")) package.add_dependency(Factory.create_dependency("A", "~1.0")) @@ -2556,11 +2671,11 @@ def test_installer_does_not_write_lock_file_when_installation_fails( @pytest.mark.parametrize("quiet", [True, False]) def test_run_with_dependencies_quiet( - installer: Installer, - locker: Locker, - repo: Repository, - package: ProjectPackage, - quiet: bool, + installer: Installer, + locker: Locker, + repo: Repository, + package: ProjectPackage, + quiet: bool, ) -> None: package_a = get_package("A", "1.0") package_b = get_package("B", "1.1") @@ -2587,7 +2702,7 @@ def test_run_with_dependencies_quiet( def test_installer_should_use_the_locked_version_of_git_dependencies( - installer: Installer, locker: Locker, package: ProjectPackage, repo: Repository + installer: Installer, locker: Locker, package: ProjectPackage, repo: Repository ) -> None: locker.locked(True) locker.mock_lock_data( @@ -2651,11 +2766,11 @@ def test_installer_should_use_the_locked_version_of_git_dependencies( @pytest.mark.parametrize("is_locked", [False, True]) def test_installer_should_use_the_locked_version_of_git_dependencies_with_extras( - installer: Installer, - locker: Locker, - package: ProjectPackage, - repo: Repository, - is_locked: bool, + installer: Installer, + locker: Locker, + package: ProjectPackage, + repo: Repository, + is_locked: bool, ) -> None: if is_locked: locker.locked(True) @@ -2695,11 +2810,11 @@ def test_installer_should_use_the_locked_version_of_git_dependencies_with_extras @pytest.mark.parametrize("is_locked", [False, True]) def test_installer_should_use_the_locked_version_of_git_dependencies_without_reference( - installer: Installer, - locker: Locker, - package: ProjectPackage, - repo: Repository, - is_locked: bool, + installer: Installer, + locker: Locker, + package: ProjectPackage, + repo: Repository, + is_locked: bool, ) -> None: """ If there is no explicit reference (branch or tag or rev) in pyproject.toml, @@ -2735,13 +2850,13 @@ def test_installer_should_use_the_locked_version_of_git_dependencies_without_ref @pytest.mark.parametrize("env_platform", ["darwin", "linux"]) def test_installer_distinguishes_locked_packages_with_local_version_by_source( - pool: RepositoryPool, - locker: Locker, - installed: CustomInstalledRepository, - config: Config, - repo: Repository, - package: ProjectPackage, - env_platform: str, + pool: RepositoryPool, + locker: Locker, + installed: CustomInstalledRepository, + config: Config, + repo: Repository, + 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 @@ -2838,13 +2953,13 @@ def test_installer_distinguishes_locked_packages_with_local_version_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, + 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( @@ -2942,13 +3057,13 @@ def test_installer_distinguishes_locked_packages_with_same_version_by_source( @pytest.mark.parametrize("env_platform", ["darwin", "linux"]) def test_explicit_source_dependency_with_direct_origin_dependency( - pool: RepositoryPool, - locker: Locker, - installed: CustomInstalledRepository, - config: Config, - repo: Repository, - package: ProjectPackage, - env_platform: str, + pool: RepositoryPool, + locker: Locker, + installed: CustomInstalledRepository, + config: Config, + repo: Repository, + package: ProjectPackage, + env_platform: str, ) -> None: """ A dependency with explicit source should not be satisfied by From 1326833b925820e2451a7c8a42b5e25c60bb2670 Mon Sep 17 00:00:00 2001 From: Reese Hyde Date: Thu, 14 Nov 2024 16:07:57 -0500 Subject: [PATCH 20/39] add overrides for different-extra duplicates to fix conflicting transitive deps case --- src/poetry/puzzle/provider.py | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/src/poetry/puzzle/provider.py b/src/poetry/puzzle/provider.py index ecadc022e4b..63f5a737908 100644 --- a/src/poetry/puzzle/provider.py +++ b/src/poetry/puzzle/provider.py @@ -608,7 +608,7 @@ def complete_package( # • pypiwin32 (219); sys_platform == "win32" and python_version < "3.6" duplicates: dict[str, list[Dependency]] = defaultdict(list) for dep in dependencies: - duplicates[dep.complete_name].append(dep) + duplicates[dep.name].append(dep) dependencies = [] for dep_name, deps in duplicates.items(): @@ -619,16 +619,19 @@ def complete_package( self.debug(f"Duplicate dependencies for {dep_name}") # For dependency resolution, markers of duplicate dependencies must be - # mutually exclusive. - active_extras = ( - self._active_root_extras if package.is_root() else dependency.extras - ) - deps = self._resolve_overlapping_markers(package, deps, active_extras) + # mutually exclusive. Perform overlapping marker resolution if the + # duplicates share all share a complete_name (i.e. are the same exact + # package, including in their extra definitions) + if len(set(d.complete_name for d in deps)) == 1: + active_extras = ( + self._active_root_extras if package.is_root() else dependency.extras + ) + deps = self._resolve_overlapping_markers(package, deps, active_extras) - if len(deps) == 1: - self.debug(f"Merging requirements for {dep_name}") - dependencies.append(deps[0]) - continue + if len(deps) == 1: + self.debug(f"Merging requirements for {dep_name}") + dependencies.append(deps[0]) + continue # At this point, we raise an exception that will # tell the solver to make new resolutions with specific overrides. From 449a2f029bccf7f35b19328d0962bf2f8e4c9b9c Mon Sep 17 00:00:00 2001 From: Reese Hyde Date: Thu, 14 Nov 2024 16:36:39 -0500 Subject: [PATCH 21/39] test cleanup: improve docstrings and make extra specs harder to solve --- .../with-conflicting-dependency-extras-root.test | 4 ++-- ...-conflicting-dependency-extras-transitive.test | 8 ++++---- tests/installation/test_installer.py | 15 +++++++++------ 3 files changed, 15 insertions(+), 12 deletions(-) diff --git a/tests/installation/fixtures/with-conflicting-dependency-extras-root.test b/tests/installation/fixtures/with-conflicting-dependency-extras-root.test index 1d8f0db385c..4ed9440f6d4 100644 --- a/tests/installation/fixtures/with-conflicting-dependency-extras-root.test +++ b/tests/installation/fixtures/with-conflicting-dependency-extras-root.test @@ -15,8 +15,8 @@ python-versions = "*" files = [ ] [extras] -extra-one = [ "conflicting-dep" ] -extra-two = [ "conflicting-dep" ] +extra-one = [ "conflicting-dep", "conflicting-dep" ] +extra-two = [ "conflicting-dep", "conflicting-dep" ] [metadata] lock-version = "2.0" diff --git a/tests/installation/fixtures/with-conflicting-dependency-extras-transitive.test b/tests/installation/fixtures/with-conflicting-dependency-extras-transitive.test index 2550d8dbc4f..3012d7de9a6 100644 --- a/tests/installation/fixtures/with-conflicting-dependency-extras-transitive.test +++ b/tests/installation/fixtures/with-conflicting-dependency-extras-transitive.test @@ -33,12 +33,12 @@ optional = true markers = 'extra != "extra-one" and extra == "extra-two"' [package.extras] - extra-one = [ "conflicting-dep (==1.1.0)" ] - extra-two = [ "conflicting-dep (==1.2.0)" ] + extra-one = [ "conflicting-dep (==1.1.0)", "conflicting-dep (==1.2.0)" ] + extra-two = [ "conflicting-dep (==1.1.0)", "conflicting-dep (==1.2.0)" ] [extras] -root-extra-one = [ "intermediate-dep" ] -root-extra-two = [ "intermediate-dep" ] +root-extra-one = [ "intermediate-dep", "intermediate-dep" ] +root-extra-two = [ "intermediate-dep", "intermediate-dep" ] [metadata] lock-version = "2.0" diff --git a/tests/installation/test_installer.py b/tests/installation/test_installer.py index 560344696e2..1f36827e430 100644 --- a/tests/installation/test_installer.py +++ b/tests/installation/test_installer.py @@ -1042,8 +1042,9 @@ def test_run_with_conflicting_dependency_extras( root: bool, ) -> None: """https://github.com/python-poetry/poetry/issues/834 - Tests resolution of extras in both root ('extra-one', 'extra-two') and transitive - ('demo-extra-one', 'demo-extra-two') dependencies + + Tests resolution of extras with conflicting dependencies. Tests in both as direct dependencies of + root package and as transitive dependencies. """ # A package with two optional dependencies, one for each extra # If root, this is the root package, otherwise an intermediate package @@ -1070,9 +1071,10 @@ def test_run_with_conflicting_dependency_extras( } ) + # Include both just for extra validation that our marker validation works as expected main_package.extras = { - canonicalize_name("extra-one"): [conflicting_dep_one], - canonicalize_name("extra-two"): [conflicting_dep_two], + canonicalize_name("extra-one"): [conflicting_dep_one, conflicting_dep_two], + canonicalize_name("extra-two"): [conflicting_dep_one, conflicting_dep_two], } main_package.add_dependency(conflicting_dep_one) @@ -1106,9 +1108,10 @@ def test_run_with_conflicting_dependency_extras( ) package.add_dependency(extra_one_dep) package.add_dependency(extra_two_dep) + # Include both just for extra validation that our marker validation works as expected package.extras = { - canonicalize_name("root-extra-one"): [extra_one_dep], - canonicalize_name("root-extra-two"): [extra_two_dep], + canonicalize_name("root-extra-one"): [extra_one_dep, extra_two_dep], + canonicalize_name("root-extra-two"): [extra_one_dep, extra_two_dep], } fixture_name = "with-conflicting-dependency-extras-" + ("root" if root else "transitive") From 84482a2518d4481ac8e696a0b57de81b9aaf8e66 Mon Sep 17 00:00:00 2001 From: Reese Hyde Date: Thu, 14 Nov 2024 16:40:43 -0500 Subject: [PATCH 22/39] remove redundant test --- tests/installation/test_installer.py | 80 +--------------------------- 1 file changed, 1 insertion(+), 79 deletions(-) diff --git a/tests/installation/test_installer.py b/tests/installation/test_installer.py index 1f36827e430..7213e5f249c 100644 --- a/tests/installation/test_installer.py +++ b/tests/installation/test_installer.py @@ -1077,10 +1077,8 @@ def test_run_with_conflicting_dependency_extras( canonicalize_name("extra-two"): [conflicting_dep_one, conflicting_dep_two], } main_package.add_dependency(conflicting_dep_one) + main_package.add_dependency(conflicting_dep_two) - main_package.add_dependency( - conflicting_dep_two - ) repo.add_package(conflicting_dep_one_pkg) repo.add_package(conflicting_dep_two_pkg) if not root: @@ -1254,82 +1252,6 @@ def test_run_with_exclusive_extras_different_sources( ) -@pytest.mark.parametrize("locked", [True, False]) -@pytest.mark.parametrize("extra", [None, "extra-one", "extra-two"]) -def test_run_with_conflicting_root_dependency_extras( - installer: Installer, - pool: RepositoryPool, - locker: Locker, - installed: CustomInstalledRepository, - repo: Repository, - config: Config, - package: ProjectPackage, - extra: str | None, - locked: bool, -) -> None: - """https://github.com/python-poetry/poetry/issues/834 - - Tests resolution of extras in both root ('extra-one', 'extra-two') and transitive - ('demo-extra-one', 'demo-extra-two') dependencies - """ - dep_one = get_package("dependency", "1.1.0") - dep_two = get_package("dependency", "1.2.0") - package.extras = { - canonicalize_name("extra-one"): [ - get_dependency("dependency", constraint={"version": "1.1.0", "optional": True}) - ], - canonicalize_name("extra-two"): [ - get_dependency("dependency", constraint={"version": "1.2.0", "optional": True}) - ], - } - package.add_dependency( - Factory.create_dependency( - "dependency", - { - "version": "1.1.0", - "markers": "extra == 'extra-one' and extra != 'extra-two'", - "optional": True, - } - ) - ) - package.add_dependency( - Factory.create_dependency( - "dependency", - { - "version": "1.2.0", - "markers": "extra != 'extra-one' and extra == 'extra-two'", - "optional": True, - } - ) - ) - repo.add_package(dep_one) - repo.add_package(dep_two) - - locker.locked(locked) - if locked: - locker.mock_lock_data(dict(fixture("with-conflicting-dependencies-root-extras"))) - - if extra is not None: - installer.extras([extra]) - result = installer.run() - assert result == 0 - - if not locked: - expected = fixture("with-conflicting-dependencies-root-extras") - assert locker.written_data == expected - - # Results of installation are consistent with the 'extra' input - assert isinstance(installer.executor, Executor) - if extra is None: - assert len(installer.executor.installations) == 0 - elif extra == "extra-one": - assert installer.executor.installations == [dep_one] - elif extra == "extra-two": - assert installer.executor.installations == [dep_two] - else: - raise ValueError(f"Unexpected extra value {extra}") - - @pytest.mark.parametrize("locked", [True, False]) @pytest.mark.parametrize("extra", [None, "extra-one", "extra-two"]) def test_run_with_different_dependency_extras( From 4e097a972ff37533c92b954cc6287bb15636fcea Mon Sep 17 00:00:00 2001 From: Reese Hyde Date: Thu, 14 Nov 2024 16:43:26 -0500 Subject: [PATCH 23/39] update issue links in unit tests --- tests/installation/test_installer.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/installation/test_installer.py b/tests/installation/test_installer.py index 7213e5f249c..0c66b57385b 100644 --- a/tests/installation/test_installer.py +++ b/tests/installation/test_installer.py @@ -1041,7 +1041,8 @@ def test_run_with_conflicting_dependency_extras( locked: bool, root: bool, ) -> None: - """https://github.com/python-poetry/poetry/issues/834 + """ + - https://github.com/python-poetry/poetry/issues/6419 Tests resolution of extras with conflicting dependencies. Tests in both as direct dependencies of root package and as transitive dependencies. @@ -1156,6 +1157,7 @@ def test_run_with_exclusive_extras_different_sources( - https://github.com/python-poetry/poetry/issues/6409 - https://github.com/python-poetry/poetry/issues/6419 - https://github.com/python-poetry/poetry/issues/7748 + - https://github.com/python-poetry/poetry/issues/9537 """ # Setup repo for each of our sources cpu_repo = Repository("pytorch-cpu") @@ -1265,7 +1267,9 @@ def test_run_with_different_dependency_extras( extra: str | None, locked: bool, ) -> None: - """https://github.com/python-poetry/poetry/issues/834 + """ + - https://github.com/python-poetry/poetry/issues/834 + - https://github.com/python-poetry/poetry/issues/7748 This tests different sets of extras in a dependency of the root project. These different dependency extras are themselves conditioned on extras in the root project. From 73cf6cdc411d5fc87ed598099d4742600d921877 Mon Sep 17 00:00:00 2001 From: Reese Hyde Date: Thu, 14 Nov 2024 16:48:43 -0500 Subject: [PATCH 24/39] apply pre-commit hooks --- src/poetry/puzzle/provider.py | 9 ++-- tests/installation/test_installer.py | 76 +++++++++++++++------------- 2 files changed, 47 insertions(+), 38 deletions(-) diff --git a/src/poetry/puzzle/provider.py b/src/poetry/puzzle/provider.py index 63f5a737908..21d033a8cf3 100644 --- a/src/poetry/puzzle/provider.py +++ b/src/poetry/puzzle/provider.py @@ -557,7 +557,8 @@ def complete_package( if self._env: marker_values = ( - self._marker_values(self._active_root_extras) if package.is_root() + self._marker_values(self._active_root_extras) + if package.is_root() else self._env.marker_env ) if not dep.marker.validate(marker_values): @@ -622,7 +623,7 @@ def complete_package( # mutually exclusive. Perform overlapping marker resolution if the # duplicates share all share a complete_name (i.e. are the same exact # package, including in their extra definitions) - if len(set(d.complete_name for d in deps)) == 1: + if len({d.complete_name for d in deps}) == 1: active_extras = ( self._active_root_extras if package.is_root() else dependency.extras ) @@ -988,6 +989,8 @@ def _marker_values( """ result = self._env.marker_env.copy() if self._env is not None else {} if extras is not None: - assert "extra" not in result, "'extra' marker key is already present in environment" + assert ( + "extra" not in result + ), "'extra' marker key is already present in environment" result["extra"] = set(extras) return result diff --git a/tests/installation/test_installer.py b/tests/installation/test_installer.py index 0c66b57385b..09ba29bd39c 100644 --- a/tests/installation/test_installer.py +++ b/tests/installation/test_installer.py @@ -1030,16 +1030,16 @@ def test_run_with_dependencies_nested_extras( @pytest.mark.parametrize("locked", [False, True]) @pytest.mark.parametrize("extra", [None, "extra-one", "extra-two"]) def test_run_with_conflicting_dependency_extras( - installer: Installer, - pool: RepositoryPool, - locker: Locker, - installed: CustomInstalledRepository, - repo: Repository, - config: Config, - package: ProjectPackage, - extra: str | None, - locked: bool, - root: bool, + installer: Installer, + pool: RepositoryPool, + locker: Locker, + installed: CustomInstalledRepository, + repo: Repository, + config: Config, + package: ProjectPackage, + extra: str | None, + locked: bool, + root: bool, ) -> None: """ - https://github.com/python-poetry/poetry/issues/6419 @@ -1061,7 +1061,7 @@ def test_run_with_conflicting_dependency_extras( "version": "1.1.0", "markers": "extra == 'extra-one' and extra != 'extra-two'", "optional": True, - } + }, ) conflicting_dep_two = Factory.create_dependency( "conflicting-dep", @@ -1069,7 +1069,7 @@ def test_run_with_conflicting_dependency_extras( "version": "1.2.0", "markers": "extra != 'extra-one' and extra == 'extra-two'", "optional": True, - } + }, ) # Include both just for extra validation that our marker validation works as expected @@ -1113,7 +1113,9 @@ def test_run_with_conflicting_dependency_extras( canonicalize_name("root-extra-two"): [extra_one_dep, extra_two_dep], } - fixture_name = "with-conflicting-dependency-extras-" + ("root" if root else "transitive") + fixture_name = "with-conflicting-dependency-extras-" + ( + "root" if root else "transitive" + ) locker.locked(locked) if locked: locker.mock_lock_data(dict(fixture(fixture_name))) @@ -1145,13 +1147,13 @@ def test_run_with_conflicting_dependency_extras( @pytest.mark.parametrize("locked", [True, False]) @pytest.mark.parametrize("extra", [None, "cpu", "cuda"]) def test_run_with_exclusive_extras_different_sources( - installer: Installer, - locker: Locker, - installed: CustomInstalledRepository, - config: Config, - package: ProjectPackage, - extra: str | None, - locked: bool, + installer: Installer, + locker: Locker, + installed: CustomInstalledRepository, + config: Config, + package: ProjectPackage, + extra: str | None, + locked: bool, ) -> None: """ - https://github.com/python-poetry/poetry/issues/6409 @@ -1257,15 +1259,15 @@ def test_run_with_exclusive_extras_different_sources( @pytest.mark.parametrize("locked", [True, False]) @pytest.mark.parametrize("extra", [None, "extra-one", "extra-two"]) def test_run_with_different_dependency_extras( - installer: Installer, - pool: RepositoryPool, - locker: Locker, - installed: CustomInstalledRepository, - repo: Repository, - config: Config, - package: ProjectPackage, - extra: str | None, - locked: bool, + installer: Installer, + pool: RepositoryPool, + locker: Locker, + installed: CustomInstalledRepository, + repo: Repository, + config: Config, + package: ProjectPackage, + extra: str | None, + locked: bool, ) -> None: """ - https://github.com/python-poetry/poetry/issues/834 @@ -1286,7 +1288,7 @@ def test_run_with_different_dependency_extras( "version": "1.1.0", "markers": "extra == 'demo-extra-one' and extra != 'demo-extra-two'", "optional": True, - } + }, ) transitive_dep_two = Factory.create_dependency( "transitive-dep-two", @@ -1294,14 +1296,18 @@ def test_run_with_different_dependency_extras( "version": "1.2.0", "markers": "extra != 'demo-extra-one' and extra == 'demo-extra-two'", "optional": True, - } + }, ) # Include both packages in both demo extras, to validate that they're filtered out based on extra markers alone demo_pkg.extras = { - canonicalize_name("demo-extra-one"): [get_dependency("transitive-dep-one"), - get_dependency("transitive-dep-two")], - canonicalize_name("demo-extra-two"): [get_dependency("transitive-dep-one"), - get_dependency("transitive-dep-two")], + canonicalize_name("demo-extra-one"): [ + get_dependency("transitive-dep-one"), + get_dependency("transitive-dep-two"), + ], + canonicalize_name("demo-extra-two"): [ + get_dependency("transitive-dep-one"), + get_dependency("transitive-dep-two"), + ], } demo_pkg.add_dependency(transitive_dep_one) demo_pkg.add_dependency(transitive_dep_two) From 3811ab58c92fc5c3f36de685939ef61841550b0c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Randy=20D=C3=B6ring?= <30527984+radoering@users.noreply.github.com> Date: Sat, 16 Nov 2024 08:20:23 +0100 Subject: [PATCH 25/39] pre-commit --- tests/installation/test_installer.py | 445 ++++++++++++++------------- 1 file changed, 223 insertions(+), 222 deletions(-) diff --git a/tests/installation/test_installer.py b/tests/installation/test_installer.py index 09ba29bd39c..58dbe101559 100644 --- a/tests/installation/test_installer.py +++ b/tests/installation/test_installer.py @@ -34,6 +34,7 @@ from tests.helpers import get_dependency from tests.helpers import get_package + if TYPE_CHECKING: from pytest_mock import MockerFixture @@ -86,7 +87,7 @@ def _execute_uninstall(self, operation: Operation) -> int: class CustomInstalledRepository(InstalledRepository): @classmethod def load( - cls, env: Env, with_dependencies: bool = False + cls, env: Env, with_dependencies: bool = False ) -> CustomInstalledRepository: return cls() @@ -179,12 +180,12 @@ def env(tmp_path: Path) -> NullEnv: @pytest.fixture() def installer( - package: ProjectPackage, - pool: RepositoryPool, - locker: Locker, - env: NullEnv, - installed: CustomInstalledRepository, - config: Config, + package: ProjectPackage, + pool: RepositoryPool, + locker: Locker, + env: NullEnv, + installed: CustomInstalledRepository, + config: Config, ) -> Installer: return Installer( NullIO(), @@ -216,17 +217,17 @@ def test_run_no_dependencies(installer: Installer, locker: Locker) -> None: def test_not_fresh_lock(installer: Installer, locker: Locker) -> None: locker.locked().fresh(False) with pytest.raises( - ValueError, - match=re.escape( - "pyproject.toml changed significantly since poetry.lock was last generated. " - "Run `poetry lock [--no-update]` to fix the lock file." - ), + ValueError, + match=re.escape( + "pyproject.toml changed significantly since poetry.lock was last generated. " + "Run `poetry lock [--no-update]` to fix the lock file." + ), ): installer.run() def test_run_with_dependencies( - installer: Installer, locker: Locker, repo: Repository, package: ProjectPackage + installer: Installer, locker: Locker, repo: Repository, package: ProjectPackage ) -> None: package_a = get_package("A", "1.0") package_b = get_package("B", "1.1") @@ -244,11 +245,11 @@ def test_run_with_dependencies( def test_run_update_after_removing_dependencies( - installer: Installer, - locker: Locker, - repo: Repository, - package: ProjectPackage, - installed: CustomInstalledRepository, + installer: Installer, + locker: Locker, + repo: Repository, + package: ProjectPackage, + installed: CustomInstalledRepository, ) -> None: locker.locked(True) locker.mock_lock_data( @@ -314,12 +315,12 @@ def test_run_update_after_removing_dependencies( def _configure_run_install_dev( - locker: Locker, - repo: Repository, - package: ProjectPackage, - installed: CustomInstalledRepository, - with_optional_group: bool = False, - with_packages_installed: bool = False, + locker: Locker, + repo: Repository, + package: ProjectPackage, + installed: CustomInstalledRepository, + with_optional_group: bool = False, + with_packages_installed: bool = False, ) -> None: """ Perform common test setup for `test_run_install_*dev*()` methods. @@ -397,16 +398,16 @@ def _configure_run_install_dev( ], ) def test_run_install_with_dependency_groups( - groups: list[str] | None, - installs: int, - updates: int, - removals: int, - with_packages_installed: bool, - installer: Installer, - locker: Locker, - repo: Repository, - package: ProjectPackage, - installed: CustomInstalledRepository, + groups: list[str] | None, + installs: int, + updates: int, + removals: int, + with_packages_installed: bool, + installer: Installer, + locker: Locker, + repo: Repository, + package: ProjectPackage, + installed: CustomInstalledRepository, ) -> None: _configure_run_install_dev( locker, @@ -430,11 +431,11 @@ def test_run_install_with_dependency_groups( def test_run_install_does_not_remove_locked_packages_if_installed_but_not_required( - installer: Installer, - locker: Locker, - repo: Repository, - package: ProjectPackage, - installed: CustomInstalledRepository, + installer: Installer, + locker: Locker, + repo: Repository, + package: ProjectPackage, + installed: CustomInstalledRepository, ) -> None: package_a = get_package("a", "1.0") package_b = get_package("b", "1.1") @@ -500,11 +501,11 @@ def test_run_install_does_not_remove_locked_packages_if_installed_but_not_requir def test_run_install_removes_locked_packages_if_installed_and_synchronization_is_required( - installer: Installer, - locker: Locker, - repo: Repository, - package: ProjectPackage, - installed: CustomInstalledRepository, + installer: Installer, + locker: Locker, + repo: Repository, + package: ProjectPackage, + installed: CustomInstalledRepository, ) -> None: package_a = get_package("a", "1.0") package_b = get_package("b", "1.1") @@ -570,11 +571,11 @@ def test_run_install_removes_locked_packages_if_installed_and_synchronization_is def test_run_install_removes_no_longer_locked_packages_if_installed( - installer: Installer, - locker: Locker, - repo: Repository, - package: ProjectPackage, - installed: CustomInstalledRepository, + installer: Installer, + locker: Locker, + repo: Repository, + package: ProjectPackage, + installed: CustomInstalledRepository, ) -> None: package_a = get_package("a", "1.0") package_b = get_package("b", "1.1") @@ -645,12 +646,12 @@ def test_run_install_removes_no_longer_locked_packages_if_installed( [(), ("pip",)], ) def test_run_install_with_synchronization( - managed_reserved_package_names: tuple[str, ...], - installer: Installer, - locker: Locker, - repo: Repository, - package: ProjectPackage, - installed: CustomInstalledRepository, + managed_reserved_package_names: tuple[str, ...], + installer: Installer, + locker: Locker, + repo: Repository, + package: ProjectPackage, + installed: CustomInstalledRepository, ) -> None: package_a = get_package("a", "1.0") package_b = get_package("b", "1.1") @@ -721,7 +722,7 @@ def test_run_install_with_synchronization( def test_run_whitelist_add( - installer: Installer, locker: Locker, repo: Repository, package: ProjectPackage + installer: Installer, locker: Locker, repo: Repository, package: ProjectPackage ) -> None: locker.locked(True) locker.mock_lock_data( @@ -765,11 +766,11 @@ def test_run_whitelist_add( def test_run_whitelist_remove( - installer: Installer, - locker: Locker, - repo: Repository, - package: ProjectPackage, - installed: CustomInstalledRepository, + installer: Installer, + locker: Locker, + repo: Repository, + package: ProjectPackage, + installed: CustomInstalledRepository, ) -> None: locker.locked(True) locker.mock_lock_data( @@ -822,7 +823,7 @@ def test_run_whitelist_remove( def test_add_with_sub_dependencies( - installer: Installer, locker: Locker, repo: Repository, package: ProjectPackage + installer: Installer, locker: Locker, repo: Repository, package: ProjectPackage ) -> None: package_a = get_package("A", "1.0") package_b = get_package("B", "1.1") @@ -847,7 +848,7 @@ def test_add_with_sub_dependencies( def test_run_with_python_versions( - installer: Installer, locker: Locker, repo: Repository, package: ProjectPackage + installer: Installer, locker: Locker, repo: Repository, package: ProjectPackage ) -> None: package.python_versions = "~2.7 || ^3.4" @@ -875,7 +876,7 @@ def test_run_with_python_versions( def test_run_with_optional_and_python_restricted_dependencies( - installer: Installer, locker: Locker, repo: Repository, package: ProjectPackage + installer: Installer, locker: Locker, repo: Repository, package: ProjectPackage ) -> None: package.python_versions = "~2.7 || ^3.4" @@ -919,11 +920,11 @@ def test_run_with_optional_and_python_restricted_dependencies( def test_run_with_optional_and_platform_restricted_dependencies( - installer: Installer, - locker: Locker, - repo: Repository, - package: ProjectPackage, - mocker: MockerFixture, + installer: Installer, + locker: Locker, + repo: Repository, + package: ProjectPackage, + mocker: MockerFixture, ) -> None: mocker.patch("sys.platform", "darwin") @@ -967,7 +968,7 @@ def test_run_with_optional_and_platform_restricted_dependencies( def test_run_with_dependencies_extras( - installer: Installer, locker: Locker, repo: Repository, package: ProjectPackage + installer: Installer, locker: Locker, repo: Repository, package: ProjectPackage ) -> None: package_a = get_package("A", "1.0") package_b = get_package("B", "1.0") @@ -995,7 +996,7 @@ def test_run_with_dependencies_extras( def test_run_with_dependencies_nested_extras( - installer: Installer, locker: Locker, repo: Repository, package: ProjectPackage + installer: Installer, locker: Locker, repo: Repository, package: ProjectPackage ) -> None: package_a = get_package("A", "1.0") package_b = get_package("B", "1.0") @@ -1382,15 +1383,15 @@ def test_run_with_different_dependency_extras( @pytest.mark.parametrize("with_extras", [False, True]) @pytest.mark.parametrize("do_sync", [False, True]) def test_run_installs_extras_with_deps_if_requested( - installer: Installer, - locker: Locker, - repo: Repository, - installed: CustomInstalledRepository, - package: ProjectPackage, - is_locked: bool, - is_installed: bool, - with_extras: bool, - do_sync: bool, + installer: Installer, + locker: Locker, + repo: Repository, + installed: CustomInstalledRepository, + package: ProjectPackage, + is_locked: bool, + is_installed: bool, + with_extras: bool, + do_sync: bool, ) -> None: package.extras = {canonicalize_name("foo"): [get_dependency("C")]} package_a = get_package("A", "1.0") @@ -1446,12 +1447,12 @@ def test_run_installs_extras_with_deps_if_requested( def test_installer_with_pypi_repository( - package: ProjectPackage, - locker: Locker, - installed: CustomInstalledRepository, - config: Config, - env: NullEnv, - pypi_repository: PyPiRepository, + package: ProjectPackage, + locker: Locker, + installed: CustomInstalledRepository, + config: Config, + env: NullEnv, + pypi_repository: PyPiRepository, ) -> None: pool = RepositoryPool() pool.add_repository(pypi_repository) @@ -1470,11 +1471,11 @@ def test_installer_with_pypi_repository( def test_run_installs_with_local_file( - installer: Installer, - locker: Locker, - repo: Repository, - package: ProjectPackage, - fixture_dir: FixtureDirGetter, + installer: Installer, + locker: Locker, + repo: Repository, + package: ProjectPackage, + fixture_dir: FixtureDirGetter, ) -> None: root_dir = Path(__file__).parent.parent.parent package.root_dir = root_dir @@ -1498,11 +1499,11 @@ def test_run_installs_with_local_file( def test_run_installs_wheel_with_no_requires_dist( - installer: Installer, - locker: Locker, - repo: Repository, - package: ProjectPackage, - fixture_dir: FixtureDirGetter, + installer: Installer, + locker: Locker, + repo: Repository, + package: ProjectPackage, + fixture_dir: FixtureDirGetter, ) -> None: root_dir = Path(__file__).parent.parent.parent package.root_dir = root_dir @@ -1527,12 +1528,12 @@ def test_run_installs_wheel_with_no_requires_dist( def test_run_installs_with_local_poetry_directory_and_extras( - installer: Installer, - locker: Locker, - repo: Repository, - package: ProjectPackage, - tmpdir: Path, - fixture_dir: FixtureDirGetter, + installer: Installer, + locker: Locker, + repo: Repository, + package: ProjectPackage, + tmpdir: Path, + fixture_dir: FixtureDirGetter, ) -> None: root_dir = Path(__file__).parent.parent.parent package.root_dir = root_dir @@ -1559,12 +1560,12 @@ def test_run_installs_with_local_poetry_directory_and_extras( @pytest.mark.parametrize("skip_directory", [True, False]) def test_run_installs_with_local_poetry_directory_and_skip_directory_flag( - installer: Installer, - locker: Locker, - repo: Repository, - package: ProjectPackage, - fixture_dir: FixtureDirGetter, - skip_directory: bool, + installer: Installer, + locker: Locker, + repo: Repository, + package: ProjectPackage, + fixture_dir: FixtureDirGetter, + skip_directory: bool, ) -> None: """When we set Installer.skip_directory(True) no path dependencies should be installed (including transitive dependencies). @@ -1607,12 +1608,12 @@ def test_run_installs_with_local_poetry_directory_and_skip_directory_flag( def test_run_installs_with_local_poetry_file_transitive( - installer: Installer, - locker: Locker, - repo: Repository, - package: ProjectPackage, - tmpdir: str, - fixture_dir: FixtureDirGetter, + installer: Installer, + locker: Locker, + repo: Repository, + package: ProjectPackage, + tmpdir: str, + fixture_dir: FixtureDirGetter, ) -> None: root_dir = fixture_dir("directory") package.root_dir = root_dir @@ -1642,12 +1643,12 @@ def test_run_installs_with_local_poetry_file_transitive( def test_run_installs_with_local_setuptools_directory( - installer: Installer, - locker: Locker, - repo: Repository, - package: ProjectPackage, - tmp_path: Path, - fixture_dir: FixtureDirGetter, + installer: Installer, + locker: Locker, + repo: Repository, + package: ProjectPackage, + tmp_path: Path, + fixture_dir: FixtureDirGetter, ) -> None: root_dir = tmp_path / "root" package.root_dir = root_dir @@ -1674,7 +1675,7 @@ def test_run_installs_with_local_setuptools_directory( def test_run_with_prereleases( - installer: Installer, locker: Locker, repo: Repository, package: ProjectPackage + installer: Installer, locker: Locker, repo: Repository, package: ProjectPackage ) -> None: locker.locked(True) locker.mock_lock_data( @@ -1718,7 +1719,7 @@ def test_run_with_prereleases( def test_run_update_all_with_lock( - installer: Installer, locker: Locker, repo: Repository, package: ProjectPackage + installer: Installer, locker: Locker, repo: Repository, package: ProjectPackage ) -> None: locker.locked(True) locker.mock_lock_data( @@ -1757,7 +1758,7 @@ def test_run_update_all_with_lock( def test_run_update_with_locked_extras( - installer: Installer, locker: Locker, repo: Repository, package: ProjectPackage + installer: Installer, locker: Locker, repo: Repository, package: ProjectPackage ) -> None: locker.locked(True) locker.mock_lock_data( @@ -1828,7 +1829,7 @@ def test_run_update_with_locked_extras( def test_run_install_duplicate_dependencies_different_constraints( - installer: Installer, locker: Locker, repo: Repository, package: ProjectPackage + installer: Installer, locker: Locker, repo: Repository, package: ProjectPackage ) -> None: package.add_dependency(Factory.create_dependency("A", "*")) @@ -1873,7 +1874,7 @@ def test_run_install_duplicate_dependencies_different_constraints( def test_run_install_duplicate_dependencies_different_constraints_with_lock( - installer: Installer, locker: Locker, repo: Repository, package: ProjectPackage + installer: Installer, locker: Locker, repo: Repository, package: ProjectPackage ) -> None: locker.locked(True) locker.mock_lock_data( @@ -1976,11 +1977,11 @@ def test_run_install_duplicate_dependencies_different_constraints_with_lock( def test_run_update_uninstalls_after_removal_transitive_dependency( - installer: Installer, - locker: Locker, - repo: Repository, - package: ProjectPackage, - installed: CustomInstalledRepository, + installer: Installer, + locker: Locker, + repo: Repository, + package: ProjectPackage, + installed: CustomInstalledRepository, ) -> None: locker.locked(True) locker.mock_lock_data( @@ -2037,11 +2038,11 @@ def test_run_update_uninstalls_after_removal_transitive_dependency( def test_run_install_duplicate_dependencies_different_constraints_with_lock_update( - installer: Installer, - locker: Locker, - repo: Repository, - package: ProjectPackage, - installed: CustomInstalledRepository, + installer: Installer, + locker: Locker, + repo: Repository, + package: ProjectPackage, + installed: CustomInstalledRepository, ) -> None: locker.locked(True) locker.mock_lock_data( @@ -2142,11 +2143,11 @@ def test_run_install_duplicate_dependencies_different_constraints_with_lock_upda def test_installer_test_solver_finds_compatible_package_for_dependency_python_not_fully_compatible_with_package_python( - installer: Installer, - locker: Locker, - repo: Repository, - package: ProjectPackage, - installed: CustomInstalledRepository, + installer: Installer, + locker: Locker, + repo: Repository, + package: ProjectPackage, + installed: CustomInstalledRepository, ) -> None: package.python_versions = "~2.7 || ^3.4" package.add_dependency( @@ -2171,14 +2172,14 @@ def test_installer_test_solver_finds_compatible_package_for_dependency_python_no def test_installer_required_extras_should_not_be_removed_when_updating_single_dependency( - installer: Installer, - locker: Locker, - repo: Repository, - package: ProjectPackage, - installed: CustomInstalledRepository, - env: NullEnv, - pool: RepositoryPool, - config: Config, + installer: Installer, + locker: Locker, + repo: Repository, + package: ProjectPackage, + installed: CustomInstalledRepository, + env: NullEnv, + pool: RepositoryPool, + config: Config, ) -> None: package.add_dependency(Factory.create_dependency("A", {"version": "^1.0"})) @@ -2238,14 +2239,14 @@ def test_installer_required_extras_should_not_be_removed_when_updating_single_de def test_installer_required_extras_should_not_be_removed_when_updating_single_dependency_pypi_repository( - locker: Locker, - repo: Repository, - package: ProjectPackage, - installed: CustomInstalledRepository, - env: NullEnv, - mocker: MockerFixture, - config: Config, - pypi_repository: PyPiRepository, + locker: Locker, + repo: Repository, + package: ProjectPackage, + installed: CustomInstalledRepository, + env: NullEnv, + mocker: MockerFixture, + config: Config, + pypi_repository: PyPiRepository, ) -> None: mocker.patch("sys.platform", "darwin") @@ -2307,13 +2308,13 @@ def test_installer_required_extras_should_not_be_removed_when_updating_single_de def test_installer_required_extras_should_be_installed( - locker: Locker, - repo: Repository, - package: ProjectPackage, - installed: CustomInstalledRepository, - env: NullEnv, - config: Config, - pypi_repository: PyPiRepository, + locker: Locker, + repo: Repository, + package: ProjectPackage, + installed: CustomInstalledRepository, + env: NullEnv, + config: Config, + pypi_repository: PyPiRepository, ) -> None: pool = RepositoryPool() pool.add_repository(pypi_repository) @@ -2365,7 +2366,7 @@ def test_installer_required_extras_should_be_installed( def test_update_multiple_times_with_split_dependencies_is_idempotent( - installer: Installer, locker: Locker, repo: Repository, package: ProjectPackage + installer: Installer, locker: Locker, repo: Repository, package: ProjectPackage ) -> None: locker.locked(True) locker.mock_lock_data( @@ -2445,13 +2446,13 @@ def test_update_multiple_times_with_split_dependencies_is_idempotent( def test_installer_can_install_dependencies_from_forced_source( - locker: Locker, - package: ProjectPackage, - installed: CustomInstalledRepository, - env: NullEnv, - config: Config, - legacy_repository: LegacyRepository, - pypi_repository: PyPiRepository, + locker: Locker, + package: ProjectPackage, + installed: CustomInstalledRepository, + env: NullEnv, + config: Config, + legacy_repository: LegacyRepository, + pypi_repository: PyPiRepository, ) -> None: package.python_versions = "^3.7" package.add_dependency( @@ -2482,7 +2483,7 @@ def test_installer_can_install_dependencies_from_forced_source( def test_run_installs_with_url_file( - installer: Installer, locker: Locker, repo: Repository, package: ProjectPackage + installer: Installer, locker: Locker, repo: Repository, package: ProjectPackage ) -> None: url = "https://files.pythonhosted.org/distributions/demo-0.1.0-py2.py3-none-any.whl" package.add_dependency(Factory.create_dependency("demo", {"url": url})) @@ -2501,13 +2502,13 @@ def test_run_installs_with_url_file( @pytest.mark.parametrize("env_platform", ["linux", "win32"]) def test_run_installs_with_same_version_url_files( - pool: RepositoryPool, - locker: Locker, - installed: CustomInstalledRepository, - config: Config, - repo: Repository, - package: ProjectPackage, - env_platform: str, + pool: RepositoryPool, + locker: Locker, + installed: CustomInstalledRepository, + config: Config, + repo: Repository, + package: ProjectPackage, + env_platform: str, ) -> None: urls = { "linux": "https://files.pythonhosted.org/distributions/demo-0.1.0.tar.gz", @@ -2551,7 +2552,7 @@ def test_run_installs_with_same_version_url_files( def test_installer_uses_prereleases_if_they_are_compatible( - installer: Installer, locker: Locker, package: ProjectPackage, repo: Repository + installer: Installer, locker: Locker, package: ProjectPackage, repo: Repository ) -> None: package.python_versions = "~2.7 || ^3.4" package.add_dependency( @@ -2582,11 +2583,11 @@ def test_installer_uses_prereleases_if_they_are_compatible( def test_installer_does_not_write_lock_file_when_installation_fails( - installer: Installer, - locker: Locker, - repo: Repository, - package: ProjectPackage, - mocker: MockerFixture, + installer: Installer, + locker: Locker, + repo: Repository, + package: ProjectPackage, + mocker: MockerFixture, ) -> None: repo.add_package(get_package("A", "1.0")) package.add_dependency(Factory.create_dependency("A", "~1.0")) @@ -2606,11 +2607,11 @@ def test_installer_does_not_write_lock_file_when_installation_fails( @pytest.mark.parametrize("quiet", [True, False]) def test_run_with_dependencies_quiet( - installer: Installer, - locker: Locker, - repo: Repository, - package: ProjectPackage, - quiet: bool, + installer: Installer, + locker: Locker, + repo: Repository, + package: ProjectPackage, + quiet: bool, ) -> None: package_a = get_package("A", "1.0") package_b = get_package("B", "1.1") @@ -2637,7 +2638,7 @@ def test_run_with_dependencies_quiet( def test_installer_should_use_the_locked_version_of_git_dependencies( - installer: Installer, locker: Locker, package: ProjectPackage, repo: Repository + installer: Installer, locker: Locker, package: ProjectPackage, repo: Repository ) -> None: locker.locked(True) locker.mock_lock_data( @@ -2701,11 +2702,11 @@ def test_installer_should_use_the_locked_version_of_git_dependencies( @pytest.mark.parametrize("is_locked", [False, True]) def test_installer_should_use_the_locked_version_of_git_dependencies_with_extras( - installer: Installer, - locker: Locker, - package: ProjectPackage, - repo: Repository, - is_locked: bool, + installer: Installer, + locker: Locker, + package: ProjectPackage, + repo: Repository, + is_locked: bool, ) -> None: if is_locked: locker.locked(True) @@ -2745,11 +2746,11 @@ def test_installer_should_use_the_locked_version_of_git_dependencies_with_extras @pytest.mark.parametrize("is_locked", [False, True]) def test_installer_should_use_the_locked_version_of_git_dependencies_without_reference( - installer: Installer, - locker: Locker, - package: ProjectPackage, - repo: Repository, - is_locked: bool, + installer: Installer, + locker: Locker, + package: ProjectPackage, + repo: Repository, + is_locked: bool, ) -> None: """ If there is no explicit reference (branch or tag or rev) in pyproject.toml, @@ -2785,13 +2786,13 @@ def test_installer_should_use_the_locked_version_of_git_dependencies_without_ref @pytest.mark.parametrize("env_platform", ["darwin", "linux"]) def test_installer_distinguishes_locked_packages_with_local_version_by_source( - pool: RepositoryPool, - locker: Locker, - installed: CustomInstalledRepository, - config: Config, - repo: Repository, - package: ProjectPackage, - env_platform: str, + pool: RepositoryPool, + locker: Locker, + installed: CustomInstalledRepository, + config: Config, + repo: Repository, + 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 @@ -2888,13 +2889,13 @@ def test_installer_distinguishes_locked_packages_with_local_version_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, + 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( @@ -2992,13 +2993,13 @@ def test_installer_distinguishes_locked_packages_with_same_version_by_source( @pytest.mark.parametrize("env_platform", ["darwin", "linux"]) def test_explicit_source_dependency_with_direct_origin_dependency( - pool: RepositoryPool, - locker: Locker, - installed: CustomInstalledRepository, - config: Config, - repo: Repository, - package: ProjectPackage, - env_platform: str, + pool: RepositoryPool, + locker: Locker, + installed: CustomInstalledRepository, + config: Config, + repo: Repository, + package: ProjectPackage, + env_platform: str, ) -> None: """ A dependency with explicit source should not be satisfied by From b4466750edfe93dfa122642765d11cf4e6be3485 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Randy=20D=C3=B6ring?= <30527984+radoering@users.noreply.github.com> Date: Sat, 16 Nov 2024 14:29:10 +0100 Subject: [PATCH 26/39] fix logic for adding overrides for different-extra duplicates --- src/poetry/puzzle/provider.py | 41 ++++++++++++++++++++++++++++------- 1 file changed, 33 insertions(+), 8 deletions(-) diff --git a/src/poetry/puzzle/provider.py b/src/poetry/puzzle/provider.py index 21d033a8cf3..b299def9ae8 100644 --- a/src/poetry/puzzle/provider.py +++ b/src/poetry/puzzle/provider.py @@ -620,20 +620,45 @@ def complete_package( self.debug(f"Duplicate dependencies for {dep_name}") # For dependency resolution, markers of duplicate dependencies must be - # mutually exclusive. Perform overlapping marker resolution if the - # duplicates share all share a complete_name (i.e. are the same exact - # package, including in their extra definitions) - if len({d.complete_name for d in deps}) == 1: + # mutually exclusive. However, we have to take care about duplicates + # with differing extras. + duplicates_by_extras: dict[str, list[Dependency]] = defaultdict(list) + for dep in deps: + duplicates_by_extras[dep.complete_name].append(dep) + + if len(duplicates_by_extras) == 1: active_extras = ( self._active_root_extras if package.is_root() else dependency.extras ) deps = self._resolve_overlapping_markers(package, deps, active_extras) - - if len(deps) == 1: - self.debug(f"Merging requirements for {dep_name}") - dependencies.append(deps[0]) + else: + # There are duplicates with different extras. + for complete_dep_name, deps_by_extra in duplicates_by_extras.items(): + if len(deps_by_extra) > 1: + duplicates_by_extras[complete_dep_name] = ( + self._resolve_overlapping_markers(package, deps, None) + ) + if all(len(d) == 1 for d in duplicates_by_extras.values()) and all( + d1[0].marker.intersect(d2[0].marker).is_empty() + for d1, d2 in itertools.combinations( + duplicates_by_extras.values(), 2 + ) + ): + # Since all markers are mutually exclusive, + # we can trigger overrides. + deps = list(itertools.chain(*duplicates_by_extras.values())) + else: + # Too complicated to handle with overrides, + # fallback to basic handling without overrides. + for d in duplicates_by_extras.values(): + dependencies.extend(d) continue + if len(deps) == 1: + self.debug(f"Merging requirements for {dep_name}") + dependencies.append(deps[0]) + continue + # At this point, we raise an exception that will # tell the solver to make new resolutions with specific overrides. # From 33eed996a197b2ce3cd4c6a83fb1f22ed93a289e Mon Sep 17 00:00:00 2001 From: Reese Hyde Date: Mon, 18 Nov 2024 10:07:32 -0500 Subject: [PATCH 27/39] pr feedback: remove redundant filtering and test fixture, improve solver test --- src/poetry/puzzle/provider.py | 11 +-------- ...-conflicting-dependencies-root-extras.test | 24 ------------------- tests/puzzle/test_solver.py | 7 ++++-- 3 files changed, 6 insertions(+), 36 deletions(-) delete mode 100644 tests/installation/fixtures/with-conflicting-dependencies-root-extras.test diff --git a/src/poetry/puzzle/provider.py b/src/poetry/puzzle/provider.py index b299def9ae8..b5e726a80a0 100644 --- a/src/poetry/puzzle/provider.py +++ b/src/poetry/puzzle/provider.py @@ -462,16 +462,7 @@ def incompatibilities_for( for dep in dependencies if dep.name not in self.UNSAFE_PACKAGES and self._python_constraint.allows_any(dep.python_constraint) - and ( - not self._env - or dep.marker.validate( - self._marker_values( - self._active_root_extras - if dependency_package.package.is_root() - else dependency_package.dependency.extras - ) - ) - ) + and (not self._env or dep.marker.validate(self._env.marker_env)) ] dependencies = self._get_dependencies_with_overrides(_dependencies, package) diff --git a/tests/installation/fixtures/with-conflicting-dependencies-root-extras.test b/tests/installation/fixtures/with-conflicting-dependencies-root-extras.test deleted file mode 100644 index 0534be4a068..00000000000 --- a/tests/installation/fixtures/with-conflicting-dependencies-root-extras.test +++ /dev/null @@ -1,24 +0,0 @@ -[[package]] -name = "dependency" -version = "1.1.0" -description = "" -optional = true -python-versions = "*" -files = [] - -[[package]] -name = "dependency" -version = "1.2.0" -description = "" -optional = true -python-versions = "*" -files = [] - -[extras] -extra-one = ["dependency"] -extra-two = ["dependency"] - -[metadata] -python-versions = "*" -lock-version = "2.0" -content-hash = "123456789" diff --git a/tests/puzzle/test_solver.py b/tests/puzzle/test_solver.py index 05e3b7ce65a..ab32abac2b9 100644 --- a/tests/puzzle/test_solver.py +++ b/tests/puzzle/test_solver.py @@ -4792,8 +4792,11 @@ def test_solver_resolves_duplicate_dependency_in_root_extra( repo.add_package(package_a1) repo.add_package(package_a2) - solver = Solver(package, pool, [], [], io, active_root_extras=extra) - transaction = solver.solve() + solver = Solver( + package, pool, [], [package_a1, package_a2], io, active_root_extras=extra + ) + with solver.use_environment(MockEnv()): + transaction = solver.solve() check_solver_result( transaction, From f4de278b3c4c2ba683fc6a488aedded80da3f8da Mon Sep 17 00:00:00 2001 From: Reese Hyde Date: Mon, 18 Nov 2024 11:10:51 -0500 Subject: [PATCH 28/39] run pre-commit --- src/poetry/puzzle/solver.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/poetry/puzzle/solver.py b/src/poetry/puzzle/solver.py index 2fab298ec5e..234518fc52e 100644 --- a/src/poetry/puzzle/solver.py +++ b/src/poetry/puzzle/solver.py @@ -57,7 +57,13 @@ def __init__( self._locked_packages = locked self._io = io - self._provider = Provider(self._package, self._pool, self._io, locked=locked, active_root_extras=active_root_extras) + self._provider = Provider( + self._package, + self._pool, + self._io, + locked=locked, + active_root_extras=active_root_extras, + ) self._overrides: list[dict[Package, dict[str, Dependency]]] = [] @property From 8efb89d7427424501ad57267878c366f63c333e3 Mon Sep 17 00:00:00 2001 From: Reese Hyde Date: Mon, 18 Nov 2024 11:27:40 -0500 Subject: [PATCH 29/39] update test fixtures for new locker features :) --- .../fixtures/with-conflicting-dependency-extras-root.test | 6 +++++- .../with-conflicting-dependency-extras-transitive.test | 8 +++++++- .../fixtures/with-dependencies-differing-extras.test | 8 +++++++- tests/installation/fixtures/with-exclusive-extras.test | 6 +++++- 4 files changed, 24 insertions(+), 4 deletions(-) diff --git a/tests/installation/fixtures/with-conflicting-dependency-extras-root.test b/tests/installation/fixtures/with-conflicting-dependency-extras-root.test index 4ed9440f6d4..c99c37a5694 100644 --- a/tests/installation/fixtures/with-conflicting-dependency-extras-root.test +++ b/tests/installation/fixtures/with-conflicting-dependency-extras-root.test @@ -5,6 +5,8 @@ description = "" optional = true python-versions = "*" files = [ ] +groups = [ "main" ] +markers = "extra == \"extra-one\" and extra != \"extra-two\"" [[package]] name = "conflicting-dep" @@ -13,12 +15,14 @@ description = "" optional = true python-versions = "*" files = [ ] +groups = [ "main" ] +markers = "extra != \"extra-one\" and extra == \"extra-two\"" [extras] extra-one = [ "conflicting-dep", "conflicting-dep" ] extra-two = [ "conflicting-dep", "conflicting-dep" ] [metadata] -lock-version = "2.0" +lock-version = "2.1" python-versions = "*" content-hash = "123456789" diff --git a/tests/installation/fixtures/with-conflicting-dependency-extras-transitive.test b/tests/installation/fixtures/with-conflicting-dependency-extras-transitive.test index 3012d7de9a6..5e6f11ab7f9 100644 --- a/tests/installation/fixtures/with-conflicting-dependency-extras-transitive.test +++ b/tests/installation/fixtures/with-conflicting-dependency-extras-transitive.test @@ -5,6 +5,8 @@ description = "" optional = true python-versions = "*" files = [ ] +groups = [ "main" ] +markers = "extra == \"root-extra-one\" and extra != \"root-extra-two\"" [[package]] name = "conflicting-dep" @@ -13,6 +15,8 @@ description = "" optional = true python-versions = "*" files = [ ] +groups = [ "main" ] +markers = "extra != \"root-extra-one\" and extra == \"root-extra-two\"" [[package]] name = "intermediate-dep" @@ -21,6 +25,8 @@ description = "" optional = true python-versions = "*" files = [ ] +groups = [ "main" ] +markers = "extra == \"root-extra-one\" and extra != \"root-extra-two\" or extra == \"root-extra-two\" and extra != \"root-extra-one\"" [[package.dependencies.conflicting-dep]] version = "1.1.0" @@ -41,6 +47,6 @@ root-extra-one = [ "intermediate-dep", "intermediate-dep" ] root-extra-two = [ "intermediate-dep", "intermediate-dep" ] [metadata] -lock-version = "2.0" +lock-version = "2.1" python-versions = "*" content-hash = "123456789" diff --git a/tests/installation/fixtures/with-dependencies-differing-extras.test b/tests/installation/fixtures/with-dependencies-differing-extras.test index 69827c4db28..7dcaacf26ae 100644 --- a/tests/installation/fixtures/with-dependencies-differing-extras.test +++ b/tests/installation/fixtures/with-dependencies-differing-extras.test @@ -5,6 +5,8 @@ description = "" optional = true python-versions = "*" files = [ ] +groups = [ "main" ] +markers = "extra == \"extra-one\" and extra != \"extra-two\" or extra == \"extra-two\" and extra != \"extra-one\"" [package.dependencies.transitive-dep-one] version = "1.1.0" @@ -27,6 +29,8 @@ description = "" optional = true python-versions = "*" files = [ ] +groups = [ "main" ] +markers = "extra == \"extra-one\" and extra != \"extra-two\"" [[package]] name = "transitive-dep-two" @@ -35,12 +39,14 @@ description = "" optional = true python-versions = "*" files = [ ] +groups = [ "main" ] +markers = "extra != \"extra-one\" and extra == \"extra-two\"" [extras] extra-one = [ "demo", "demo" ] extra-two = [ "demo", "demo" ] [metadata] -lock-version = "2.0" +lock-version = "2.1" python-versions = "*" content-hash = "123456789" diff --git a/tests/installation/fixtures/with-exclusive-extras.test b/tests/installation/fixtures/with-exclusive-extras.test index 4d05775b13c..c4764e4a8af 100644 --- a/tests/installation/fixtures/with-exclusive-extras.test +++ b/tests/installation/fixtures/with-exclusive-extras.test @@ -5,6 +5,8 @@ description = "" optional = true python-versions = "*" files = [] +groups = [ "main" ] +markers = "extra == \"cpu\" and extra != \"cuda\"" [package.source] reference = "pytorch-cpu" @@ -18,6 +20,8 @@ description = "" optional = true python-versions = "*" files = [] +groups = [ "main" ] +markers = "extra != \"cpu\" and extra == \"cuda\"" [package.source] reference = "pytorch-cuda" @@ -30,5 +34,5 @@ cuda = ["torch", "torch"] [metadata] python-versions = "*" -lock-version = "2.0" +lock-version = "2.1" content-hash = "123456789" From 2070521efd2510c09326d4d16fd177f0981aa514 Mon Sep 17 00:00:00 2001 From: Reese Hyde Date: Mon, 18 Nov 2024 11:53:10 -0500 Subject: [PATCH 30/39] fix test: don't validate installation order --- tests/installation/test_installer.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/installation/test_installer.py b/tests/installation/test_installer.py index 3d4ae15574f..a8e3b726c36 100644 --- a/tests/installation/test_installer.py +++ b/tests/installation/test_installer.py @@ -1192,7 +1192,8 @@ def test_run_with_conflicting_dependency_extras( if not root and extra is not None: expected_installations.append(get_package("intermediate-dep", "1.0.0")) - assert installer.executor.installations == expected_installations + assert len(installer.executor.installations) == len(expected_installations) + assert set(installer.executor.installations) == set(expected_installations) @pytest.mark.parametrize("locked", [True, False]) From 357340372a464b58fbfd68d49042e17f709f7bf4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Randy=20D=C3=B6ring?= <30527984+radoering@users.noreply.github.com> Date: Fri, 22 Nov 2024 14:09:34 +0100 Subject: [PATCH 31/39] remove redundant filtering completely --- src/poetry/puzzle/provider.py | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/src/poetry/puzzle/provider.py b/src/poetry/puzzle/provider.py index 4b59a7f0fed..19dbda2442d 100644 --- a/src/poetry/puzzle/provider.py +++ b/src/poetry/puzzle/provider.py @@ -421,21 +421,12 @@ def incompatibilities_for( ) ] - _dependencies = [ - dep - for dep in dependencies - if dep.name not in self.UNSAFE_PACKAGES - and self._python_constraint.allows_any(dep.python_constraint) - and (not self._env or dep.marker.validate(self._env.marker_env)) - ] - dependencies = self._get_dependencies_with_overrides(_dependencies, package) - return [ Incompatibility( [Term(package.to_dependency(), True), Term(dep, False)], DependencyCauseError(), ) - for dep in dependencies + for dep in self._get_dependencies_with_overrides(dependencies, package) ] def complete_package( From 5ad4419ed3c1d88a4fbd6f14036ed3546fb46698 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Randy=20D=C3=B6ring?= <30527984+radoering@users.noreply.github.com> Date: Fri, 22 Nov 2024 14:20:41 +0100 Subject: [PATCH 32/39] fix link, improve wording --- docs/dependency-specification.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/dependency-specification.md b/docs/dependency-specification.md index b2a694e59ec..83dc32643a6 100644 --- a/docs/dependency-specification.md +++ b/docs/dependency-specification.md @@ -473,14 +473,14 @@ dependencies = [ ### `extra` environment marker -Poetry populates the `extra` marker with each of the selected extras for the parent declaring the dependency. For -example, consider the following dependency in your root package: +Poetry populates the `extra` marker with each of the selected extras of the root package. +For example, consider the following dependency: ```toml [tool.poetry.dependencies] pathlib2 = { version = "^2.2", markers = "extra == 'paths' and sys_platform == 'win32'", optional = true} ``` `pathlib2` will be installed when you install your package with `--extras paths` on a `win32` machine. -You'll also need to [define the `paths` extra in your project](./pyproject.md#extras). +You will also need to [define the `paths` extra in your project]({{< relref "pyproject.md#extras" >}}). #### Exclusive extras From 9624a363f6bd4cd1ada6dcd4b4bf245610752a97 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Randy=20D=C3=B6ring?= <30527984+radoering@users.noreply.github.com> Date: Sat, 23 Nov 2024 09:22:43 +0100 Subject: [PATCH 33/39] avoid redundant uninstalls of extras --- src/poetry/puzzle/transaction.py | 14 ++++++--- tests/puzzle/test_transaction.py | 52 ++++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 4 deletions(-) diff --git a/src/poetry/puzzle/transaction.py b/src/poetry/puzzle/transaction.py index 3f18e7f5095..aa17492e144 100644 --- a/src/poetry/puzzle/transaction.py +++ b/src/poetry/puzzle/transaction.py @@ -74,7 +74,7 @@ def calculate_operations( else: priorities = defaultdict(int) relevant_result_packages: set[NormalizedName] = set() - uninstalls: set[NormalizedName] = set() + pending_extra_uninstalls: list[Package] = [] # list for deterministic order for result_package in self._result_packages: is_unsolicited_extra = False if self._marker_env: @@ -91,11 +91,12 @@ def calculate_operations( else: continue else: - relevant_result_packages.add(result_package.name) is_unsolicited_extra = extras is not None and ( result_package.optional and result_package.name not in extra_packages ) + if not is_unsolicited_extra: + relevant_result_packages.add(result_package.name) installed = False for installed_package in self._installed_packages: @@ -104,8 +105,7 @@ 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)) + pending_extra_uninstalls.append(installed_package) # We have to perform an update if the version or another # attribute of the package has changed (source type, url, ref, ...). @@ -148,6 +148,12 @@ def calculate_operations( op.skip("Not required") operations.append(op) + uninstalls: set[NormalizedName] = set() + for package in pending_extra_uninstalls: + if package.name not in (relevant_result_packages | uninstalls): + uninstalls.add(package.name) + operations.append(Uninstall(package)) + if with_uninstalls: for current_package in self._current_packages: found = current_package.name in (relevant_result_packages | uninstalls) diff --git a/tests/puzzle/test_transaction.py b/tests/puzzle/test_transaction.py index a8874ebe4eb..ea1dcb6df03 100644 --- a/tests/puzzle/test_transaction.py +++ b/tests/puzzle/test_transaction.py @@ -372,3 +372,55 @@ def test_calculate_operations_extras( ), ops, ) + + +@pytest.mark.parametrize("extra", ["", "foo", "bar"]) +def test_calculate_operations_extras_no_redundant_uninstall(extra: str) -> None: + extra1 = canonicalize_name("foo") + extra2 = canonicalize_name("bar") + package = ProjectPackage("root", "1.0") + dep_a1 = Dependency("a", "1", optional=True) + dep_a1._in_extras = ["foo"] + dep_a1.marker = parse_marker("extra != 'bar'") + dep_a2 = Dependency("a", "2", optional=True) + dep_a2._in_extras = ["bar"] + dep_a2.marker = parse_marker("extra != 'foo'") + package.add_dependency(dep_a1) + package.add_dependency(dep_a2) + package.extras = {extra1: [dep_a1], extra2: [dep_a2]} + opt_a1 = Package("a", "1") + opt_a1.optional = True + opt_a2 = Package("a", "2") + opt_a2.optional = True + + transaction = Transaction( + [Package("a", "1")], + { + opt_a1: TransitivePackageInfo( + 0, {"main"}, {"main": parse_marker("extra == 'foo' and extra != 'bar'")} + ), + opt_a2: TransitivePackageInfo( + 0, {"main"}, {"main": parse_marker("extra == 'bar' and extra != 'foo'")} + ), + }, + [Package("a", "1")], + package, + {"python_version": "3.9"}, + {"main"}, + ) + + if not extra: + ops = [{"job": "remove", "package": Package("a", "1")}] + elif extra == "foo": + ops = [{"job": "install", "package": Package("a", "1"), "skipped": True}] + elif extra == "bar": + ops = [{"job": "update", "from": Package("a", "1"), "to": Package("a", "2")}] + else: + raise NotImplementedError + + check_operations( + transaction.calculate_operations( + extras=set() if not extra else {canonicalize_name(extra)}, + ), + ops, + ) From 96c19ce0f319450ed5bb3fc98eeb0d05f5e954d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Randy=20D=C3=B6ring?= <30527984+radoering@users.noreply.github.com> Date: Sat, 23 Nov 2024 07:10:26 +0100 Subject: [PATCH 34/39] fix dependency resolution for extra dependencies without explicit extra marker --- src/poetry/puzzle/provider.py | 21 ++++++++- tests/puzzle/test_solver.py | 89 ++++++++++++++++++++++++++++++++++- 2 files changed, 108 insertions(+), 2 deletions(-) diff --git a/src/poetry/puzzle/provider.py b/src/poetry/puzzle/provider.py index 19dbda2442d..b3b29e5f6c6 100644 --- a/src/poetry/puzzle/provider.py +++ b/src/poetry/puzzle/provider.py @@ -18,6 +18,7 @@ from poetry.core.constraints.version import VersionRange from poetry.core.packages.utils.utils import get_python_constraint_from_marker from poetry.core.version.markers import AnyMarker +from poetry.core.version.markers import parse_marker from poetry.core.version.markers import union as marker_union from poetry.mixology.incompatibility import Incompatibility @@ -476,7 +477,7 @@ def complete_package( package = dependency_package.package dependency = dependency_package.dependency new_dependency = package.without_features().to_dependency() - new_dependency.marker = AnyMarker() + new_dependency.marker = dependency.marker # When adding dependency foo[extra] -> foo, preserve foo's source, if it's # specified. This prevents us from trying to get foo from PyPI @@ -511,6 +512,24 @@ def complete_package( ): continue + # For normal dependency resolution, we have to make sure that root extras + # are represented in the markers. This is required to identify mutually + # exclusive markers in cases like 'extra == "foo"' and 'extra != "foo"'. + # However, for installation with re-resolving (installer.re-resolve=true, + # which results in self._env being not None), this spoils the result + # because we have to keep extras so that they are uninstalled + # when calculating the operations of the transaction. + if self._env is None and package.is_root() and dep.in_extras: + # The clone is required for installation with re-resolving + # without an existing lock file because the root package is used + # once for solving and a second time for re-resolving for installation. + dep = dep.clone() + dep.marker = dep.marker.intersect( + parse_marker( + " or ".join(f'extra == "{extra}"' for extra in dep.in_extras) + ) + ) + _dependencies.append(dep) if self._load_deferred: diff --git a/tests/puzzle/test_solver.py b/tests/puzzle/test_solver.py index 222f9c8db86..a566e5c588e 100644 --- a/tests/puzzle/test_solver.py +++ b/tests/puzzle/test_solver.py @@ -4755,8 +4755,95 @@ def test_solver_resolves_duplicate_dependency_in_extra( ) +def test_solver_resolves_conflicting_dependency_in_root_extra( + package: ProjectPackage, + pool: RepositoryPool, + repo: Repository, + io: NullIO, +) -> None: + package_a1 = get_package("A", "1.0") + package_a2 = get_package("A", "2.0") + + dep = get_dependency("A", {"version": "1.0", "markers": "extra != 'foo'"}) + package.add_dependency(dep) + + dep_extra = get_dependency("A", "2.0", optional=True) + dep_extra._in_extras = [canonicalize_name("foo")] + package.extras = {canonicalize_name("foo"): [dep_extra]} + package.add_dependency(dep_extra) + + repo.add_package(package_a1) + repo.add_package(package_a2) + + solver = Solver(package, pool, [], [], io) + transaction = solver.solve() + + check_solver_result( + transaction, + ( + [ + {"job": "install", "package": package_a1}, + {"job": "install", "package": package_a2}, + ] + ), + ) + solved_packages = transaction.get_solved_packages() + assert solved_packages[package_a1].markers["main"] == parse_marker("extra != 'foo'") + assert solved_packages[package_a2].markers["main"] == parse_marker("extra == 'foo'") + + +def test_solver_resolves_conflicting_dependency_in_root_extras( + package: ProjectPackage, + pool: RepositoryPool, + repo: Repository, + io: NullIO, +) -> None: + package_a1 = get_package("A", "1.0") + package_a2 = get_package("A", "2.0") + + dep_extra1 = get_dependency( # extra == 'foo' is implicit via _in_extras! + "A", {"version": "1.0", "markers": "extra != 'bar'"}, optional=True + ) + dep_extra1._in_extras = [canonicalize_name("foo")] + package.add_dependency(dep_extra1) + + dep_extra2 = get_dependency( # extra == 'bar' is implicit via _in_extras! + "A", {"version": "2.0", "markers": "extra != 'foo'"}, optional=True + ) + dep_extra2._in_extras = [canonicalize_name("bar")] + package.extras = { + canonicalize_name("foo"): [dep_extra1], + canonicalize_name("bar"): [dep_extra2], + } + package.add_dependency(dep_extra1) + package.add_dependency(dep_extra2) + + repo.add_package(package_a1) + repo.add_package(package_a2) + + solver = Solver(package, pool, [], [], io) + transaction = solver.solve() + + check_solver_result( + transaction, + ( + [ + {"job": "install", "package": package_a1}, + {"job": "install", "package": package_a2}, + ] + ), + ) + solved_packages = transaction.get_solved_packages() + assert solved_packages[package_a1].markers["main"] == parse_marker( + "extra != 'bar' and extra == 'foo'" + ) + assert solved_packages[package_a2].markers["main"] == parse_marker( + "extra != 'foo' and extra == 'bar'" + ) + + @pytest.mark.parametrize("with_extra", [False, True]) -def test_solver_resolves_duplicate_dependency_in_root_extra( +def test_solver_resolves_duplicate_dependency_in_root_extra_for_installation( package: ProjectPackage, pool: RepositoryPool, repo: Repository, From cfdf903cec9999194cc4d61c603370a65bf3db91 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Randy=20D=C3=B6ring?= <30527984+radoering@users.noreply.github.com> Date: Sat, 23 Nov 2024 13:32:01 +0100 Subject: [PATCH 35/39] `tool.poetry.extras` will be deprecated in the next version -> update docs to new syntax --- docs/dependency-specification.md | 55 +++++++++++++++++++++++++------- 1 file changed, 44 insertions(+), 11 deletions(-) diff --git a/docs/dependency-specification.md b/docs/dependency-specification.md index 83dc32643a6..58d9b2a5cd4 100644 --- a/docs/dependency-specification.md +++ b/docs/dependency-specification.md @@ -476,11 +476,13 @@ dependencies = [ Poetry populates the `extra` marker with each of the selected extras of the root package. For example, consider the following dependency: ```toml -[tool.poetry.dependencies] -pathlib2 = { version = "^2.2", markers = "extra == 'paths' and sys_platform == 'win32'", optional = true} +[project.optional-dependencies] +paths = [ + "pathlib2 (>=2.2,<3.0) ; sys_platform == 'win32'" +] ``` + `pathlib2` will be installed when you install your package with `--extras paths` on a `win32` machine. -You will also need to [define the `paths` extra in your project]({{< relref "pyproject.md#extras" >}}). #### Exclusive extras @@ -491,15 +493,22 @@ when the `cuda` extra is *not* specified, while the other installs from another for GPUs when the `cuda` extra *is* specified: ```toml +[project] +dependencies = [ + "torch (==2.3.1+cpu) ; extra != 'cuda'", +] + +[project.optional-dependencies] +cuda = [ + "torch (==2.3.1+cu118)", +] + [tool.poetry.dependencies] torch = [ - { markers = "extra != 'cuda'", version = "2.3.1+cpu", source = "pytorch-cpu", optional = true}, - { markers = "extra == 'cuda'", version = "2.3.1+cu118", source = "pytorch-cu118", optional = true}, + { markers = "extra != 'cuda'", source = "pytorch-cpu"}, + { markers = "extra == 'cuda'", source = "pytorch-cu118"}, ] -[tool.poetry.extras] -cuda = ["torch"] - [[tool.poetry.source]] name = "pytorch-cpu" url = "https://download.pytorch.org/whl/cpu" @@ -514,9 +523,33 @@ priority = "explicit" For the CPU case, we have to specify `"extra != 'cuda'"` because the version specified is not compatible with the GPU (`cuda`) version. -This same logic applies when you want either-or extras. If a dependency for `extra-one` and -`extra-two` conflict, they will need to be restricted using `markers = "extra == 'extra-one' and extra != 'extra-two'"` -and vice versa. +This same logic applies when you want either-or extras: + +```toml +[project.optional-dependencies] +cuda = [ + "torch (==2.3.1+cu118) ; extra != 'cpu'", +] +cpu = [ + "torch (==2.3.1+cpu) ; extra != 'cuda'", +] + +[tool.poetry.dependencies] +torch = [ + { markers = "extra == 'cpu' and extra != 'cuda'", source = "pytorch-cpu"}, + { markers = "extra == 'cuda' and extra != 'cpu'", source = "pytorch-cu118"}, + ] + +[[tool.poetry.source]] +name = "pytorch-cpu" +url = "https://download.pytorch.org/whl/cpu" +priority = "explicit" + +[[tool.poetry.source]] +name = "pytorch-cu118" +url = "https://download.pytorch.org/whl/cu118" +priority = "explicit" +``` ## Multiple constraints dependencies From e3a87bf5a8d435ba23c1456938ae7a6b1bac1a8d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Randy=20D=C3=B6ring?= <30527984+radoering@users.noreply.github.com> Date: Sat, 23 Nov 2024 13:34:25 +0100 Subject: [PATCH 36/39] temp: use poetry-core with fix for dependency merging (see examples from docs) --- poetry.lock | 8 ++++---- pyproject.toml | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/poetry.lock b/poetry.lock index 7ce68c3733a..89bb7eee5a6 100644 --- a/poetry.lock +++ b/poetry.lock @@ -958,9 +958,9 @@ develop = false [package.source] type = "git" -url = "https://github.com/python-poetry/poetry-core.git" -reference = "main" -resolved_reference = "616d7bfaf018d50bd09bc24c630b697b6368d5a3" +url = "https://github.com/radoering/poetry-core.git" +reference = "fix-dependency-merge-by-extra-marker" +resolved_reference = "fc7b524967386003bc143ef71ca740e3c84e72ac" [[package]] name = "pre-commit" @@ -1472,4 +1472,4 @@ test = ["big-O", "importlib-resources", "jaraco.functools", "jaraco.itertools", [metadata] lock-version = "2.0" python-versions = "^3.9" -content-hash = "abcf5a628d4f501a0a8c84fbaa96906dd7985317613f176bb2d4eee15e06313d" +content-hash = "640e463c5fc83d34bd4058225a67e133fe8e40bb140b3efe69b7fa5ec67fffd1" diff --git a/pyproject.toml b/pyproject.toml index dbe95782146..5e93fb13746 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -31,7 +31,7 @@ Changelog = "https://python-poetry.org/history/" [tool.poetry.dependencies] python = "^3.9" -poetry-core = { git = "https://github.com/python-poetry/poetry-core.git", branch = "main" } +poetry-core = { git = "https://github.com/radoering/poetry-core.git", branch = "fix-dependency-merge-by-extra-marker" } build = "^1.2.1" cachecontrol = { version = "^0.14.0", extras = ["filecache"] } cleo = "^2.2.1" From 6696453ab981fc79da54d57cbc553c56bda660b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Randy=20D=C3=B6ring?= <30527984+radoering@users.noreply.github.com> Date: Sat, 23 Nov 2024 14:04:49 +0100 Subject: [PATCH 37/39] fix typing --- tests/puzzle/test_transaction.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/puzzle/test_transaction.py b/tests/puzzle/test_transaction.py index ea1dcb6df03..1d133f47a55 100644 --- a/tests/puzzle/test_transaction.py +++ b/tests/puzzle/test_transaction.py @@ -380,10 +380,10 @@ def test_calculate_operations_extras_no_redundant_uninstall(extra: str) -> None: extra2 = canonicalize_name("bar") package = ProjectPackage("root", "1.0") dep_a1 = Dependency("a", "1", optional=True) - dep_a1._in_extras = ["foo"] + dep_a1._in_extras = [canonicalize_name("foo")] dep_a1.marker = parse_marker("extra != 'bar'") dep_a2 = Dependency("a", "2", optional=True) - dep_a2._in_extras = ["bar"] + dep_a2._in_extras = [canonicalize_name("bar")] dep_a2.marker = parse_marker("extra != 'foo'") package.add_dependency(dep_a1) package.add_dependency(dep_a2) From a6980a3be70828e66aee02ab17b5cf6b4951fe90 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Randy=20D=C3=B6ring?= <30527984+radoering@users.noreply.github.com> Date: Fri, 29 Nov 2024 16:51:39 +0100 Subject: [PATCH 38/39] use poetry-core from main branch again (after fix has been merged) --- poetry.lock | 8 ++++---- pyproject.toml | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/poetry.lock b/poetry.lock index 8eac73d7396..5a6b6a44b07 100644 --- a/poetry.lock +++ b/poetry.lock @@ -968,9 +968,9 @@ develop = false [package.source] type = "git" -url = "https://github.com/radoering/poetry-core.git" -reference = "fix-dependency-merge-by-extra-marker" -resolved_reference = "fc7b524967386003bc143ef71ca740e3c84e72ac" +url = "https://github.com/python-poetry/poetry-core.git" +reference = "main" +resolved_reference = "b95ec5321f1842286b042ac206c9f5395850c684" [[package]] name = "pre-commit" @@ -1582,4 +1582,4 @@ test = ["big-O", "importlib-resources", "jaraco.functools", "jaraco.itertools", [metadata] lock-version = "2.0" python-versions = "^3.9" -content-hash = "a58493823486e0349c5c441363134c7f558e3d3e795e01cee48f60fe952703cc" +content-hash = "95b92662eda25340562e13daba629f975222cc1baeafa1f78d9ceaeb652a7849" diff --git a/pyproject.toml b/pyproject.toml index 2e4d12a20df..8fac358c767 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -31,7 +31,7 @@ Changelog = "https://python-poetry.org/history/" [tool.poetry.dependencies] python = "^3.9" -poetry-core = { git = "https://github.com/radoering/poetry-core.git", branch = "fix-dependency-merge-by-extra-marker" } +poetry-core = { git = "https://github.com/python-poetry/poetry-core.git", branch = "main" } build = "^1.2.1" cachecontrol = { version = "^0.14.0", extras = ["filecache"] } cleo = "^2.1.0" From 8b36cbd4f22081cb5d5e71c865b308c0cf2a8d19 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Randy=20D=C3=B6ring?= <30527984+radoering@users.noreply.github.com> Date: Fri, 29 Nov 2024 17:27:49 +0100 Subject: [PATCH 39/39] update examples and add warning --- docs/dependency-specification.md | 39 +++++++++++++++++++++++++------- 1 file changed, 31 insertions(+), 8 deletions(-) diff --git a/docs/dependency-specification.md b/docs/dependency-specification.md index 71adbc43169..500999394e5 100644 --- a/docs/dependency-specification.md +++ b/docs/dependency-specification.md @@ -587,6 +587,17 @@ paths = [ #### Exclusive extras +{{% warning %}} +The first example will only work completely if you configure Poetry to not re-resolve for installation: + +```bash +poetry config installer.re-resolve false +``` + +This is a new feature of Poetry 2.0 that may become the default in a future version of Poetry. + +{{% /warning %}} + Keep in mind that all combinations of possible extras available in your project need to be compatible with each other. This means that in order to use differing or incompatible versions across different combinations, you need to make your extra markers *exclusive*. For example, the following installs PyTorch from one source repository with CPU versions @@ -595,6 +606,8 @@ for GPUs when the `cuda` extra *is* specified: ```toml [project] +name = "torch-example" +requires-python = ">=3.10" dependencies = [ "torch (==2.3.1+cpu) ; extra != 'cuda'", ] @@ -604,10 +617,13 @@ cuda = [ "torch (==2.3.1+cu118)", ] +[tool.poetry] +package-mode = false + [tool.poetry.dependencies] torch = [ { markers = "extra != 'cuda'", source = "pytorch-cpu"}, - { markers = "extra == 'cuda'", source = "pytorch-cu118"}, + { markers = "extra == 'cuda'", source = "pytorch-cuda"}, ] [[tool.poetry.source]] @@ -616,7 +632,7 @@ url = "https://download.pytorch.org/whl/cpu" priority = "explicit" [[tool.poetry.source]] -name = "pytorch-cu118" +name = "pytorch-cuda" url = "https://download.pytorch.org/whl/cu118" priority = "explicit" ``` @@ -627,18 +643,25 @@ GPU (`cuda`) version. This same logic applies when you want either-or extras: ```toml +[project] +name = "torch-example" +requires-python = ">=3.10" + [project.optional-dependencies] -cuda = [ - "torch (==2.3.1+cu118) ; extra != 'cpu'", -] cpu = [ - "torch (==2.3.1+cpu) ; extra != 'cuda'", + "torch (==2.3.1+cpu)", ] +cuda = [ + "torch (==2.3.1+cu118)", +] + +[tool.poetry] +package-mode = false [tool.poetry.dependencies] torch = [ { markers = "extra == 'cpu' and extra != 'cuda'", source = "pytorch-cpu"}, - { markers = "extra == 'cuda' and extra != 'cpu'", source = "pytorch-cu118"}, + { markers = "extra == 'cuda' and extra != 'cpu'", source = "pytorch-cuda"}, ] [[tool.poetry.source]] @@ -647,7 +670,7 @@ url = "https://download.pytorch.org/whl/cpu" priority = "explicit" [[tool.poetry.source]] -name = "pytorch-cu118" +name = "pytorch-cuda" url = "https://download.pytorch.org/whl/cu118" priority = "explicit" ```