From 85bfe40c8728303b342c10a84cb4233192125960 Mon Sep 17 00:00:00 2001 From: Sunil Mohan Adapa Date: Tue, 6 Jun 2023 07:46:27 -0700 Subject: [PATCH] packages: Purge packages on uninstall The primary reason for the existence of uninstall feature is to clean up the existing working or non-working setup and re-install freshly. It is not to remove the effects of the installed app since disable works well for that. It is seldom to free up space since even on a microSD card, the space occupied by most apps is negligible. Tests: - Unit tests pass - Functional tests for all apps pass (except for known failures). Signed-off-by: Sunil Mohan Adapa Reviewed-by: James Valleroy --- plinth/package.py | 14 ++++++++------ plinth/privileged/packages.py | 5 +++-- plinth/tests/test_package.py | 11 ++++++----- 3 files changed, 17 insertions(+), 13 deletions(-) diff --git a/plinth/package.py b/plinth/package.py index 3c4ea5b33..dee7b9baf 100644 --- a/plinth/package.py +++ b/plinth/package.py @@ -175,7 +175,7 @@ class Packages(app_module.FollowerComponent): self.conflicts_action not in (None, self.ConflictsAction.IGNORE): logger.info('Removing conflicting packages: %s', packages_to_remove) - uninstall(packages_to_remove) + uninstall(packages_to_remove, purge=False) install(self.get_actual_packages(), skip_recommends=self.skip_recommends) @@ -198,7 +198,8 @@ class Packages(app_module.FollowerComponent): 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]) + uninstall([package for package in packages if package in packages_set], + purge=True) def diagnose(self): """Run diagnostics and return results.""" @@ -341,10 +342,11 @@ class Transaction: logger.exception('Error installing package: %s', exception) raise - def uninstall(self): + def uninstall(self, purge): """Run an apt-get transaction to uninstall given packages.""" try: - privileged.remove(app_id=self.app_id, packages=self.package_names) + privileged.remove(app_id=self.app_id, packages=self.package_names, + purge=purge) except Exception as exception: logger.exception('Error uninstalling package: %s', exception) raise @@ -416,7 +418,7 @@ def install(package_names, skip_recommends=False, force_configuration=None, force_missing_configuration) -def uninstall(package_names): +def uninstall(package_names, purge): """Uninstall a set of packages.""" try: operation = operation_module.Operation.get_operation() @@ -437,7 +439,7 @@ def uninstall(package_names): from . import package transaction = package.Transaction(operation.app_id, package_names) operation.thread_data['transaction'] = transaction - transaction.uninstall() + transaction.uninstall(purge) def is_package_manager_busy(): diff --git a/plinth/privileged/packages.py b/plinth/privileged/packages.py index 76f63a764..a315f9144 100644 --- a/plinth/privileged/packages.py +++ b/plinth/privileged/packages.py @@ -71,7 +71,7 @@ def install(app_id: str, packages: list[str], skip_recommends: bool = False, @privileged -def remove(app_id: str, packages: list[str]): +def remove(app_id: str, packages: list[str], purge: bool): """Remove packages using apt-get.""" try: _assert_managed_packages(app_id, packages) @@ -81,7 +81,8 @@ def remove(app_id: str, packages: list[str]): subprocess.run(['dpkg', '--configure', '-a'], check=False) with action_utils.apt_hold_freedombox(): run_apt_command(['--fix-broken', 'install']) - returncode = run_apt_command(['remove'] + packages) + options = [] if not purge else ['--purge'] + returncode = run_apt_command(['remove'] + options + packages) if returncode: raise RuntimeError( diff --git a/plinth/tests/test_package.py b/plinth/tests/test_package.py index 31c0a3e04..8f5232e25 100644 --- a/plinth/tests/test_package.py +++ b/plinth/tests/test_package.py @@ -130,7 +130,7 @@ def test_packages_setup_with_conflicts(install, uninstall, packages_installed): component = Packages('test-component', ['bash'], conflicts=['exim4-base'], conflicts_action=Packages.ConflictsAction.REMOVE) component.setup(old_version=0) - uninstall.assert_has_calls([call(['exim4-base'])]) + uninstall.assert_has_calls([call(['exim4-base'], purge=False)]) install.assert_has_calls([call(['bash'], skip_recommends=False)]) uninstall.reset_mock() @@ -162,7 +162,7 @@ def test_packages_uninstall(uninstall): app = TestApp() app.add(component) app.uninstall() - uninstall.assert_has_calls([call(['python3', 'bash'])]) + uninstall.assert_has_calls([call(['python3', 'bash'], purge=True)]) @patch('plinth.package.uninstall') @@ -218,9 +218,10 @@ def test_packages_uninstall_exclusion(cache, uninstall): TestApp2() TestApp3() app1.uninstall() - uninstall.assert_has_calls( - [call(['package11', 'package3']), - call(['package12', 'package3'])]) + uninstall.assert_has_calls([ + call(['package11', 'package3'], purge=True), + call(['package12', 'package3'], purge=True) + ]) @patch('apt.Cache')