temp: do not uninstall extras that are not required and previous lock…

…ed packages (does not work for remove...)

src/poetry/installation/installer.py

@@ -299,29 +299,11 @@ def _do_install(self) -> int:
 
         with solver.use_environment(self._env):
             ops = solver.solve(use_latest=self._whitelist).calculate_operations(
-                with_uninstalls=self._requires_synchronization,
                 synchronize=self._requires_synchronization,
                 skip_directory=self._skip_directory,
             )
 
-        if not self._requires_synchronization:
-            # If no packages synchronisation has been requested we need
-            # to calculate the uninstall operations
-            from poetry.puzzle.transaction import Transaction
-
-            transaction = Transaction(
-                locked_repository.packages,
-                [(package, 0) for package in 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
-        else:
+        if self._requires_synchronization:
             ops = uninstalls + ops
 
         # We need to filter operations so that packages

src/poetry/puzzle/solver.py

@@ -45,7 +45,6 @@ def __init__(
         self._package = package
         self._pool = pool
         self._installed_packages = installed
-        self._locked_packages = locked
         self._io = io
 
         self._provider = Provider(
@@ -95,7 +94,6 @@ def solve(
                 self._io.write_error_line(f"<warning>Warning: {message}</warning>")
 
         return Transaction(
-            self._locked_packages,
             list(zip(packages, depths)),
             installed_packages=self._installed_packages,
             root_package=self._package,

src/poetry/puzzle/transaction.py

@@ -12,12 +12,10 @@
 class Transaction:
     def __init__(
         self,
-        current_packages: list[Package],
         result_packages: list[tuple[Package, int]],
         installed_packages: list[Package] | None = None,
         root_package: Package | None = None,
     ) -> None:
-        self._current_packages = current_packages
         self._result_packages = result_packages
 
         if installed_packages is None:
@@ -28,7 +26,6 @@ def __init__(
 
     def calculate_operations(
         self,
-        with_uninstalls: bool = True,
         synchronize: bool = False,
         *,
         skip_directory: bool = False,
@@ -80,44 +77,31 @@ def calculate_operations(
             ):
                 operations.append(Install(result_package, priority=priority))
 
-        if with_uninstalls:
+        if synchronize:
+            result_package_names = {
+                result_package.name for result_package, _ in self._result_packages
+            }
+            # We preserve pip when not managed by poetry, this is done to avoid
+            # externally managed virtual environments causing unnecessary removals.
+            preserved_package_names = {"pip"} - result_package_names
+
             uninstalls: set[str] = set()
-            for current_package in self._current_packages:
-                found = any(
-                    current_package.name == result_package.name
-                    for result_package, _ in self._result_packages
-                )
-
-                if not found:
-                    for installed_package in self._installed_packages:
-                        if installed_package.name == current_package.name:
-                            uninstalls.add(installed_package.name)
-                            operations.append(Uninstall(current_package))
-
-            if synchronize:
-                result_package_names = {
-                    result_package.name for result_package, _ in self._result_packages
-                }
-                # We preserve pip when not managed by poetry, this is done to avoid
-                # externally managed virtual environments causing unnecessary removals.
-                preserved_package_names = {"pip"} - result_package_names
-
-                for installed_package in self._installed_packages:
-                    if installed_package.name in uninstalls:
-                        continue
-
-                    if (
-                        self._root_package
-                        and installed_package.name == self._root_package.name
-                    ):
-                        continue
+            for installed_package in self._installed_packages:
+                if installed_package.name in uninstalls:
+                    continue
+
+                if (
+                    self._root_package
+                    and installed_package.name == self._root_package.name
+                ):
+                    continue
 
-                    if installed_package.name in preserved_package_names:
-                        continue
+                if installed_package.name in preserved_package_names:
+                    continue
 
-                    if installed_package.name not in result_package_names:
-                        uninstalls.add(installed_package.name)
-                        operations.append(Uninstall(installed_package))
+                if installed_package.name not in result_package_names:
+                    uninstalls.add(installed_package.name)
+                    operations.append(Uninstall(installed_package))
 
         return sorted(
             operations,

tests/installation/test_installer.py

@@ -244,12 +244,14 @@ def test_run_with_dependencies(
     assert locker.written_data == expected
 
 
+@pytest.mark.parametrize("sync", [True, False])
 def test_run_update_after_removing_dependencies(
     installer: Installer,
     locker: Locker,
     repo: Repository,
     package: ProjectPackage,
     installed: CustomInstalledRepository,
+    sync: bool,
 ) -> None:
     locker.locked(True)
     locker.mock_lock_data(
@@ -303,6 +305,7 @@ def test_run_update_after_removing_dependencies(
     package.add_dependency(Factory.create_dependency("B", "~1.1"))
 
     installer.update(True)
+    installer.requires_synchronization(sync)
     result = installer.run()
     assert result == 0
 
@@ -311,7 +314,7 @@ def test_run_update_after_removing_dependencies(
 
     assert installer.executor.installations_count == 0
     assert installer.executor.updates_count == 0
-    assert installer.executor.removals_count == 1
+    assert installer.executor.removals_count == (1 if sync else 0)
 
 
 def _configure_run_install_dev(
@@ -570,12 +573,14 @@ def test_run_install_removes_locked_packages_if_installed_and_synchronization_is
     assert installer.executor.removals_count == 2
 
 
+@pytest.mark.parametrize("sync", [True, False])
 def test_run_install_removes_no_longer_locked_packages_if_installed(
     installer: Installer,
     locker: Locker,
     repo: Repository,
     package: ProjectPackage,
     installed: CustomInstalledRepository,
+    sync: bool,
 ) -> None:
     package_a = get_package("a", "1.0")
     package_b = get_package("b", "1.1")
@@ -633,12 +638,13 @@ def test_run_install_removes_no_longer_locked_packages_if_installed(
     )
 
     installer.update(True)
+    installer.requires_synchronization(sync)
     result = installer.run()
     assert result == 0
 
     assert installer.executor.installations_count == 0
     assert installer.executor.updates_count == 0
-    assert installer.executor.removals_count == 2
+    assert installer.executor.removals_count == (2 if sync else 0)
 
 
 @pytest.mark.parametrize(
@@ -709,13 +715,9 @@ def test_run_install_with_synchronization(
 
     assert installer.executor.installations_count == 0
     assert installer.executor.updates_count == 0
-    assert 2 + len(managed_reserved_packages) == installer.executor.removals_count
+    assert installer.executor.removals_count == 2
 
-    expected_removals = {
-        package_b.name,
-        package_c.name,
-        *managed_reserved_package_names,
-    }
+    expected_removals = {package_b.name, package_c.name}
 
     assert isinstance(installer.executor, Executor)
     assert {r.name for r in installer.executor.removals} == expected_removals
@@ -765,12 +767,14 @@ def test_run_whitelist_add(
     assert locker.written_data == expected
 
 
+@pytest.mark.parametrize("sync", [True, False])
 def test_run_whitelist_remove(
     installer: Installer,
     locker: Locker,
     repo: Repository,
     package: ProjectPackage,
     installed: CustomInstalledRepository,
+    sync: bool,
 ) -> None:
     locker.locked(True)
     locker.mock_lock_data(
@@ -810,6 +814,7 @@ def test_run_whitelist_remove(
     package.add_dependency(Factory.create_dependency("A", "~1.0"))
 
     installer.update(True)
+    installer.requires_synchronization(sync)
     installer.whitelist(["B"])
 
     result = installer.run()
@@ -819,7 +824,7 @@ def test_run_whitelist_remove(
     assert locker.written_data == expected
     assert installer.executor.installations_count == 1
     assert installer.executor.updates_count == 0
-    assert installer.executor.removals_count == 1
+    assert installer.executor.removals_count == (1 if sync else 0)
 
 
 def test_add_with_sub_dependencies(
@@ -1087,9 +1092,9 @@ def test_run_installs_extras_with_deps_if_requested(
     else:
         # A, B
         expected_installations_count = 0 if is_installed else 2
-        # We only want to uninstall extras if we do a "poetry install" without extras,
-        # not if we do a "poetry update" or "poetry add".
-        expected_removals_count = 2 if is_installed and is_locked else 0
+        # We only want to uninstall extras if we do a "poetry install --sync" without
+        # extras, not if we do a "poetry install", "poetry update" or "poetry add".
+        expected_removals_count = 2 if is_installed and is_locked and do_sync else 0
 
     assert installer.executor.installations_count == expected_installations_count
     assert installer.executor.removals_count == expected_removals_count
@@ -1625,12 +1630,14 @@ def test_run_install_duplicate_dependencies_different_constraints_with_lock(
     assert installer.executor.removals_count == 0
 
 
+@pytest.mark.parametrize("sync", [True, False])
 def test_run_update_uninstalls_after_removal_transitive_dependency(
     installer: Installer,
     locker: Locker,
     repo: Repository,
     package: ProjectPackage,
     installed: CustomInstalledRepository,
+    sync: bool,
 ) -> None:
     locker.locked(True)
     locker.mock_lock_data(
@@ -1678,12 +1685,13 @@ def test_run_update_uninstalls_after_removal_transitive_dependency(
     installed.add_package(get_package("B", "1.0"))
 
     installer.update(True)
+    installer.requires_synchronization(sync)
     result = installer.run()
     assert result == 0
 
     assert installer.executor.installations_count == 0
     assert installer.executor.updates_count == 0
-    assert installer.executor.removals_count == 1
+    assert installer.executor.removals_count == (1 if sync else 0)
 
 
 def test_run_install_duplicate_dependencies_different_constraints_with_lock_update(

tests/puzzle/test_solver.py

@@ -136,7 +136,7 @@ def test_solver_remove_if_no_longer_locked(
     solver = Solver(package, pool, [package_a], [package_a], io)
     transaction = solver.solve()
 
-    check_solver_result(transaction, [{"job": "remove", "package": package_a}])
+    check_solver_result(transaction, [])
 
 
 def test_remove_non_installed(

tests/puzzle/test_transaction.py

@@ -42,7 +42,6 @@ def check_operations(ops: list[Operation], expected: list[dict[str, Any]]) -> No
 
 def test_it_should_calculate_operations_in_correct_order() -> None:
     transaction = Transaction(
-        [Package("a", "1.0.0"), Package("b", "2.0.0"), Package("c", "3.0.0")],
         [
             (Package("a", "1.0.0"), 1),
             (Package("b", "2.1.0"), 2),
@@ -62,7 +61,6 @@ def test_it_should_calculate_operations_in_correct_order() -> None:
 
 def test_it_should_calculate_operations_for_installed_packages() -> None:
     transaction = Transaction(
-        [Package("a", "1.0.0"), Package("b", "2.0.0"), Package("c", "3.0.0")],
         [
             (Package("a", "1.0.0"), 1),
             (Package("b", "2.1.0"), 2),
@@ -79,7 +77,6 @@ def test_it_should_calculate_operations_for_installed_packages() -> None:
     check_operations(
         transaction.calculate_operations(),
         [
-            {"job": "remove", "package": Package("c", "3.0.0")},
             {
                 "job": "update",
                 "from": Package("b", "2.0.0"),
@@ -93,7 +90,6 @@ def test_it_should_calculate_operations_for_installed_packages() -> None:
 
 def test_it_should_remove_installed_packages_if_required() -> None:
     transaction = Transaction(
-        [Package("a", "1.0.0"), Package("b", "2.0.0"), Package("c", "3.0.0")],
         [
             (Package("a", "1.0.0"), 1),
             (Package("b", "2.1.0"), 2),
@@ -125,7 +121,6 @@ def test_it_should_remove_installed_packages_if_required() -> None:
 
 def test_it_should_not_remove_installed_packages_that_are_in_result() -> None:
     transaction = Transaction(
-        [],
         [
             (Package("a", "1.0.0"), 1),
             (Package("b", "2.0.0"), 2),
@@ -150,7 +145,6 @@ def test_it_should_not_remove_installed_packages_that_are_in_result() -> None:
 
 def test_it_should_update_installed_packages_if_sources_are_different() -> None:
     transaction = Transaction(
-        [Package("a", "1.0.0")],
         [
             (
                 Package(