From 3a22e1717e6f0d3b5fa673b41ba4531a144b94a5 Mon Sep 17 00:00:00 2001 From: Sunil Mohan Adapa Date: Wed, 1 Apr 2015 22:40:27 +0530 Subject: [PATCH] packages: refresh package list before install and handle errors - Refresh package list before trying to install packages. Also lookup the ID of the package, including the version, using the newly fetched package list. - Call on_install() callback requested by modules only if the package installation was successful. Handle any exception raised in the callback itself. - Handle exceptions raised during packagekit operations. Also check the returned results for error code. - Capture error/success status during a transaction. Don't destroy the transaction object until the error/success status has been collected. When a view is refreshed after completion of a transaction collect the result of the transaction and show it to the user. - Handle cases where package searches in the package list fails. Simply show the package name without its description. --- plinth/package.py | 136 +++++++++++++++++++++++--- plinth/templates/package_install.html | 4 +- plinth/views.py | 5 +- 3 files changed, 127 insertions(+), 18 deletions(-) diff --git a/plinth/package.py b/plinth/package.py index bce510d0c..76feb3ff6 100644 --- a/plinth/package.py +++ b/plinth/package.py @@ -19,7 +19,10 @@ Framework for installing and updating distribution packages """ +from django.contrib import messages import functools +from gettext import gettext as _ +from gi.repository import GLib as glib from gi.repository import PackageKitGlib as packagekit import logging import threading @@ -32,6 +35,17 @@ transactions = {} packages_resolved = {} +class PackageException(Exception): + """A package operation has failed.""" + + def __init__(self, error_string=None, error_details=None, *args, **kwargs): + """Store packagekit error string and details.""" + super(PackageException, self).__init__(*args, **kwargs) + + self.error_string = error_string + self.error_details = error_details + + class Transaction(object): """Information about an ongoing transaction.""" @@ -57,6 +71,10 @@ class Transaction(object): self.caller_active = None self.download_size_remaining = None + # Completion + self.is_finished = False + self.exception = None + def get_id(self): """Return a identifier to use as a key in a map of transactions.""" return frozenset(self.package_names) @@ -78,13 +96,70 @@ class Transaction(object): def _install(self): """Run a PackageKit transaction to install given packages.""" - package_ids = [packages_resolved[package_name].get_id() - for package_name in self.package_names] + try: + self._do_install() + except PackageException as exception: + self.finish(exception) + return + except glib.Error as exception: + self.finish(PackageException(exception.message)) + return + + try: + if self.on_install: + self.on_install() + except Exception as exception: + logger.exception('Error during setup - %s', exception) + self.finish(exception) + return + + self.finish() + + def _do_install(self): + """Run a PackageKit transaction to install given packages. + + Raise exception in case of error. + """ client = packagekit.Client() client.set_interactive(False) - client.install_packages(packagekit.TransactionFlagEnum.ONLY_TRUSTED, - package_ids + [None], None, - self.progress_callback, self) + + # Refresh package cache from all enabled repositories + results = client.refresh_cache( + False, None, self.progress_callback, self) + self._assert_success(results) + + # Resolve packages again to get the latest versions after refresh + results = client.resolve(packagekit.FilterEnum.INSTALLED, + tuple(self.package_names) + (None, ), + None, self.progress_callback, self) + self._assert_success(results) + + for package in results.get_package_array(): + packages_resolved[package.get_name()] = package + + package_ids = [] + for package_name in self.package_names: + if package_name not in packages_resolved or \ + not packages_resolved[package_name]: + raise PackageException(_('packages not found')) + + package_ids.append(packages_resolved[package_name].get_id()) + + # Start package installation + results = client.install_packages( + packagekit.TransactionFlagEnum.ONLY_TRUSTED, package_ids + [None], + None, self.progress_callback, self) + self._assert_success(results) + + def _assert_success(self, results): + """Check that the most recent operation was a success.""" + # XXX: Untested code + if results and results.get_error_code() is not None: + error = results.get_error_code() + error_code = error.get_code() if error else None + error_string = error_code.to_string() if error_code else None + error_details = error.get_details() if error else None + raise PackageException(error_string, error_details) def progress_callback(self, progress, progress_type, user_data): """Process progress updates on package resolve operation""" @@ -102,8 +177,6 @@ class Transaction(object): self.status = progress.props.status self.status_string = \ packagekit.StatusEnum.to_string(progress.props.status) - if self.status == packagekit.StatusEnum.FINISHED: - self.finish() elif progress_type == packagekit.ProgressType.TRANSACTION_FLAGS: self.flags = progress.props.transaction_flags elif progress_type == packagekit.ProgressType.ROLE: @@ -117,15 +190,20 @@ class Transaction(object): logger.info('Unhandle packagekit progress callback - %s, %s', progress, progress_type) - def finish(self): - """Perform clean up operations on the transaction. + def finish(self, exception=None): + """Mark transaction as complected and store exception if any.""" + self.is_finished = True + self.exception = exception - Remove self from global transactions list. + def collect_result(self): + """Retrieve the result of this transaction. + + Also remove self from global transactions list. """ - if self.status == packagekit.StatusEnum.FINISHED: - self.on_install() + assert self.is_finished del transactions[self.get_id()] + return self.exception def required(package_names, on_install=None): @@ -137,8 +215,7 @@ def required(package_names, on_install=None): @functools.wraps(func) def wrapper(request, *args, **kwargs): """Check and install packages required by a view.""" - if not is_installing(package_names) and \ - check_installed(package_names): + if not _should_show_install_view(request, package_names): return func(request, *args, **kwargs) view = plinth.views.PackageInstallView.as_view() @@ -150,6 +227,32 @@ def required(package_names, on_install=None): return wrapper2 +def _should_show_install_view(request, package_names): + """Return whether the installation view should be shown.""" + transaction_id = frozenset(package_names) + + # No transaction in progress + if transaction_id not in transactions: + is_installed = check_installed(package_names) + return not is_installed + + # Installing + transaction = transactions[transaction_id] + if not transaction.is_finished: + return True + + # Transaction finished, waiting to show the result + exception = transaction.collect_result() + if not exception: + messages.success(request, + _('Installed and configured packages successfully')) + return False + else: + messages.error(request, _('Error installing packages: {details}') + .format(details=exception.error_string)) + return True + + def check_installed(package_names): """Return a boolean installed status of package. @@ -171,6 +274,11 @@ def check_installed(package_names): packages_resolved[package.get_name()] = package + # When package names could not be resolved + for package_name in package_names: + if package_name not in packages_resolved: + packages_resolved[package_name] = None + return set(installed_package_names) == set(package_names) diff --git a/plinth/templates/package_install.html b/plinth/templates/package_install.html index f853a3e2c..05663d23b 100644 --- a/plinth/templates/package_install.html +++ b/plinth/templates/package_install.html @@ -44,9 +44,9 @@ PackageSummary - {% for package in packages %} + {% for package_name, package in packages.items %} - {{ package.get_name }} + {{ package_name }} {{ package.get_summary }} {% endfor %} diff --git a/plinth/views.py b/plinth/views.py index 9dc1af75d..df6c5dd77 100644 --- a/plinth/views.py +++ b/plinth/views.py @@ -44,8 +44,9 @@ class PackageInstallView(TemplateView): if 'packages_names' not in context: context['package_names'] = self.kwargs.get('package_names', []) - context['packages'] = [package_module.packages_resolved[package_name] - for package_name in context['package_names']] + context['packages'] = { + package_name: package_module.packages_resolved[package_name] + for package_name in context['package_names']} context['is_installing'] = \ package_module.is_installing(context['package_names']) context['transactions'] = package_module.transactions