jw.pkg: Fix "make check" static code check fallout

The previous commits have put rules for linting and formatting via
ruff, yapf, mypy and pyright into place. They are checked with the
make check target, and this commit adds the fixes for the target to
succeed.

It does some refactoring where type checking dug up dirty bits, and
also adds lots of churn in the Python code. To a good deal, that's
owed to mere formatting changes. It would have been better to
seperate those from syntax and refactoring fixes into multiple
commits, so that the interesting changes don't drown in the
formatting nose. However, that would have been a lot of additional
work only to be thrown away by later commits, hence this commit has a
big diff in one piece. The size of the diff is regrettable but
hopefully a one-off: What it buys is automatic format checking for CI
and predictble formats for smaller diffs in the future.

Rules that "make check" enforces are, in the following order

   - Syntax checkers:

      - ruff check .
      - mypy .
      - pyright

   - Format check:

     - yapf --diff --recursive .

The refactoring includes:

  - Turn the Result class into a more elaborate object, capable of
    doing more heavy lifting around stderr and stdout decoding,
    summarizing outcome, and matching error strings.

    Aside from fixing broken type checks, this also removes lots of
    boilerplate calling code which is currently used for handling
    possible call outcome scenarios. Trying to access an inexistent,
    decoded string should raise a meaningful exception by itself now,
    which removes lots of code with case distinctions.

  - Fix Cmd type hierarchy:

    - Add the AbstractCmd class above Cmd. This is necessary because
      the checker rightfully complains it can't instantiate a Cmd
      instance where constructor arguments were needed. They never
      were, but the type used at the instantiating code's location in
      jw.pkg.App so claims.

    - Lots of sub- and sub-subcommands are derived from the base
      class of the invoking command. That provides some properties
      shared across the ancestor hierarchy of a command, but is
      semantically unsound. Fix that by introducing jw.pkg.BaseCmd
      class as a place to provide basic helpers shared across all
      commands used in a jw.pkg.App's context, and derive all command
      classes from that afresh. The parent command is still reachable
      via a common parent property.

Formatting changes are conforming to PEP-8, mostly, with minor
tweaks. All in all they include the following changes.

  - Remove # -*- coding: utf-8 -*-

    The line was needed by Python 2 which is not supported anylonger.
    For Python 3, the default encoding is UTF-8, anyway.

  - Allow to run "make py-format" without having it produce any
    changes. It's basically "yapf --in-place --recursive ." with some
    code style settings, see conf/topdir/pyproject.toml. The settings
    may be debatable. I've had custom tweaks in place on that target,
    too, but then again, IDEs would have more hassle to integrate
    that.

  - Introduce a 88 character line length limit

  - One import per line, reshuffle them semantically, see
    [tool.isort] in pyproject.toml.

  - Hide imports needed for type-checking only behind

      if TYPE_CHECKING

  - Spaces around assignments accounts for much churn. Having having
    no spaces in inline parameter list assignments and default
    parameter values would arguably be more compact where it's
    useful. On the other hand, I have not found a code formatter
    which allows spaces around assignments in parameter lists broken
    into one per line and that's often better than a wall of text.

  - Add two spaces before # export, as this seems to be mandated by
    PEP-8

  - Use single quotes by default

Signed-off-by: Jan Lindemann <jan@janware.com>

src/python/jw/pkg/lib/Distro.py

@@ -1,38 +1,41 @@
-# -*- coding: utf-8 -*-
-
 from __future__ import annotations
 
-from typing import TYPE_CHECKING
+import abc
+import importlib
+import re
+import sys
+
 from functools import cached_property
+from typing import TYPE_CHECKING
+
+from .log import ERR, INFO, WARNING, log
 
 if TYPE_CHECKING:
-    import Iterable
+    from typing import Iterable
+
+    from .base import InputMode, Result
+    from .ExecContext import ExecContext
+    from .Package import Package
     from .PackageFilter import PackageFilter
 
-import abc, importlib, re
-
-from .PackageFilter import PackageFilter
-from .ExecContext import ExecContext
-from .base import Result, InputMode
-from .Package import Package
-from .log import *
-
 class Distro(abc.ABC):
 
     def __init__(
-            self,
-            ec: ExecContext,
-            id: str|None=None,
-            os_release_str: str|None=None,
-            default_pkg_filter: PackageFilter|None=None,
-        ) -> None:
+        self,
+        ec: ExecContext,
+        id: str | None = None,
+        os_release_str: str | None = None,
+        default_pkg_filter: PackageFilter | None = None,
+    ) -> None:
         if id is None:
-            raise ValueError(f'Tried to instaniate Distro without id')
+            raise ValueError('Tried to instaniate Distro without id')
         if ec is None:
-            raise ValueError(f'Tried to instaniate Distro "{id}" without execution context')
+            raise ValueError(
+                f'Tried to instaniate Distro "{id}" without execution context'
+            )
         self.__exec_context = ec
-        self.__id: str|None = None
-        self.__os_release_str: str|None = os_release_str
+        self.__id: str | None = None
+        self.__os_release_str: str | None = os_release_str
         self.__default_pkg_filter = default_pkg_filter
 
     # Names that can be used by code outside this class to retrieve
@@ -52,47 +55,60 @@ class Distro(abc.ABC):
     # == Load
 
     @classmethod
-    async def read_os_release_str(cls, ec: ExecContext) -> None:
+    async def read_os_release_str(cls, ec: ExecContext) -> str:
         release_file = '/etc/os-release'
         try:
-            result = await ec.get(release_file, throw=True)
-            ret = result.stdout.decode().strip()
+            result = await ec.get(release_file, throw = True)
+            return result.stdout_str
         except Exception as e:
-            log(INFO, f'Failed to read {release_file} ({str(e)}), falling back to uname')
+            log(
+                INFO,
+                f'Failed to read {release_file} ({str(e)}), falling back to uname'
+            )
             result = await ec.run(
-                            ['uname', '-s'],
-                            throw=False,
-                            cmd_input=InputMode.NonInteractive
-                        )
+                ['uname', '-s'], throw = False, cmd_input = InputMode.NonInteractive
+            )
             if result.status != 0:
-                log(ERR, f'/etc/os-release and uname both failed, the latter with exit status {result.status}')
+                log(
+                    ERR,
+                    (
+                        '/etc/os-release and uname both failed, '
+                        f'the latter with {result.summary}'
+                    ),
+                )
                 raise
-            uname = result.decode().stdout.strip().lower()
+            uname = result.stdout_str.lower()
             ret = f'ID={uname}\nVERSION_CODENAME=unknown'
         return ret
 
     @classmethod
-    def parse_os_release_field(self, key: str, os_release_str: str, throw: bool=False) -> str:
-        m = re.search(r'^\s*' + key + r'\s*=\s*("?)([^"\n]+)\1\s*$', os_release_str, re.MULTILINE)
+    def parse_os_release_field(cls, key: str, os_release_str: str) -> str:
+        m = re.search(
+            r'^\s*' + key + r'\s*=\s*("?)([^"\n]+)\1\s*$', os_release_str, re.MULTILINE
+        )
         if m is None:
-            if throw:
-                raise Exception(f'Could not read "{key}=" from /etc/os-release')
-            return None
+            raise Exception(f'Could not read "{key}=" from /etc/os-release')
         return m.group(2)
 
     @classmethod
-    def parse_os_release_field_id(cls, os_release_str: str, throw: bool=False) -> str:
-        ret = cls.parse_os_release_field('ID', os_release_str, throw=throw)
+    def parse_os_release_field_id(cls, os_release_str: str) -> str:
+        ret = cls.parse_os_release_field('ID', os_release_str)
         match ret:
             case 'opensuse-tumbleweed':
                 return 'suse'
         return ret
 
     @classmethod
-    async def instantiate(cls, ec: ExecContext, *args, id: str|None=None, os_release_str: str|None=None, **kwargs):
+    async def instantiate(
+        cls,
+        ec: ExecContext,
+        id: str | None = None,
+        os_release_str: str | None = None,
+        **kwargs,
+    ):
         if id is None:
             os_release_str = await cls.read_os_release_str(ec)
-            id = cls.parse_os_release_field_id(os_release_str, throw=True)
+            id = cls.parse_os_release_field_id(os_release_str)
         backend_id = id.lower().replace('-', '_')
         match backend_id:
             case 'ubuntu' | 'raspbian' | 'kali':
@@ -108,11 +124,11 @@ class Distro(abc.ABC):
             log(ERR, f'Failed to import Distro module {module_path} ({str(e)})')
             raise
         cls = getattr(module, 'Distro')
-        ret = cls(ec, *args, id=id, os_release_str=os_release_str, **kwargs)
+        ret = cls(ec, id = id, os_release_str = os_release_str, **kwargs)
         return ret
 
-    def os_release_field(self, key: str, throw: bool=False) -> str:
-        return self.parse_os_release_field(key, self.os_release_str, throw)
+    def os_release_field(self, key: str) -> str:
+        return self.parse_os_release_field(key, self.os_release_str)
 
     async def cache(self) -> None:
         if self.__os_release_str is None:
@@ -120,10 +136,12 @@ class Distro(abc.ABC):
 
     @cached_property
     def os_cascade(self) -> list[str]:
+
         def __append(entry: str):
-            if not entry in ret:
+            if entry not in ret:
                 ret.append(entry)
-        ret = [ 'os' ]
+
+        ret = ['os']
         match self.id:
             case 'centos':
                 __append('linux')
@@ -177,27 +195,32 @@ class Distro(abc.ABC):
     @property
     def os_release_str(self) -> str:
         if self.__os_release_str is None:
-            raise Exception(f'Tried to access OS release from an incompletely loaded Distro instance. Call reacache() before')
+            raise Exception(
+                'Tried to access OS release from an incompletely loaded Distro '
+                'instance. Call cache() before'
+            )
         return self.__os_release_str
 
     @cached_property
     def name(self) -> str:
-        return self.os_release_field('NAME', throw=True)
+        return self.os_release_field('NAME')
 
     @cached_property
     def id(self) -> str:
-        return self.parse_os_release_field_id(self.__os_release_str, throw=True)
+        return self.parse_os_release_field_id(self.os_release_str)
 
     @cached_property
     def codename(self) -> str:
         match self.id:
             case 'suse':
-                return self.os_release_field('ID', throw=True).split('-')[1]
+                return self.os_release_field('ID').split('-')[1]
             case 'kali':
-                return self.os_release_field('VERSION_CODENAME', throw=True).split('-')[1]
+                return self.os_release_field('VERSION_CODENAME').split('-')[1]
             case _:
-                return self.os_release_field('VERSION_CODENAME', throw=True)
-        raise NotImplementedError(f'Can\'t determine code name from distribution ID {self.id}')
+                return self.os_release_field('VERSION_CODENAME')
+        raise NotImplementedError(
+            f"Can't determine code name from distribution ID {self.id}"
+        )
 
     @cached_property
     def os(self) -> str:
@@ -214,33 +237,35 @@ class Distro(abc.ABC):
     @cached_property
     def gnu_triplet(self) -> str:
 
-        import sysconfig
         import shutil
         import subprocess
+        import sysconfig
 
         # Best: GNU host triplet Python was built for
-        for key in ("HOST_GNU_TYPE", "BUILD_GNU_TYPE"):  # BUILD_GNU_TYPE can exist too
+        for key in ('HOST_GNU_TYPE', 'BUILD_GNU_TYPE'):  # BUILD_GNU_TYPE can exist too
             ret = sysconfig.get_config_var(key)
             if isinstance(ret, str) and ret:
                 return ret
 
         # Common on Debian/Ubuntu: multiarch component (often looks like a triplet)
-        ret = sysconfig.get_config_var("MULTIARCH")
+        ret = sysconfig.get_config_var('MULTIARCH')
         if isinstance(ret, str) and ret:
             return ret
 
         # Sometimes exposed (privately) by CPython
-        ret = getattr(sys.implementation, "_multiarch", None)
+        ret = getattr(sys.implementation, '_multiarch', None)
         if isinstance(ret, str) and ret:
             return ret
 
         # Last resort: ask the system compiler
-        for cc in ("gcc", "cc", "clang"):
+        for cc in ('gcc', 'cc', 'clang'):
             path = shutil.which(cc)
             if not path:
                 continue
             try:
-                ret = subprocess.check_output([path, "-dumpmachine"], text=True, stderr=subprocess.DEVNULL).strip()
+                ret = subprocess.check_output(
+                    [path, '-dumpmachine'], text = True, stderr = subprocess.DEVNULL
+                ).strip()
                 if ret:
                     return ret
             except Exception:
@@ -252,14 +277,21 @@ class Distro(abc.ABC):
     def macros(cls) -> list[str]:
         return ['%%{' + name + '}' for name in cls.macro_names]
 
-    def expand_macros(self, fmt: str|Iterable) -> str|Iterable:
+    def expand_macros(self, fmt: str | Iterable) -> str | list[str]:
+        ret: str | list[str]
         if not isinstance(fmt, str):
-            ret: list[str] = []
+            ret = []
             for entry in fmt:
-                ret.append(self.expand_macros(entry))
+                rv = self.expand_macros(entry)
+                if isinstance(rv, str):
+                    ret.append(rv)
+                    continue
+                raise NotImplementedError(
+                    f'Expanding macros in nested lists is not supported: {rv}'
+                )
             return ret
         ret = fmt
-        for macro in re.findall("%{([A-Za-z_-]+)}", fmt):
+        for macro in re.findall('%{([A-Za-z_-]+)}', fmt):
             try:
                 name = macro.replace('-', '_')
                 val = getattr(self, name)
@@ -279,7 +311,7 @@ class Distro(abc.ABC):
         return self.__exec_context
 
     @property
-    def default_pkg_filter(self) -> str:
+    def default_pkg_filter(self) -> PackageFilter | None:
         return self.__default_pkg_filter
 
     async def run(self, *args, **kwargs) -> Result:
@@ -289,7 +321,7 @@ class Distro(abc.ABC):
         return await self.__exec_context.sudo(*args, **kwargs)
 
     @property
-    def interactive(self) -> bool:
+    def interactive(self) -> bool | None:
         return self.__exec_context.interactive
 
     # == Distribution abstraction methods
@@ -309,8 +341,8 @@ class Distro(abc.ABC):
     async def _dup(self, download_only: bool) -> None:
         pass
 
-    async def dup(self, download_only: bool=False) -> None:
-        return await self._dup(download_only=download_only)
+    async def dup(self, download_only: bool = False) -> None:
+        return await self._dup(download_only = download_only)
 
     # -- reboot_required
 
@@ -318,10 +350,10 @@ class Distro(abc.ABC):
     async def _reboot_required(self, verbose: bool) -> bool:
         pass
 
-    async def reboot_required(self, verbose: bool|None=None) -> bool:
+    async def reboot_required(self, verbose: bool | None = None) -> bool:
         if verbose is None:
             verbose = self.ctx.verbose_default
-        return await self._reboot_required(verbose=verbose)
+        return await self._reboot_required(verbose = verbose)
 
     # -- select
 
@@ -329,11 +361,16 @@ class Distro(abc.ABC):
     async def _select_by_name(self, names: Iterable[str]) -> Iterable[Package]:
         pass
 
-    async def _select(self, names: Iterable[str], filter: PackageFilter) -> Iterable[Package]:
-        assert filter, "No filter in _select()"
+    async def _select(self, names: Iterable[str],
+                      filter: PackageFilter) -> Iterable[Package]:
+        assert filter, 'No filter in _select()'
         return [p for p in await self._select_by_name(names) if filter.match(p)]
 
-    async def select(self, names: Iterable[str] = [], filter: PackageFilter|None=None) -> Iterable[Package]:
+    async def select(
+        self,
+        names: Iterable[str] = [],
+        filter: PackageFilter | None = None
+    ) -> Iterable[Package]:
         if not filter:
             filter = self.__default_pkg_filter
         if not filter:
@@ -349,17 +386,28 @@ class Distro(abc.ABC):
 
     # Default implementation assumes package manager can handle local files.
     # Not true for all distros. Override if Distro knows better.
-    async def _install_local_files(self, paths: Iterable[str], only_update: bool) -> None:
-        await self._install(paths, only_update=only_update)
+    async def _install_local_files(
+        self, paths: Iterable[str], only_update: bool
+    ) -> None:
+        await self._install(paths, only_update = only_update)
 
     # Download first and then install. Override if Distro knows better.
     async def _install_urls(self, urls: Iterable[str], only_update: bool) -> None:
         from .util import copy
-        tmp: str|None = None
+
+        tmp: str | None = None
         try:
-            tmp = await self.__exec_context.mktemp('/tmp/jw-pkg-XXXXXX', directory=True)
-            paths = await copy(urls, self.__exec_context.uri.scheme_plus_authority + tmp)
-            await self._install_local_files(paths, only_update=only_update)
+            tmp = await self.__exec_context.mktemp(
+                '/tmp/jw-pkg-XXXXXX', directory = True
+            )
+            paths = await copy(
+                urls, self.__exec_context.uri.scheme_plus_authority + tmp
+            )
+            if isinstance(paths, Exception):
+                raise paths
+            if isinstance(paths, str):
+                paths = [paths]
+            await self._install_local_files(paths, only_update = only_update)
         finally:
             if tmp is not None:
                 await self.__exec_context.erase(tmp)
@@ -368,7 +416,9 @@ class Distro(abc.ABC):
     # - Download URLs into local directories and install
     # - Pass names to package manager
     # Override if Distro knows better.
-    async def _install_urls_and_names(self, packages: Iterable[str], only_update: bool) -> None:
+    async def _install_urls_and_names(
+        self, packages: Iterable[str], only_update: bool
+    ) -> None:
         urls: list[str] = []
         names: list[str] = []
         for package in packages:
@@ -380,15 +430,15 @@ class Distro(abc.ABC):
                 continue
             names.append(package)
         if urls:
-            await self._install_urls(urls, only_update=only_update)
+            await self._install_urls(urls, only_update = only_update)
         if names:
-            await self._install(names, only_update=only_update)
+            await self._install(names, only_update = only_update)
 
-    async def install(self, names: Iterable[str], only_update: bool=False) -> None:
+    async def install(self, names: Iterable[str], only_update: bool = False) -> None:
         if not names:
-            log(WARNING, f'No packages specified for installation')
+            log(WARNING, 'No packages specified for installation')
             return
-        await self._install_urls_and_names(names, only_update=only_update)
+        await self._install_urls_and_names(names, only_update = only_update)
 
     # -- delete
 
@@ -398,7 +448,7 @@ class Distro(abc.ABC):
 
     async def delete(self, names: Iterable[str]) -> None:
         if not names:
-            log(WARNING, f'No packages specified for deletion')
+            log(WARNING, 'No packages specified for deletion')
             return
         return await self._delete(names)
 
@@ -410,6 +460,6 @@ class Distro(abc.ABC):
 
     async def pkg_files(self, name: str) -> Iterable[str]:
         if not name:
-            log(WARNING, f'No package specified for inspection')
+            log(WARNING, 'No package specified for inspection')
             return []
         return await self._pkg_files(name)