From 5f75fdda358ca0e116f6472ffa285c7d248ab7db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Randy=20D=C3=B6ring?= <30527984+radoering@users.noreply.github.com> Date: Tue, 13 Feb 2024 17:44:51 +0100 Subject: [PATCH] provider: consider explicit source when searching for a locked package with a source reference in the repository pool (#8948) --- src/poetry/puzzle/provider.py | 13 +++++ tests/puzzle/test_provider.py | 63 ++++++++++++++++++++++ tests/puzzle/test_solver.py | 98 +++++++++++++++++++++++++++++++++++ 3 files changed, 174 insertions(+) diff --git a/src/poetry/puzzle/provider.py b/src/poetry/puzzle/provider.py index d404f84d520..b759af225d7 100644 --- a/src/poetry/puzzle/provider.py +++ b/src/poetry/puzzle/provider.py @@ -134,6 +134,7 @@ def __init__( self._locked: dict[NormalizedName, list[DependencyPackage]] = defaultdict(list) self._use_latest: Collection[NormalizedName] = [] + self._explicit_sources: dict[str, str] = {} for package in locked or []: self._locked[package.name].append( DependencyPackage(package.to_dependency(), package) @@ -682,6 +683,16 @@ def fmt_warning(d: Dependency) -> str: for dep in clean_dependencies: package.add_dependency(dep) + if self._locked and package.is_root(): + # At this point all duplicates have been eliminated via overrides + # so that explicit sources are unambiguous. + # Clear _explicit_sources because it might be filled + # from a previous override. + self._explicit_sources.clear() + for dep in clean_dependencies: + if dep.source_name: + self._explicit_sources[dep.name] = dep.source_name + return dependency_package def get_locked(self, dependency: Dependency) -> DependencyPackage | None: @@ -692,6 +703,8 @@ def get_locked(self, dependency: Dependency) -> DependencyPackage | None: for dependency_package in locked: package = dependency_package.package if package.satisfies(dependency): + if explicit_source := self._explicit_sources.get(dependency.name): + dependency.source_name = explicit_source return DependencyPackage(dependency, package) return None diff --git a/tests/puzzle/test_provider.py b/tests/puzzle/test_provider.py index 9ee1f6fc644..1e1b03acc42 100644 --- a/tests/puzzle/test_provider.py +++ b/tests/puzzle/test_provider.py @@ -21,7 +21,9 @@ from poetry.packages import DependencyPackage from poetry.puzzle.provider import IncompatibleConstraintsError from poetry.puzzle.provider import Provider +from poetry.repositories.exceptions import PackageNotFound from poetry.repositories.repository import Repository +from poetry.repositories.repository_pool import Priority from poetry.repositories.repository_pool import RepositoryPool from poetry.utils.env import EnvCommandError from poetry.utils.env import MockEnv as BaseMockEnv @@ -783,6 +785,67 @@ def test_complete_package_fetches_optional_vcs_dependency_only_if_requested( spy.assert_not_called() +def test_complete_package_finds_locked_package_in_explicit_source( + root: ProjectPackage, pool: RepositoryPool +) -> None: + package = Package("a", "1.0", source_reference="explicit") + explicit_repo = Repository("explicit") + explicit_repo.add_package(package) + pool.add_repository(explicit_repo, priority=Priority.EXPLICIT) + + root_dependency = get_dependency("a", ">0") + root_dependency.source_name = "explicit" + root.add_dependency(root_dependency) + locked_package = Package("a", "1.0", source_reference="explicit") + provider = Provider(root, pool, NullIO(), locked=[locked_package]) + provider.complete_package(DependencyPackage(root.to_dependency(), root)) + + # transitive dependency without explicit source + dependency = get_dependency("a", ">=1") + + locked = provider.get_locked(dependency) + assert locked is not None + provider.complete_package(locked) # must not fail + + +def test_complete_package_finds_locked_package_in_other_source( + root: ProjectPackage, repository: Repository, pool: RepositoryPool +) -> None: + package = Package("a", "1.0") + repository.add_package(package) + explicit_repo = Repository("explicit") + pool.add_repository(explicit_repo) + + root_dependency = get_dependency("a", ">0") # no explicit source + root.add_dependency(root_dependency) + locked_package = Package("a", "1.0", source_reference="explicit") # explicit source + provider = Provider(root, pool, NullIO(), locked=[locked_package]) + provider.complete_package(DependencyPackage(root.to_dependency(), root)) + + # transitive dependency without explicit source + dependency = get_dependency("a", ">=1") + + locked = provider.get_locked(dependency) + assert locked is not None + provider.complete_package(locked) # must not fail + + +def test_complete_package_raises_packagenotfound_if_locked_source_not_available( + root: ProjectPackage, pool: RepositoryPool, provider: Provider +) -> None: + locked_package = Package("a", "1.0", source_reference="outdated") + provider = Provider(root, pool, NullIO(), locked=[locked_package]) + provider.complete_package(DependencyPackage(root.to_dependency(), root)) + + # transitive dependency without explicit source + dependency = get_dependency("a", ">=1") + + locked = provider.get_locked(dependency) + assert locked is not None + with pytest.raises(PackageNotFound): + provider.complete_package(locked) + + def test_source_dependency_is_satisfied_by_direct_origin( provider: Provider, repository: Repository ) -> None: diff --git a/tests/puzzle/test_solver.py b/tests/puzzle/test_solver.py index dd77ff01cca..e2f4682e2b6 100644 --- a/tests/puzzle/test_solver.py +++ b/tests/puzzle/test_solver.py @@ -3417,6 +3417,104 @@ def test_direct_dependency_with_extras_from_explicit_and_transitive_dependency2( ) +@pytest.mark.parametrize("locked", [False, True]) +def test_multiple_constraints_explicit_source_transitive_locked_use_latest( + package: ProjectPackage, + repo: Repository, + pool: RepositoryPool, + io: NullIO, + locked: bool, +) -> None: + """ + The root package depends on + * lib[extra] == 1.0; sys_platform != "linux" with source=explicit1 + * lib[extra] == 2.0; sys_platform == "linux" with source=explicit2 + * other >= 1.0 + "other" depends on "lib" (without an extra and of course without an explicit source + because explicit sources can only be defined in the root package). + + If only "other" is in use_latest (equivalent to "poetry update other"), + the transitive dependency of "other" on "lib" is resolved before + the direct dependency on "lib[extra]" (if packages have been locked before). + We still have to make sure that the locked package is looked up in the explicit + source although the DependencyCache is not used for locked packages, + so we can't rely on it to propagate the correct source. + """ + package.add_dependency( + Factory.create_dependency( + "lib", + { + "version": "1.0", + "extras": ["extra"], + "source": "explicit1", + "markers": "sys_platform != 'linux'", + }, + ) + ) + package.add_dependency( + Factory.create_dependency( + "lib", + { + "version": "2.0", + "extras": ["extra"], + "source": "explicit2", + "markers": "sys_platform == 'linux'", + }, + ) + ) + package.add_dependency(Factory.create_dependency("other", {"version": ">=1.0"})) + + explicit_repo1 = Repository("explicit1") + pool.add_repository(explicit_repo1, priority=Priority.EXPLICIT) + explicit_repo2 = Repository("explicit2") + pool.add_repository(explicit_repo2, priority=Priority.EXPLICIT) + + dep_extra = get_dependency("extra", ">=1.0") + dep_extra_opt = Factory.create_dependency( + "extra", {"version": ">=1.0", "optional": True} + ) + package_lib1 = Package( + "lib", "1.0", source_type="legacy", source_reference="explicit1" + ) + package_lib1.extras = {canonicalize_name("extra"): [dep_extra]} + package_lib1.add_dependency(dep_extra_opt) + explicit_repo1.add_package(package_lib1) + package_lib2 = Package( + "lib", "2.0", source_type="legacy", source_reference="explicit2" + ) + package_lib2.extras = {canonicalize_name("extra"): [dep_extra]} + package_lib2.add_dependency(dep_extra_opt) + explicit_repo2.add_package(package_lib2) + + package_extra = Package("extra", "1.0") + repo.add_package(package_extra) + package_other = Package("other", "1.5") + package_other.add_dependency(Factory.create_dependency("lib", ">=1.0")) + repo.add_package(package_other) + + if locked: + locked_packages = [package_extra, package_lib1, package_lib2, package_other] + use_latest = [canonicalize_name("other")] + else: + locked_packages = [] + use_latest = None + solver = Solver(package, pool, [], locked_packages, io) + + transaction = solver.solve(use_latest=use_latest) + + ops = check_solver_result( + transaction, + [ + {"job": "install", "package": package_extra}, + {"job": "install", "package": package_lib1}, + {"job": "install", "package": package_lib2}, + {"job": "install", "package": package_other}, + ], + ) + assert ops[1].package.source_reference == "explicit1" + assert ops[2].package.source_reference == "explicit2" + + def test_solver_discards_packages_with_empty_markers( package: ProjectPackage, repo: Repository,