From 271e7c6371c27551d2d9736962aa67c62275c149 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ciro=20Santilli=20=E5=85=AD=E5=9B=9B=E4=BA=8B=E4=BB=B6=20?= =?UTF-8?q?=E6=B3=95=E8=BD=AE=E5=8A=9F?= Date: Thu, 29 Nov 2018 00:00:00 +0000 Subject: [PATCH] start migration to CliFunction --- build-gem5 | 72 +++++++++-------- build-linux | 4 +- cli_function.py | 210 ++++++++++++++++++++++++++++++++++++++++++++++++ common.py | 142 ++++++++++++++++---------------- 4 files changed, 318 insertions(+), 110 deletions(-) create mode 100644 cli_function.py diff --git a/build-gem5 b/build-gem5 index 4a89b61..eff6757 100755 --- a/build-gem5 +++ b/build-gem5 @@ -5,38 +5,40 @@ import pathlib import subprocess import common +from cli_function import Argument -class Gem5Component(common.Component): - def add_parser_arguments(self, parser): - parser.add_argument( - 'extra_scons_args', - default=[], - metavar='extra-scons-args', - nargs='*' - ) +class Main(common.BuildCliFunction): + def do_get_arguments(self): + return [ + Argument( + 'extra_scons_args', + metavar='extra-scons-args', + nargs='*', + ) + ] - def do_build(self, args): - build_dir = self.get_build_dir(args) - binaries_dir = os.path.join(common.gem5_system_dir, 'binaries') - disks_dir = os.path.join(common.gem5_system_dir, 'disks') + def do_main(self, **kwargs): + build_dir = self.get_build_dir(**kwargs) + binaries_dir = os.path.join(kwargs['gem5_system_dir'], 'binaries') + disks_dir = os.path.join(kwargs['gem5_system_dir'], 'disks') os.makedirs(binaries_dir, exist_ok=True) os.makedirs(disks_dir, exist_ok=True) - if args.gem5_source_dir is None: - if not os.path.exists(os.path.join(common.gem5_src_dir, '.git')): - if common.gem5_src_dir == common.gem5_default_src_dir: + if kwargs['gem5_source_dir'] is None: + if not os.path.exists(os.path.join(kwargs['gem5_src_dir'], '.git')): + if kwargs['gem5_src_dir'] == kwargs['gem5_default_src_dir']: raise Exception('gem5 submodule not checked out') common.run_cmd([ 'git', common.Newline, - '-C', common.gem5_default_src_dir, common.Newline, + '-C', kwargs['gem5_default_src_dir'], common.Newline, 'worktree', 'add', common.Newline, - '-b', os.path.join('wt', args.gem5_build_id), common.Newline, - common.gem5_src_dir, common.Newline, + '-b', os.path.join('wt', kwargs['gem5_build_id']), common.Newline, + kwargs['gem5_src_dir'], common.Newline, ]) - if args.verbose: + if kwargs['verbose']: verbose = ['--verbose', common.Newline] else: verbose = [] - if args.arch == 'x86_64': + if kwargs['arch'] == 'x86_64': dummy_img_path = os.path.join(disks_dir, 'linux-bigswap2.img') with open(dummy_img_path, 'wb') as dummy_img_file: zeroes = b'\x00' * (2 ** 16) @@ -47,12 +49,12 @@ class Gem5Component(common.Component): # This file must always be present, despite --kernel overriding that default and selecting the kernel. # I'm not even joking. No one has ever built x86 gem5 without the magic dist dir present. pass - elif args.arch == 'arm' or args.arch == 'aarch64': - gem5_system_src_dir = os.path.join(common.gem5_src_dir, 'system') + elif kwargs['arch'] == 'arm' or kwargs['arch'] == 'aarch64': + gem5_system_src_dir = os.path.join(kwargs['gem5_src_dir'], 'system') # dtb dt_src_dir = os.path.join(gem5_system_src_dir, 'arm', 'dt') - dt_build_dir = os.path.join(common.gem5_system_dir, 'arm', 'dt') + dt_build_dir = os.path.join(kwargs['gem5_system_dir'], 'arm', 'dt') common.run_cmd([ 'make', common.Newline, '-C', dt_src_dir, common.Newline, @@ -86,29 +88,29 @@ class Gem5Component(common.Component): ( [ 'scons', common.Newline, - '-j', str(args.nproc), common.Newline, + '-j', str(kwargs['nproc']), common.Newline, '--gold-linker', common.Newline, '--ignore-style', common.Newline, - common.gem5_executable, common.Newline, + kwargs['gem5_executable'], common.Newline, ] + verbose + - common.add_newlines(args.extra_scons_args) + common.add_newlines(kwargs['extra_scons_args']) ), - cwd=common.gem5_src_dir, - extra_paths=[common.ccache_dir], + cwd=kwargs['gem5_src_dir'], + extra_paths=[kwargs['ccache_dir']], ) - term_src_dir = os.path.join(common.gem5_src_dir, 'util/term') + term_src_dir = os.path.join(kwargs['gem5_src_dir'], 'util/term') m5term_build = os.path.join(term_src_dir, 'm5term') common.run_cmd(['make', '-C', term_src_dir]) - if os.path.exists(common.gem5_m5term): + if os.path.exists(kwargs['gem5_m5term']): # Otherwise common.cp would fail with "Text file busy" if you # tried to rebuild while running m5term: # https://stackoverflow.com/questions/16764946/what-generates-the-text-file-busy-message-in-unix/52427512#52427512 - os.unlink(common.gem5_m5term) - common.cp(m5term_build, common.gem5_m5term) + os.unlink(kwargs['gem5_m5term']) + common.cp(m5term_build, kwargs['gem5_m5term']) - def get_build_dir(self, args): - return common.gem5_build_dir + def get_build_dir(self, **kwargs): + return kwargs['gem5_build_dir'] if __name__ == '__main__': - Gem5Component().build() + Main().cli() diff --git a/build-linux b/build-linux index 94eaf74..b173adf 100755 --- a/build-linux +++ b/build-linux @@ -49,8 +49,8 @@ Configure the kernel, but don't build it. nargs='*' ) - def do_build(self, args): - build_dir = self.get_build_dir(args) + def do_main(self, **kwargs): + build_dir = self.get_build_dir(**kwargs) if args.initrd or args.initramfs: raise Exception('just trolling, --initrd and --initramfs are broken for now') os.makedirs(build_dir, exist_ok=True) diff --git a/cli_function.py b/cli_function.py new file mode 100644 index 0000000..ef6963c --- /dev/null +++ b/cli_function.py @@ -0,0 +1,210 @@ +#!/usr/bin/env python3 + +import argparse +import imp +import os + +class Argument: + def __init__( + self, + longname, + shortname=None, + default=None, + help=None, + nargs=None, + **kwargs + ): + self.args = [] + # argparse is crappy and cannot tell us if arguments were given or not. + # We need that information to decide if the config file should override argparse or not. + # So we just use None as a sentinel. + self.kwargs = {'default': None} + if shortname is not None: + self.args.append(shortname) + if longname[0] == '-': + self.args.append(longname) + self.key = longname.lstrip('-').replace('-', '_') + else: + self.key = longname.replace('-', '_') + self.args.append(self.key) + self.kwargs['metavar'] = longname + if default is not None and nargs is None: + self.kwargs['nargs'] = '?' + if nargs is not None: + self.kwargs['nargs'] = nargs + if default is True: + self.kwargs['action'] = 'store_false' + self.is_bool = True + elif default is False: + self.kwargs['action'] = 'store_true' + self.is_bool = True + else: + self.is_bool = False + if default is None and nargs in ('*', '+'): + default = [] + if help is not None: + if not self.is_bool and default: + help += ' Default: {}'.format(default) + self.kwargs['help'] = help + self.optional = ( + default is not None or + self.is_bool or + nargs in ('?', '*', '+') + ) + self.kwargs.update(kwargs) + self.default = default + + def __str__(self): + return str(self.args) + ' ' + str(self.kwargs) + +class CliFunction: + ''' + Represent a function that can be called either from Python code, or + from the command line. + + Features: + + * single argument description in format very similar to argparse + * handle default arguments transparently in both cases + * expose a configuration file mechanism to get default parameters from a file + * fix some argparse.ArgumentParser() annoyances: + ** allow dashes in positional arguments: + https://stackoverflow.com/questions/12834785/having-options-in-argparse-with-a-dash + ** boolean defaults automatically use store_true or store_false + + This somewhat duplicates: https://click.palletsprojects.com but: + + * that decorator API is insane + * CLI + Python for single functions was wontfixed: https://github.com/pallets/click/issues/40 + ''' + def __call__(self, **args): + ''' + Python version of the function call. + + :type arguments: Dict + ''' + arguments = self._get_arguments() + args_with_defaults = args.copy() + # Add missing args from config file. + if 'config_file' in args_with_defaults and args_with_defaults['config_file'] is not None: + config_file = args_with_defaults['config_file'] + else: + config_file = self.default_config + if os.path.exists(config_file): + all_keys = {argument.key for argument in arguments} + config_configs = {} + config = imp.load_source('config', config_file) + config.set_args(config_configs) + for key in config_configs: + if key not in all_keys: + raise Exception('Unknown key in config file: ' + key) + if (not key in args_with_defaults) or args_with_defaults[key] is None: + args_with_defaults[key] = config_configs[key] + # Add missing args from hard-coded defaults. + for argument in arguments: + key = argument.key + if (not key in args_with_defaults) or args_with_defaults[key] is None: + if argument.optional: + args_with_defaults[key] = argument.default + else: + raise Exception('Value not given for mandatory argument: ' + key) + return self.main(**args_with_defaults) + + def __init__(self): + self.default_config = 'config.py' + + def _get_arguments(self): + ''' + Define the arguments that the function takes. + + :rtype: List[Argument] + ''' + args = self.get_arguments() + config_file = self.get_config_file() + if config_file is not None: + args.append(Argument( + longname='--config-file', + default=config_file, + help='Path to the configuration file to use' + )) + return args + + def cli(self, args=None): + ''' + Call the function from the CLI. Parse command line arguments + to get all arguments. + ''' + parser = argparse.ArgumentParser( + description=self.get_description(), + formatter_class=argparse.RawTextHelpFormatter, + ) + for argument in self._get_arguments(): + parser.add_argument(*argument.args, **argument.kwargs) + args = parser.parse_args(args=args) + return self(**vars(args)) + + def get_arguments(self): + ''' + Define the arguments that the function takes. + + :rtype: List[Argument] + ''' + raise NotImplementedError + + def get_config_file(self): + ''' + :rtype: Union[None,str] + ''' + return None + + def get_description(self): + ''' + argparse.ArgumentParser() description. + + :rtype: Union[Any,str] + ''' + return None + + def main(self, arguments): + ''' + Do the main function call work. + + :type arguments: Dict + ''' + raise NotImplementedError + +if __name__ == '__main__': + class OneCliFunction(CliFunction): + def get_arguments(self): + return [ + Argument(shortname='-a', longname='--asdf', default='A', help='Help for asdf'), + Argument(shortname='-q', longname='--qwer', default='Q', help='Help for qwer'), + Argument(shortname='-b', longname='--bool', default=True, help='Help for bool'), + Argument(longname='pos-mandatory', help='Help for pos-mandatory', type=int), + Argument(longname='pos-optional', default=0, help='Help for pos-optional', type=int), + Argument(longname='args-star', help='Help for args-star', nargs='*'), + ] + def main(self, **kwargs): + return \ + kwargs['asdf'], \ + kwargs['qwer'], \ + kwargs['bool'], \ + kwargs['pos_optional'], \ + kwargs['pos_mandatory'], \ + kwargs['args_star'] + def get_config_file(self): + return 'test_config.py' + def get_description(self): + return '''\ +Description of this +amazing function! +''' + + # Code calls. + assert OneCliFunction()(pos_mandatory=1) == ('A', 'Q', True, 0, 1, []) + assert OneCliFunction()(pos_mandatory=1, asdf='B') == ('B', 'Q', True, 0, 1, []) + assert OneCliFunction()(pos_mandatory=1, bool=False) == ('A', 'Q', False, 0, 1, []) + assert OneCliFunction()(pos_mandatory=1, asdf='B', qwer='R', bool=False) == ('B', 'R', False, 0, 1, []) + + # CLI call. + print(OneCliFunction().cli()) diff --git a/common.py b/common.py index d697cbc..26e9180 100644 --- a/common.py +++ b/common.py @@ -2,6 +2,7 @@ import argparse import base64 +import cli_function import collections import copy import datetime @@ -80,78 +81,99 @@ obj_ext = '.o' config_file = os.path.join(data_dir, 'config') command_prefix = '+ ' magic_fail_string = b'lkmc_test_fail' -if os.path.exists(config_file): - config = imp.load_source('config', config_file) - configs = {x:getattr(config, x) for x in dir(config) if not x.startswith('__')} -class Component: - def __init__(self): - pass +class LkmcCliFunction(cli_function.CliFunction): + ''' + Common functionality shared across our CLI functions: - def build(self): + * command timing + * some common flags, e.g.: --arch, --dry-run + ''' + def get_arguments(self): + return [ + cli_function.Argument( + longname='--dry-run', + default=False, + help='''\ +Print the commands that would be run, but don't run them. + +We aim display every command that modifies the filesystem state, and generate +Bash equivalents even for actions taken directly in Python without shelling out. + +mkdir are generally omitted since those are obvious +''' + ) + ] + + def main(self, **kwargs): ''' Parse CLI, and to the build based on it. - The actual build work is done by do_build in implementing classes. + The actual build work is done by timed_main in implementing classes. ''' - parser = common.get_argparse( - argparse_args=self.get_argparse_args(), - default_args=self.get_default_args(), - ) - self.add_parser_arguments(parser) - parser.add_argument( - '--clean', - help='Clean the build instead of building.', - action='store_true', - ) - parser.add_argument( - '-j', '--nproc', - help='Number of processors to use for the build. Default: use all cores.', - type=int, - default=multiprocessing.cpu_count(), - ) - args = common.setup(parser) - if not common.dry_run: + if not kwargs['dry_run']: start_time = time.time() - if args.clean: - self.clean(args) - else: - self.do_build(args) - if not common.dry_run: + self.timed_main(**kwargs) + if not kwargs['dry_run']: end_time = time.time() common.print_time(end_time - start_time) - def add_parser_arguments(self, parser): - pass + def timed_main(self, **kwargs): + raise NotImplementedError() - def clean(self, args): - build_dir = self.get_build_dir(args) +class BuildCliFunction(LkmcCliFunction): + ''' + A CLI function with common facilities to build stuff, e.g.: + + * `--clean` to clean the build directory + * `--nproc` to set he number of build threads + ''' + def clean(self, **kwargs): + build_dir = self.get_build_dir(kwargs) if build_dir is not None: common.rmrf(build_dir) - def do_build(self, args): + def get_arguments(self): + return super().get_arguments() + [ + cli_function.Argument( + longname='--clean', + default=False, + help='Clean the build instead of building.', + ), + cli_function.Argument( + shortname='-j', + longname='--nproc', + default=multiprocessing.cpu_count(), + type=int, + help='Number of processors to use for the build. Default: use all cores.', + ), + ] + self.do_get_arguments() + + def do_get_arguments(self): + return [] + + def do_main(self, **kwargs): ''' Do the actual main build work. ''' raise NotImplementedError() - def get_argparse_args(self): - ''' - Extra arguments for argparse.ArgumentParser. - ''' - return {} - - def get_build_dir(self, args): + def get_build_dir(self, **kwargs): ''' Build directory, gets cleaned by --clean if not None. ''' return None - def get_default_args(self): + def timed_main(self, **kwargs): ''' - Default values for command line arguments. + Parse CLI, and to the build based on it. + + The actual build work is done by do_build in implementing classes. ''' - return {} + if kwargs['clean']: + self.clean(kwargs) + else: + self.do_main(**kwargs) class Newline: ''' @@ -160,16 +182,6 @@ class Newline: ''' pass -def add_dry_run_argument(parser): - parser.add_argument('--dry-run', default=False, action='store_true', help='''\ -Print the commands that would be run, but don't run them. - -We aim display every command that modifies the filesystem state, and generate -Bash equivalents even for actions taken directly in Python without shelling out. - -mkdir are generally omitted since those are obvious. -''') - def add_newlines(cmd): out = [] for arg in cmd: @@ -254,7 +266,7 @@ def gem_list_checkpoint_dirs(): files.sort(key=lambda x: os.path.getmtime(os.path.join(common.m5out_dir, x))) return files -def get_argparse(default_args=None, argparse_args=None): +def get_argparse(default_args=None): ''' Return an argument parser with common arguments set. @@ -265,10 +277,6 @@ def get_argparse(default_args=None, argparse_args=None): default_args = {} if argparse_args is None: argparse_args = {} - parser = argparse.ArgumentParser( - formatter_class=argparse.RawTextHelpFormatter, - **argparse_args - ) common.add_dry_run_argument(parser) emulator_group = parser.add_mutually_exclusive_group(required=False) parser.add_argument( @@ -392,17 +400,6 @@ to allow overriding configs from the CLI. '-v', '--verbose', default=False, action='store_true', help='Show full compilation commands when they are not shown by default.' ) - if hasattr(common, 'configs'): - defaults = common.configs.copy() - else: - defaults = {} - defaults.update(default_args) - # A bit ugly as it actually changes the defaults shown on --help, but we can't do any better - # because it is impossible to check if arguments were given or not... - # - https://stackoverflow.com/questions/30487767/check-if-argparse-optional-argument-is-set-or-not - # - https://stackoverflow.com/questions/3609852/which-is-the-best-way-to-allow-configuration-options-be-overridden-at-the-comman - parser.set_defaults(**defaults) - return parser def get_elf_entry(elf_file_path): readelf_header = subprocess.check_output([ @@ -708,7 +705,6 @@ def setup(parser): Parse the command line arguments, and setup several variables based on them. Typically done after getting inputs from the command line arguments. ''' - args = parser.parse_args() if args.qemu or not args.gem5: common.emulator = 'qemu' else: