From faa8cc4e0d2a45323f85f65f98b34c96bddfdf35 Mon Sep 17 00:00:00 2001 From: Vinicius Moreira Date: Sat, 5 Sep 2020 23:29:55 -0300 Subject: [PATCH] [arch] fix -> displaying wrong progress during the upgrade process when there are packages to install and upgrade --- CHANGELOG.md | 1 + bauh/gems/arch/controller.py | 33 +++++++++++++++++++++++++-------- bauh/gems/arch/output.py | 33 ++++++++++++++++++--------------- 3 files changed, 44 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 90d440ba..562f0454 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -93,6 +93,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - some environment variables are not available during the common operations (install, upgrade, downgrade, uninstall, makepkg, launch) - not displaying and uninstalling dependent packages during conflict resolutions - not retrieving all packages that would break if a given package is uninstalled + - displaying wrong progress during the upgrade process when there are packages to install and upgrade - AUR - info dialog of installed packages displays the latest PKGBUILD file instead of the one used for installation/upgrade/downgrade (the fix will only work for new installed packages) - multi-threaded download: not retrieving correctly some source files URLs (e.g: linux-xanmod-lts) diff --git a/bauh/gems/arch/controller.py b/bauh/gems/arch/controller.py index ba0a0d05..c3309f43 100644 --- a/bauh/gems/arch/controller.py +++ b/bauh/gems/arch/controller.py @@ -861,10 +861,12 @@ class ArchManager(SoftwareManager): multithread_download: bool, pkgs_data: Dict[str, dict], overwrite_files: bool = False, status_handler: TransactionStatusHandler = None, sizes: Dict[str, int] = None, download: bool = True, check_syncfirst: bool = True, skip_dependency_checks: bool = False) -> bool: + self.logger.info("Total packages to upgrade: {}".format(len(to_upgrade))) to_sync_first = None if check_syncfirst: to_sync_first = [p for p in pacman.get_packages_to_sync_first() if p.endswith('-keyring') and p in to_upgrade] + self.logger.info("Packages detected to upgrade firstly: {}".format(len(to_sync_first))) if to_sync_first: self.logger.info("Upgrading keyrings marked as 'SyncFirst'") @@ -880,9 +882,10 @@ class ArchManager(SoftwareManager): return False to_upgrade_remaining = [p for p in to_upgrade if p not in to_sync_first] if to_sync_first else to_upgrade + self.logger.info("Packages remaining to upgrade: {}".format(len(to_upgrade_remaining))) # pre-downloading all packages before removing any - if download: + if download and to_upgrade_remaining: try: downloaded = self._download_packages(pkgnames=to_upgrade_remaining, handler=handler, @@ -901,14 +904,17 @@ class ArchManager(SoftwareManager): if to_remove and not self._remove_transaction_packages(to_remove, handler, root_password): return False + if not to_upgrade_remaining: + return True + try: if status_handler: output_handler = status_handler else: - output_handler = TransactionStatusHandler(handler.watcher, self.i18n, len(to_upgrade), self.logger, downloading=len(to_upgrade_remaining)) + output_handler = TransactionStatusHandler(handler.watcher, self.i18n, {*to_upgrade_remaining}, self.logger, downloading=len(to_upgrade_remaining)) output_handler.start() - self.logger.info("Upgrading {} repository packages: {}".format(len(to_upgrade), ', '.join(to_upgrade))) + self.logger.info("Upgrading {} repository packages: {}".format(len(to_upgrade_remaining), ', '.join(to_upgrade_remaining))) success, upgrade_output = handler.handle_simple(pacman.upgrade_several(pkgnames=to_upgrade_remaining, root_password=root_password, overwrite_conflicting_files=overwrite_files, @@ -995,7 +1001,7 @@ class ArchManager(SoftwareManager): def _remove_transaction_packages(self, to_remove: Set[str], handler: ProcessHandler, root_password: str) -> bool: output_handler = TransactionStatusHandler(watcher=handler.watcher, i18n=self.i18n, - pkgs_to_sync=0, + names=set(), logger=self.logger, pkgs_to_remove=len(to_remove)) output_handler.start() @@ -1090,10 +1096,21 @@ class ArchManager(SoftwareManager): return True def _uninstall_pkgs(self, pkgs: Iterable[str], root_password: str, handler: ProcessHandler) -> bool: + status_handler = TransactionStatusHandler(watcher=handler.watcher, + i18n=self.i18n, + names={*pkgs}, + logger=self.logger, + pkgs_to_remove=len(pkgs)) + + status_handler.start() all_uninstalled, _ = handler.handle_simple(SimpleProcess(cmd=['pacman', '-R', *pkgs, '--noconfirm'], root_password=root_password, error_phrases={'error: failed to prepare transaction', - 'error: failed to commit transaction'})) + 'error: failed to commit transaction'}, + shell=True), + output_handler=status_handler.handle) + status_handler.stop_working() + status_handler.join() installed = pacman.list_installed_names() @@ -1584,7 +1601,7 @@ class ArchManager(SoftwareManager): except ArchDownloadException: return False - status_handler = TransactionStatusHandler(watcher=context.watcher, i18n=self.i18n, pkgs_to_sync=len(repo_dep_names), + status_handler = TransactionStatusHandler(watcher=context.watcher, i18n=self.i18n, names=repo_dep_names, logger=self.logger, percentage=len(repo_deps) > 1, downloading=downloaded) status_handler.start() installed, _ = context.handler.handle_simple(pacman.install_as_process(pkgpaths=repo_dep_names, @@ -2041,7 +2058,7 @@ class ArchManager(SoftwareManager): root_password=root_password) else: self.logger.info("Downloading {} repository package(s)".format(len(pkgnames))) - output_handler = TransactionStatusHandler(handler.watcher, self.i18n, len(pkgnames), self.logger) + output_handler = TransactionStatusHandler(handler.watcher, self.i18n, pkgnames, self.logger) output_handler.start() try: success, _ = handler.handle_simple(pacman.download(root_password, *pkgnames), output_handler=output_handler.handle) @@ -2123,7 +2140,7 @@ class ArchManager(SoftwareManager): return False if not context.dependency: - status_handler = TransactionStatusHandler(context.watcher, self.i18n, len(to_install), self.logger, + status_handler = TransactionStatusHandler(context.watcher, self.i18n, to_install, self.logger, percentage=len(to_install) > 1, downloading=downloaded) if not context.dependency else None status_handler.start() diff --git a/bauh/gems/arch/output.py b/bauh/gems/arch/output.py index ecaa7057..39ff0c71 100644 --- a/bauh/gems/arch/output.py +++ b/bauh/gems/arch/output.py @@ -1,6 +1,7 @@ import logging import time from threading import Thread +from typing import Set from bauh.api.abstract.handler import ProcessWatcher from bauh.commons.html import bold @@ -9,12 +10,13 @@ from bauh.view.util.translation import I18n class TransactionStatusHandler(Thread): - def __init__(self, watcher: ProcessWatcher, i18n: I18n, pkgs_to_sync: int, logger: logging.Logger, + def __init__(self, watcher: ProcessWatcher, i18n: I18n, names: Set[str], logger: logging.Logger, percentage: bool = True, downloading: int = 0, pkgs_to_remove: int = 0): super(TransactionStatusHandler, self).__init__(daemon=True) self.watcher = watcher self.i18n = i18n - self.pkgs_to_sync = pkgs_to_sync + self.pkgs_to_sync = len(names) + self.names = names self.pkgs_to_remove = pkgs_to_remove self.downloading = downloading self.upgrading = 0 @@ -43,24 +45,25 @@ class TransactionStatusHandler(Thread): def _handle(self, output: str) -> bool: if output: - if output.startswith('removing'): + output_split = output.split(' ') + + if output_split[0].lower() == 'removing' and (not self.names or output_split[1].split('.')[0] in self.names): if self.pkgs_to_remove > 0: self.removing += 1 - self.watcher.change_substatus( - '[{}/{}] {} {}'.format(self.removing, self.pkgs_to_remove, - self.i18n['uninstalling'].capitalize(), output.split(' ')[1].strip())) + self.watcher.change_substatus('[{}/{}] {} {}'.format(self.removing, self.pkgs_to_remove, + self.i18n['uninstalling'].capitalize(), output.split(' ')[1].strip())) else: - self.watcher.change_substatus('{} {}'.format(self.i18n['uninstalling'].capitalize(), output.split(' ')[1].strip())) + self.watcher.change_substatus('{} {}'.format(self.i18n['uninstalling'].capitalize(), output_split[1].strip())) - elif output.startswith('downloading'): + elif output_split[0].lower() == 'downloading' and (not self.names or (n for n in self.names if n in output_split[1])): if self.downloading < self.pkgs_to_sync: perc = self.gen_percentage() self.downloading += 1 self.watcher.change_substatus('{}[{}/{}] {} {} {}'.format(perc, self.downloading, self.pkgs_to_sync, bold('[pacman]'), - self.i18n['downloading'].capitalize(), output.split(' ')[1].strip())) - elif output.startswith('upgrading'): + self.i18n['downloading'].capitalize(), output_split[1].strip())) + elif output_split[0].lower() == 'upgrading' and (not self.names or output_split[1].split('.')[0] in self.names): if self.get_performed() < self.pkgs_to_sync: perc = self.gen_percentage() self.upgrading += 1 @@ -68,9 +71,9 @@ class TransactionStatusHandler(Thread): performed = self.upgrading + self.installing if performed <= self.pkgs_to_sync: - self.watcher.change_substatus('{}[{}/{}] {} {}'.format(perc, self.upgrading, self.pkgs_to_sync, - self.i18n['manage_window.status.upgrading'].capitalize(), output.split(' ')[1].strip())) - elif output.startswith('installing'): + self.watcher.change_substatus('{}[{}/{}] {} {}'.format(perc, performed, self.pkgs_to_sync, + self.i18n['manage_window.status.upgrading'].capitalize(), output_split[1].strip())) + elif output_split[0].lower() == 'installing' and (not self.names or output_split[1].split('.')[0] in self.names): if self.get_performed() < self.pkgs_to_sync: perc = self.gen_percentage() self.installing += 1 @@ -78,9 +81,9 @@ class TransactionStatusHandler(Thread): performed = self.upgrading + self.installing if performed <= self.pkgs_to_sync: - self.watcher.change_substatus('{}[{}/{}] {} {}'.format(perc, self.installing, self.pkgs_to_sync, + self.watcher.change_substatus('{}[{}/{}] {} {}'.format(perc, performed, self.pkgs_to_sync, self.i18n['manage_window.status.installing'].capitalize(), - output.split(' ')[1].strip())) + output_split[1].strip())) else: substatus_found = False lower_output = output.lower()