start migration to CliFunction

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()

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)

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())

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: