refactor: port all sys.exit over to kas exceptions

This patch replaces all direct invocations of sys.exit outside of the
main invocation to KasUserError based exceptions. By that, only one
method for returning is used and return codes can be handled
consistently. In addition, this makes it possible to handle specific
errors differently.

Signed-off-by: Felix Moessbauer <felix.moessbauer@siemens.com>
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>

kas/kas.py

@@ -34,7 +34,7 @@ import logging
 import signal
 import sys
 import os
-from .kasusererror import KasUserError
+from .kasusererror import KasUserError, CommandExecError
 
 try:
     import colorlog
@@ -176,6 +176,9 @@ def main():
 
     try:
         kas(sys.argv[1:])
+    except CommandExecError as err:
+        logging.error('%s', err)
+        sys.exit(err.ret_code if err.forward else 2)
     except KasUserError as err:
         logging.error('%s', err)
         sys.exit(2)

kas/kasusererror.py

@@ -43,3 +43,28 @@ class KasUserError(Exception):
     User or input error. Derive all user error exceptions from this class.
     """
     pass
+
+
+class CommandExecError(KasUserError):
+    """
+    Failure in execution of a shell command. The `forward_error_code` parameter
+    can be used to request the receiver of the exception to `sys.exit` with
+    that code instead of a generic one. Only use this in special cases, where
+    the return code can actually be related to a single shell command.
+    """
+    def __init__(self, command, ret_code,
+                 forward_ret_code=False):
+        self.ret_code = ret_code
+        self.forward = forward_ret_code
+        message = ["'{}'".format(c) if ' ' in c else c for c in command]
+        super().__init__('Command "{}" failed with error {}'
+                         .format(' '.join(message), ret_code))
+
+
+class ArgsCombinationError(KasUserError):
+    """
+    Invalid combination of CLI arguments provided
+    """
+    def __init__(self, message):
+        super().__init__('Invalid combination of arguments: {}'
+                         .format(message))

kas/libcmds.py

@@ -28,7 +28,6 @@ import logging
 import shutil
 import os
 import pprint
-import sys
 from .libkas import (ssh_cleanup_agent, ssh_setup_agent, ssh_no_host_key_check,
                      get_build_environ, repos_fetch, repos_apply_patches)
 from .includehandler import IncludeException
@@ -377,8 +376,8 @@ class SetupReposStep(Command):
         ctx.missing_repos = []
         for repo_name in ctx.missing_repo_names:
             if repo_name not in ctx.config.get_repos_config():
-                logging.error('Include references unknown repo: %s', repo_name)
-                sys.exit(1)
+                raise IncludeException('Include references unknown repo: {}'
+                                       .format(repo_name))
             ctx.missing_repos.append(ctx.config.get_repo(repo_name))
 
         repos_fetch(ctx.missing_repos)

kas/libkas.py

@@ -34,11 +34,36 @@ import pathlib
 import signal
 from subprocess import Popen, PIPE
 from .context import get_context
+from .kasusererror import KasUserError, CommandExecError
 
 __license__ = 'MIT'
 __copyright__ = 'Copyright (c) Siemens AG, 2017-2018'
 
 
+class InitBuildEnvError(KasUserError):
+    """
+    Error related to the OE / ISAR environment setup scripts
+    """
+    pass
+
+
+class EnvNotValidError(KasUserError):
+    """
+    The caller environment is not suited for the requested operation
+    """
+    pass
+
+
+class TaskExecError(KasUserError):
+    """
+    Similar to :class:`kas.kasusererror.CommandExecError` but for kas
+    internal tasks
+    """
+    def __init__(self, command, ret_code):
+        self.ret_code = ret_code
+        super().__init__('{} failed: error code {}'.format(command, ret_code))
+
+
 class LogOutput:
     """
         Handles the log output of executed applications
@@ -144,7 +169,7 @@ def run_cmd(cmd, cwd, env=None, fail=True, liveupdate=True):
     (ret, output) = loop.run_until_complete(
         run_cmd_async(cmd, cwd, env, fail, liveupdate))
     if ret and fail:
-        sys.exit(ret)
+        raise CommandExecError(cmd, ret)
     return (ret, output)
 
 
@@ -175,7 +200,7 @@ def repos_fetch(repos):
 
     for task in tasks:
         if task.result():
-            sys.exit(task.result())
+            raise TaskExecError('fetch repos', task.result())
 
 
 def repos_apply_patches(repos):
@@ -194,7 +219,7 @@ def repos_apply_patches(repos):
 
     for task in tasks:
         if task.result():
-            sys.exit(task.result())
+            raise TaskExecError('apply patches', task.result())
 
 
 def get_build_environ(build_system):
@@ -217,15 +242,15 @@ def get_build_environ(build_system):
     for (repo, script) in permutations:
         if os.path.exists(repo.path + '/' + script):
             if init_repo:
-                logging.error('Multiple init scripts found (%s vs. %s). ',
-                              repo.name, init_repo.name)
-                logging.error('Resolve ambiguity by removing one of the repos')
-                sys.exit(1)
+                raise InitBuildEnvError(
+                    'Multiple init scripts found ({} vs. {}). '
+                    'Resolve ambiguity by removing one of the repos'
+                    .format(repo.name, init_repo.name))
+
             init_repo = repo
             init_script = script
     if not init_repo:
-        logging.error('Did not find any init-build-env script')
-        sys.exit(1)
+        raise InitBuildEnvError('Did not find any init-build-env script')
 
     with tempfile.TemporaryDirectory() as temp_dir:
         script = """#!/bin/bash
@@ -412,9 +437,8 @@ def run_handle_preserve_env_arg(ctx, os, args, SetupHome):
                                 var)
 
         if not os.isatty(sys.stdout.fileno()):
-            logging.error("Error: --preserve-env can only be "
-                          "run from a tty")
-            sys.exit(1)
+            raise EnvNotValidError(
+                '--preserve-env can only be run from a tty')
 
         ctx.environ = os.environ.copy()
 

kas/plugins/build.py

@@ -40,6 +40,7 @@ from kas.config import Config
 from kas.libkas import find_program, run_cmd
 from kas.libcmds import Macro, Command
 from kas.libkas import setup_parser_common_args
+from kas.kasusererror import CommandExecError
 
 __license__ = 'MIT'
 __copyright__ = 'Copyright (c) Siemens AG, 2017-2018'
@@ -114,8 +115,7 @@ class BuildCommand(Command):
             logging.info('%s$ %s', ctx.build_dir, ' '.join(cmd))
             ret = subprocess.call(cmd, env=ctx.environ, cwd=ctx.build_dir)
             if ret != 0:
-                logging.error('Command returned non-zero exit status %d', ret)
-                sys.exit(ret)
+                raise CommandExecError(cmd, ret)
         else:
             run_cmd(cmd, cwd=ctx.build_dir)
 

kas/plugins/dump.py

@@ -63,7 +63,6 @@
     Note, that the lockfiles should be checked-in into the VCS.
 """
 
-import logging
 import sys
 import json
 import yaml
@@ -71,11 +70,17 @@ from typing import TypeVar, TextIO
 from collections import OrderedDict
 from kas.context import get_context
 from kas.plugins.checkout import Checkout
+from kas.kasusererror import KasUserError, ArgsCombinationError
 
 __license__ = 'MIT'
 __copyright__ = 'Copyright (c) Siemens AG, 2022'
 
 
+class OutputFormatError(KasUserError):
+    def __init__(self, format):
+        super().__init__('invalid format {}'.format(format))
+
+
 class IoTarget:
     StrOrTextIO = TypeVar('StrOrTextIO', str, TextIO)
 
@@ -184,8 +189,7 @@ class Dump(Checkout):
         output = IoTarget(target=sys.stdout, managed=False)
 
         if args.inplace and not args.lock:
-            logging.error('--inplace requires --lock')
-            sys.exit(1)
+            raise ArgsCombinationError('--inplace requires --lock')
 
         if args.lock:
             args.resolve_refs = True
@@ -220,8 +224,7 @@ class Dump(Checkout):
                     indent=args.indent,
                     Dumper=self.KasYamlDumper)
             else:
-                logging.error('invalid format %s', args.format)
-                sys.exit(1)
+                raise OutputFormatError(args.format)
 
 
 __KAS_PLUGINS__ = [Dump]

kas/plugins/for_all_repos.py

@@ -56,13 +56,13 @@
 import logging
 import os
 import subprocess
-import sys
 from kas.context import create_global_context
 from kas.config import Config
 from kas.libcmds import Macro, Command, SetupHome
 from kas.libkas import setup_parser_common_args
 from kas.libkas import setup_parser_preserve_env_arg
 from kas.libkas import run_handle_preserve_env_arg
+from kas.kasusererror import CommandExecError
 
 __license__ = 'MIT'
 __copyright__ = 'Copyright (c) Siemens AG, 2017-2018'
@@ -113,8 +113,7 @@ class ForAllReposCommand(Command):
             retcode = subprocess.call(self.command, shell=True, cwd=repo.path,
                                       env=env)
             if retcode != 0:
-                logging.error('Command failed with return code %d', retcode)
-                sys.exit(retcode)
+                raise CommandExecError(self.command, retcode)
 
 
 __KAS_PLUGINS__ = [ForAllRepos]

kas/plugins/menu.py

@@ -69,7 +69,6 @@
 import logging
 import os
 import pprint
-import sys
 import yaml
 from kconfiglib import Kconfig, Symbol, Choice, expr_value, TYPE_TO_STR, \
     MENU, COMMENT, STRING, BOOL, INT, HEX, UNKNOWN
@@ -78,6 +77,7 @@ from kas.context import create_global_context
 from kas.config import CONFIG_YAML_FILE
 from kas.includehandler import load_config as load_config_yaml
 from kas.plugins.build import Build
+from kas.kasusererror import KasUserError
 
 try:
     from snack import SnackScreen, EntryWindow, ButtonChoiceWindow, \
@@ -92,10 +92,18 @@ __copyright__ = \
     'Copyright (c) Siemens AG, 2021'
 
 
+class VariableTypeError(KasUserError):
+    pass
+
+
+class MissingModuleError(KasUserError):
+    pass
+
+
 def check_sym_is_string(sym):
     if sym.type != STRING:
-        logging.error('Variable %s must be of string type', sym.name)
-        sys.exit(1)
+        raise VariableTypeError('Variable {} must be of string type'
+                                .format(sym.name))
 
 
 def str_representer(dumper, data):
@@ -175,10 +183,9 @@ class Menu:
                 elif sym.type == HEX:
                     menu_configuration[symname] = int(symvalue, 16)
                 else:
-                    logging.error(
-                        'Configuration variable %s uses unsupported type',
-                        symname)
-                    sys.exit(1)
+                    raise VariableTypeError(
+                        'Configuration variable {} uses unsupported type'
+                        .format(symname))
 
             if symname.startswith('KAS_INCLUDE_'):
                 check_sym_is_string(sym)
@@ -241,9 +248,8 @@ class Menu:
 
     def run(self, args):
         if not newt_available:
-            logging.error(
+            raise MissingModuleError(
                 'Menu plugin requires \'python3-newt\' distribution package.')
-            sys.exit(1)
 
         ctx = create_global_context(args)
 

kas/plugins/shell.py

@@ -40,13 +40,13 @@
 import logging
 import os
 import subprocess
-import sys
 from kas.context import create_global_context
 from kas.config import Config
 from kas.libcmds import Macro, Command, SetupHome
 from kas.libkas import setup_parser_common_args
 from kas.libkas import setup_parser_preserve_env_arg
 from kas.libkas import run_handle_preserve_env_arg
+from kas.kasusererror import CommandExecError
 
 __license__ = 'MIT'
 __copyright__ = 'Copyright (c) Siemens AG, 2017-2018'
@@ -124,8 +124,8 @@ class ShellCommand(Command):
             cmd.append(self.cmd)
         ret = subprocess.call(cmd, env=ctx.environ, cwd=ctx.build_dir)
         if ret != 0:
-            logging.error('Shell returned non-zero exit status %d', ret)
-            sys.exit(ret)
+            logging.error('Shell returned non-zero exit status')
+            raise CommandExecError(cmd, ret, True)
 
 
 __KAS_PLUGINS__ = [Shell]

kas/repos.py

@@ -39,7 +39,14 @@ __copyright__ = 'Copyright (c) Siemens AG, 2017-2018'
 
 class UnsupportedRepoTypeError(KasUserError, NotImplementedError):
     """
-    Exception for unsupported / not implemented repository types
+    The requested repo type is unsupported / not implemented
+    """
+    pass
+
+
+class RepoRefError(KasUserError):
+    """
+    The requested repo reference is invalid, missing or could not be found
     """
     pass
 
@@ -51,6 +58,13 @@ class PatchFileNotFound(KasUserError, FileNotFoundError):
     pass
 
 
+class PatchMappingError(KasUserError):
+    """
+    The requested patch can not be related to a repo
+    """
+    pass
+
+
 class Repo:
     """
         Represents a repository in the kas configuration.
@@ -127,9 +141,10 @@ class Repo:
                 'path': patches_dict[p]['path'],
             }
             if this_patch['repo'] is None:
-                logging.error('No repo specified for patch entry "%s" and no '
-                              'default repo specified.', p)
-                sys.exit(1)
+                raise PatchMappingError(
+                    'No repo specified for patch entry "{}" and no '
+                    'default repo specified.'.format(p))
+
             patches.append(this_patch)
 
         url = repo_config.get('url', None)
@@ -138,9 +153,10 @@ class Repo:
         refspec = repo_overrides.get('refspec', repo_config.get('refspec',
                                      repo_defaults.get('refspec', None)))
         if refspec is None and url is not None:
-            logging.error('No refspec specified for repository "%s". This is '
-                          'only allowed for local repositories.', name)
-            sys.exit(1)
+            raise RepoRefError('No refspec specified for repository "{}". '
+                               'This is only allowed for local repositories.'
+                               .format(name))
+
         path = repo_config.get('path', None)
         disable_operations = False
 

tests/test_refspec.py

@@ -25,6 +25,7 @@ import pytest
 import shutil
 from kas import kas
 from kas.libkas import run_cmd
+from kas.repos import RepoRefError
 
 
 def test_refspec_switch(changedir, tmpdir):
@@ -95,5 +96,5 @@ def test_url_no_refspec(changedir, tmpdir):
     tdir = str(tmpdir / 'test_url_no_refspec')
     shutil.copytree('tests/test_refspec', tdir)
     os.chdir(tdir)
-    with pytest.raises(SystemExit):
+    with pytest.raises(RepoRefError):
         kas.kas(['shell', 'test4.yml', '-c', 'true'])