* [wic patch 0/5] Add option to wic and use argparse @ 2017-04-21 12:11 Andreas J. Reichel 2017-04-21 12:11 ` [wic patch 1/5] wic: Catch errors during image files clean-up Andreas J. Reichel ` (5 more replies) 0 siblings, 6 replies; 17+ messages in thread From: Andreas J. Reichel @ 2017-04-21 12:11 UTC (permalink / raw) To: openembedded-core; +Cc: Jan Kiszka, Daniel Wagner, Andreas Reichel This series consists of the following changes to wic: * wic previously introduced deletion of all single partition image files. In some cases, this deletion can fail, however this is no reason for wic to fail. Thus, exception handling was added to the clean-up function of the direct imager plugin. * wic uses optparse and a rather complicated approach of its initialization. It is much easier to switch from the deprecated optparse to the argparse library, which also supports sub-commands, which wic heavily makes use of. The second patch replaces optparse by argparse. * For some embedded projects it is necessary to keep the single partition image files to further process them. This is the case if a user has no rights to mount and cannot extract the images from the already merged .direct file, for example. For this purpose, a new option is introduced: -P, --keep-partition-images which prevents deletion of .p* files created by the direct imager plugin. * Maintainability improvement in engine.py by comparing parameters with enum-like string constants rather than repeating the strings and fixes in help texts. The new argument parser was tested with random sampling manually, since variable names were not changed inside the core of the subcommands and transition from optparse to argparse is straight forward regarding access to argument values. Signed-off-by: Andreas Reichel <andreas.reichel.ext@siemens.com> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> Signed-off-by: Daniel Wagner <daniel.wagner@siemens.com> Reichel Andreas (5): wic: Catch errors during image files clean-up wic: Use argparse instead of optparse wic: Add missing text to usage and help strings wic: Add option to keep partition images wic: Use enum like dicts for string constants scripts/lib/wic/engine.py | 21 ++- scripts/lib/wic/help.py | 30 +++-- scripts/lib/wic/plugins/imager/direct.py | 23 +++- scripts/wic | 215 +++++++++++++++++++------------ 4 files changed, 191 insertions(+), 98 deletions(-) -- 2.11.0 ^ permalink raw reply [flat|nested] 17+ messages in thread
* [wic patch 1/5] wic: Catch errors during image files clean-up 2017-04-21 12:11 [wic patch 0/5] Add option to wic and use argparse Andreas J. Reichel @ 2017-04-21 12:11 ` Andreas J. Reichel 2017-05-02 12:56 ` Ed Bartosh 2017-04-21 12:11 ` [wic patch 2/5] wic: Use argparse instead of optparse Andreas J. Reichel ` (4 subsequent siblings) 5 siblings, 1 reply; 17+ messages in thread From: Andreas J. Reichel @ 2017-04-21 12:11 UTC (permalink / raw) To: openembedded-core; +Cc: Jan Kiszka, Daniel Wagner, Andreas Reichel Handle exception if a file could not be deleted during clean-up of unwanted files, thus preventing a failure of wic in this case. Signed-off-by: Andreas Reichel <andreas.reichel.ext@siemens.com> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> Signed-off-by: Daniel Wagner <daniel.wagner@siemens.com> --- scripts/lib/wic/plugins/imager/direct.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/scripts/lib/wic/plugins/imager/direct.py b/scripts/lib/wic/plugins/imager/direct.py index f2e6127331..d6b47ff0bb 100644 --- a/scripts/lib/wic/plugins/imager/direct.py +++ b/scripts/lib/wic/plugins/imager/direct.py @@ -541,7 +541,12 @@ class PartitionedImage(): def cleanup(self): # remove partition images for image in set(self.partimages): - os.remove(image) + try: + os.remove(image) + except IOError as e: + logger.warning( + "Could not delete file. {0}: I/O error ({1}): {2}\n".format( + image, e.errno, e.strerror)) def assemble(self): logger.debug("Installing partitions") -- 2.11.0 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [wic patch 1/5] wic: Catch errors during image files clean-up 2017-04-21 12:11 ` [wic patch 1/5] wic: Catch errors during image files clean-up Andreas J. Reichel @ 2017-05-02 12:56 ` Ed Bartosh 2017-05-03 8:45 ` Andreas Reichel 0 siblings, 1 reply; 17+ messages in thread From: Ed Bartosh @ 2017-05-02 12:56 UTC (permalink / raw) To: Andreas J. Reichel Cc: Jan Kiszka, Andreas Reichel, Daniel Wagner, openembedded-core On Fri, Apr 21, 2017 at 02:11:41PM +0200, Andreas J. Reichel wrote: > Handle exception if a file could not be deleted during clean-up of > unwanted files, thus preventing a failure of wic in this case. Can you explain why partition images can't be deleted? As wic creates them it's not obvious why it can't remove them. > Signed-off-by: Andreas Reichel <andreas.reichel.ext@siemens.com> > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> > Signed-off-by: Daniel Wagner <daniel.wagner@siemens.com> > > --- > scripts/lib/wic/plugins/imager/direct.py | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/scripts/lib/wic/plugins/imager/direct.py b/scripts/lib/wic/plugins/imager/direct.py > index f2e6127331..d6b47ff0bb 100644 > --- a/scripts/lib/wic/plugins/imager/direct.py > +++ b/scripts/lib/wic/plugins/imager/direct.py > @@ -541,7 +541,12 @@ class PartitionedImage(): > def cleanup(self): > # remove partition images > for image in set(self.partimages): > - os.remove(image) > + try: > + os.remove(image) > + except IOError as e: > + logger.warning( > + "Could not delete file. {0}: I/O error ({1}): {2}\n".format( > + image, e.errno, e.strerror)) > > def assemble(self): > logger.debug("Installing partitions") > -- > 2.11.0 > > -- > _______________________________________________ > Openembedded-core mailing list > Openembedded-core@lists.openembedded.org > http://lists.openembedded.org/mailman/listinfo/openembedded-core -- -- Regards, Ed ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [wic patch 1/5] wic: Catch errors during image files clean-up 2017-05-02 12:56 ` Ed Bartosh @ 2017-05-03 8:45 ` Andreas Reichel 2017-05-03 10:32 ` Ed Bartosh 0 siblings, 1 reply; 17+ messages in thread From: Andreas Reichel @ 2017-05-03 8:45 UTC (permalink / raw) To: Ed Bartosh; +Cc: Jan Kiszka, Andreas Reichel, Daniel Wagner, openembedded-core [-- Attachment #1: message --] [-- Type: text/plain, Size: 3069 bytes --] On Tue, May 02, 2017 at 03:56:38PM +0300, Ed Bartosh wrote: > On Fri, Apr 21, 2017 at 02:11:41PM +0200, Andreas J. Reichel wrote: > > Handle exception if a file could not be deleted during clean-up of > > unwanted files, thus preventing a failure of wic in this case. > > Can you explain why partition images can't be deleted? > As wic creates them it's not obvious why it can't remove them. > As part of our internal project, we generate artifacts to be packaged in further archives and only contain one single partition image. The image generation process is solely done by wic and post-processing scripts, therefore we always used the .p2 file for the root partition, that had already peen patched (for example fstab by wic). Suddenly the file was missing and CI was not working anymore. Here, instead of just stating, that *nobody* needs these files, my idea was to introduce a parameter, so that people can chose if they want them or not. We do not want to generate any image artifacts with bitbake to keep things separated better. Just saving space is not a valid argument to me because building a system with bitbake needs up to 50 GB and if you do it for several machines, a few hundred MB should not matter. However, no problem for me to delete them per standard, if you say *most* people don' use them. > > Signed-off-by: Andreas Reichel <andreas.reichel.ext@siemens.com> > > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> > > Signed-off-by: Daniel Wagner <daniel.wagner@siemens.com> > > > > --- > > scripts/lib/wic/plugins/imager/direct.py | 7 ++++++- > > 1 file changed, 6 insertions(+), 1 deletion(-) > > > > diff --git a/scripts/lib/wic/plugins/imager/direct.py b/scripts/lib/wic/plugins/imager/direct.py > > index f2e6127331..d6b47ff0bb 100644 > > --- a/scripts/lib/wic/plugins/imager/direct.py > > +++ b/scripts/lib/wic/plugins/imager/direct.py > > @@ -541,7 +541,12 @@ class PartitionedImage(): > > def cleanup(self): > > # remove partition images > > for image in set(self.partimages): > > - os.remove(image) > > + try: > > + os.remove(image) > > + except IOError as e: > > + logger.warning( > > + "Could not delete file. {0}: I/O error ({1}): {2}\n".format( > > + image, e.errno, e.strerror)) > > > > def assemble(self): > > logger.debug("Installing partitions") > > -- > > 2.11.0 > > > > -- > > _______________________________________________ > > Openembedded-core mailing list > > Openembedded-core@lists.openembedded.org > > http://lists.openembedded.org/mailman/listinfo/openembedded-core > > -- > -- > Regards, > Ed -- Andreas Reichel Dipl.-Phys. (Univ.) Software Consultant Andreas.Reichel@tngtech.com +49-174-3180074 TNG Technology Consulting GmbH, Betastr. 13a, 85774 Unterföhring Geschäftsführer: Henrik Klagges, Christoph Stock, Dr. Robert Dahlke Sitz: Unterföhring * Amtsgericht München * HRB 135082 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 849 bytes --] ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [wic patch 1/5] wic: Catch errors during image files clean-up 2017-05-03 8:45 ` Andreas Reichel @ 2017-05-03 10:32 ` Ed Bartosh 0 siblings, 0 replies; 17+ messages in thread From: Ed Bartosh @ 2017-05-03 10:32 UTC (permalink / raw) To: Andreas Reichel Cc: Jan Kiszka, Andreas Reichel, Daniel Wagner, openembedded-core On Wed, May 03, 2017 at 10:45:52AM +0200, Andreas Reichel wrote: > On Tue, May 02, 2017 at 03:56:38PM +0300, Ed Bartosh wrote: > > On Fri, Apr 21, 2017 at 02:11:41PM +0200, Andreas J. Reichel wrote: > > > Handle exception if a file could not be deleted during clean-up of > > > unwanted files, thus preventing a failure of wic in this case. > > > > Can you explain why partition images can't be deleted? > > As wic creates them it's not obvious why it can't remove them. > > > As part of our internal project, we generate artifacts to be packaged > in further archives and only contain one single partition image. The > image generation process is solely done by wic and post-processing > scripts, therefore we always used the .p2 file for the root partition, > that had already peen patched (for example fstab by wic). Suddenly the > file was missing and CI was not working anymore. I'd suggest to investigate this further and find out why it disappeared. > Here, instead of > just stating, that *nobody* needs these files, my idea was to > introduce a parameter, so that people can chose if they want them or > not. This makes sense to do. However, it doesn't explain nor justify this particual change. > We do not want to generate any image artifacts with bitbake to keep > things separated better. > Just saving space is not a valid argument to me because building > a system with bitbake needs up to 50 GB and if you do it for several > machines, a few hundred MB should not matter. However, no problem for me > to delete them per standard, if you say *most* people don' use them. I'm ok with the option to preserve partition files. I don't understand the reason for this change though. It looks like a workaround to me. It's better to find a real reason and fix it. > > > Signed-off-by: Andreas Reichel <andreas.reichel.ext@siemens.com> > > > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> > > > Signed-off-by: Daniel Wagner <daniel.wagner@siemens.com> > > > > > > --- > > > scripts/lib/wic/plugins/imager/direct.py | 7 ++++++- > > > 1 file changed, 6 insertions(+), 1 deletion(-) > > > > > > diff --git a/scripts/lib/wic/plugins/imager/direct.py b/scripts/lib/wic/plugins/imager/direct.py > > > index f2e6127331..d6b47ff0bb 100644 > > > --- a/scripts/lib/wic/plugins/imager/direct.py > > > +++ b/scripts/lib/wic/plugins/imager/direct.py > > > @@ -541,7 +541,12 @@ class PartitionedImage(): > > > def cleanup(self): > > > # remove partition images > > > for image in set(self.partimages): > > > - os.remove(image) > > > + try: > > > + os.remove(image) > > > + except IOError as e: > > > + logger.warning( > > > + "Could not delete file. {0}: I/O error ({1}): {2}\n".format( > > > + image, e.errno, e.strerror)) > > > > > > def assemble(self): > > > logger.debug("Installing partitions") > > > -- > > > 2.11.0 > > > > > > -- > > > _______________________________________________ > > > Openembedded-core mailing list > > > Openembedded-core@lists.openembedded.org > > > http://lists.openembedded.org/mailman/listinfo/openembedded-core > > > > -- > > -- > > Regards, > > Ed > > -- > Andreas Reichel > Dipl.-Phys. (Univ.) > Software Consultant > > Andreas.Reichel@tngtech.com > +49-174-3180074 > > TNG Technology Consulting GmbH, Betastr. 13a, 85774 Unterföhring > Geschäftsführer: Henrik Klagges, Christoph Stock, Dr. Robert Dahlke > Sitz: Unterföhring * Amtsgericht München * HRB 135082 -- -- Regards, Ed ^ permalink raw reply [flat|nested] 17+ messages in thread
* [wic patch 2/5] wic: Use argparse instead of optparse 2017-04-21 12:11 [wic patch 0/5] Add option to wic and use argparse Andreas J. Reichel 2017-04-21 12:11 ` [wic patch 1/5] wic: Catch errors during image files clean-up Andreas J. Reichel @ 2017-04-21 12:11 ` Andreas J. Reichel 2017-04-23 19:58 ` Burton, Ross 2017-04-21 12:11 ` [wic patch 3/5] wic: Add missing text to usage and help strings Andreas J. Reichel ` (3 subsequent siblings) 5 siblings, 1 reply; 17+ messages in thread From: Andreas J. Reichel @ 2017-04-21 12:11 UTC (permalink / raw) To: openembedded-core; +Cc: Jan Kiszka, Daniel Wagner, Andreas Reichel * optparse is deprecated and will not be developed further (see: https://docs.python.org/2/library/optparse.html) * argparse supports subcommands, which simplifies definition of arguments and options * reimplement help mechanism through sub-subcommands Signed-off-by: Andreas Reichel <andreas.reichel.ext@siemens.com> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> Signed-off-by: Daniel Wagner <daniel.wagner@siemens.com> --- scripts/lib/wic/engine.py | 11 +-- scripts/lib/wic/help.py | 23 +++-- scripts/wic | 213 ++++++++++++++++++++++++++++------------------ 3 files changed, 154 insertions(+), 93 deletions(-) diff --git a/scripts/lib/wic/engine.py b/scripts/lib/wic/engine.py index f59821fea6..647358287f 100644 --- a/scripts/lib/wic/engine.py +++ b/scripts/lib/wic/engine.py @@ -201,17 +201,18 @@ def wic_list(args, scripts_path): """ Print the list of images or source plugins. """ - if len(args) < 1: + if args.list_type is None: return False - if args == ["images"]: + if args.list_type == "images": + list_canned_images(scripts_path) return True - elif args == ["source-plugins"]: + elif args.list_type == "source-plugins": list_source_plugins() return True - elif len(args) == 2 and args[1] == "help": - wks_file = args[0] + elif len(args.help_for) == 1 and args.help_for[0] == 'help': + wks_file = args.list_type fullpath = find_canned_image(scripts_path, wks_file) if not fullpath: raise WicError("No image named %s found, exiting. " diff --git a/scripts/lib/wic/help.py b/scripts/lib/wic/help.py index aee2451a72..2c2dbdc3b5 100644 --- a/scripts/lib/wic/help.py +++ b/scripts/lib/wic/help.py @@ -56,7 +56,7 @@ def wic_help(args, usage_str, subcommands): """ Subcommand help dispatcher. """ - if len(args) == 1 or not display_help(args[1], subcommands): + if args.help_topic == None or not display_help(args.help_topic, subcommands): print(usage_str) @@ -82,19 +82,20 @@ def invoke_subcommand(args, parser, main_command_usage, subcommands): Dispatch to subcommand handler borrowed from combo-layer. Should use argparse, but has to work in 2.6. """ - if not args: + if not args.command: logger.error("No subcommand specified, exiting") parser.print_help() return 1 - elif args[0] == "help": + elif args.command == "help": wic_help(args, main_command_usage, subcommands) - elif args[0] not in subcommands: - logger.error("Unsupported subcommand %s, exiting\n", args[0]) + elif args.command not in subcommands: + logger.error("Unsupported subcommand %s, exiting\n", args.command) parser.print_help() return 1 else: - usage = subcommands.get(args[0], subcommand_error)[1] - subcommands.get(args[0], subcommand_error)[0](args[1:], usage) + subcmd = subcommands.get(args.command, subcommand_error) + usage = subcmd[1] + subcmd[0](args, usage) ## @@ -795,3 +796,11 @@ DESCRIPTION .wks files. """ + +wic_help_help = """ +NAME + wic help - display a help topic + +DESCRIPTION + Specify a help topic to display it. Topics are shown above. +""" diff --git a/scripts/wic b/scripts/wic index a5f2dbfc6f..49cad869e2 100755 --- a/scripts/wic +++ b/scripts/wic @@ -33,7 +33,7 @@ __version__ = "0.2.0" # Python Standard Library modules import os import sys -import optparse +import argparse import logging from distutils import spawn @@ -85,66 +85,30 @@ def rootfs_dir_to_args(krootfs_dir): rootfs_dir += '='.join([key, val]) return rootfs_dir.strip() -def callback_rootfs_dir(option, opt, value, parser): - """ - Build a dict using --rootfs_dir connection=dir - """ - if not type(parser.values.rootfs_dir) is dict: - parser.values.rootfs_dir = dict() - if '=' in value: - (key, rootfs_dir) = value.split('=') - else: - key = 'ROOTFS_DIR' - rootfs_dir = value +class RootfsArgAction(argparse.Action): + def __init__(self, **kwargs): + super().__init__(**kwargs) + + def __call__(self, parser, namespace, value, option_string=None): + if not "rootfs_dir" in vars(namespace) or \ + not type(namespace.__dict__['rootfs_dir']) is dict: + namespace.__dict__['rootfs_dir'] = {} + + if '=' in value: + (key, rootfs_dir) = value.split('=') + else: + key = 'ROOTFS_DIR' + rootfs_dir = value - parser.values.rootfs_dir[key] = rootfs_dir + namespace.__dict__['rootfs_dir'][key] = rootfs_dir -def wic_create_subcommand(args, usage_str): + +def wic_create_subcommand(options, usage_str): """ Command-line handling for image creation. The real work is done by image.engine.wic_create() """ - parser = optparse.OptionParser(usage=usage_str) - - parser.add_option("-o", "--outdir", dest="outdir", default='.', - help="name of directory to create image in") - parser.add_option("-e", "--image-name", dest="image_name", - help="name of the image to use the artifacts from " - "e.g. core-image-sato") - parser.add_option("-r", "--rootfs-dir", dest="rootfs_dir", type="string", - action="callback", callback=callback_rootfs_dir, - help="path to the /rootfs dir to use as the " - ".wks rootfs source") - parser.add_option("-b", "--bootimg-dir", dest="bootimg_dir", - help="path to the dir containing the boot artifacts " - "(e.g. /EFI or /syslinux dirs) to use as the " - ".wks bootimg source") - parser.add_option("-k", "--kernel-dir", dest="kernel_dir", - help="path to the dir containing the kernel to use " - "in the .wks bootimg") - parser.add_option("-n", "--native-sysroot", dest="native_sysroot", - help="path to the native sysroot containing the tools " - "to use to build the image") - parser.add_option("-s", "--skip-build-check", dest="build_check", - action="store_false", default=True, help="skip the build check") - parser.add_option("-f", "--build-rootfs", action="store_true", help="build rootfs") - parser.add_option("-c", "--compress-with", choices=("gzip", "bzip2", "xz"), - dest='compressor', - help="compress image with specified compressor") - parser.add_option("-m", "--bmap", action="store_true", help="generate .bmap") - parser.add_option("-v", "--vars", dest='vars_dir', - help="directory with <image>.env files that store " - "bitbake variables") - parser.add_option("-D", "--debug", dest="debug", action="store_true", - default=False, help="output debug information") - - (options, args) = parser.parse_args(args) - - if len(args) != 1: - parser.print_help() - raise WicError("Wrong number of arguments, exiting") - if options.build_rootfs and not bitbake_main: raise WicError("Can't build rootfs as bitbake is not in the $PATH") @@ -206,14 +170,14 @@ def wic_create_subcommand(args, usage_str): raise WicError("Unable to find the location of the native " "tools sysroot to use") - wks_file = args[0] + wks_file = options.wks_file if not wks_file.endswith(".wks"): wks_file = engine.find_canned_image(scripts_path, wks_file) if not wks_file: raise WicError("No image named %s found, exiting. (Use 'wic list images' " "to list available images, or specify a fully-qualified OE " - "kickstart (.wks) filename)" % args[0]) + "kickstart (.wks) filename)" % options.wks_file) if not options.image_name: rootfs_dir = '' @@ -264,59 +228,146 @@ def wic_list_subcommand(args, usage_str): Command-line handling for listing available images. The real work is done by image.engine.wic_list() """ - parser = optparse.OptionParser(usage=usage_str) - args = parser.parse_args(args)[1] - if not engine.wic_list(args, scripts_path): - parser.print_help() raise WicError("Bad list arguments, exiting") -def wic_help_topic_subcommand(args, usage_str): +def wic_help_subcommand(args, usage_str): """ - Command-line handling for help-only 'subcommands'. This is - essentially a dummy command that doesn nothing but allow users to - use the existing subcommand infrastructure to display help on a - particular topic not attached to any particular subcommand. + Command-line handling for help subcommand to keep the current + structure of the function definitions. """ pass +def wic_help_topic_subcommand(usage_str, help_str): + """ + Display function for help 'sub-subcommands'. + """ + print(help_str) + return + + wic_help_topic_usage = """ """ -subcommands = { - "create": [wic_create_subcommand, - hlp.wic_create_usage, - hlp.wic_create_help], - "list": [wic_list_subcommand, - hlp.wic_list_usage, - hlp.wic_list_help], +helptopics = { "plugins": [wic_help_topic_subcommand, wic_help_topic_usage, - hlp.get_wic_plugins_help], + hlp.wic_plugins_help], "overview": [wic_help_topic_subcommand, wic_help_topic_usage, hlp.wic_overview_help], "kickstart": [wic_help_topic_subcommand, wic_help_topic_usage, hlp.wic_kickstart_help], + "create": [wic_help_topic_subcommand, + wic_help_topic_usage, + hlp.wic_create_help], + "list": [wic_help_topic_subcommand, + wic_help_topic_usage, + hlp.wic_list_help] } +def wic_init_parser_create(subparser): + subparser.add_argument("wks_file") + + subparser.add_argument("-o", "--outdir", dest="outdir", default='.', + help="name of directory to create image in") + subparser.add_argument("-e", "--image-name", dest="image_name", + help="name of the image to use the artifacts from " + "e.g. core-image-sato") + subparser.add_argument("-r", "--rootfs-dir", action=RootfsArgAction, + help="path to the /rootfs dir to use as the " + ".wks rootfs source") + subparser.add_argument("-b", "--bootimg-dir", dest="bootimg_dir", + help="path to the dir containing the boot artifacts " + "(e.g. /EFI or /syslinux dirs) to use as the " + ".wks bootimg source") + subparser.add_argument("-k", "--kernel-dir", dest="kernel_dir", + help="path to the dir containing the kernel to use " + "in the .wks bootimg") + subparser.add_argument("-n", "--native-sysroot", dest="native_sysroot", + help="path to the native sysroot containing the tools " + "to use to build the image") + subparser.add_argument("-s", "--skip-build-check", dest="build_check", + action="store_false", default=True, help="skip the build check") + subparser.add_argument("-f", "--build-rootfs", action="store_true", help="build rootfs") + subparser.add_argument("-c", "--compress-with", choices=("gzip", "bzip2", "xz"), + dest='compressor', + help="compress image with specified compressor") + subparser.add_argument("-m", "--bmap", action="store_true", help="generate .bmap") + subparser.add_argument("-v", "--vars", dest='vars_dir', + help="directory with <image>.env files that store " + "bitbake variables") + subparser.add_argument("-D", "--debug", dest="debug", action="store_true", + default=False, help="output debug information") + return + + +def wic_init_parser_list(subparser): + subparser.add_argument("list_type", + help="can be 'images' or 'source-plugins' " + "to obtain a list. " + "If value is a valid .wks image file") + subparser.add_argument("help_for", default=[], nargs='*', + help="If 'list_type' is a valid .wks image file " + "this value can be 'help' to show the help information " + "defined inside the .wks file") + return + + +def wic_init_parser_help(subparser): + helpparsers = subparser.add_subparsers(dest='help_topic', help=hlp.wic_usage) + for helptopic in helptopics: + helpparsers.add_parser(helptopic, help=helptopics[helptopic][2]) + return + + +subcommands = { + "create": [wic_create_subcommand, + hlp.wic_create_usage, + hlp.wic_create_help, + wic_init_parser_create], + "list": [wic_list_subcommand, + hlp.wic_list_usage, + hlp.wic_list_help, + wic_init_parser_list], + "help": [wic_help_subcommand, + wic_help_topic_usage, + hlp.wic_help_help, + wic_init_parser_help] +} + + +def init_parser(parser): + parser.add_argument("--version", action="version", + version="%(prog)s {version}".format(version=__version__)) + subparsers = parser.add_subparsers(dest='command', help=hlp.wic_usage) + for subcmd in subcommands: + subparser = subparsers.add_parser(subcmd, help=subcommands[subcmd][2]) + subcommands[subcmd][3](subparser) + + def main(argv): - parser = optparse.OptionParser(version="wic version %s" % __version__, - usage=hlp.wic_usage) + parser = argparse.ArgumentParser( + description="wic version %s" % __version__) - parser.disable_interspersed_args() + init_parser(parser) - args = parser.parse_args(argv)[1] + args = parser.parse_args(argv) - if len(args): - if args[0] == "help": - if len(args) == 1: + if "command" in vars(args): + if args.command == "help": + if args.help_topic is None: parser.print_help() - raise WicError("help command requires parameter") + print() + print("Please specify a help topic") + elif args.help_topic in helptopics: + hlpt = helptopics[args.help_topic] + hlpt[0](hlpt[1], hlpt[2]) + return 0 return hlp.invoke_subcommand(args, parser, hlp.wic_help_usage, subcommands) -- 2.11.0 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [wic patch 2/5] wic: Use argparse instead of optparse 2017-04-21 12:11 ` [wic patch 2/5] wic: Use argparse instead of optparse Andreas J. Reichel @ 2017-04-23 19:58 ` Burton, Ross 2017-04-26 10:34 ` Andreas Reichel 0 siblings, 1 reply; 17+ messages in thread From: Burton, Ross @ 2017-04-23 19:58 UTC (permalink / raw) To: Andreas J. Reichel; +Cc: Jan Kiszka, Andreas Reichel, Daniel Wagner, OE-core [-- Attachment #1: Type: text/plain, Size: 1006 bytes --] On 21 April 2017 at 13:11, Andreas J. Reichel <andreas.reichel@tngtech.com> wrote: > * optparse is deprecated and will not be developed further > (see: https://docs.python.org/2/library/optparse.html) > * argparse supports subcommands, which simplifies definition of > arguments and options > * reimplement help mechanism through sub-subcommands > This results in a behaviour change and breaks the selftest: FAIL [0.535s]: test_unsupported_subcommand (oeqa.selftest.wic.Wic) ---------------------------------------------------------------------- Traceback (most recent call last): File "/home/pokybuild/yocto-autobuilder/yocto-worker/nightly-oe-selftest/build/meta/lib/oeqa/utils/decorators.py", line 109, in wrapped_f return func(*args, **kwargs) File "/home/pokybuild/yocto-autobuilder/yocto-worker/nightly-oe-selftest/build/meta/lib/oeqa/selftest/wic.py", line 163, in test_unsupported_subcommand ignore_status=True).status) AssertionError: 1 != 2 Ross [-- Attachment #2: Type: text/html, Size: 1680 bytes --] ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [wic patch 2/5] wic: Use argparse instead of optparse 2017-04-23 19:58 ` Burton, Ross @ 2017-04-26 10:34 ` Andreas Reichel 2017-04-26 13:03 ` Burton, Ross 0 siblings, 1 reply; 17+ messages in thread From: Andreas Reichel @ 2017-04-26 10:34 UTC (permalink / raw) To: Burton, Ross; +Cc: Jan Kiszka, Andreas Reichel, Daniel Wagner, OE-core On Sun, Apr 23, 2017 at 08:58:44PM +0100, Burton, Ross wrote: > > On 21 April 2017 at 13:11, Andreas J. Reichel <andreas.reichel@tngtech.com> > wrote: > > * optparse is deprecated and will not be developed further > (see: https://docs.python.org/2/library/optparse.html) > * argparse supports subcommands, which simplifies definition of > arguments and options > * reimplement help mechanism through sub-subcommands > > > This results in a behaviour change and breaks the selftest: > > FAIL [0.535s]: test_unsupported_subcommand (oeqa.selftest.wic.Wic) > ---------------------------------------------------------------------- > Traceback (most recent call last): > File "/home/pokybuild/yocto-autobuilder/yocto-worker/nightly-oe-selftest/ > build/meta/lib/oeqa/utils/decorators.py", line 109, in wrapped_f > return func(*args, **kwargs) > File "/home/pokybuild/yocto-autobuilder/yocto-worker/nightly-oe-selftest/ > build/meta/lib/oeqa/selftest/wic.py", line 163, in test_unsupported_subcommand > ignore_status=True).status) > AssertionError: 1 != 2 > Dear Ross, I see three possible strategies in fixing this problem: - Change the code to keep current return values / behavior - Adapt the unit test to fit the new code - Write a less complex patch for just adding the option "--keep-partition-images" without migration to argparse Which way would you prefer it? Andreas > Ross -- Andreas Reichel Dipl.-Phys. (Univ.) Software Consultant Andreas.Reichel@tngtech.com +49-174-3180074 TNG Technology Consulting GmbH, Betastr. 13a, 85774 Unterföhring Geschäftsführer: Henrik Klagges, Christoph Stock, Dr. Robert Dahlke Sitz: Unterföhring * Amtsgericht München * HRB 135082 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [wic patch 2/5] wic: Use argparse instead of optparse 2017-04-26 10:34 ` Andreas Reichel @ 2017-04-26 13:03 ` Burton, Ross 0 siblings, 0 replies; 17+ messages in thread From: Burton, Ross @ 2017-04-26 13:03 UTC (permalink / raw) To: Andreas Reichel; +Cc: Jan Kiszka, Andreas Reichel, Daniel Wagner, OE-core [-- Attachment #1: Type: text/plain, Size: 526 bytes --] On 26 April 2017 at 11:34, Andreas Reichel <Andreas.Reichel@tngtech.com> wrote: > - Change the code to keep current return values / behavior > - Adapt the unit test to fit the new code > - Write a less complex patch for just adding the option > "--keep-partition-images" without migration to argparse > It looks like the exit 2 is deeply nested into the argparse code, so I guess the test should be updated. The test could be made more resilient by checking that the output wasn't 0 instead of was 1. Ross [-- Attachment #2: Type: text/html, Size: 1038 bytes --] ^ permalink raw reply [flat|nested] 17+ messages in thread
* [wic patch 3/5] wic: Add missing text to usage and help strings 2017-04-21 12:11 [wic patch 0/5] Add option to wic and use argparse Andreas J. Reichel 2017-04-21 12:11 ` [wic patch 1/5] wic: Catch errors during image files clean-up Andreas J. Reichel 2017-04-21 12:11 ` [wic patch 2/5] wic: Use argparse instead of optparse Andreas J. Reichel @ 2017-04-21 12:11 ` Andreas J. Reichel 2017-04-21 12:11 ` [wic patch 4/5] wic: Add option to keep partition images Andreas J. Reichel ` (2 subsequent siblings) 5 siblings, 0 replies; 17+ messages in thread From: Andreas J. Reichel @ 2017-04-21 12:11 UTC (permalink / raw) To: openembedded-core; +Cc: Jan Kiszka, Daniel Wagner, Andreas Reichel Add missing parameters -c to and remove non-existent parameter -i from usage string for 'wic create'. Signed-off-by: Andreas Reichel <andreas.reichel.ext@siemens.com> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> Signed-off-by: Daniel Wagner <daniel.wagner@siemens.com> --- scripts/lib/wic/help.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/lib/wic/help.py b/scripts/lib/wic/help.py index 2c2dbdc3b5..ba0c8e5eea 100644 --- a/scripts/lib/wic/help.py +++ b/scripts/lib/wic/help.py @@ -131,10 +131,10 @@ wic_create_usage = """ Create a new OpenEmbedded image usage: wic create <wks file or image name> [-o <DIRNAME> | --outdir <DIRNAME>] - [-i <JSON PROPERTY FILE> | --infile <JSON PROPERTY_FILE>] [-e | --image-name] [-s, --skip-build-check] [-D, --debug] [-r, --rootfs-dir] [-b, --bootimg-dir] [-k, --kernel-dir] [-n, --native-sysroot] [-f, --build-rootfs] + [-c, --compress-with] [-m, --bmap] This command creates an OpenEmbedded image based on the 'OE kickstart commands' found in the <wks file>. -- 2.11.0 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [wic patch 4/5] wic: Add option to keep partition images 2017-04-21 12:11 [wic patch 0/5] Add option to wic and use argparse Andreas J. Reichel ` (2 preceding siblings ...) 2017-04-21 12:11 ` [wic patch 3/5] wic: Add missing text to usage and help strings Andreas J. Reichel @ 2017-04-21 12:11 ` Andreas J. Reichel 2017-04-21 12:11 ` [wic patch 5/5] wic: Use enum like dicts for string constants Andreas J. Reichel 2017-05-02 14:37 ` [wic patch 0/5] Add option to wic and use argparse Ed Bartosh 5 siblings, 0 replies; 17+ messages in thread From: Andreas J. Reichel @ 2017-04-21 12:11 UTC (permalink / raw) To: openembedded-core; +Cc: Jan Kiszka, Daniel Wagner, Andreas Reichel Add new option -P, --keep-partition-images to prevent deletion of .p* files created by wic's direct imager plugin. Signed-off-by: Andreas Reichel <andreas.reichel.ext@siemens.com> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> Signed-off-by: Daniel Wagner <daniel.wagner@siemens.com> --- scripts/lib/wic/help.py | 5 +++++ scripts/lib/wic/plugins/imager/direct.py | 28 +++++++++++++++++++--------- scripts/wic | 2 ++ 3 files changed, 26 insertions(+), 9 deletions(-) diff --git a/scripts/lib/wic/help.py b/scripts/lib/wic/help.py index ba0c8e5eea..026a383155 100644 --- a/scripts/lib/wic/help.py +++ b/scripts/lib/wic/help.py @@ -134,6 +134,7 @@ wic_create_usage = """ [-e | --image-name] [-s, --skip-build-check] [-D, --debug] [-r, --rootfs-dir] [-b, --bootimg-dir] [-k, --kernel-dir] [-n, --native-sysroot] [-f, --build-rootfs] + [-P, --keep-partition-images] [-c, --compress-with] [-m, --bmap] This command creates an OpenEmbedded image based on the 'OE kickstart @@ -155,6 +156,7 @@ SYNOPSIS [-e | --image-name] [-s, --skip-build-check] [-D, --debug] [-r, --rootfs-dir] [-b, --bootimg-dir] [-k, --kernel-dir] [-n, --native-sysroot] [-f, --build-rootfs] + [-P, --keep-partition-images] [-c, --compress-with] [-m, --bmap] DESCRIPTION @@ -227,6 +229,9 @@ DESCRIPTION The -m option is used to produce .bmap file for the image. This file can be used to flash image using bmaptool utility. + + The -p option is used to keep the individual partition image files .p*, + created by the direct imager plugin. """ wic_list_usage = """ diff --git a/scripts/lib/wic/plugins/imager/direct.py b/scripts/lib/wic/plugins/imager/direct.py index d6b47ff0bb..b27584bdbe 100644 --- a/scripts/lib/wic/plugins/imager/direct.py +++ b/scripts/lib/wic/plugins/imager/direct.py @@ -84,8 +84,12 @@ class DirectPlugin(ImagerPlugin): break image_path = self._full_path(self.workdir, self.parts[0].disk, "direct") + self.keep_partimages = options.keep_partimages + self._image = PartitionedImage(image_path, self.ptable_format, - self.parts, self.native_sysroot) + self.parts, self.native_sysroot, + keep_partimages=self.keep_partimages) + def do_create(self): """ @@ -287,7 +291,9 @@ class PartitionedImage(): Partitioned image in a file. """ - def __init__(self, path, ptable_format, partitions, native_sysroot=None): + def __init__(self, path, ptable_format, partitions, native_sysroot=None, + keep_partimages=False): + self.path = path # Path to the image file self.numpart = 0 # Number of allocated partitions self.realpart = 0 # Number of partitions in the partition table @@ -325,6 +331,8 @@ class PartitionedImage(): else: # msdos partition table part.uuid = '%08x-%02d' % (self.identifier, part.realnum) + self.keep_partimages = keep_partimages + def prepare(self, imager): """Prepare an image. Call prepare method of all image partitions.""" for part in self.partitions: @@ -540,13 +548,15 @@ class PartitionedImage(): def cleanup(self): # remove partition images - for image in set(self.partimages): - try: - os.remove(image) - except IOError as e: - logger.warning( - "Could not delete file. {0}: I/O error ({1}): {2}\n".format( - image, e.errno, e.strerror)) + if not self.keep_partimages: + # remove partition images + for image in set(self.partimages): + try: + os.remove(image) + except IOError as e: + logger.warning( + "Could not delete file. {0}: I/O error ({1}): {2}\n".format( + image, e.errno, e.strerror)) def assemble(self): logger.debug("Installing partitions") diff --git a/scripts/wic b/scripts/wic index 49cad869e2..98f7405702 100755 --- a/scripts/wic +++ b/scripts/wic @@ -303,6 +303,8 @@ def wic_init_parser_create(subparser): "bitbake variables") subparser.add_argument("-D", "--debug", dest="debug", action="store_true", default=False, help="output debug information") + subparser.add_argument("-P", "--keep-partition-images", dest="keep_partimages", + action="store_true", default=False) return -- 2.11.0 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [wic patch 5/5] wic: Use enum like dicts for string constants 2017-04-21 12:11 [wic patch 0/5] Add option to wic and use argparse Andreas J. Reichel ` (3 preceding siblings ...) 2017-04-21 12:11 ` [wic patch 4/5] wic: Add option to keep partition images Andreas J. Reichel @ 2017-04-21 12:11 ` Andreas J. Reichel 2017-05-02 13:36 ` Ed Bartosh 2017-05-02 14:37 ` [wic patch 0/5] Add option to wic and use argparse Ed Bartosh 5 siblings, 1 reply; 17+ messages in thread From: Andreas J. Reichel @ 2017-04-21 12:11 UTC (permalink / raw) To: openembedded-core; +Cc: Jan Kiszka, Daniel Wagner, Andreas Reichel To increase code maintainability, use dictionaries as enum-like container for parameter string comparisons. Signed-off-by: Andreas Reichel <andreas.reichel.ext@siemens.com> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> Signed-off-by: Daniel Wagner <daniel.wagner@siemens.com> --- scripts/lib/wic/engine.py | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/scripts/lib/wic/engine.py b/scripts/lib/wic/engine.py index 647358287f..1428a73ba8 100644 --- a/scripts/lib/wic/engine.py +++ b/scripts/lib/wic/engine.py @@ -35,6 +35,16 @@ from wic import WicError from wic.pluginbase import PluginMgr from wic.utils.misc import get_bitbake_var +class StrEnum(dict): + __getattr__ = dict.get + +ListType = StrEnum({ + 'LIST_IMAGES': "images", + 'LIST_SRC_PLUGINS': "source-plugins" }) + +HelpArg = StrEnum({ + 'HELP': "help" }) + logger = logging.getLogger('wic') def verify_build_env(): @@ -204,14 +214,14 @@ def wic_list(args, scripts_path): if args.list_type is None: return False - if args.list_type == "images": + if args.list_type == ListType.LIST_IMAGES: list_canned_images(scripts_path) return True - elif args.list_type == "source-plugins": + elif args.list_type == ListType.LIST_SRC_PLUGINS: list_source_plugins() return True - elif len(args.help_for) == 1 and args.help_for[0] == 'help': + elif len(args.help_for) == 1 and args.help_for[0] == HelpArg.HELP: wks_file = args.list_type fullpath = find_canned_image(scripts_path, wks_file) if not fullpath: -- 2.11.0 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [wic patch 5/5] wic: Use enum like dicts for string constants 2017-04-21 12:11 ` [wic patch 5/5] wic: Use enum like dicts for string constants Andreas J. Reichel @ 2017-05-02 13:36 ` Ed Bartosh 2017-05-03 8:47 ` Andreas Reichel 0 siblings, 1 reply; 17+ messages in thread From: Ed Bartosh @ 2017-05-02 13:36 UTC (permalink / raw) To: Andreas J. Reichel Cc: Jan Kiszka, Andreas Reichel, Daniel Wagner, openembedded-core On Fri, Apr 21, 2017 at 02:11:45PM +0200, Andreas J. Reichel wrote: > To increase code maintainability, use dictionaries > as enum-like container for parameter string comparisons. > > Signed-off-by: Andreas Reichel <andreas.reichel.ext@siemens.com> > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> > Signed-off-by: Daniel Wagner <daniel.wagner@siemens.com> > > --- > scripts/lib/wic/engine.py | 16 +++++++++++++--- > 1 file changed, 13 insertions(+), 3 deletions(-) > > diff --git a/scripts/lib/wic/engine.py b/scripts/lib/wic/engine.py > index 647358287f..1428a73ba8 100644 > --- a/scripts/lib/wic/engine.py > +++ b/scripts/lib/wic/engine.py > @@ -35,6 +35,16 @@ from wic import WicError > from wic.pluginbase import PluginMgr > from wic.utils.misc import get_bitbake_var > > +class StrEnum(dict): > + __getattr__ = dict.get > + > +ListType = StrEnum({ > + 'LIST_IMAGES': "images", > + 'LIST_SRC_PLUGINS': "source-plugins" }) > + > +HelpArg = StrEnum({ > + 'HELP': "help" }) > + > logger = logging.getLogger('wic') > > def verify_build_env(): > @@ -204,14 +214,14 @@ def wic_list(args, scripts_path): > if args.list_type is None: > return False > > - if args.list_type == "images": > + if args.list_type == ListType.LIST_IMAGES: > > list_canned_images(scripts_path) > return True > - elif args.list_type == "source-plugins": > + elif args.list_type == ListType.LIST_SRC_PLUGINS: > list_source_plugins() > return True > - elif len(args.help_for) == 1 and args.help_for[0] == 'help': > + elif len(args.help_for) == 1 and args.help_for[0] == HelpArg.HELP: > wks_file = args.list_type > fullpath = find_canned_image(scripts_path, wks_file) > if not fullpath: I'm not sure if this increases maintainability, but it definitely increases code complexity and decreases readability. Can you explain the idea in a bit more detailed way? -- Regards, Ed ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [wic patch 5/5] wic: Use enum like dicts for string constants 2017-05-02 13:36 ` Ed Bartosh @ 2017-05-03 8:47 ` Andreas Reichel 2017-05-03 12:18 ` Ed Bartosh 0 siblings, 1 reply; 17+ messages in thread From: Andreas Reichel @ 2017-05-03 8:47 UTC (permalink / raw) To: Ed Bartosh; +Cc: Jan Kiszka, Andreas Reichel, Daniel Wagner, openembedded-core [-- Attachment #1: message --] [-- Type: text/plain, Size: 2816 bytes --] On Tue, May 02, 2017 at 04:36:22PM +0300, Ed Bartosh wrote: > On Fri, Apr 21, 2017 at 02:11:45PM +0200, Andreas J. Reichel wrote: > > To increase code maintainability, use dictionaries > > as enum-like container for parameter string comparisons. > > > > Signed-off-by: Andreas Reichel <andreas.reichel.ext@siemens.com> > > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> > > Signed-off-by: Daniel Wagner <daniel.wagner@siemens.com> > > > > --- > > scripts/lib/wic/engine.py | 16 +++++++++++++--- > > 1 file changed, 13 insertions(+), 3 deletions(-) > > > > diff --git a/scripts/lib/wic/engine.py b/scripts/lib/wic/engine.py > > index 647358287f..1428a73ba8 100644 > > --- a/scripts/lib/wic/engine.py > > +++ b/scripts/lib/wic/engine.py > > @@ -35,6 +35,16 @@ from wic import WicError > > from wic.pluginbase import PluginMgr > > from wic.utils.misc import get_bitbake_var > > > > +class StrEnum(dict): > > + __getattr__ = dict.get > > + > > +ListType = StrEnum({ > > + 'LIST_IMAGES': "images", > > + 'LIST_SRC_PLUGINS': "source-plugins" }) > > + > > +HelpArg = StrEnum({ > > + 'HELP': "help" }) > > + > > logger = logging.getLogger('wic') > > > > def verify_build_env(): > > @@ -204,14 +214,14 @@ def wic_list(args, scripts_path): > > if args.list_type is None: > > return False > > > > - if args.list_type == "images": > > + if args.list_type == ListType.LIST_IMAGES: > > > > list_canned_images(scripts_path) > > return True > > - elif args.list_type == "source-plugins": > > + elif args.list_type == ListType.LIST_SRC_PLUGINS: > > list_source_plugins() > > return True > > - elif len(args.help_for) == 1 and args.help_for[0] == 'help': > > + elif len(args.help_for) == 1 and args.help_for[0] == HelpArg.HELP: > > wks_file = args.list_type > > fullpath = find_canned_image(scripts_path, wks_file) > > if not fullpath: > > I'm not sure if this increases maintainability, but it definitely increases code complexity and > decreases readability. Can you explain the idea in a bit more detailed way? > In general, if string constants are used as keys, it is better do define them as constants. Maintainability increases because if you have to change them, you only change them in one place. You have a central place for this definition and not scattered them all over the code. That's the idea. > -- > Regards, > Ed -- Andreas Reichel Dipl.-Phys. (Univ.) Software Consultant Andreas.Reichel@tngtech.com +49-174-3180074 TNG Technology Consulting GmbH, Betastr. 13a, 85774 Unterföhring Geschäftsführer: Henrik Klagges, Christoph Stock, Dr. Robert Dahlke Sitz: Unterföhring * Amtsgericht München * HRB 135082 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 849 bytes --] ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [wic patch 5/5] wic: Use enum like dicts for string constants 2017-05-03 8:47 ` Andreas Reichel @ 2017-05-03 12:18 ` Ed Bartosh 0 siblings, 0 replies; 17+ messages in thread From: Ed Bartosh @ 2017-05-03 12:18 UTC (permalink / raw) To: Andreas Reichel Cc: Jan Kiszka, Andreas Reichel, Daniel Wagner, openembedded-core On Wed, May 03, 2017 at 10:47:45AM +0200, Andreas Reichel wrote: > On Tue, May 02, 2017 at 04:36:22PM +0300, Ed Bartosh wrote: > > On Fri, Apr 21, 2017 at 02:11:45PM +0200, Andreas J. Reichel wrote: > > > To increase code maintainability, use dictionaries > > > as enum-like container for parameter string comparisons. > > > > > > Signed-off-by: Andreas Reichel <andreas.reichel.ext@siemens.com> > > > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> > > > Signed-off-by: Daniel Wagner <daniel.wagner@siemens.com> > > > > > > --- > > > scripts/lib/wic/engine.py | 16 +++++++++++++--- > > > 1 file changed, 13 insertions(+), 3 deletions(-) > > > > > > diff --git a/scripts/lib/wic/engine.py b/scripts/lib/wic/engine.py > > > index 647358287f..1428a73ba8 100644 > > > --- a/scripts/lib/wic/engine.py > > > +++ b/scripts/lib/wic/engine.py > > > @@ -35,6 +35,16 @@ from wic import WicError > > > from wic.pluginbase import PluginMgr > > > from wic.utils.misc import get_bitbake_var > > > > > > +class StrEnum(dict): > > > + __getattr__ = dict.get > > > + > > > +ListType = StrEnum({ > > > + 'LIST_IMAGES': "images", > > > + 'LIST_SRC_PLUGINS': "source-plugins" }) > > > + > > > +HelpArg = StrEnum({ > > > + 'HELP': "help" }) > > > + > > > logger = logging.getLogger('wic') > > > > > > def verify_build_env(): > > > @@ -204,14 +214,14 @@ def wic_list(args, scripts_path): > > > if args.list_type is None: > > > return False > > > > > > - if args.list_type == "images": > > > + if args.list_type == ListType.LIST_IMAGES: > > > > > > list_canned_images(scripts_path) > > > return True > > > - elif args.list_type == "source-plugins": > > > + elif args.list_type == ListType.LIST_SRC_PLUGINS: > > > list_source_plugins() > > > return True > > > - elif len(args.help_for) == 1 and args.help_for[0] == 'help': > > > + elif len(args.help_for) == 1 and args.help_for[0] == HelpArg.HELP: > > > wks_file = args.list_type > > > fullpath = find_canned_image(scripts_path, wks_file) > > > if not fullpath: > > > > I'm not sure if this increases maintainability, but it definitely increases code complexity and > > decreases readability. Can you explain the idea in a bit more detailed way? > > > In general, if string constants are used as keys, it is better do define them as > constants. Maintainability increases because if you have to change them, > you only change them in one place. You have a central place for this > definition and not scattered them all over the code. That's the idea. > Not sure I'm convinced. From my point of view this patch changes quite simple and readable code to something more complex and less readable for no visible reason. If I have to change 'images' or 'source-plugins'(why would I want to do that?) I'd just go and change them in one place as they're not used anywhere else. -- Regards, Ed ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [wic patch 0/5] Add option to wic and use argparse 2017-04-21 12:11 [wic patch 0/5] Add option to wic and use argparse Andreas J. Reichel ` (4 preceding siblings ...) 2017-04-21 12:11 ` [wic patch 5/5] wic: Use enum like dicts for string constants Andreas J. Reichel @ 2017-05-02 14:37 ` Ed Bartosh 2017-05-03 8:49 ` Andreas Reichel 5 siblings, 1 reply; 17+ messages in thread From: Ed Bartosh @ 2017-05-02 14:37 UTC (permalink / raw) To: Andreas J. Reichel Cc: Jan Kiszka, Andreas Reichel, Daniel Wagner, openembedded-core On Fri, Apr 21, 2017 at 02:11:40PM +0200, Andreas J. Reichel wrote: > * wic uses optparse and a rather complicated approach of its > initialization. It is much easier to switch from the > deprecated optparse to the argparse library, which also > supports sub-commands, which wic heavily makes use of. > The second patch replaces optparse by argparse. Thank you for the patchset! I'm going to fix failing test case and test argparse-related patches as a separate patchset. Feel free to update and re-send the rest of the patches. -- Regards, Ed ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [wic patch 0/5] Add option to wic and use argparse 2017-05-02 14:37 ` [wic patch 0/5] Add option to wic and use argparse Ed Bartosh @ 2017-05-03 8:49 ` Andreas Reichel 0 siblings, 0 replies; 17+ messages in thread From: Andreas Reichel @ 2017-05-03 8:49 UTC (permalink / raw) To: Ed Bartosh; +Cc: Jan Kiszka, Andreas Reichel, Daniel Wagner, openembedded-core [-- Attachment #1: message --] [-- Type: text/plain, Size: 1115 bytes --] On Tue, May 02, 2017 at 05:37:31PM +0300, Ed Bartosh wrote: > On Fri, Apr 21, 2017 at 02:11:40PM +0200, Andreas J. Reichel wrote: > > * wic uses optparse and a rather complicated approach of its > > initialization. It is much easier to switch from the > > deprecated optparse to the argparse library, which also > > supports sub-commands, which wic heavily makes use of. > > The second patch replaces optparse by argparse. > > Thank you for the patchset! > > I'm going to fix failing test case and test argparse-related patches > as a separate patchset. > > Feel free to update and re-send the rest of the patches. > Thank you. So then I wait until argparse is successfully integrated and rebase the option-patch. Andreas > -- > Regards, > Ed -- Andreas Reichel Dipl.-Phys. (Univ.) Software Consultant Andreas.Reichel@tngtech.com +49-174-3180074 TNG Technology Consulting GmbH, Betastr. 13a, 85774 Unterföhring Geschäftsführer: Henrik Klagges, Christoph Stock, Dr. Robert Dahlke Sitz: Unterföhring * Amtsgericht München * HRB 135082 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 849 bytes --] ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2017-05-03 12:31 UTC | newest] Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-04-21 12:11 [wic patch 0/5] Add option to wic and use argparse Andreas J. Reichel 2017-04-21 12:11 ` [wic patch 1/5] wic: Catch errors during image files clean-up Andreas J. Reichel 2017-05-02 12:56 ` Ed Bartosh 2017-05-03 8:45 ` Andreas Reichel 2017-05-03 10:32 ` Ed Bartosh 2017-04-21 12:11 ` [wic patch 2/5] wic: Use argparse instead of optparse Andreas J. Reichel 2017-04-23 19:58 ` Burton, Ross 2017-04-26 10:34 ` Andreas Reichel 2017-04-26 13:03 ` Burton, Ross 2017-04-21 12:11 ` [wic patch 3/5] wic: Add missing text to usage and help strings Andreas J. Reichel 2017-04-21 12:11 ` [wic patch 4/5] wic: Add option to keep partition images Andreas J. Reichel 2017-04-21 12:11 ` [wic patch 5/5] wic: Use enum like dicts for string constants Andreas J. Reichel 2017-05-02 13:36 ` Ed Bartosh 2017-05-03 8:47 ` Andreas Reichel 2017-05-03 12:18 ` Ed Bartosh 2017-05-02 14:37 ` [wic patch 0/5] Add option to wic and use argparse Ed Bartosh 2017-05-03 8:49 ` Andreas Reichel
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.