From 1908bd5366b2c3c95c22f92b7756e1dcb68e04ea Mon Sep 17 00:00:00 2001 From: Sunil Mohan Adapa Date: Thu, 18 Aug 2022 22:02:24 -0700 Subject: [PATCH] package: Implement low-level methods for uninstalling Signed-off-by: Sunil Mohan Adapa Reviewed-by: James Valleroy --- actions/packages | 19 +++++++++++-- plinth/modules/email/__init__.py | 4 +-- plinth/package.py | 47 ++++++++++++++++++++++++-------- plinth/tests/test_package.py | 20 ++------------ 4 files changed, 57 insertions(+), 33 deletions(-) diff --git a/actions/packages b/actions/packages index d5762431f..8ac12a27b 100755 --- a/actions/packages +++ b/actions/packages @@ -51,6 +51,8 @@ def parse_arguments(): help='list of packages to install') subparser = subparsers.add_parser('remove', help='remove the package(s)') + subparser.add_argument( + 'app_id', help='ID of app for which package is being uninstalled') subparser.add_argument('--packages', required=True, help='List of packages to remove', nargs='+') @@ -107,8 +109,19 @@ def subcommand_install(arguments): def subcommand_remove(arguments): - """Remove apt package(s).""" - sys.exit(run_apt_command(['remove'] + arguments.packages)) + """Remove packages using apt-get.""" + try: + _assert_managed_packages(arguments.app_id, arguments.packages) + except Exception as exception: + print('Access check failed:', exception, file=sys.stderr) + sys.exit(99) + + subprocess.run(['dpkg', '--configure', '-a'], check=False) + with apt_hold_freedombox(): + run_apt_command(['--fix-broken', 'install']) + returncode = run_apt_command(['remove'] + arguments.packages) + + sys.exit(returncode) def _assert_managed_packages(app_id, packages): @@ -118,7 +131,7 @@ def _assert_managed_packages(app_id, packages): app = app_module.App.get(app_id) managed_packages = [] for component in app.get_components_of_type(Packages): - managed_packages += component.possible_packages + managed_packages += component.possible_packages + component.conflicts for package in packages: assert package in managed_packages diff --git a/plinth/modules/email/__init__.py b/plinth/modules/email/__init__.py index 71abd5097..2f9236af0 100644 --- a/plinth/modules/email/__init__.py +++ b/plinth/modules/email/__init__.py @@ -16,7 +16,7 @@ from plinth.modules.backups.components import BackupRestore from plinth.modules.config import get_domainname from plinth.modules.firewall.components import Firewall from plinth.modules.letsencrypt.components import LetsEncrypt -from plinth.package import Packages, remove +from plinth.package import Packages, uninstall from plinth.signals import domain_added, domain_removed from plinth.utils import format_lazy @@ -173,7 +173,7 @@ class EmailApp(plinth.app.App): if packages_to_remove: logger.info('Removing conflicting packages: %s', packages_to_remove) - remove(packages_to_remove) + uninstall(packages_to_remove) # Install _clear_conflicts() diff --git a/plinth/package.py b/plinth/package.py index c03223eb4..9531b9e53 100644 --- a/plinth/package.py +++ b/plinth/package.py @@ -17,7 +17,7 @@ from django.utils.translation import gettext as _ from django.utils.translation import gettext_lazy from plinth import actions, app -from plinth.errors import ActionError, MissingPackageError +from plinth.errors import MissingPackageError from plinth.utils import format_lazy from . import operation as operation_module @@ -144,7 +144,7 @@ class Packages(app.FollowerComponent): self._packages.append(package) self.skip_recommends = skip_recommends - self.conflicts = conflicts + self.conflicts = conflicts or [] self.conflicts_action = conflicts_action @property @@ -324,6 +324,15 @@ class Transaction: logger.exception('Error installing package: %s', exception) raise + def uninstall(self): + """Run an apt-get transaction to uninstall given packages.""" + try: + self._run_apt_command(['remove', self.app_id, '--packages'] + + self.package_names) + except subprocess.CalledProcessError as exception: + logger.exception('Error uninstalling package: %s', exception) + raise + def refresh_package_lists(self): """Refresh apt package lists.""" try: @@ -351,7 +360,7 @@ class Transaction: return_code = process.wait() if return_code != 0: - raise PackageException(_('Error during installation'), self.stderr) + raise PackageException(_('Error running apt-get'), self.stderr) def _read_stdout(self, process): """Read the stdout of the process and update progress.""" @@ -421,6 +430,30 @@ def install(package_names, skip_recommends=False, force_configuration=None, force_missing_configuration) +def uninstall(package_names): + """Uninstall a set of packages.""" + try: + operation = operation_module.Operation.get_operation() + except AttributeError: + raise RuntimeError( + 'uninstall() must be called from within an operation.') + + start_time = time.time() + while is_package_manager_busy(): + if time.time() - start_time >= 24 * 3600: # One day + raise PackageException(_('Timeout waiting for package manager')) + + time.sleep(3) # seconds + + logger.info('Running uninstall for app - %s, packages - %s', + operation.app_id, package_names) + + from . import package + transaction = package.Transaction(operation.app_id, package_names) + operation.thread_data['transaction'] = transaction + transaction.uninstall() + + def is_package_manager_busy(): """Return whether a package manager is running.""" try: @@ -467,11 +500,3 @@ def packages_installed(candidates: Union[list, tuple]) -> list: pass return installed_packages - - -def remove(packages: Union[list, tuple]) -> None: - """Remove packages.""" - try: - actions.superuser_run('packages', ['remove', '--packages'] + packages) - except ActionError: - pass diff --git a/plinth/tests/test_package.py b/plinth/tests/test_package.py index 2f171a57b..7e6cf12d5 100644 --- a/plinth/tests/test_package.py +++ b/plinth/tests/test_package.py @@ -9,8 +9,8 @@ from unittest.mock import Mock, call, patch import pytest from plinth.app import App -from plinth.errors import ActionError, MissingPackageError -from plinth.package import Package, Packages, packages_installed, remove +from plinth.errors import MissingPackageError +from plinth.package import Package, Packages, packages_installed class TestPackageExpressions(unittest.TestCase): @@ -54,7 +54,7 @@ def test_packages_init(): assert component.possible_packages == ['foo', 'bar'] assert component.component_id == 'test-component' assert not component.skip_recommends - assert component.conflicts is None + assert component.conflicts == [] assert component.conflicts_action is None with pytest.raises(ValueError): @@ -189,17 +189,3 @@ def test_packages_installed(): assert len(packages_installed(())) == 0 assert len(packages_installed(('unknown-package', ))) == 0 assert len(packages_installed(('python3', ))) == 1 - - -@patch('plinth.actions.superuser_run') -def test_remove(run): - """Test removing packages.""" - remove(['package1', 'package2']) - run.assert_has_calls( - [call('packages', ['remove', '--packages', 'package1', 'package2'])]) - - run.reset_mock() - run.side_effect = ActionError() - remove(['package1']) - run.assert_has_calls( - [call('packages', ['remove', '--packages', 'package1'])])