From: Mark Hatle <mark.hatle@windriver.com>
To: <openembedded-core@lists.openembedded.org>
Subject: Re: [PATCH] python-smartpm: improve error reporting
Date: Tue, 18 Dec 2012 12:42:00 -0600 [thread overview]
Message-ID: <50D0B8F8.60906@windriver.com> (raw)
In-Reply-To: <1355758664-912-1-git-send-email-bogdan.a.marinescu@intel.com>
On 12/17/12 9:37 AM, Bogdan Marinescu wrote:
> Add code to check proper command line arguments for various
> smart commands. Exit with error if erroneous/additional arguments
> are given in the command line.
>
> Signed-off-by: Bogdan Marinescu <bogdan.a.marinescu@intel.com>
I've reviewed this. It looks like a good addition to 'smart'.
Signed-off-by: Mark Hatle <mark.hatle@windriver.com>
--Mark
> ---
> .../smart-improve-error-reporting.patch | 253 ++++++++++++++++++++
> .../python/python-smartpm_1.4.1.bb | 3 +-
> 2 files changed, 255 insertions(+), 1 deletion(-)
> create mode 100644 meta/recipes-devtools/python/python-smartpm/smart-improve-error-reporting.patch
>
> diff --git a/meta/recipes-devtools/python/python-smartpm/smart-improve-error-reporting.patch b/meta/recipes-devtools/python/python-smartpm/smart-improve-error-reporting.patch
> new file mode 100644
> index 0000000..fece5b9
> --- /dev/null
> +++ b/meta/recipes-devtools/python/python-smartpm/smart-improve-error-reporting.patch
> @@ -0,0 +1,253 @@
> +Improve error reporting in smart
> +
> +Add code to check proper command line arguments for various
> +smart commands. Exit with error if erroneous/additional arguments
> +are given in the command line.
> +
> +Upstream-Status: Pending
> +
> +Signed-off-by: Bogdan Marinescu <bogdan.a.marinescu@intel.com>
> +
> +diff --git a/smart/commands/channel.py b/smart/commands/channel.py
> +index aa76f91..63fbb35 100644
> +--- a/smart/commands/channel.py
> ++++ b/smart/commands/channel.py
> +@@ -157,7 +157,17 @@ def main(ctrl, opts):
> + opts.show is None and opts.yaml is None):
> + iface.warning(_("Can't edit channels information."))
> + raise Error, _("Configuration is in readonly mode.")
> +-
> ++
> ++ # Argument check
> ++ opts.check_args_of_option("set", -1)
> ++ opts.check_args_of_option("remove", -1)
> ++ opts.check_args_of_option("edit", 0)
> ++ opts.check_args_of_option("enable", -1)
> ++ opts.check_args_of_option("disable", -1)
> ++ opts.ensure_action("channel", ["add", "set", "remove", "remove-all",
> ++ "list", "show", "yaml", "enable", "disable"])
> ++ opts.check_remaining_args()
> ++
> + if opts.add is not None:
> + if not opts.add and opts.args == ["-"]:
> + newchannels = []
> +diff --git a/smart/commands/check.py b/smart/commands/check.py
> +index b08608a..506e852 100644
> +--- a/smart/commands/check.py
> ++++ b/smart/commands/check.py
> +@@ -72,6 +72,9 @@ def parse_options(argv):
> +
> + def main(ctrl, opts, reloadchannels=True):
> +
> ++ # Argument check
> ++ opts.check_args_of_option("channels", 1)
> ++
> + if sysconf.get("auto-update"):
> + from smart.commands import update
> + updateopts = update.parse_options([])
> +diff --git a/smart/commands/config.py b/smart/commands/config.py
> +index dd50dee..4fe4366 100644
> +--- a/smart/commands/config.py
> ++++ b/smart/commands/config.py
> +@@ -80,6 +80,12 @@ def main(ctrl, opts):
> + globals["false"] = False
> + globals["no"] = False
> +
> ++ # Check arguments
> ++ opts.check_args_of_option("set", -1)
> ++ opts.check_args_of_option("remove", -1)
> ++ opts.ensure_action("config", ["set", "show", "yaml", "remove"])
> ++ opts.check_remaining_args()
> ++
> + if opts.set:
> + for opt in opts.set:
> + m = SETRE.match(opt)
> +diff --git a/smart/commands/download.py b/smart/commands/download.py
> +index 6837993..b853c61 100644
> +--- a/smart/commands/download.py
> ++++ b/smart/commands/download.py
> +@@ -81,6 +81,14 @@ def parse_options(argv):
> +
> + def main(ctrl, opts):
> +
> ++ # Argument check
> ++ opts.check_args_of_option("target", 1)
> ++ opts.check_args_of_option("output", 1)
> ++ opts.check_args_of_option("from_urls", -1)
> ++ opts.check_args_of_option("from_metalink", -1)
> ++ if not opts.args and not opts.from_metalink and not opts.from_urls:
> ++ raise Error, _("no package(s) given")
> ++
> + packages = []
> + if opts.args:
> + if sysconf.get("auto-update"):
> +diff --git a/smart/commands/info.py b/smart/commands/info.py
> +index 12f74f0..59fbe98 100644
> +--- a/smart/commands/info.py
> ++++ b/smart/commands/info.py
> +@@ -58,6 +58,10 @@ def parse_options(argv):
> +
> + def main(ctrl, opts, reloadchannels=True):
> +
> ++ # Argument check
> ++ if not opts.args:
> ++ raise Error, _("No package(s) given")
> ++
> + if sysconf.get("auto-update"):
> + from smart.commands import update
> + updateopts = update.parse_options([])
> +diff --git a/smart/commands/install.py b/smart/commands/install.py
> +index 8a45954..590222c 100644
> +--- a/smart/commands/install.py
> ++++ b/smart/commands/install.py
> +@@ -76,6 +76,10 @@ def parse_options(argv):
> +
> + def main(ctrl, opts):
> +
> ++ # Argument check
> ++ if not opts.args:
> ++ raise Error, _("no package(s) given")
> ++
> + if opts.explain:
> + sysconf.set("explain-changesets", True, soft=True)
> +
> +diff --git a/smart/commands/reinstall.py b/smart/commands/reinstall.py
> +index e59d896..32da3e6 100644
> +--- a/smart/commands/reinstall.py
> ++++ b/smart/commands/reinstall.py
> +@@ -68,7 +68,11 @@ def parse_options(argv):
> + return opts
> +
> + def main(ctrl, opts):
> +-
> ++
> ++ # Argument check
> ++ if not opts.args:
> ++ raise Error, _("no package(s) given")
> ++
> + if opts.explain:
> + sysconf.set("explain-changesets", True, soft=True)
> +
> +diff --git a/smart/commands/remove.py b/smart/commands/remove.py
> +index b4823a6..acd3bbd 100644
> +--- a/smart/commands/remove.py
> ++++ b/smart/commands/remove.py
> +@@ -74,6 +74,10 @@ def parse_options(argv):
> +
> + def main(ctrl, opts):
> +
> ++ # Argument check
> ++ if not opts.args:
> ++ raise Error, _("no package(s) given")
> ++
> + if opts.explain:
> + sysconf.set("explain-changesets", True, soft=True)
> +
> +diff --git a/smart/commands/search.py b/smart/commands/search.py
> +index 0d0b573..44806b8 100644
> +--- a/smart/commands/search.py
> ++++ b/smart/commands/search.py
> +@@ -44,6 +44,8 @@ def option_parser():
> + def parse_options(argv):
> + opts = query.parse_options(argv, usage=USAGE, \
> + description=DESCRIPTION, examples=EXAMPLES)
> ++ if not argv:
> ++ raise Error, _("Search expression not specified")
> + opts.name = opts.args
> + opts.summary = opts.args
> + opts.description = opts.args
> +diff --git a/smart/commands/upgrade.py b/smart/commands/upgrade.py
> +index ec86290..7e290d8 100644
> +--- a/smart/commands/upgrade.py
> ++++ b/smart/commands/upgrade.py
> +@@ -91,6 +91,9 @@ def parse_options(argv):
> +
> + def main(ctrl, opts):
> +
> ++ # Argument check
> ++ opts.check_args_of_option("flag", 1)
> ++
> + if opts.explain:
> + sysconf.set("explain-changesets", True, soft=True)
> +
> +diff --git a/smart/util/optparse.py b/smart/util/optparse.py
> +index 4a3d3a8..279b0bf 100644
> +--- a/smart/util/optparse.py
> ++++ b/smart/util/optparse.py
> +@@ -70,6 +70,8 @@ import sys, os
> + import types
> + import textwrap
> + from gettext import gettext as _
> ++from smart import Error
> ++import re
> +
> + def _repr(self):
> + return "<%s at 0x%x: %s>" % (self.__class__.__name__, id(self), self)
> +@@ -708,6 +710,12 @@ class Option:
> + self.action, self.dest, opt, value, values, parser)
> +
> + def take_action(self, action, dest, opt, value, values, parser):
> ++ # Keep all the options in the command line in the '_given_opts' array
> ++ # This will be used later to validate the command line
> ++ given_opts = getattr(parser.values, "_given_opts", [])
> ++ user_opt = re.sub(r"^\-*", "", opt).replace("-", "_")
> ++ given_opts.append(user_opt)
> ++ setattr(parser.values, "_given_opts", given_opts)
> + if action == "store":
> + setattr(values, dest, value)
> + elif action == "store_const":
> +@@ -819,6 +827,54 @@ class Values:
> + setattr(self, attr, value)
> + return getattr(self, attr)
> +
> ++ # Check if the given option has the specified number of arguments
> ++ # Raise an error if the option has an invalid number of arguments
> ++ # A negative number for 'nargs' means "at least |nargs| arguments are needed"
> ++ def check_args_of_option(self, opt, nargs, err=None):
> ++ given_opts = getattr(self, "_given_opts", [])
> ++ if not opt in given_opts:
> ++ return
> ++ values = getattr(self, opt, [])
> ++ if type(values) != type([]):
> ++ return
> ++ if nargs < 0:
> ++ nargs = -nargs
> ++ if len(values) >= nargs:
> ++ return
> ++ if not err:
> ++ if nargs == 1:
> ++ err = _("Option '%s' requires at least one argument") % opt
> ++ else:
> ++ err = _("Option '%s' requires at least %d arguments") % (opt, nargs)
> ++ raise Error, err
> ++ elif nargs == 0:
> ++ if len( values ) == 0:
> ++ return
> ++ raise Error, err
> ++ else:
> ++ if len(values) == nargs:
> ++ return
> ++ if not err:
> ++ if nargs == 1:
> ++ err = _("Option '%s' requires one argument") % opt
> ++ else:
> ++ err = _("Option '%s' requires %d arguments") % (opt, nargs)
> ++ raise Error, err
> ++
> ++ # Check that at least one of the options in 'actlist' was given as an argument
> ++ # to the command 'cmdname'
> ++ def ensure_action(self, cmdname, actlist):
> ++ given_opts = getattr(self, "_given_opts", [])
> ++ for action in actlist:
> ++ if action in given_opts:
> ++ return
> ++ raise Error, _("No action specified for command '%s'") % cmdname
> ++
> ++ # Check if there are any other arguments left after parsing the command line and
> ++ # raise an error if such arguments are found
> ++ def check_remaining_args(self):
> ++ if self.args:
> ++ raise Error, _("Invalid argument(s) '%s'" % str(self.args))
> +
> + class OptionContainer:
> +
> diff --git a/meta/recipes-devtools/python/python-smartpm_1.4.1.bb b/meta/recipes-devtools/python/python-smartpm_1.4.1.bb
> index 53f232b..ce1f435 100644
> --- a/meta/recipes-devtools/python/python-smartpm_1.4.1.bb
> +++ b/meta/recipes-devtools/python/python-smartpm_1.4.1.bb
> @@ -11,7 +11,7 @@ LICENSE = "GPLv2"
> LIC_FILES_CHKSUM = "file://LICENSE;md5=393a5ca445f6965873eca0259a17f833"
>
> DEPENDS = "python rpm"
> -PR = "r4"
> +PR = "r5"
> SRCNAME = "smart"
>
> SRC_URI = "\
> @@ -24,6 +24,7 @@ SRC_URI = "\
> file://smart-rpm-md-parse.patch \
> file://smart-tmpdir.patch \
> file://smart-metadata-match.patch \
> + file://smart-improve-error-reporting.patch \
> "
>
> SRC_URI[md5sum] = "573ef32ba177a6b3c4bf7ef04873fcb6"
>
prev parent reply other threads:[~2012-12-18 18:56 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-12-17 15:37 [PATCH] python-smartpm: improve error reporting Bogdan Marinescu
2012-12-18 18:42 ` Mark Hatle [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=50D0B8F8.60906@windriver.com \
--to=mark.hatle@windriver.com \
--cc=openembedded-core@lists.openembedded.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.