From c74ba2fe740560f3bf8096190be5ea6624eb3c8d Mon Sep 17 00:00:00 2001 From: Ian Wienand Date: Tue, 2 May 2017 13:29:34 +1000 Subject: [PATCH] Move to subparsers Move argument parsing to subparsers, rather than positional arguments. This better reflects the tool's role as a driver and allows sub-commands to deal with arguments in a natural way. Change-Id: Iae8c368e0f3fe47abfddb9e0a1558bd5b3423aee --- diskimage_builder/block_device/blockdevice.py | 14 +-- diskimage_builder/block_device/cmd.py | 102 +++++++++++------- diskimage_builder/lib/disk-image-create | 17 ++- diskimage_builder/lib/img-functions | 2 +- 4 files changed, 80 insertions(+), 55 deletions(-) diff --git a/diskimage_builder/block_device/blockdevice.py b/diskimage_builder/block_device/blockdevice.py index c8fad739..c9d43a8c 100644 --- a/diskimage_builder/block_device/blockdevice.py +++ b/diskimage_builder/block_device/blockdevice.py @@ -40,7 +40,7 @@ class BlockDevice(object): A typical call sequence: - cmd_init: initialized the block device level config. After this + cmd_init: initialize the block device level config. After this call it is possible to e.g. query information from the (partially automatic generated) internal state like root-label. @@ -74,19 +74,19 @@ class BlockDevice(object): In a script this should be called in the following way: - dib-block-device --phase=init ... + dib-block-device init ... # From that point the database can be queried, like - ROOT_LABEL=$(dib-block-device --phase=getval --symbol=root-label ...) + ROOT_LABEL=$(dib-block-device getval root-label) Please note that currently the dib-block-device executable can only be used outside the chroot. - dib-block-device --phase=create ... - trap "dib-block-device --phase=delete ..." EXIT + dib-block-device create ... + trap "dib-block-device delete ..." EXIT # copy / install files - dib-block-device --phase=umount ... + dib-block-device umount ... # convert image(s) - dib-block-device --phase=cleanup ... + dib-block-device cleanup ... trap - EXIT """ diff --git a/diskimage_builder/block_device/cmd.py b/diskimage_builder/block_device/cmd.py index 2be316e8..a795e0f1 100644 --- a/diskimage_builder/block_device/cmd.py +++ b/diskimage_builder/block_device/cmd.py @@ -26,67 +26,93 @@ logger = logging.getLogger(__name__) class BlockDeviceCmd(object): - def generate_phase_doc(self): - phase_doc = "" - bdattrs = dir(BlockDevice) - for attr in bdattrs: - if attr.startswith("cmd_"): - phase_doc += " '" + attr[4:] + "'\n" - method = getattr(BlockDevice, attr, None) - # The first line is the line that is used - phase_doc += " " + method.__doc__.split("\n")[0] + "\n" - return phase_doc + def cmd_init(self): + self.bd.cmd_init() + + def cmd_getval(self): + self.bd.cmd_getval() + + def cmd_create(self): + self.bd.cmd_create() + + def cmd_umount(self): + self.bd.cmd_umount() + + def cmd_cleanup(self): + self.bd.cmd_cleanup() + + def cmd_delete(self): + self.bd.cmd_delete() + + def cmd_writefstab(self): + self.bd.cmd_writefstab() def main(self): logging_config.setup() - phase_doc = self.generate_phase_doc() - parser = argparse.ArgumentParser( - formatter_class=argparse.RawDescriptionHelpFormatter, - description="Create block device layer", - epilog="Available phases:\n" + phase_doc) - parser.add_argument('--phase', required=True, - help="phase to execute") + parser = argparse.ArgumentParser(description="DIB Block Device helper") parser.add_argument('--params', required=False, - help="YAML file containing parameters for " + help="YAML file containing parameters for" "block-device handling. Default is " "DIB_BLOCK_DEVICE_PARAMS_YAML") - parser.add_argument('--symbol', required=False, - help="symbol to query for getval") - args = parser.parse_args() + + subparsers = parser.add_subparsers(title='commands', + description='valid commands', + dest='command', + help='additional help') + + cmd_init = subparsers.add_parser('init', + help='Initialize configuration') + cmd_init.set_defaults(func=self.cmd_init) + + cmd_getval = subparsers.add_parser('getval', + help='Retrieve information about' + 'internal state') + cmd_getval.set_defaults(func=self.cmd_getval) + cmd_getval.add_argument('symbol', help='symbol to print') + + cmd_create = subparsers.add_parser('create', + help='Create the block device') + cmd_create.set_defaults(func=self.cmd_create) + + cmd_umount = subparsers.add_parser('umount', + help='Unmount blockdevice and' + 'cleanup resources') + cmd_umount.set_defaults(func=self.cmd_umount) + + cmd_cleanup = subparsers.add_parser('cleanup', help='Final cleanup') + cmd_cleanup.set_defaults(func=self.cmd_cleanup) + + cmd_delete = subparsers.add_parser('delete', help='Error cleanup') + cmd_delete.set_defaults(func=self.cmd_delete) + + cmd_writefstab = subparsers.add_parser('writefstab', + help='Create fstab for system') + cmd_writefstab.set_defaults(func=self.cmd_writefstab) + + self.args = parser.parse_args() # Find, open and parse the parameters file - if not args.params: + if not self.args.params: if 'DIB_BLOCK_DEVICE_PARAMS_YAML' in os.environ: param_file = os.environ['DIB_BLOCK_DEVICE_PARAMS_YAML'] else: parser.error( "DIB_BLOCK_DEVICE_PARAMS_YAML or --params not set") else: - param_file = args.params + param_file = self.args.params logger.info("params [%s]" % param_file) try: with open(param_file) as f: - params = yaml.safe_load(f) + self.params = yaml.safe_load(f) except Exception: logger.exception("Failed to open parameter YAML") sys.exit(1) - logger.info("phase [%s]" % args.phase) + # Setup main BlockDevice object from args + self.bd = BlockDevice(self.params, self.args) - if args.symbol: - logger.info("symbol [%s]" % args.symbol) - - bd = BlockDevice(params, args) - - # Check if the method is available - method = getattr(bd, "cmd_" + args.phase, None) - if callable(method): - # If so: call it. - return method() - else: - logger.error("phase [%s] does not exists" % args.phase) - return 1 + self.args.func() def main(): diff --git a/diskimage_builder/lib/disk-image-create b/diskimage_builder/lib/disk-image-create index 57ab7fa2..2fe6a039 100644 --- a/diskimage_builder/lib/disk-image-create +++ b/diskimage_builder/lib/disk-image-create @@ -299,7 +299,7 @@ mount-base: ${TMP_BUILD_DIR}/mnt build-dir: ${TMP_BUILD_DIR} EOF -dib-block-device --phase=init +dib-block-device init create_base # This variable needs to be propagated into the chroot @@ -424,14 +424,13 @@ if [ -z ${IMAGE_BLOCK_DEVICE} ] ; then # After changeing the parameters, there is the need to # re-run dib-block-device init because some value might # change based on the new set parameters. - dib-block-device --phase=init + dib-block-device init # values to dib-block-device: using the YAML config and - dib-block-device --phase=create + dib-block-device create # It's called 'DEVICE' but it's the partition. - IMAGE_BLOCK_DEVICE=$(dib-block-device \ - --phase=getval --symbol=image-block-partition) + IMAGE_BLOCK_DEVICE=$(dib-block-device getval image-block-partition) fi export IMAGE_BLOCK_DEVICE LOOPDEV=${IMAGE_BLOCK_DEVICE} @@ -441,7 +440,7 @@ IMAGE_BLOCK_DEVICE_WITHOUT_PART=$(echo ${IMAGE_BLOCK_DEVICE} \ export IMAGE_BLOCK_DEVICE_WITHOUT_PART export EXTRA_DETACH="detach_loopback ${IMAGE_BLOCK_DEVICE_WITHOUT_PART}" -export EXTRA_UNMOUNT="dib-block-device --phase=cleanup" +export EXTRA_UNMOUNT="dib-block-device cleanup" sudo mkfs -t $FS_TYPE $MKFS_OPTS -L ${DIB_ROOT_LABEL} ${IMAGE_BLOCK_DEVICE} # Tuning the rootfs uuid works only for ext filesystems. @@ -505,12 +504,12 @@ fi export EXTRA_UNMOUNT="" unmount_image -TMP_IMAGE_PATH=$(dib-block-device --phase=getval --symbol=image-path) +TMP_IMAGE_PATH=$(dib-block-device getval image-path) export TMP_IMAGE_PATH -dib-block-device --phase=umount +dib-block-device umount -dib-block-device --phase=cleanup +dib-block-device cleanup cleanup_build_dir diff --git a/diskimage_builder/lib/img-functions b/diskimage_builder/lib/img-functions index 5e64e86c..4fdf3011 100644 --- a/diskimage_builder/lib/img-functions +++ b/diskimage_builder/lib/img-functions @@ -50,7 +50,7 @@ function trap_cleanup() { } function cleanup () { - dib-block-device --phase=umount + dib-block-device umount unmount_image cleanup_build_dir cleanup_image_dir