Skip to content

Commit 30fe6c7

Browse files
radoeringabn
authored andcommitted
fix: update, add and remove with --only shall not uninstall dependencies from all other groups
Reintroduce some logic removed in bd3500d 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`.
1 parent d6dbe8c commit 30fe6c7

File tree

4 files changed

+53
-13
lines changed

4 files changed

+53
-13
lines changed

src/poetry/installation/installer.py

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -329,14 +329,31 @@ def _do_install(self) -> int:
329329
)
330330

331331
ops = transaction.calculate_operations(
332-
with_uninstalls=self._requires_synchronization or self._update,
332+
with_uninstalls=(
333+
self._requires_synchronization or (self._update and not reresolve)
334+
),
333335
synchronize=self._requires_synchronization,
334336
skip_directory=self._skip_directory,
335337
extras=set(self._extras),
336338
system_site_packages={
337339
p.name for p in self._installed_repository.system_site_packages
338340
},
339341
)
342+
if reresolve and not self._requires_synchronization:
343+
# If no packages synchronisation has been requested we need
344+
# to calculate the uninstall operations
345+
transaction = Transaction(
346+
locked_repository.packages,
347+
lockfile_repo.packages,
348+
installed_packages=self._installed_repository.packages,
349+
root_package=root,
350+
)
351+
352+
ops = [
353+
op
354+
for op in transaction.calculate_operations(with_uninstalls=True)
355+
if op.job_type == "uninstall"
356+
] + ops
340357

341358
# Validate the dependencies
342359
for op in ops:

src/poetry/puzzle/transaction.py

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -149,10 +149,9 @@ def calculate_operations(
149149
operations.append(Uninstall(package))
150150

151151
if with_uninstalls:
152+
result_packages = {package.name for package in self._result_packages}
152153
for current_package in self._current_packages:
153-
if current_package.name not in (
154-
relevant_result_packages | uninstalls
155-
) and (
154+
if current_package.name not in (result_packages | uninstalls) and (
156155
installed_package := self._installed_packages.get(
157156
current_package.name
158157
)

tests/installation/test_installer.py

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -373,6 +373,8 @@ def _configure_run_install_dev(
373373

374374

375375
@pytest.mark.parametrize("lock_version", ("1.1", "2.1"))
376+
@pytest.mark.parametrize("update", [False, True])
377+
@pytest.mark.parametrize("requires_synchronization", [False, True])
376378
@pytest.mark.parametrize(
377379
("groups", "installs", "updates", "removals", "with_packages_installed"),
378380
[
@@ -399,6 +401,8 @@ def test_run_install_with_dependency_groups(
399401
repo: Repository,
400402
package: ProjectPackage,
401403
installed: CustomInstalledRepository,
404+
update: bool,
405+
requires_synchronization: bool,
402406
lock_version: str,
403407
) -> None:
404408
_configure_run_install_dev(
@@ -414,10 +418,13 @@ def test_run_install_with_dependency_groups(
414418
if groups is not None:
415419
installer.only_groups(groups)
416420

417-
installer.requires_synchronization(True)
421+
installer.update(update)
422+
installer.requires_synchronization(requires_synchronization)
418423
result = installer.run()
419424
assert result == 0
420425

426+
if not requires_synchronization:
427+
removals = 0
421428
assert installer.executor.installations_count == installs
422429
assert installer.executor.updates_count == updates
423430
assert installer.executor.removals_count == removals

tests/puzzle/test_transaction.py

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -258,12 +258,17 @@ def test_it_should_update_installed_packages_if_sources_are_different() -> None:
258258
],
259259
)
260260
@pytest.mark.parametrize("installed", [False, True])
261+
@pytest.mark.parametrize("with_uninstalls", [False, True])
261262
@pytest.mark.parametrize("sync", [False, True])
262263
def test_calculate_operations_with_groups(
263-
installed: bool, sync: bool, groups: set[str], expected: list[str]
264+
installed: bool,
265+
with_uninstalls: bool,
266+
sync: bool,
267+
groups: set[str],
268+
expected: list[str],
264269
) -> None:
265270
transaction = Transaction(
266-
[Package("a", "1"), Package("b", "1"), Package("c", "1")],
271+
[Package("a", "1"), Package("b", "1"), Package("c", "1"), Package("d", "1")],
267272
{
268273
Package("a", "1"): TransitivePackageInfo(
269274
0, {"main"}, {"main": AnyMarker()}
@@ -273,7 +278,11 @@ def test_calculate_operations_with_groups(
273278
0, {"main", "dev"}, {"main": AnyMarker(), "dev": AnyMarker()}
274279
),
275280
},
276-
[Package("a", "1"), Package("b", "1"), Package("c", "1")] if installed else [],
281+
(
282+
[Package("a", "1"), Package("b", "1"), Package("c", "1"), Package("d", "1")]
283+
if installed
284+
else []
285+
),
277286
None,
278287
{"python_version": "3.8"},
279288
groups,
@@ -285,12 +294,19 @@ def test_calculate_operations_with_groups(
285294
if installed:
286295
for op in expected_ops:
287296
op["skipped"] = True
288-
if sync:
289-
for name in sorted({"a", "b", "c"}.difference(expected), reverse=True):
290-
expected_ops.insert(0, {"job": "remove", "package": Package(name, "1")})
297+
if with_uninstalls:
298+
expected_ops.insert(0, {"job": "remove", "package": Package("d", "1")})
299+
if sync:
300+
for name in sorted({"a", "b", "c"}.difference(expected), reverse=True):
301+
expected_ops.insert(
302+
0, {"job": "remove", "package": Package(name, "1")}
303+
)
291304

292305
check_operations(
293-
transaction.calculate_operations(with_uninstalls=sync), expected_ops
306+
transaction.calculate_operations(
307+
with_uninstalls=with_uninstalls, synchronize=sync
308+
),
309+
expected_ops,
294310
)
295311

296312

@@ -329,7 +345,8 @@ def test_calculate_operations_with_markers(
329345
expected_ops.insert(0, {"job": "remove", "package": Package(name, "1")})
330346

331347
check_operations(
332-
transaction.calculate_operations(with_uninstalls=sync), expected_ops
348+
transaction.calculate_operations(with_uninstalls=sync, synchronize=sync),
349+
expected_ops,
333350
)
334351

335352

0 commit comments

Comments
 (0)