From 1bb9da610a550c05a554285b8ffc110f65af0892 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Randy=20D=C3=B6ring?= <30527984+radoering@users.noreply.github.com> Date: Fri, 10 Jan 2025 18:22:37 +0100 Subject: [PATCH] fix: `update`, `add` and `remove` with `--only` shall not uninstall dependencies from all other groups Reintroduce some logic removed in bd3500d07cd0ab0d10d26917fdda3abdd8e67704 and add tests for it. Fix logic with `reresolve = False`, which had the same issue. - The change in installer is the fix for `reresolve = True` (reintroducing old logic). - The change in transaction is the fix for `reresolve = False`. --- src/poetry/installation/installer.py | 19 +++++++++++++++- src/poetry/puzzle/transaction.py | 5 ++--- tests/installation/test_installer.py | 9 +++++++- tests/puzzle/test_transaction.py | 33 +++++++++++++++++++++------- 4 files changed, 53 insertions(+), 13 deletions(-) diff --git a/src/poetry/installation/installer.py b/src/poetry/installation/installer.py index c51a7db04bf..e86fb540a0a 100644 --- a/src/poetry/installation/installer.py +++ b/src/poetry/installation/installer.py @@ -329,7 +329,9 @@ def _do_install(self) -> int: ) ops = transaction.calculate_operations( - with_uninstalls=self._requires_synchronization or self._update, + with_uninstalls=( + self._requires_synchronization or (self._update and not reresolve) + ), synchronize=self._requires_synchronization, skip_directory=self._skip_directory, extras=set(self._extras), @@ -337,6 +339,21 @@ def _do_install(self) -> int: p.name for p in self._installed_repository.system_site_packages }, ) + if reresolve and not self._requires_synchronization: + # If no packages synchronisation has been requested we need + # to calculate the uninstall operations + transaction = Transaction( + locked_repository.packages, + lockfile_repo.packages, + installed_packages=self._installed_repository.packages, + root_package=root, + ) + + ops = [ + op + for op in transaction.calculate_operations(with_uninstalls=True) + if op.job_type == "uninstall" + ] + ops # Validate the dependencies for op in ops: diff --git a/src/poetry/puzzle/transaction.py b/src/poetry/puzzle/transaction.py index b7cc361186a..df94dc6ae48 100644 --- a/src/poetry/puzzle/transaction.py +++ b/src/poetry/puzzle/transaction.py @@ -149,10 +149,9 @@ def calculate_operations( operations.append(Uninstall(package)) if with_uninstalls: + result_packages = {package.name for package in self._result_packages} for current_package in self._current_packages: - if current_package.name not in ( - relevant_result_packages | uninstalls - ) and ( + if current_package.name not in (result_packages | uninstalls) and ( installed_package := self._installed_packages.get( current_package.name ) diff --git a/tests/installation/test_installer.py b/tests/installation/test_installer.py index 87098eb3249..daeb9e52796 100644 --- a/tests/installation/test_installer.py +++ b/tests/installation/test_installer.py @@ -373,6 +373,8 @@ def _configure_run_install_dev( @pytest.mark.parametrize("lock_version", ("1.1", "2.1")) +@pytest.mark.parametrize("update", [False, True]) +@pytest.mark.parametrize("requires_synchronization", [False, True]) @pytest.mark.parametrize( ("groups", "installs", "updates", "removals", "with_packages_installed"), [ @@ -399,6 +401,8 @@ def test_run_install_with_dependency_groups( repo: Repository, package: ProjectPackage, installed: CustomInstalledRepository, + update: bool, + requires_synchronization: bool, lock_version: str, ) -> None: _configure_run_install_dev( @@ -414,10 +418,13 @@ def test_run_install_with_dependency_groups( if groups is not None: installer.only_groups(groups) - installer.requires_synchronization(True) + installer.update(update) + installer.requires_synchronization(requires_synchronization) result = installer.run() assert result == 0 + if not requires_synchronization: + removals = 0 assert installer.executor.installations_count == installs assert installer.executor.updates_count == updates assert installer.executor.removals_count == removals diff --git a/tests/puzzle/test_transaction.py b/tests/puzzle/test_transaction.py index 2dcea971e79..a368adf0d31 100644 --- a/tests/puzzle/test_transaction.py +++ b/tests/puzzle/test_transaction.py @@ -258,12 +258,17 @@ def test_it_should_update_installed_packages_if_sources_are_different() -> None: ], ) @pytest.mark.parametrize("installed", [False, True]) +@pytest.mark.parametrize("with_uninstalls", [False, True]) @pytest.mark.parametrize("sync", [False, True]) def test_calculate_operations_with_groups( - installed: bool, sync: bool, groups: set[str], expected: list[str] + installed: bool, + with_uninstalls: bool, + sync: bool, + groups: set[str], + expected: list[str], ) -> None: transaction = Transaction( - [Package("a", "1"), Package("b", "1"), Package("c", "1")], + [Package("a", "1"), Package("b", "1"), Package("c", "1"), Package("d", "1")], { Package("a", "1"): TransitivePackageInfo( 0, {"main"}, {"main": AnyMarker()} @@ -273,7 +278,11 @@ def test_calculate_operations_with_groups( 0, {"main", "dev"}, {"main": AnyMarker(), "dev": AnyMarker()} ), }, - [Package("a", "1"), Package("b", "1"), Package("c", "1")] if installed else [], + ( + [Package("a", "1"), Package("b", "1"), Package("c", "1"), Package("d", "1")] + if installed + else [] + ), None, {"python_version": "3.8"}, groups, @@ -285,12 +294,19 @@ def test_calculate_operations_with_groups( if installed: for op in expected_ops: op["skipped"] = True - if sync: - for name in sorted({"a", "b", "c"}.difference(expected), reverse=True): - expected_ops.insert(0, {"job": "remove", "package": Package(name, "1")}) + if with_uninstalls: + expected_ops.insert(0, {"job": "remove", "package": Package("d", "1")}) + if sync: + for name in sorted({"a", "b", "c"}.difference(expected), reverse=True): + expected_ops.insert( + 0, {"job": "remove", "package": Package(name, "1")} + ) check_operations( - transaction.calculate_operations(with_uninstalls=sync), expected_ops + transaction.calculate_operations( + with_uninstalls=with_uninstalls, synchronize=sync + ), + expected_ops, ) @@ -329,7 +345,8 @@ def test_calculate_operations_with_markers( expected_ops.insert(0, {"job": "remove", "package": Package(name, "1")}) check_operations( - transaction.calculate_operations(with_uninstalls=sync), expected_ops + transaction.calculate_operations(with_uninstalls=sync, synchronize=sync), + expected_ops, )