diff --git a/src/poetry/installation/installer.py b/src/poetry/installation/installer.py index 975b7526135..acafc8bb9bb 100644 --- a/src/poetry/installation/installer.py +++ b/src/poetry/installation/installer.py @@ -12,6 +12,7 @@ from poetry.repositories import Pool from poetry.repositories import Repository from poetry.repositories.installed_repository import InstalledRepository +from poetry.repositories.lockfile_repository import LockfileRepository from poetry.utils.extras import get_extra_package_names from poetry.utils.helpers import canonicalize_name from poetry.utils.helpers import pluralize @@ -107,9 +108,7 @@ def run(self) -> int: self._write_lock = False self._execute_operations = False - local_repo = Repository() - - return self._do_install(local_repo) + return self._do_install() def dry_run(self, dry_run: bool = True) -> Installer: self._dry_run = dry_run @@ -204,14 +203,14 @@ def _do_refresh(self) -> int: ): ops = solver.solve(use_latest=[]).calculate_operations() - local_repo = Repository() - self._populate_local_repo(local_repo, ops) + lockfile_repo = LockfileRepository() + self._populate_lockfile_repo(lockfile_repo, ops) - self._write_lock_file(local_repo, force=True) + self._write_lock_file(lockfile_repo, force=True) return 0 - def _do_install(self, local_repo: Repository) -> int: + def _do_install(self) -> int: from poetry.puzzle.solver import Solver locked_repository = Repository() @@ -266,10 +265,11 @@ def _do_install(self, local_repo: Repository) -> int: # currently installed ops = self._get_operations_from_lock(locked_repository) - self._populate_local_repo(local_repo, ops) + lockfile_repo = LockfileRepository() + self._populate_lockfile_repo(lockfile_repo, ops) if self._update: - self._write_lock_file(local_repo) + self._write_lock_file(lockfile_repo) if self._lock: # If we are only in lock mode, no need to go any further @@ -292,8 +292,8 @@ def _do_install(self, local_repo: Repository) -> int: # Making a new repo containing the packages # newly resolved and the ones from the current lock file repo = Repository() - for package in local_repo.packages + locked_repository.packages: - if not repo.has_package(package): + for package in lockfile_repo.packages + locked_repository.packages: + if not package.is_direct_origin() and not repo.has_package(package): repo.add_package(package) pool.add_repository(repo) @@ -318,7 +318,7 @@ def _do_install(self, local_repo: Repository) -> int: transaction = Transaction( locked_repository.packages, - [(package, 0) for package in local_repo.packages], + [(package, 0) for package in lockfile_repo.packages], installed_packages=self._installed_repository.packages, root_package=root, ) @@ -332,12 +332,12 @@ def _do_install(self, local_repo: Repository) -> int: # We need to filter operations so that packages # not compatible with the current system, # or optional and not requested, are dropped - self._filter_operations(ops, local_repo) + self._filter_operations(ops, lockfile_repo) # Execute operations return self._execute(ops) - def _write_lock_file(self, repo: Repository, force: bool = False) -> None: + def _write_lock_file(self, repo: LockfileRepository, force: bool = False) -> None: if self._write_lock and (force or self._update): updated_lock = self._locker.set_lock_data(self._package, repo.packages) @@ -460,8 +460,8 @@ def _execute_uninstall(self, operation: Uninstall) -> None: self._installer.remove(operation.package) - def _populate_local_repo( - self, local_repo: Repository, ops: Sequence[Operation] + def _populate_lockfile_repo( + self, repo: LockfileRepository, ops: Sequence[Operation] ) -> None: for op in ops: if isinstance(op, Uninstall): @@ -471,8 +471,8 @@ def _populate_local_repo( else: package = op.package - if not local_repo.has_package(package): - local_repo.add_package(package) + if not repo.has_package(package): + repo.add_package(package) def _get_operations_from_lock( self, locked_repository: Repository diff --git a/src/poetry/packages/locker.py b/src/poetry/packages/locker.py index 7ff91f2e45d..e57e60b83b7 100644 --- a/src/poetry/packages/locker.py +++ b/src/poetry/packages/locker.py @@ -483,7 +483,18 @@ def _get_lock_data(self) -> TOMLDocument: def _lock_packages(self, packages: list[Package]) -> list[dict[str, Any]]: locked = [] - for package in sorted(packages, key=lambda x: (x.name, x.version)): + for package in sorted( + packages, + key=lambda x: ( + x.name, + x.version, + x.source_type or "", + x.source_url or "", + x.source_subdirectory or "", + x.source_reference or "", + x.source_resolved_reference or "", + ), + ): spec = self._dump_package(package) locked.append(spec) diff --git a/src/poetry/puzzle/provider.py b/src/poetry/puzzle/provider.py index ed0a425d53a..b0084549b77 100644 --- a/src/poetry/puzzle/provider.py +++ b/src/poetry/puzzle/provider.py @@ -612,39 +612,31 @@ def complete_package(self, package: DependencyPackage) -> DependencyPackage: # An example of this is: # - pypiwin32 (220); sys_platform == "win32" and python_version >= "3.6" # - pypiwin32 (219); sys_platform == "win32" and python_version < "3.6" - # - # Additional care has to be taken to ensure that hidden constraints like that - # of source type, reference etc. are taking into consideration when duplicates - # are identified. - duplicates: dict[ - tuple[str, str | None, str | None, str | None, str | None], list[Dependency] - ] = {} + duplicates: dict[str, list[Dependency]] = defaultdict(list) for dep in dependencies: - key = ( - dep.complete_name, - dep.source_type, - dep.source_url, - dep.source_reference, - dep.source_subdirectory, - ) - if key not in duplicates: - duplicates[key] = [] - duplicates[key].append(dep) + duplicates[dep.complete_name].append(dep) dependencies = [] - for key, deps in duplicates.items(): + for dep_name, deps in duplicates.items(): if len(deps) == 1: dependencies.append(deps[0]) continue - extra_keys = ", ".join(k for k in key[1:] if k is not None) - dep_name = f"{key[0]} ({extra_keys})" if extra_keys else key[0] - self.debug(f"Duplicate dependencies for {dep_name}") - deps = self._merge_dependencies_by_marker(deps) - deps = self._merge_dependencies_by_constraint(deps) - + non_direct_origin_deps: list[Dependency] = [] + direct_origin_deps: list[Dependency] = [] + for dep in deps: + if dep.is_direct_origin(): + direct_origin_deps.append(dep) + else: + non_direct_origin_deps.append(dep) + deps = ( + self._merge_dependencies_by_constraint( + self._merge_dependencies_by_marker(non_direct_origin_deps) + ) + + direct_origin_deps + ) if len(deps) == 1: self.debug(f"Merging requirements for {deps[0]!s}") dependencies.append(deps[0]) diff --git a/src/poetry/repositories/lockfile_repository.py b/src/poetry/repositories/lockfile_repository.py new file mode 100644 index 00000000000..6aa44aff8c7 --- /dev/null +++ b/src/poetry/repositories/lockfile_repository.py @@ -0,0 +1,29 @@ +from __future__ import annotations + +from typing import TYPE_CHECKING + +from poetry.repositories import Repository + + +if TYPE_CHECKING: + from poetry.core.packages.package import Package + + +class LockfileRepository(Repository): + """ + Special repository that distinguishes packages not only by name and version, + but also by source type, url, etc. + """ + + def has_package(self, package: Package) -> bool: + return any(p == package for p in self.packages) + + def remove_package(self, package: Package) -> None: + index = None + for i, repo_package in enumerate(self.packages): + if repo_package == package: + index = i + break + + if index is not None: + del self._packages[index] diff --git a/tests/installation/fixtures/with-same-version-url-dependencies.test b/tests/installation/fixtures/with-same-version-url-dependencies.test new file mode 100644 index 00000000000..aec9fefa911 --- /dev/null +++ b/tests/installation/fixtures/with-same-version-url-dependencies.test @@ -0,0 +1,54 @@ +[[package]] +name = "demo" +version = "0.1.0" +description = "" +category = "main" +optional = false +python-versions = ">=2.7, !=3.0.*, !=3.1.*, !=3.2.*, !=3.3.*" + +[package.source] +type = "url" +url = "https://python-poetry.org/distributions/demo-0.1.0-py2.py3-none-any.whl" + +[package.dependencies] +pendulum = ">=1.4.4" + +[package.extras] +bar = ["tomlkit"] +foo = ["cleo"] + +[[package]] +name = "demo" +version = "0.1.0" +description = "" +category = "main" +optional = false +python-versions = ">=2.7, !=3.0.*, !=3.1.*, !=3.2.*, !=3.3.*" + +[package.source] +type = "url" +url = "https://python-poetry.org/distributions/demo-0.1.0.tar.gz" + +[package.dependencies] +pendulum = ">=1.4.4" + +[package.extras] +bar = ["tomlkit"] +foo = ["cleo"] + +[[package]] +name = "pendulum" +version = "1.4.4" +description = "" +category = "main" +optional = false +python-versions = "*" + +[metadata] +python-versions = "*" +lock-version = "1.1" +content-hash = "123456789" + +[metadata.files] +demo = [] +pendulum = [] diff --git a/tests/installation/test_installer.py b/tests/installation/test_installer.py index a153fa9386f..ce2f683411a 100644 --- a/tests/installation/test_installer.py +++ b/tests/installation/test_installer.py @@ -2231,6 +2231,56 @@ def test_run_installs_with_url_file( assert installer.executor.installations_count == 2 +@pytest.mark.parametrize("env_platform", ["linux", "win32"]) +def test_run_installs_with_same_version_url_files( + pool: Pool, + locker: Locker, + installed: CustomInstalledRepository, + config: Config, + repo: Repository, + package: ProjectPackage, + env_platform: str, +) -> None: + urls = { + "linux": "https://python-poetry.org/distributions/demo-0.1.0.tar.gz", + "win32": ( + "https://python-poetry.org/distributions/demo-0.1.0-py2.py3-none-any.whl" + ), + } + for platform, url in urls.items(): + package.add_dependency( + Factory.create_dependency( + "demo", + {"url": url, "markers": f"sys_platform == '{platform}'"}, + ) + ) + repo.add_package(get_package("pendulum", "1.4.4")) + + installer = Installer( + NullIO(), + MockEnv(platform=env_platform), + package, + locker, + pool, + config, + installed=installed, + executor=Executor( + MockEnv(platform=env_platform), + pool, + config, + NullIO(), + ), + ) + installer.use_executor(True) + installer.run() + + expected = fixture("with-same-version-url-dependencies") + assert locker.written_data == expected + assert installer.executor.installations_count == 2 + demo_package = next(p for p in installer.executor.installations if p.name == "demo") + assert demo_package.source_url == urls[env_platform] + + def test_installer_uses_prereleases_if_they_are_compatible( installer: Installer, locker: Locker, package: ProjectPackage, repo: Repository ): diff --git a/tests/packages/test_locker.py b/tests/packages/test_locker.py index 24efbcaffb2..0e79817d4f3 100644 --- a/tests/packages/test_locker.py +++ b/tests/packages/test_locker.py @@ -55,7 +55,26 @@ def test_lock_file_data_is_ordered(locker: Locker, root: ProjectPackage): source_reference="develop", source_resolved_reference="123456", ) - packages = [package_a2, package_a, get_package("B", "1.2"), package_git] + package_url_linux = Package( + "url-package", + "1.0", + source_type="url", + source_url="https://example.org/url-package-1.0-cp39-manylinux_2_17_x86_64.whl", + ) + package_url_win32 = Package( + "url-package", + "1.0", + source_type="url", + source_url="https://example.org/url-package-1.0-cp39-win_amd64.whl", + ) + packages = [ + package_a2, + package_a, + get_package("B", "1.2"), + package_git, + package_url_win32, + package_url_linux, + ] locker.set_lock_data(root, packages) @@ -105,6 +124,30 @@ def test_lock_file_data_is_ordered(locker: Locker, root: ProjectPackage): reference = "develop" resolved_reference = "123456" +[[package]] +name = "url-package" +version = "1.0" +description = "" +category = "main" +optional = false +python-versions = "*" + +[package.source] +type = "url" +url = "https://example.org/url-package-1.0-cp39-manylinux_2_17_x86_64.whl" + +[[package]] +name = "url-package" +version = "1.0" +description = "" +category = "main" +optional = false +python-versions = "*" + +[package.source] +type = "url" +url = "https://example.org/url-package-1.0-cp39-win_amd64.whl" + [metadata] lock-version = "1.1" python-versions = "*" @@ -118,6 +161,7 @@ def test_lock_file_data_is_ordered(locker: Locker, root: ProjectPackage): ] B = [] git-package = [] +url-package = [] """ assert content == expected diff --git a/tests/puzzle/test_provider.py b/tests/puzzle/test_provider.py index a1f7076ae74..a31c6d82dad 100644 --- a/tests/puzzle/test_provider.py +++ b/tests/puzzle/test_provider.py @@ -514,27 +514,25 @@ def test_search_for_file_wheel_with_extras(provider: Provider): } -def test_complete_package_preserves_source_type(provider: Provider) -> None: +def test_complete_package_preserves_source_type( + provider: Provider, root: ProjectPackage +) -> None: fixtures = Path(__file__).parent.parent / "fixtures" project_dir = fixtures.joinpath("with_conditional_path_deps") - poetry = Factory().create_poetry(cwd=project_dir) + for folder in ["demo_one", "demo_two"]: + path = (project_dir / folder).as_posix() + root.add_dependency(Factory.create_dependency("demo", {"path": path})) complete_package = provider.complete_package( - DependencyPackage(poetry.package.to_dependency(), poetry.package) + DependencyPackage(root.to_dependency(), root) ) requires = complete_package.package.all_requires - assert len(requires) == 2 - assert {requires[0].source_url, requires[1].source_url} == { project_dir.joinpath("demo_one").as_posix(), project_dir.joinpath("demo_two").as_posix(), } - assert {str(requires[0].marker), str(requires[1].marker)} == { - 'sys_platform == "linux"', - 'sys_platform == "win32"', - } def test_complete_package_preserves_source_type_with_subdirectories( @@ -545,7 +543,6 @@ def test_complete_package_preserves_source_type_with_subdirectories( { "git": "https://github.com/demo/subdirectories.git", "subdirectory": "one", - "platform": "linux", }, ) dependency_one_copy = Factory.create_dependency( @@ -553,7 +550,6 @@ def test_complete_package_preserves_source_type_with_subdirectories( { "git": "https://github.com/demo/subdirectories.git", "subdirectory": "one-copy", - "platform": "win32", }, ) dependency_two = Factory.create_dependency( @@ -567,7 +563,6 @@ def test_complete_package_preserves_source_type_with_subdirectories( { "git": "https://github.com/demo/subdirectories.git", "subdirectory": "one", - "platform": "linux", }, ) ) diff --git a/tests/puzzle/test_solver.py b/tests/puzzle/test_solver.py index b61f5c36764..931d56f641e 100644 --- a/tests/puzzle/test_solver.py +++ b/tests/puzzle/test_solver.py @@ -1418,7 +1418,7 @@ def test_solver_duplicate_dependencies_different_sources_types_are_preserved( assert len(complete_package.all_requires) == 2 - git, pypi = complete_package.all_requires + pypi, git = complete_package.all_requires assert isinstance(pypi, Dependency) assert pypi == dependency_pypi @@ -1632,6 +1632,63 @@ def test_solver_duplicate_dependencies_sub_dependencies( ) +def test_duplicate_path_dependencies(solver: Solver, package: ProjectPackage) -> None: + solver.provider.set_package_python_versions("^3.7") + fixtures = Path(__file__).parent.parent / "fixtures" + project_dir = fixtures / "with_conditional_path_deps" + + path1 = (project_dir / "demo_one").as_posix() + demo1 = Package("demo", "1.2.3", source_type="directory", source_url=path1) + package.add_dependency( + Factory.create_dependency( + "demo", {"path": path1, "markers": "sys_platform == 'linux'"} + ) + ) + + path2 = (project_dir / "demo_two").as_posix() + demo2 = Package("demo", "1.2.3", source_type="directory", source_url=path2) + package.add_dependency( + Factory.create_dependency( + "demo", {"path": path2, "markers": "sys_platform == 'win32'"} + ) + ) + + transaction = solver.solve() + + check_solver_result( + transaction, + [ + {"job": "install", "package": demo1}, + {"job": "install", "package": demo2}, + ], + ) + + +def test_duplicate_path_dependencies_same_path( + solver: Solver, package: ProjectPackage +) -> None: + solver.provider.set_package_python_versions("^3.7") + fixtures = Path(__file__).parent.parent / "fixtures" + project_dir = fixtures / "with_conditional_path_deps" + + path1 = (project_dir / "demo_one").as_posix() + demo1 = Package("demo", "1.2.3", source_type="directory", source_url=path1) + package.add_dependency( + Factory.create_dependency( + "demo", {"path": path1, "markers": "sys_platform == 'linux'"} + ) + ) + package.add_dependency( + Factory.create_dependency( + "demo", {"path": path1, "markers": "sys_platform == 'win32'"} + ) + ) + + transaction = solver.solve() + + check_solver_result(transaction, [{"job": "install", "package": demo1}]) + + def test_solver_fails_if_dependency_name_does_not_match_package( solver: Solver, repo: Repository, package: ProjectPackage ): diff --git a/tests/repositories/test_lockfile_repository.py b/tests/repositories/test_lockfile_repository.py new file mode 100644 index 00000000000..200a48f5bb7 --- /dev/null +++ b/tests/repositories/test_lockfile_repository.py @@ -0,0 +1,60 @@ +from __future__ import annotations + +from copy import deepcopy + +from poetry.core.packages.package import Package + +from poetry.repositories.lockfile_repository import LockfileRepository + + +def test_has_package(): + repo = LockfileRepository() + + url_package = Package( + "a", "1.0", source_type="url", source_url="https://example.org/a.whl" + ) + assert not repo.has_package(url_package) + repo.add_package(url_package) + + pypi_package = Package("a", "1.0") + assert not repo.has_package(pypi_package) + repo.add_package(pypi_package) + + url_package_2 = Package( + "a", "1.0", source_type="url", source_url="https://example.org/a-1.whl" + ) + assert not repo.has_package(url_package_2) + repo.add_package(url_package_2) + + assert len(repo.packages) == 3 + assert repo.has_package(deepcopy(url_package)) + assert repo.has_package(deepcopy(pypi_package)) + assert repo.has_package(deepcopy(url_package_2)) + + +def test_remove_package(): + url_package = Package( + "a", "1.0", source_type="url", source_url="https://example.org/a.whl" + ) + pypi_package = Package("a", "1.0") + url_package_2 = Package( + "a", "1.0", source_type="url", source_url="https://example.org/a-1.whl" + ) + + repo = LockfileRepository() + repo.add_package(url_package) + repo.add_package(pypi_package) + repo.add_package(url_package_2) + + assert len(repo.packages) == 3 + + repo.remove_package(deepcopy(pypi_package)) + assert len(repo.packages) == 2 + repo.remove_package(pypi_package) + assert len(repo.packages) == 2 + + repo.remove_package(deepcopy(url_package_2)) + assert len(repo.packages) == 1 + assert repo.packages[0] == url_package + repo.remove_package(url_package_2) + assert len(repo.packages) == 1