diff --git a/plinth/package.py b/plinth/package.py index 17274eacb..6bb0a71ae 100644 --- a/plinth/package.py +++ b/plinth/package.py @@ -6,6 +6,7 @@ import logging import pathlib import time +import apt import apt.cache from django.utils.translation import gettext as _ from django.utils.translation import gettext_lazy, gettext_noop @@ -183,24 +184,15 @@ class Packages(app_module.FollowerComponent): def uninstall(self): """Uninstall and purge the packages.""" + # Ensure package list is update-to-date before looking at dependencies. + refresh_package_lists() + + # List of packages to purge from the system packages = self.get_actual_packages() - packages_set = set(packages) - for app in app_module.App.list(): - # uninstall() will be called on Packages of this app separately - # for uninstalling this app. - if app == self.app: - continue + logger.info('App\'s list of packages to remove: %s', packages) - if app.get_setup_state() == app_module.App.SetupState.NEEDS_SETUP: - continue - - # Remove packages used by other installed apps - for component in app.get_components_of_type(Packages): - packages_set -= set(component.get_actual_packages()) - - # Preserve order of packages for ease of testing - uninstall([package for package in packages if package in packages_set], - purge=True) + packages = self._filter_packages_to_keep(packages) + uninstall(packages, purge=True) def diagnose(self) -> list[DiagnosticCheck]: """Run diagnostics and return results.""" @@ -271,6 +263,60 @@ class Packages(app_module.FollowerComponent): return False + def _filter_packages_to_keep(self, packages: list[str]) -> list[str]: + """Filter out the list of packages to keep from given list. + + Packages to keep are packages needed by other installed apps and their + dependencies (PreDepends, Depends, Recommends). + """ + packages_set: set[str] = set(packages) + + # Get list of packages needed by other installed apps (packages to + # keep). + keep_packages: set[str] = set() + for app in app_module.App.list(): + # uninstall() will be called on Packages of this app separately + # for uninstalling this app. + if app == self.app: + continue + + if app.get_setup_state() == app_module.App.SetupState.NEEDS_SETUP: + continue + + # Remove packages used by other installed apps + for component in app.get_components_of_type(Packages): + keep_packages |= set(component.get_actual_packages()) + + # Get list of all the dependencies of packages to keep. + keep_packages_with_deps: set[str] = set() + cache = apt.Cache() + while keep_packages: + package_name = keep_packages.pop() + if package_name in keep_packages_with_deps: + continue # Already processed + + keep_packages_with_deps.add(package_name) + if package_name not in cache: + continue # Package is not available in sources + + if not cache[package_name].is_installed: + continue # Package is not installed + + version = cache[package_name].installed + if not version: + continue + + dependencies = version.dependencies + version.recommends + for dependency in dependencies: + for or_dependency in dependency.or_dependencies: + keep_packages.add(or_dependency.name) + + # Filter out any packages that are to be kept or their dependencies. + packages_set -= keep_packages_with_deps + + # Preserve order of packages for ease of testing. + return [package for package in packages if package in packages_set] + class PackageException(Exception): """A package operation has failed.""" diff --git a/plinth/tests/test_package.py b/plinth/tests/test_package.py index 993484d08..d5308d88b 100644 --- a/plinth/tests/test_package.py +++ b/plinth/tests/test_package.py @@ -151,8 +151,9 @@ def test_packages_setup_with_conflicts(install, uninstall, packages_installed): install.assert_has_calls([call(['bash'], skip_recommends=False)]) +@patch('plinth.package.refresh_package_lists') @patch('plinth.package.uninstall') -def test_packages_uninstall(uninstall): +def test_packages_uninstall(uninstall, _refresh_package_lists): """Test uninstalling packages component.""" class TestApp(App): @@ -166,15 +167,53 @@ def test_packages_uninstall(uninstall): uninstall.assert_has_calls([call(['python3', 'bash'], purge=True)]) +@patch('plinth.package.refresh_package_lists') @patch('plinth.package.uninstall') @patch('apt.Cache') -def test_packages_uninstall_exclusion(cache, uninstall): +def test_packages_uninstall_exclusion(cache, uninstall, + _refresh_package_lists): """Test excluding packages from other installed apps when uninstalling.""" + + def _get_mock_package(installed_version='1.0', dependencies=None, + recommends=None): + mock_dependencies = [] + for or_dependencies in (dependencies or []): + mock_or_dependency = Mock(or_dependencies=[]) + mock_dependencies.append(mock_or_dependency) + for dependency in or_dependencies: + mock = Mock() + mock.name = dependency + mock_or_dependency.or_dependencies.append(mock) + + mock_recommends = [] + for or_dependencies in (recommends or []): + mock_or_dependency = Mock(or_dependencies=[]) + mock_recommends.append(mock_or_dependency) + for dependency in or_dependencies: + mock = Mock() + mock.name = dependency + mock_or_dependency.or_dependencies.append(mock) + + mock = Mock( + version=installed_version or '1.0', + is_installed=bool(installed_version), + installed=Mock(dependencies=mock_dependencies, + recommends=mock_recommends)) + return mock + + package2 = _get_mock_package('4.0', [['dep1', 'dep2'], ['dep3'], ['dep4']], + [['dep5']]) cache.return_value = { - 'package11': Mock(candidate=Mock(version='2.0', is_installed=True)), - 'package12': Mock(candidate=Mock(version='3.0', is_installed=False)), - 'package2': Mock(candidate=Mock(version='4.0', is_installed=True)), - 'package3': Mock(candidate=Mock(version='5.0', is_installed=True)), + 'package11': _get_mock_package('2.0'), + 'package12': _get_mock_package(None), + 'package2': package2, + 'package3': _get_mock_package('5.0', ['unknown-dep1']), + 'dep1': _get_mock_package('6.0'), + 'dep2': _get_mock_package('6.1'), + 'dep3': _get_mock_package('6.2'), + 'dep4': _get_mock_package(None), + 'dep5': _get_mock_package('6.4'), + 'dep6': _get_mock_package('6.5'), } class TestApp1(App): @@ -183,8 +222,10 @@ def test_packages_uninstall_exclusion(cache, uninstall): def __init__(self): super().__init__() - component = Packages('test-component11', - ['package11', 'package2', 'package3']) + component = Packages('test-component11', [ + 'package11', 'package2', 'package3', 'dep1', 'dep2', 'dep3', + 'dep4', 'dep6' + ]) self.add(component) component = Packages('test-component12', @@ -220,7 +261,7 @@ def test_packages_uninstall_exclusion(cache, uninstall): TestApp3() app1.uninstall() uninstall.assert_has_calls([ - call(['package11', 'package3'], purge=True), + call(['package11', 'package3', 'dep6'], purge=True), call(['package12', 'package3'], purge=True) ])