diff --git a/plinth/actions.py b/plinth/actions.py index 01de248f9..535d11fae 100644 --- a/plinth/actions.py +++ b/plinth/actions.py @@ -109,12 +109,12 @@ def run(action, options=None, input=None, async=False): return _run(action, options, input, async, False) -def superuser_run(action, options=None, input=None, async=False): +def superuser_run(action, options=None, input=None, async=False, log_error=True): """Safely run a specific action as root. See actions._run for more information. """ - return _run(action, options, input, async, True) + return _run(action, options, input, async, True, log_error=log_error) def run_as_user(action, options=None, input=None, async=False, @@ -127,7 +127,7 @@ def run_as_user(action, options=None, input=None, async=False, def _run(action, options=None, input=None, async=False, run_as_root=False, - become_user=None): + become_user=None, log_error=True): """Safely run a specific action as a normal user or root. Actions are pulled from the actions directory. @@ -186,8 +186,9 @@ def _run(action, options=None, input=None, async=False, run_as_root=False, output, error = proc.communicate(input=input) output, error = output.decode(), error.decode() if proc.returncode != 0: - LOGGER.error('Error executing command - %s, %s, %s', cmd, output, - error) + if log_error: + LOGGER.error('Error executing command - %s, %s, %s', cmd, output, + error) raise ActionError(action, output, error) return output diff --git a/plinth/package.py b/plinth/package.py index c195117b2..08ae4161e 100644 --- a/plinth/package.py +++ b/plinth/package.py @@ -92,9 +92,9 @@ class Transaction(object): process.stdin.close() stdout_thread = threading.Thread(target=self._read_stdout, - args=(process,)) + args=(process, )) stderr_thread = threading.Thread(target=self._read_stderr, - args=(process,)) + args=(process, )) stdout_thread.start() stderr_thread.start() stdout_thread.join() @@ -123,11 +123,14 @@ class Transaction(object): return status_map = { - 'pmstatus': _('installing'), - 'dlstatus': _('downloading'), - 'media-change': _('media change'), - 'pmconffile': _('configuration file: {file}').format( - file=parts[1]), + 'pmstatus': + _('installing'), + 'dlstatus': + _('downloading'), + 'media-change': + _('media change'), + 'pmconffile': + _('configuration file: {file}').format(file=parts[1]), } self.status_string = status_map.get(parts[0], '') self.percentage = int(float(parts[2])) @@ -136,7 +139,8 @@ class Transaction(object): def is_package_manager_busy(): """Return whether a package manager is running.""" try: - actions.superuser_run('packages', ['is-package-manager-busy']) + actions.superuser_run('packages', ['is-package-manager-busy'], + log_error=False) return True except actions.ActionError: return False diff --git a/plinth/tests/test_actions.py b/plinth/tests/test_actions.py index 5f7e28d21..e4f69ad21 100644 --- a/plinth/tests/test_actions.py +++ b/plinth/tests/test_actions.py @@ -19,10 +19,13 @@ Test module for actions utilities that modify configuration. """ +import apt_pkg +import os import shutil import tempfile import unittest +from plinth.errors import ActionError from plinth.actions import superuser_run, run from plinth import cfg @@ -40,6 +43,7 @@ class TestPrivileged(unittest.TestCase): cls.action_directory = tempfile.TemporaryDirectory() cfg.actions_dir = cls.action_directory.name + shutil.copy(os.path.join(os.path.dirname(__file__), '..', '..', 'actions', 'packages'), cfg.actions_dir) shutil.copy('/bin/echo', cfg.actions_dir) shutil.copy('/usr/bin/id', cfg.actions_dir) @@ -143,3 +147,15 @@ class TestPrivileged(unittest.TestCase): output = run('echo', tuple(options.split())) output = output.rstrip('\n') self.assertEqual(options, output) + + def test_error_handling_for_superuser(self): + """Test that errors are raised only when expected.""" + with apt_pkg.SystemLock(): + with self.assertRaises(ActionError): + superuser_run('packages', ['is-package-manager-busy'], + log_error=True) + with self.assertRaises(ActionError): + superuser_run('packages', ['is-package-manager-busy'], + log_error=False) + with self.assertRaises(ActionError): + superuser_run('packages', ['is-package-manager-busy'])