From 244ee69b2c9fc72cc3eeeaa26908a0cbc98ec468 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Randy=20D=C3=B6ring?= <30527984+radoering@users.noreply.github.com> Date: Sun, 4 Feb 2024 18:10:57 +0100 Subject: [PATCH 1/3] provider: consider explicit source when searching for a locked package with a source reference in the repository pool --- src/poetry/puzzle/provider.py | 4 +++- tests/puzzle/test_provider.py | 19 +++++++++++++++++++ tests/puzzle/test_solver.py | 2 +- 3 files changed, 23 insertions(+), 2 deletions(-) diff --git a/src/poetry/puzzle/provider.py b/src/poetry/puzzle/provider.py index d404f84d520..47c4c128afb 100644 --- a/src/poetry/puzzle/provider.py +++ b/src/poetry/puzzle/provider.py @@ -487,7 +487,9 @@ def complete_package( package.pretty_name, package.version, extras=list(dependency.extras), - repository_name=dependency.source_name, + repository_name=( + dependency.source_name or package.source_reference + ), ), ) except PackageNotFound as e: diff --git a/tests/puzzle/test_provider.py b/tests/puzzle/test_provider.py index 9ee1f6fc644..71f43a0a96c 100644 --- a/tests/puzzle/test_provider.py +++ b/tests/puzzle/test_provider.py @@ -22,6 +22,7 @@ from poetry.puzzle.provider import IncompatibleConstraintsError from poetry.puzzle.provider import Provider 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 +784,24 @@ 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( + pool: RepositoryPool, provider: Provider +) -> 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) + + dependency = package.to_dependency() + # complete_package() must succeed even if the dependency does not have an explicit + # source. This can be the case if the dependency is a transitive dependency and + # there is a direct dependency with an explicit source. + dependency.source_name = None + + # must not fail + provider.complete_package(DependencyPackage(dependency, package)) + + 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..6de5a69128a 100644 --- a/tests/puzzle/test_solver.py +++ b/tests/puzzle/test_solver.py @@ -3826,7 +3826,7 @@ def test_solver_should_not_update_same_version_packages_if_installed_has_no_sour "1.0.0", source_type="legacy", source_url="https://foo.bar", - source_reference="custom", + source_reference="repo", ) repo.add_package(foo) From 84c776dcf0a5cb6de20d3f6270006d442ad4ea40 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Randy=20D=C3=B6ring?= <30527984+radoering@users.noreply.github.com> Date: Wed, 7 Feb 2024 20:00:14 +0100 Subject: [PATCH 2/3] better approach --- src/poetry/puzzle/provider.py | 17 ++++++++-- tests/puzzle/test_provider.py | 60 ++++++++++++++++++++++++++++++----- tests/puzzle/test_solver.py | 2 +- 3 files changed, 67 insertions(+), 12 deletions(-) diff --git a/src/poetry/puzzle/provider.py b/src/poetry/puzzle/provider.py index 47c4c128afb..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) @@ -487,9 +488,7 @@ def complete_package( package.pretty_name, package.version, extras=list(dependency.extras), - repository_name=( - dependency.source_name or package.source_reference - ), + repository_name=dependency.source_name, ), ) except PackageNotFound as e: @@ -684,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: @@ -694,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 71f43a0a96c..1e1b03acc42 100644 --- a/tests/puzzle/test_provider.py +++ b/tests/puzzle/test_provider.py @@ -21,6 +21,7 @@ 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 @@ -785,21 +786,64 @@ def test_complete_package_fetches_optional_vcs_dependency_only_if_requested( def test_complete_package_finds_locked_package_in_explicit_source( - pool: RepositoryPool, provider: Provider + 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) - dependency = package.to_dependency() - # complete_package() must succeed even if the dependency does not have an explicit - # source. This can be the case if the dependency is a transitive dependency and - # there is a direct dependency with an explicit source. - dependency.source_name = None + 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)) - # must not fail - provider.complete_package(DependencyPackage(dependency, package)) + # 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( diff --git a/tests/puzzle/test_solver.py b/tests/puzzle/test_solver.py index 6de5a69128a..dd77ff01cca 100644 --- a/tests/puzzle/test_solver.py +++ b/tests/puzzle/test_solver.py @@ -3826,7 +3826,7 @@ def test_solver_should_not_update_same_version_packages_if_installed_has_no_sour "1.0.0", source_type="legacy", source_url="https://foo.bar", - source_reference="repo", + source_reference="custom", ) repo.add_package(foo) From 0520cf336c3145d63e91dd34f9d4e224243e5dd7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Randy=20D=C3=B6ring?= <30527984+radoering@users.noreply.github.com> Date: Fri, 9 Feb 2024 17:27:48 +0100 Subject: [PATCH 3/3] add test with overrides and extras --- tests/puzzle/test_solver.py | 98 +++++++++++++++++++++++++++++++++++++ 1 file changed, 98 insertions(+) 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,