Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

installer: remove unrequested extras if requires_synchronization is set #8621

Merged
merged 1 commit into from
Nov 21, 2023
Merged

installer: remove unrequested extras if requires_synchronization is set #8621

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 9 additions & 6 deletions src/poetry/installation/installer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ def _do_install(self) -> int:
ops = self._get_operations_from_lock(locked_repository)

lockfile_repo = LockfileRepository()
self._populate_lockfile_repo(lockfile_repo, ops)
uninstalls = self._populate_lockfile_repo(lockfile_repo, ops)

if not self.executor.enabled:
# If we are only in lock mode, no need to go any further
Expand Down Expand Up @@ -324,6 +324,8 @@ def _do_install(self) -> int:
for op in transaction.calculate_operations(with_uninstalls=True)
if op.job_type == "uninstall"
] + ops
else:
ops = uninstalls + ops

# We need to filter operations so that packages
# not compatible with the current system,
Expand Down Expand Up @@ -359,18 +361,19 @@ def _execute(self, operations: list[Operation]) -> int:

def _populate_lockfile_repo(
self, repo: LockfileRepository, ops: Iterable[Operation]
) -> None:
) -> list[Uninstall]:
uninstalls = []
for op in ops:
if isinstance(op, Uninstall):
uninstalls.append(op)
continue
elif isinstance(op, Update):
package = op.target_package
else:
package = op.package

package = op.target_package if isinstance(op, Update) else op.package
if not repo.has_package(package):
repo.add_package(package)

return uninstalls

def _get_operations_from_lock(
self, locked_repository: Repository
) -> list[Operation]:
Expand Down
143 changes: 40 additions & 103 deletions tests/installation/test_installer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -993,73 +993,20 @@ def test_run_with_dependencies_nested_extras(
assert locker.written_data == expected


def test_run_does_not_install_extras_if_not_requested(
installer: Installer, locker: Locker, repo: Repository, package: ProjectPackage
) -> None:
package.extras[canonicalize_name("foo")] = [get_dependency("D")]
package_a = get_package("A", "1.0")
package_b = get_package("B", "1.0")
package_c = get_package("C", "1.0")
package_d = get_package("D", "1.1")

repo.add_package(package_a)
repo.add_package(package_b)
repo.add_package(package_c)
repo.add_package(package_d)

package.add_dependency(Factory.create_dependency("A", "^1.0"))
package.add_dependency(Factory.create_dependency("B", "^1.0"))
package.add_dependency(Factory.create_dependency("C", "^1.0"))
package.add_dependency(
Factory.create_dependency("D", {"version": "^1.0", "optional": True})
)

result = installer.run()
assert result == 0

expected = fixture("extras")
# Extras are pinned in lock
assert locker.written_data == expected

# But should not be installed
assert installer.executor.installations_count == 3 # A, B, C


def test_run_installs_extras_if_requested(
installer: Installer, locker: Locker, repo: Repository, package: ProjectPackage
) -> None:
package.extras[canonicalize_name("foo")] = [get_dependency("D")]
package_a = get_package("A", "1.0")
package_b = get_package("B", "1.0")
package_c = get_package("C", "1.0")
package_d = get_package("D", "1.1")

repo.add_package(package_a)
repo.add_package(package_b)
repo.add_package(package_c)
repo.add_package(package_d)

package.add_dependency(Factory.create_dependency("A", "^1.0"))
package.add_dependency(Factory.create_dependency("B", "^1.0"))
package.add_dependency(Factory.create_dependency("C", "^1.0"))
package.add_dependency(
Factory.create_dependency("D", {"version": "^1.0", "optional": True})
)

installer.extras(["foo"])
result = installer.run()
assert result == 0

# Extras are pinned in lock
expected = fixture("extras")
assert locker.written_data == expected

# But should not be installed
assert installer.executor.installations_count == 4 # A, B, C, D


@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_sync", [False, True])
def test_run_installs_extras_with_deps_if_requested(
installer: Installer, locker: Locker, repo: Repository, package: ProjectPackage
installer: Installer,
locker: Locker,
repo: Repository,
installed: CustomInstalledRepository,
package: ProjectPackage,
is_locked: bool,
is_installed: bool,
with_extras: bool,
do_sync: bool,
) -> None:
package.extras[canonicalize_name("foo")] = [get_dependency("C")]
package_a = get_package("A", "1.0")
Expand All @@ -1080,48 +1027,38 @@ def test_run_installs_extras_with_deps_if_requested(

package_c.add_dependency(Factory.create_dependency("D", "^1.0"))

installer.extras(["foo"])
result = installer.run()
assert result == 0

expected = fixture("extras-with-dependencies")

# Extras are pinned in lock
assert locker.written_data == expected

# But should not be installed
assert installer.executor.installations_count == 4 # A, B, C, D


def test_run_installs_extras_with_deps_if_requested_locked(
installer: Installer, locker: Locker, repo: Repository, package: ProjectPackage
) -> None:
locker.locked(True)
locker.mock_lock_data(fixture("extras-with-dependencies"))
package.extras[canonicalize_name("foo")] = [get_dependency("C")]
package_a = get_package("A", "1.0")
package_b = get_package("B", "1.0")
package_c = get_package("C", "1.0")
package_d = get_package("D", "1.1")

repo.add_package(package_a)
repo.add_package(package_b)
repo.add_package(package_c)
repo.add_package(package_d)

package.add_dependency(Factory.create_dependency("A", "^1.0"))
package.add_dependency(Factory.create_dependency("B", "^1.0"))
package.add_dependency(
Factory.create_dependency("C", {"version": "^1.0", "optional": True})
)
if is_locked:
locker.locked(True)
locker.mock_lock_data(fixture("extras-with-dependencies"))

package_c.add_dependency(Factory.create_dependency("D", "^1.0"))
if is_installed:
installed.add_package(package_a)
installed.add_package(package_b)
installed.add_package(package_c)
installed.add_package(package_d)

installer.extras(["foo"])
if with_extras:
installer.extras(["foo"])
installer.requires_synchronization(do_sync)
result = installer.run()
assert result == 0

assert installer.executor.installations_count == 4 # A, B, C, D
if not is_locked:
assert locker.written_data == fixture("extras-with-dependencies")

if with_extras:
# A, B, C, D
expected_installations_count = 0 if is_installed else 4
expected_removals_count = 0
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

assert installer.executor.installations_count == expected_installations_count
assert installer.executor.removals_count == expected_removals_count


@pytest.mark.network
Expand Down