Skip to content

Commit

Permalink
fix: update, add and remove shall not uninstall extra dependencies
Browse files Browse the repository at this point in the history
With this change unrequested extras dependencies will also be kept when running `install` and are only removed when running `sync`!
  • Loading branch information
radoering committed Jan 11, 2025
1 parent 30fe6c7 commit 861dd4a
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 13 deletions.
14 changes: 4 additions & 10 deletions src/poetry/puzzle/transaction.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,6 @@ def calculate_operations(
else:
priorities = defaultdict(int)
relevant_result_packages: set[NormalizedName] = set()
pending_extra_uninstalls: list[Package] = [] # list for deterministic order
for result_package in self._result_packages:
is_unsolicited_extra = False
if self._marker_env:
Expand All @@ -103,9 +102,9 @@ def calculate_operations(
relevant_result_packages.add(result_package.name)

if installed_package := self._installed_packages.get(result_package.name):
# Extras that were not requested are always uninstalled.
# Extras that were not requested are not relevant.
if is_unsolicited_extra:
pending_extra_uninstalls.append(installed_package)
pass

# We have to perform an update if the version or another
# attribute of the package has changed (source type, url, ref, ...).
Expand Down Expand Up @@ -141,14 +140,9 @@ def calculate_operations(
op.skip("Not required")
operations.append(op)

uninstalls: set[NormalizedName] = set()
for package in pending_extra_uninstalls:
if package.name not in (relevant_result_packages | uninstalls):
uninstalls.add(package.name)
if package.name not in system_site_packages:
operations.append(Uninstall(package))

if with_uninstalls:
uninstalls: set[NormalizedName] = set()

result_packages = {package.name for package in self._result_packages}
for current_package in self._current_packages:
if current_package.name not in (result_packages | uninstalls) and (
Expand Down
5 changes: 4 additions & 1 deletion tests/installation/test_installer.py
Original file line number Diff line number Diff line change
Expand Up @@ -1400,6 +1400,7 @@ def test_run_with_different_dependency_extras(
@pytest.mark.parametrize("is_locked", [False, True])
@pytest.mark.parametrize("is_installed", [False, True])
@pytest.mark.parametrize("with_extras", [False, True])
@pytest.mark.parametrize("do_update", [False, True])
@pytest.mark.parametrize("do_sync", [False, True])
def test_run_installs_extras_with_deps_if_requested(
installer: Installer,
Expand All @@ -1410,6 +1411,7 @@ def test_run_installs_extras_with_deps_if_requested(
is_locked: bool,
is_installed: bool,
with_extras: bool,
do_update: bool,
do_sync: bool,
) -> None:
package.extras = {canonicalize_name("foo"): [get_dependency("C")]}
Expand Down Expand Up @@ -1443,6 +1445,7 @@ def test_run_installs_extras_with_deps_if_requested(

if with_extras:
installer.extras(["foo"])
installer.update(do_update)
installer.requires_synchronization(do_sync)
result = installer.run()
assert result == 0
Expand All @@ -1459,7 +1462,7 @@ def test_run_installs_extras_with_deps_if_requested(
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 else 0
expected_removals_count = 2 if is_installed and do_sync else 0

assert installer.executor.installations_count == expected_installations_count
assert installer.executor.removals_count == expected_removals_count
Expand Down
7 changes: 5 additions & 2 deletions tests/puzzle/test_transaction.py
Original file line number Diff line number Diff line change
Expand Up @@ -433,8 +433,10 @@ def test_calculate_operations_extras(
if extras:
ops = [{"job": "install", "package": Package("a", "1"), "skipped": installed}]
elif installed:
# extras are always removed, even if with_uninstalls is False
ops = [{"job": "remove", "package": Package("a", "1")}]
if with_uninstalls and sync:
ops = [{"job": "remove", "package": Package("a", "1")}]
else:
ops = []
else:
ops = [{"job": "install", "package": Package("a", "1"), "skipped": True}]

Expand Down Expand Up @@ -494,6 +496,7 @@ def test_calculate_operations_extras_no_redundant_uninstall(extra: str) -> None:

check_operations(
transaction.calculate_operations(
synchronize=True,
extras=set() if not extra else {canonicalize_name(extra)},
),
ops,
Expand Down

0 comments on commit 861dd4a

Please sign in to comment.