From a961c83c8494620a583ae09fe285cb010dec0ae8 Mon Sep 17 00:00:00 2001 From: Zapta Date: Sun, 17 Nov 2024 09:45:17 -0800 Subject: [PATCH 1/3] Added handling of broken package installations. The 'apio packages --list' command now prints also package errors and the command 'apio packages --fix' fixes them automatically. The four category of package error are listed in the type definition of the class PackageScanResults. --- apio/commands/packages.py | 177 ++++++++++++++++++++++++--------- apio/managers/installer.py | 45 ++++++++- apio/pkg_util.py | 168 +++++++++++++++++++++++++++++++ apio/util.py | 20 +++- test/commands/test_packages.py | 7 +- test/test_end_to_end.py | 5 +- 6 files changed, 366 insertions(+), 56 deletions(-) diff --git a/apio/commands/packages.py b/apio/commands/packages.py index 9193df48..f33f9675 100644 --- a/apio/commands/packages.py +++ b/apio/commands/packages.py @@ -8,16 +8,116 @@ """Implementation of 'apio packages' command""" from pathlib import Path -from typing import Tuple +from typing import Tuple, List from varname import nameof import click from click.core import Context from apio.managers import installer from apio.resources import Resources -from apio import cmd_util +from apio import cmd_util, pkg_util, util from apio.commands import options +def _install( + resources: Resources, packages: List[str], force: bool, verbose: bool +) -> int: + """Handles the --install operation. Returns exit code.""" + click.secho(f"Platform id '{resources.platform_id}'") + + # -- If packages where specified, install all packages that are valid + # -- for this platform. + if not packages: + packages = resources.platform_packages.keys() + + # -- Install the packages, one by one. + for package in packages: + installer.install_package( + resources, package_spec=package, force=force, verbose=verbose + ) + + return 0 + + +def _uninstall( + resources: Resources, packages: List[str], verbose: bool, sayyes: bool +) -> int: + """Handles the --uninstall operation. Returns exit code.""" + + # -- If packages where specified, uninstall all packages that are valid + # -- for this platform. + if not packages: + packages = resources.platform_packages.keys() + + # -- Ask the user for confirmation + if not ( + sayyes + or click.confirm( + "Do you want to uninstall " f"{util.count(packages, 'package')}?" + ) + ): + # -- User doesn't want to continue. + click.secho("User said no", fg="red") + return 1 + + # -- Here when going on with the uninstallation. + click.secho(f"Platform id '{resources.platform_id}'") + + # -- Uninstall the packages, one by one + for package in packages: + installer.uninstall_package( + resources, package_spec=package, verbose=verbose + ) + + return 0 + + +def _fix(resources: Resources, verbose: bool) -> int: + """Handles the --fix operation. Returns exit code.""" + + # -- Scan the availeable and installed packages. + scan = pkg_util.scan_packages(resources) + + # -- Fix any errors. + if scan.num_errors(): + installer.fix_packages(resources, scan, verbose) + else: + click.secho("No errors to fix") + + # -- Show the new state + new_scan = pkg_util.scan_packages(resources) + pkg_util.list_packages(resources, new_scan) + + return 0 + + +def _list(resources: Resources, verbose: bool) -> int: + """Handles the --list operation. Returns exit code.""" + + if verbose: + click.secho(f"Platform id '{resources.platform_id}'") + + # -- Scan the available and installed packages. + scan = pkg_util.scan_packages(resources) + + # -- List the findings. + pkg_util.list_packages(resources, scan) + + # -- Print an hint or summary based on the findings. + if scan.num_errors(): + click.secho( + "[Hint] run 'apio packages -c' to fix the errors.", fg="yellow" + ) + elif scan.uninstalled_package_ids: + click.secho( + "[Hint] run 'apio packages -i' to install all available packages.", + fg="yellow", + ) + else: + click.secho("All available messages are installed.", fg="green") + + return 0 + + # --------------------------- # -- COMMAND # --------------------------- @@ -38,6 +138,7 @@ apio packages --install examples@0.0.32 # Install a specific version. apio packages --uninstall # Uninstall all packages. apio packages --uninstall oss-cad-suite # Uninstall only given package(s). + apio packages --fix # Fix package errors. Adding --force to --install forces the reinstallation of existing packages, otherwise, packages that are already installed correctly are left with no @@ -48,7 +149,7 @@ """ install_option = click.option( - "install", # Var name. Deconflicting from Python'g builtin 'all'. + "install", # Var name. "-i", "--install", is_flag=True, @@ -57,7 +158,7 @@ ) uninstall_option = click.option( - "uninstall", # Var name. Deconflicting from Python'g builtin 'all'. + "uninstall", # Var name. "-u", "--uninstall", is_flag=True, @@ -65,6 +166,14 @@ cls=cmd_util.ApioOption, ) +fix_option = click.option( + "fix", # Var name. + "--fix", + is_flag=True, + help="Fix package errors.", + cls=cmd_util.ApioOption, +) + # pylint: disable=duplicate-code # pylint: disable=too-many-arguments @@ -80,6 +189,7 @@ @options.list_option_gen(help="List packages.") @install_option @uninstall_option +@fix_option @options.force_option_gen(help="Force installation.") @options.project_dir_option @options.platform_option @@ -91,8 +201,9 @@ def cli( packages: Tuple[str], # Options list_: bool, - install, - uninstall, + install: bool, + uninstall: bool, + fix: bool, force: bool, platform: str, project_dir: Path, @@ -104,10 +215,14 @@ def cli( """ # Validate the option combination. - cmd_util.check_exactly_one_param(ctx, nameof(list_, install, uninstall)) + cmd_util.check_exactly_one_param( + ctx, nameof(list_, install, uninstall, fix) + ) cmd_util.check_at_most_one_param(ctx, nameof(list_, force)) cmd_util.check_at_most_one_param(ctx, nameof(uninstall, force)) + cmd_util.check_at_most_one_param(ctx, nameof(fix, force)) cmd_util.check_at_most_one_param(ctx, nameof(list_, packages)) + cmd_util.check_at_most_one_param(ctx, nameof(fix, packages)) # -- Load the resources. We don't care about project specific resources. resources = Resources( @@ -117,47 +232,17 @@ def cli( ) if install: - click.secho(f"Platform id '{resources.platform_id}'") - - # -- If packages not specified, use all. - if not packages: - packages = resources.platform_packages.keys() - # -- Install the packages. - for package in packages: - installer.install_package( - resources, package_spec=package, force=force, verbose=verbose - ) - - ctx.exit(0) + exit_code = _install(resources, packages, force, verbose) + ctx.exit(exit_code) if uninstall: - # -- If packages not specified, use all. - if not packages: - packages = resources.platform_packages.keys() + exit_code = _uninstall(resources, packages, verbose, sayyes) + ctx.exit(exit_code) - # -- Ask the user for confirmation - num_packages = ( - "1 package" if len(packages) == 1 else f"{len(packages)} packages" - ) - if sayyes or click.confirm( - f"Do you want to uninstall {num_packages}?" - ): - - click.secho(f"Platform id '{resources.platform_id}'") - - # -- Uninstall packages, one by one - for package in packages: - installer.uninstall_package( - resources, package_spec=package, verbose=verbose - ) - - # -- User quit! - else: - click.secho("User said no", fg="red") - ctx.exit(0) + if fix: + exit_code = _fix(resources, verbose) + ctx.exit(exit_code) # -- Here it must be --list. - if verbose: - click.secho(f"Platform id '{resources.platform_id}'") - resources.list_packages() - ctx.exit(0) + exit_code = _list(resources, verbose) + ctx.exit(exit_code) diff --git a/apio/managers/installer.py b/apio/managers/installer.py index bd927814..773578b0 100644 --- a/apio/managers/installer.py +++ b/apio/managers/installer.py @@ -13,7 +13,7 @@ import shutil import click import requests -from apio import util +from apio import util, pkg_util from apio.resources import Resources from apio.managers.downloader import FileDownloader from apio.managers.unpacker import FileUnpacker @@ -208,12 +208,12 @@ def _parse_package_spec(package_spec: str) -> Tuple[str, str]: def _delete_package_dir( - resources: Resources, package_name: str, verbose: bool + resources: Resources, package_id: str, verbose: bool ) -> bool: """Delete the directory of the package with given name. Returns True if the packages existed. Exits with an error message on error.""" - package_dir = resources.get_package_dir(package_name) + package_dir = resources.get_package_dir(package_id) dir_found = package_dir.is_dir() if dir_found: @@ -221,7 +221,7 @@ def _delete_package_dir( click.secho(f"Deleting {str(package_dir)}") # -- Sanity check the path and delete. - package_folder_name = resources.get_package_folder_name(package_name) + package_folder_name = resources.get_package_folder_name(package_id) assert package_folder_name in str(package_dir), package_dir shutil.rmtree(package_dir) @@ -396,3 +396,40 @@ def uninstall_package( else: # -- Package not installed. We treat it as a success. click.secho(f"Package '{package_name}' was not installed", fg="green") + + +def fix_packages( + resources: Resources, scan: pkg_util.PackageScanResults, verbose: bool +) -> None: + """If the package scan result contains errors, fix them.""" + + # -- If non verbose, print a summary message. + if not verbose: + click.secho( + f"Fixing {util.count(scan.num_errors(), 'package error')}." + ) + + # -- Fix broken packages. + for package_id in scan.broken_package_ids: + if verbose: + print(f"Uninstalling broken package '{package_id}'") + _delete_package_dir(resources, package_id, verbose=False) + resources.profile.remove_package(package_id) + resources.profile.save() + + for package_id in scan.orphan_package_ids: + if verbose: + print(f"Uninstalling unknown package '{package_id}'") + resources.profile.remove_package(package_id) + resources.profile.save() + + for dir_name in scan.orphan_dir_names: + if verbose: + print(f"Deleting unknown dir '{dir_name}'") + shutil.rmtree(util.get_packages_dir() / dir_name) + + for file_name in scan.orphan_file_names: + if verbose: + print(f"Deleting unknown file '{file_name}'") + file_path = util.get_packages_dir() / file_name + file_path.unlink() diff --git a/apio/pkg_util.py b/apio/pkg_util.py index 106acf0d..450fa661 100644 --- a/apio/pkg_util.py +++ b/apio/pkg_util.py @@ -17,6 +17,7 @@ import click import semantic_version from apio.resources import Resources +from apio import util @dataclass(frozen=True) @@ -250,3 +251,170 @@ def _show_package_install_instructions(package_name: str): " apio packages --install --force", fg="yellow", ) + + +@dataclass +class PackageScanResults: + """Represents results of packages scan.""" + + # -- Normal. Packages in platform_packages that are instaleld properly. + installed_package_ids: List[str] + # -- Normal. Packages in platform_packages that are uninstaleld properly. + uninstalled_package_ids: List[str] + # -- Error. Packages in platform_packages with broken installation. E.g, + # -- registered in profile but package directory is missing. + broken_package_ids: List[str] + # -- Error. Packages that are marked in profile as registered but are not + # -- in platform_packages. + orphan_package_ids: List[str] + # -- Error. Basenames of directories in packages dir that don't match + # -- folder_name of packages in platform_packates. + orphan_dir_names: List[str] + # -- Error. Basenames of all files in packages directory. That directory is + # -- expected to contain only directories for packages.a + orphan_file_names: List[str] + + def num_errors(self) -> bool: + """Returns the number of errors..""" + return ( + len(self.broken_package_ids) + + len(self.orphan_package_ids) + + len(self.orphan_dir_names) + + len(self.orphan_file_names) + ) + + def dump(self): + """Dump the content of this object. For debugging.""" + print("Package scan results:") + print(f" Installed {self.installed_package_ids}") + print(f" Uninstalled {self.uninstalled_package_ids}") + print(f" Broken {self.broken_package_ids}") + print(f" Orphan ids {self.orphan_package_ids}") + print(f" Orphan dirs {self.orphan_dir_names}") + print(f" Orphan files {self.orphan_file_names}") + + +def scan_packages(resources: Resources) -> PackageScanResults: + """Scans the available and installed packages and returns + the findings as a PackageScanResults object.""" + + # Initialize the result with empty data. + result = PackageScanResults([], [], [], [], [], []) + + # -- A helper set that we populate with the 'folder_name' values of the + # -- all the packages for this platform. + platform_folder_names = set() + + # -- Scan packages ids in platform_packages and populate + # -- the installed/uninstall/broken packages lists. + for package_id in resources.platform_packages.keys(): + # -- Collect package's folder names in a set. For a later use. + package_folder_name = resources.get_package_folder_name(package_id) + platform_folder_names.add(package_folder_name) + + # -- Classify the package as one of three cases. + in_profile = package_id in resources.profile.packages + has_dir = resources.get_package_dir(package_id).is_dir() + if in_profile and has_dir: + result.installed_package_ids.append(package_id) + elif not in_profile and not has_dir: + result.uninstalled_package_ids.append(package_id) + else: + result.broken_package_ids.append(package_id) + + # -- Scan the packagtes ids that are registered in profile as installed + # -- the ones that are not platform_packages as orphans. + for package_id in resources.profile.packages: + if package_id not in resources.platform_packages: + result.orphan_package_ids.append(package_id) + + # -- Scan the packages directory and identify orphan dirs and files. + for path in util.get_packages_dir().glob("*"): + base_name = os.path.basename(path) + if path.is_dir(): + if base_name not in platform_folder_names: + result.orphan_dir_names.append(base_name) + else: + result.orphan_file_names.append(base_name) + + # -- Return results + return result + + +def _list_section(title: str, items: List[List[str]], color: str) -> None: + """A helper function for printing one serction of list_packages().""" + # -- Construct horizontal lines at terminal width. + output_config = util.get_terminal_config() + line = "─" * output_config.terminal_width + dline = "═" * output_config.terminal_width + + # -- Print the section. + click.secho() + click.secho(dline, fg=color) + click.secho(title, fg=color, bold=True) + for item in items: + click.secho(line) + for sub_item in item: + click.secho(sub_item) + click.secho(dline, fg=color) + + +def list_packages(resources: Resources, scan: PackageScanResults) -> None: + """Prints in a user friendly format the results of a packages scan.""" + + # -- Shortcuts to reduce clutter. + get_package_version = resources.profile.get_package_installed_version + get_package_info = resources.get_package_info + + # --Print the installed packages, if any. + if scan.installed_package_ids: + items = [] + for package_id in scan.installed_package_ids: + name = click.style(f"{package_id}", fg="cyan", bold=True) + version = get_package_version(package_id) + description = get_package_info(package_id)["description"] + items.append([f"{name} {version}", f"{description}"]) + _list_section("Installed packages:", items, "green") + + # -- Print the uninstalled packages, if any, + if scan.uninstalled_package_ids: + items = [] + for package_id in scan.uninstalled_package_ids: + name = click.style(f"{package_id}", fg="cyan", bold=True) + description = get_package_info(package_id)["description"] + items.append([f"{name} {description}"]) + _list_section("Available packages (Not installed):", items, "yellow") + + # -- Print the broken packages, if any, + if scan.broken_package_ids: + items = [] + for package_id in scan.broken_package_ids: + name = click.style(f"{package_id}", fg="red", bold=True) + description = get_package_info(package_id)["description"] + items.append([f"{name} {description}"]) + _list_section("[Error] Broken packages:", items, None) + + # -- Print the orphan packages, if any, + if scan.orphan_package_ids: + items = [] + for package_id in scan.orphan_package_ids: + name = click.style(f"{package_id}", fg="red", bold=True) + items.append([name]) + _list_section("[Error] Unknown packages:", items, None) + + # -- Print orphan directories and files, if any, + if scan.orphan_dir_names or scan.orphan_file_names: + items = [] + for name in sorted(scan.orphan_dir_names + scan.orphan_file_names): + name = click.style(f"{name}", fg="red", bold=True) + items.append([name]) + _list_section("[Error] Unknown files and directories:", items, None) + + # -- Print an error summary + if scan.num_errors(): + click.secho(f"Total of {util.count(scan.num_errors(), 'error')}") + else: + click.secho("No errors.") + + # -- A line seperator. For asthetic reasons. + click.secho() diff --git a/apio/util.py b/apio/util.py index fdf6ed1b..fe19752c 100644 --- a/apio/util.py +++ b/apio/util.py @@ -15,7 +15,7 @@ import shutil from enum import Enum from dataclasses import dataclass -from typing import Optional +from typing import Optional, Any import subprocess from threading import Thread from pathlib import Path @@ -601,3 +601,21 @@ def safe_click(text, *args, **kwargs): # most common character error cleaned_text = "".join([ch if ord(ch) < 128 else "=" for ch in text]) click.secho(cleaned_text, err=error_flag, *args, **kwargs) + + +def count(obj: Any, singular: str, plural: str = None) -> str: + """Returns singular or plural based on the size of the object.""" + # -- Figure out the size of the object + if isinstance(obj, int): + n = obj + else: + n = len(obj) + + # -- For value of 1 return the signgular form. + if n == 1: + return f"{n} {singular}" + + # -- For all other values, return the plural form. + if plural is None: + plural = singular + "s" + return f"{n} {plural}" diff --git a/test/commands/test_packages.py b/test/commands/test_packages.py index 2320139d..54baec90 100644 --- a/test/commands/test_packages.py +++ b/test/commands/test_packages.py @@ -6,7 +6,7 @@ from apio.commands.packages import cli as cmd_packages -def test_packages(clirunner, configenv, validate_cliresult): +def test_packages(clirunner, configenv): """Test "apio packages" with different parameters""" with clirunner.isolated_filesystem(): @@ -18,13 +18,14 @@ def test_packages(clirunner, configenv, validate_cliresult): result = clirunner.invoke(cmd_packages) assert result.exit_code == 1, result.output assert ( - "One of [--list, --install, --uninstall] " + "One of [--list, --install, --uninstall, --fix] " "must be specified" in result.output ) # -- Execute "apio packages --list" result = clirunner.invoke(cmd_packages, ["--list"]) - validate_cliresult(result) + assert result.exit_code == 0, result.output + assert "No errors" in result.output # -- Execute "apio packages --install missing_package" result = clirunner.invoke( diff --git a/test/test_end_to_end.py b/test/test_end_to_end.py index a1b5f8a2..e26f2bcd 100644 --- a/test/test_end_to_end.py +++ b/test/test_end_to_end.py @@ -107,7 +107,8 @@ def test_end_to_end1(clirunner, validate_cliresult, configenv, offline): # -- Execute "apio packages --install --list" result = clirunner.invoke(cmd_packages, ["--list"]) - validate_cliresult(result) + assert result.exit_code == 0, result.output + assert "No errors" in result.output assert "Installed packages:" in result.output assert "examples" in result.output @@ -244,7 +245,7 @@ def test_end_to_end3(clirunner, validate_cliresult, configenv, offline): result = clirunner.invoke( cmd_packages, ["--uninstall", "examples"], input="n" ) - validate_cliresult(result) + assert result.exit_code != 0, result.output assert "User said no" in result.output # -- Execute "apio packages --uninstall examples" From 0bac48f1eaf49905feeb3a055e4e758f3ee27eed Mon Sep 17 00:00:00 2001 From: Zapta Date: Sun, 17 Nov 2024 09:58:09 -0800 Subject: [PATCH 2/3] Fixed an error when 'apio packages --list' is piped out. --- apio/pkg_util.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/apio/pkg_util.py b/apio/pkg_util.py index 450fa661..2367cba2 100644 --- a/apio/pkg_util.py +++ b/apio/pkg_util.py @@ -344,9 +344,10 @@ def scan_packages(resources: Resources) -> PackageScanResults: def _list_section(title: str, items: List[List[str]], color: str) -> None: """A helper function for printing one serction of list_packages().""" # -- Construct horizontal lines at terminal width. - output_config = util.get_terminal_config() - line = "─" * output_config.terminal_width - dline = "═" * output_config.terminal_width + config = util.get_terminal_config() + line_width = config.terminal_width if config.terminal_mode() else 80 + line = "─" * line_width + dline = "═" * line_width # -- Print the section. click.secho() From 461f73a39fb08f19dbde9c695c6cd4cdf5bd1589 Mon Sep 17 00:00:00 2001 From: Zapta Date: Sun, 17 Nov 2024 10:17:36 -0800 Subject: [PATCH 3/3] Fixed a typo in a user message. --- apio/commands/packages.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/apio/commands/packages.py b/apio/commands/packages.py index f33f9675..0e66e6f4 100644 --- a/apio/commands/packages.py +++ b/apio/commands/packages.py @@ -105,11 +105,12 @@ def _list(resources: Resources, verbose: bool) -> int: # -- Print an hint or summary based on the findings. if scan.num_errors(): click.secho( - "[Hint] run 'apio packages -c' to fix the errors.", fg="yellow" + "[Hint] run 'apio packages -fix' to fix the errors.", fg="yellow" ) elif scan.uninstalled_package_ids: click.secho( - "[Hint] run 'apio packages -i' to install all available packages.", + "[Hint] run 'apio packages -install' to install all " + "available packages.", fg="yellow", ) else: