From 222f07de69a3f12d6282dad63bdaf6f869871558 Mon Sep 17 00:00:00 2001 From: Felix Moessbauer Date: Thu, 4 May 2023 04:50:18 +0200 Subject: [PATCH] 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 Signed-off-by: Jan Kiszka --- kas/kas.py | 5 +++- kas/kasusererror.py | 25 +++++++++++++++++++ kas/libcmds.py | 5 ++-- kas/libkas.py | 48 +++++++++++++++++++++++++++--------- kas/plugins/build.py | 4 +-- kas/plugins/dump.py | 13 ++++++---- kas/plugins/for_all_repos.py | 5 ++-- kas/plugins/menu.py | 24 +++++++++++------- kas/plugins/shell.py | 6 ++--- kas/repos.py | 30 ++++++++++++++++------ tests/test_refspec.py | 3 ++- 11 files changed, 122 insertions(+), 46 deletions(-) diff --git a/kas/kas.py b/kas/kas.py index 4d7edbf..ac6e20f 100644 --- a/kas/kas.py +++ b/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) diff --git a/kas/kasusererror.py b/kas/kasusererror.py index c9518d4..5862385 100644 --- a/kas/kasusererror.py +++ b/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)) diff --git a/kas/libcmds.py b/kas/libcmds.py index e02de44..b5a621f 100644 --- a/kas/libcmds.py +++ b/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) diff --git a/kas/libkas.py b/kas/libkas.py index 96ba946..68b92e5 100644 --- a/kas/libkas.py +++ b/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() diff --git a/kas/plugins/build.py b/kas/plugins/build.py index 3896f66..83289cc 100644 --- a/kas/plugins/build.py +++ b/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) diff --git a/kas/plugins/dump.py b/kas/plugins/dump.py index e59d75c..65f86ab 100644 --- a/kas/plugins/dump.py +++ b/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] diff --git a/kas/plugins/for_all_repos.py b/kas/plugins/for_all_repos.py index dd87346..2581ae7 100644 --- a/kas/plugins/for_all_repos.py +++ b/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] diff --git a/kas/plugins/menu.py b/kas/plugins/menu.py index 08a2041..9618e6b 100644 --- a/kas/plugins/menu.py +++ b/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) diff --git a/kas/plugins/shell.py b/kas/plugins/shell.py index 50e101a..4475f22 100644 --- a/kas/plugins/shell.py +++ b/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] diff --git a/kas/repos.py b/kas/repos.py index 7b4af5d..3000321 100644 --- a/kas/repos.py +++ b/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 diff --git a/tests/test_refspec.py b/tests/test_refspec.py index 53aebd7..f012031 100644 --- a/tests/test_refspec.py +++ b/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'])