From 6c005cb0ad278a5edf2387752a5207335426f2ab Mon Sep 17 00:00:00 2001 From: Markus Lehtonen Date: Thu, 18 Jan 2018 11:04:19 +0200 Subject: [PATCH] pq: use argparse subparsers Rework pq and pq-rpm commands to utilize the subparsers (or subcommands) functionality of Python argparse. This changes the calling convention slightly: now the action (export, import, ...) must be the first argument - all other cmdline arguments/options must be given after that. E.g. it is not possible to do 'gbp pq -v export' anymore, but, one must use 'gbp pq export -v'. Signed-off-by: Markus Lehtonen --- gbp/config.py | 12 +++ gbp/scripts/pq.py | 131 ++++++++++++++-------------- gbp/scripts/pq_rpm.py | 133 +++++++++++++++-------------- tests/component/rpm/test_pq_rpm.py | 33 +++---- 4 files changed, 163 insertions(+), 146 deletions(-) diff --git a/gbp/config.py b/gbp/config.py index 9cd1aedd..0d23d333 100644 --- a/gbp/config.py +++ b/gbp/config.py @@ -690,6 +690,18 @@ def add_mutually_exclusive_group(self, *args, **kwargs): return self._wrap_generator('add_mutually_exclusive_group', *args, **kwargs) + def add_subparsers(self, *args, **kwargs): + """Add subparsers""" + return self._wrap_generator('add_subparsers', *args, **kwargs) + + def add_parser(self, *args, **kwargs): + """Add parser. Only valid for subparser instances!""" + if 'parents' in kwargs: + for parser in kwargs['parents']: + self.conf_file_args.update(parser.conf_file_args) + return self._wrap_generator('add_parser', + *args, **kwargs) + def __getattr__(self, name): return self.wrapped.__getattribute__(name) diff --git a/gbp/scripts/pq.py b/gbp/scripts/pq.py index be99b6aa..21f8a468 100755 --- a/gbp/scripts/pq.py +++ b/gbp/scripts/pq.py @@ -23,7 +23,6 @@ import sys import tempfile import re -from argparse import ArgumentDefaultsHelpFormatter, RawDescriptionHelpFormatter from gbp.config import GbpConfArgParserDebian from gbp.deb.source import DebianSource from gbp.deb.git import DebianGitRepository @@ -397,93 +396,95 @@ def switch_pq(repo, branch, options): switch_to_pq_branch(repo, branch) -class GbpPqHelpFormatter(RawDescriptionHelpFormatter, - ArgumentDefaultsHelpFormatter): - pass - - -def descr_msg(): - return """Maintain patches on a patch queue branch -Actions: - export export the patch queue associated to the current branch - into a quilt patch series in debian/patches/ and update the - series file. - import create a patch queue branch from quilt patches in debian/patches. - rebase switch to patch queue branch associated to the current - branch and rebase against current branch. - drop drop (delete) the patch queue associated to the current branch. - apply apply a patch - switch switch to patch-queue branch and vice versa""" - - def build_parser(name): - usage = "%(prog)s [options] action" + description = "Maintain patches on a patch queue branch" + usage = "%(prog)s [-h] [--version] ACTION [options]" + epilog = "See '%(prog)s ACTION --help' for action-specific options" try: parser = GbpConfArgParserDebian.create_parser(prog=name, usage=usage, - description=descr_msg(), - formatter_class=GbpPqHelpFormatter) + description=description, + epilog=epilog) + _parent = GbpConfArgParserDebian.create_parser(prog=name, + add_help=False) except GbpError as err: gbp.log.err(err) return None - parser.add_bool_conf_file_arg("--patch-numbers", dest="patch_numbers") - parser.add_conf_file_arg("--patch-num-format", dest="patch_num_format") - parser.add_bool_conf_file_arg("--renumber", dest="renumber") - parser.add_arg("-v", "--verbose", action="store_true", dest="verbose", default=False, - help="verbose command execution") - parser.add_arg("--topic", dest="topic", help="in case of 'apply' topic (subdir) to put patch into") - parser.add_conf_file_arg("--time-machine", dest="time_machine", type=int) - parser.add_bool_conf_file_arg("--drop", dest='drop') - parser.add_bool_conf_file_arg("--commit", dest="commit") - parser.add_conf_file_arg("--abbrev", dest="abbrev", type=int) - parser.add_arg("--force", dest="force", action="store_true", default=False, - help="in case of import even import if the branch already exists") - parser.add_conf_file_arg("--color", dest="color", type='tristate') - parser.add_conf_file_arg("--color-scheme", - dest="color_scheme") - parser.add_conf_file_arg("--meta-closes", dest="meta_closes") - parser.add_conf_file_arg("--meta-closes-bugnum", dest="meta_closes_bugnum") - parser.add_conf_file_arg("--pq-from", dest="pq_from", choices=['DEBIAN', 'TAG']) - parser.add_conf_file_arg("--upstream-tag", dest="upstream_tag") + # Add common arguments + _parent.add_arg("-v", "--verbose", action="store_true", dest="verbose", default=False, + help="verbose command execution") + _parent.add_conf_file_arg("--color", dest="color", type='tristate') + _parent.add_conf_file_arg("--color-scheme", + dest="color_scheme") + _parent.add_conf_file_arg("--pq-from", dest="pq_from", choices=['DEBIAN', 'TAG']) + _parent.add_conf_file_arg("--upstream-tag", dest="upstream_tag") + _parent.add_arg("--force", dest="force", action="store_true", default=False, + help="in case of import even import if the branch already exists") + _parent.add_conf_file_arg("--time-machine", dest="time_machine", type=int) + + # Add subcommands + subparsers = parser.add_subparsers(title='actions', dest='action') + + # Export + _parser = subparsers.add_parser('export', parents=[_parent], + help="export the patch queue associated to the current " + "branch into a quilt patch series in debian/patches/ " + "and update the series file.") + _parser.add_bool_conf_file_arg("--patch-numbers", dest="patch_numbers") + _parser.add_conf_file_arg("--patch-num-format", dest="patch_num_format") + _parser.add_bool_conf_file_arg("--renumber", dest="renumber") + _parser.add_bool_conf_file_arg("--drop", dest='drop') + _parser.add_bool_conf_file_arg("--commit", dest="commit") + _parser.add_conf_file_arg("--abbrev", dest="abbrev", type=int) + _parser.add_conf_file_arg("--meta-closes", dest="meta_closes") + _parser.add_conf_file_arg("--meta-closes-bugnum", dest="meta_closes_bugnum") + # Import + _parser = subparsers.add_parser('import', parents=[_parent], + help="create a patch queue branch from" + "quilt patches in debian/patches.") + # Rebase + _parser = subparsers.add_parser('rebase', parents=[_parent], + help="switch to patch queue branch associated to the current " + "branch and rebase against current branch.") + # Drop + _parser = subparsers.add_parser('drop', parents=[_parent], + help="drop (delete) the patch queue " + "associated to the current branch.") + # Apply + _parser = subparsers.add_parser('apply', parents=[_parent], + help="apply a patch") + _parser.add_arg("--topic", dest="topic", help="in case of 'apply' topic (subdir) to put patch into") + _parser.add_argument("patch", metavar="PATCH", help="Patch to apply") + # Switch + _parser = subparsers.add_parser('switch', parents=[_parent], + help="switch to patch-queue branch and vice versa") + return parser def parse_args(argv): parser = build_parser(os.path.basename(argv[0])) if not parser: - return None, None - return parser.parse_known_args(argv[1:]) + return None + + args = parser.parse_args(argv[1:]) + + if args.action is None: + parser.error("You need to specify an action") + return args def main(argv): retval = 0 - (options, args) = parse_args(argv) + options = parse_args(argv) if not options: return ExitCodes.parse_error gbp.log.setup(options.color, options.verbose, options.color_scheme) - if len(args) < 1: - gbp.log.err("No action given.") - return 1 - else: - action = args[0] - - if action in ["export", "import", "rebase", "drop", "switch"]: - if len(args) != 1: - gbp.log.err("Invalid options: %s" % ", ".join(args[1:])) - return 1 - elif action in ["apply"]: - if len(args) != 2: - gbp.log.err("No patch name given.") - return 1 - else: - patchfile = args[1] - else: - gbp.log.err("Unknown action '%s'." % action) - return 1 + action = options.action try: repo = DebianGitRepository(os.path.curdir) @@ -502,7 +503,7 @@ def main(argv): elif action == "rebase": rebase_pq(repo, current, options) elif action == "apply": - patch = Patch(patchfile) + patch = Patch(options.patch) maintainer = get_maintainer_from_control(repo) apply_single_patch(repo, current, patch, maintainer, options.topic) elif action == "switch": diff --git a/gbp/scripts/pq_rpm.py b/gbp/scripts/pq_rpm.py index 2aa03130..089add74 100755 --- a/gbp/scripts/pq_rpm.py +++ b/gbp/scripts/pq_rpm.py @@ -24,7 +24,6 @@ import os import re import sys -from argparse import ArgumentDefaultsHelpFormatter, RawDescriptionHelpFormatter import gbp.log from gbp.tmpfile import init_tmpdir, del_tmpdir, tempfile @@ -363,57 +362,72 @@ def switch_pq(repo, current): switch_to_pq_branch(repo, current) -class GbpPqRpmHelpFormatter(RawDescriptionHelpFormatter, - ArgumentDefaultsHelpFormatter): - pass - - -def descr_msg(): - return """Maintain patches on a patch queue branch -Actions: -export Export the patch queue / devel branch associated to the - current branch into a patch series in and update the spec file -import Create a patch queue / devel branch from spec file - and patches in current dir. -rebase Switch to patch queue / devel branch associated to the current - branch and rebase against upstream. -drop Drop (delete) the patch queue /devel branch associated to - the current branch. -apply Apply a patch -switch Switch to patch-queue branch and vice versa.""" - - def build_parser(name): """Construct command line parser""" - usage = "%(prog)s [options] action" + description = "maintain patches on a patch queue branch" + usage = "%(prog)s [-h] [--version] ACTION [options]" + epilog = "See '%(prog)s ACTION --help' for action-specific options" + try: parser = GbpConfArgParserRpm.create_parser(prog=name, usage=usage, - description=descr_msg(), - formatter_class=GbpPqRpmHelpFormatter) + description=description, + epilog=epilog) + _parent = GbpConfArgParserRpm.create_parser(prog=name, + add_help=False) except GbpError as err: gbp.log.err(err) return None - parser.add_bool_conf_file_arg("--patch-numbers", - dest="patch_numbers") - parser.add_arg("-v", "--verbose", action="store_true", dest="verbose", - default=False, help="Verbose command execution") - parser.add_arg("--force", dest="force", action="store_true", - default=False, - help="In case of import even import if the branch already exists") - parser.add_bool_conf_file_arg("--drop", dest="drop") - parser.add_conf_file_arg("--color", dest="color", - type='tristate') - parser.add_conf_file_arg("--color-scheme", - dest="color_scheme") - parser.add_conf_file_arg("--tmp-dir", dest="tmp_dir") - parser.add_conf_file_arg("--abbrev", dest="abbrev", type=int) - parser.add_conf_file_arg("--upstream-tag", - dest="upstream_tag") - parser.add_conf_file_arg("--spec-file", dest="spec_file") - parser.add_conf_file_arg("--packaging-dir", - dest="packaging_dir") + # Add common arguments + _parent.add_arg("-v", "--verbose", action="store_true", dest="verbose", + default=False, help="Verbose command execution") + _parent.add_conf_file_arg("--color", dest="color", + type='tristate') + _parent.add_conf_file_arg("--color-scheme", + dest="color_scheme") + _parent.add_conf_file_arg("--tmp-dir", dest="tmp_dir") + _parent.add_conf_file_arg("--upstream-tag", + dest="upstream_tag") + _parent.add_conf_file_arg("--spec-file", dest="spec_file") + _parent.add_conf_file_arg("--packaging-dir", + dest="packaging_dir") + + # Add subcommands + subparsers = parser.add_subparsers(title='actions', dest='action') + + # Export + _parser = subparsers.add_parser('export', parents=[_parent], + help="Export the patch queue / devel branch associated to the " + "current branch into a patch series in and update the spec " + "file") + _parser.add_bool_conf_file_arg("--patch-numbers", + dest="patch_numbers") + _parser.add_conf_file_arg("--abbrev", dest="abbrev", type=int) + _parser.add_bool_conf_file_arg("--drop", dest="drop") + # Import + _parser = subparsers.add_parser('import', parents=[_parent], + help="Create a patch queue / devel branch from spec file " + "and patches in current dir.") + _parser.add_arg("--force", dest="force", action="store_true", + default=False, + help="In case of import even import if the branch already exists") + # Rebase + _parser = subparsers.add_parser('rebase', parents=[_parent], + help="Switch to patch queue / devel branch associated to the " + "current branch and rebase against upstream.") + # Drop + _parser = subparsers.add_parser('drop', parents=[_parent], + help="Drop (delete) the patch queue /devel branch associated " + "to the current branch.") + # Apply + _parser = subparsers.add_parser('apply', parents=[_parent], + help="Apply a patch") + _parser.add_argument("patch", metavar="PATCH", help="Patch to apply") + # Switch + _parser = subparsers.add_parser('switch', parents=[_parent], + help="Switch to patch-queue branch and vice versa.") + return parser @@ -421,39 +435,26 @@ def parse_args(argv): """Parse command line arguments""" parser = build_parser(os.path.basename(argv[0])) if not parser: - return None, None - return parser.parse_known_args(argv[1:]) + return None + + args = parser.parse_args(argv[1:]) + + if args.action is None: + parser.error("You need to specify an action") + return args def main(argv): """Main function for the gbp pq-rpm command""" retval = 0 - (options, args) = parse_args(argv) + options = parse_args(argv) if not options: return ExitCodes.parse_error gbp.log.setup(options.color, options.verbose, options.color_scheme) - if len(args) < 1: - gbp.log.err("No action given.") - return 1 - else: - action = args[0] - - if action in ["export", "import", "rebase", "drop", "switch", "convert"]: - if len(args) != 1: - gbp.log.err("Invalid options: %s" % ", ".join(args[1:])) - return 1 - elif action in ["apply"]: - if len(args) != 2: - gbp.log.err("No patch name given.") - return 1 - else: - patchfile = args[1] - else: - gbp.log.err("Unknown action '%s'." % action) - return 1 + action = options.action try: repo = RpmGitRepository(os.path.curdir) @@ -474,7 +475,7 @@ def main(argv): elif action == "rebase": rebase_pq(repo, options) elif action == "apply": - patch = Patch(patchfile) + patch = Patch(options.patch) apply_single_patch(repo, current, patch, fallback_author=None) elif action == "switch": switch_pq(repo, current) diff --git a/tests/component/rpm/test_pq_rpm.py b/tests/component/rpm/test_pq_rpm.py index 3eff5218..1dac04aa 100644 --- a/tests/component/rpm/test_pq_rpm.py +++ b/tests/component/rpm/test_pq_rpm.py @@ -18,7 +18,7 @@ import os import tempfile -from nose.tools import eq_, ok_ # pylint: disable=E0611 +from nose.tools import assert_raises, eq_, ok_ # pylint: disable=E0611 from gbp.scripts.pq_rpm import main as pq from gbp.git import GitRepository @@ -49,20 +49,21 @@ def _has_patches(self, specfile, patches): def test_invalid_args(self): """See that pq-rpm fails gracefully when called with invalid args""" GitRepository.create('.') - # Test empty args - eq_(mock_pq([]), 1) - self._check_log(0, 'gbp:error: No action given.') - self._clear_log() + with capture_stderr(): + # Test empty args + with assert_raises(SystemExit) as err: + eq_(mock_pq([]), 1) + eq_(err.code, 2) - # Test invalid command - eq_(mock_pq(['mycommand']), 1) - self._check_log(0, "gbp:error: Unknown action 'mycommand'") - self._clear_log() + # Test invalid command + with assert_raises(SystemExit) as err: + eq_(mock_pq(['mycommand']), 1) + eq_(err.code, 2) - # Test invalid cmdline options - with capture_stderr(): - eq_(mock_pq(['export', '--invalid-arg=123']), 1) - self._check_log(0, "gbp:error: Invalid options: --invalid-arg=123") + # Test invalid cmdline options + with assert_raises(SystemExit) as err: + eq_(mock_pq(['export', '--invalid-arg=123']), 1) + eq_(err.code, 2) def test_import_outside_repo(self): """Run pq-rpm when not in a git repository""" @@ -234,8 +235,10 @@ def test_apply(self): branches = repo.get_local_branches() + ['patch-queue/master'] # No patch given - eq_(mock_pq(['apply']), 1) - self._check_log(-1, "gbp:error: No patch name given.") + with capture_stderr(): + with assert_raises(SystemExit) as err: + eq_(mock_pq(['apply']), 1) + eq_(err.code, 2) # Create a pristine pq-branch repo.create_branch('patch-queue/master', 'upstream')