Plumb in subcommand options
Enable passing of options to subcommands by avoiding any parsing of these
at the top level.
BUG=chromium-os:11785
TEST=chromite build --clean x86-generic, check it does clean
python main_unittest.py (run tests)
Change-Id: I4bfe86bc02d12aa207a294d143201e20875fba03
Review URL: http://codereview.chromium.org/6720024
diff --git a/shell/main.py b/shell/main.py
index c847e9d..5a2f603 100755
--- a/shell/main.py
+++ b/shell/main.py
@@ -93,6 +93,58 @@
return possible_cmds[choice]
+def _ParseArguments(parser, argv):
+ '''Helper function to separate arguments for a main program and sub-command.
+
+ We split arguments into ones we understand, and ones to pass on to
+ sub-commands. For the former, we put them through the given optparse and
+ return the result options and sub-command name. For the latter, we just
+ return a list of options and arguments not intended for us.
+
+ We want to parse only the options that we understand at the top level of
+ Chromite. Our heuristic here is that anything after the first positional
+ parameter (which we assume is the command) is ignored at this level, and
+ is passed down to the command level to handle.
+
+ TODO(sjg): Revisit this to include tolerant option parser instead
+ http://codereview.chromium.org/6469035/
+
+ Args:
+ parser: Option parser.
+ argv: List of program arguments
+
+ Returns:
+ options: Top level options (returned from optparse).
+ cmd_str: Subcommand to run
+ cmd_args: Arguments intended for subcommands.
+ '''
+ our_args = []
+ cmd_args = list(argv)
+ cmd_str = ''
+ args = [] # Nothing until we call optparse
+ while not cmd_str:
+ if our_args:
+ (options, args) = parser.parse_args(our_args)
+ if len(args) > 1:
+ cmd_str = args[1].lower()
+ elif cmd_args:
+ # We don't have a command yet. Transfer a positional arg from from
+ # cmd_args to our_args to see if that does the trick. We move over any
+ # options we find also.
+ while cmd_args:
+ arg = cmd_args.pop(0)
+ our_args.append(arg)
+ if not arg.startswith( '-'):
+ break
+ else:
+ # No more cmd_args to consume.
+ break
+
+ # We must run the parser, even if it dies due to lack of arguments
+ if not args:
+ (options, args) = parser.parse_args(our_args)
+ return options, cmd_str, cmd_args
+
def main():
"""Main function for the chromite shell."""
@@ -128,7 +180,7 @@
# Verbose defaults to full for now, just to keep people acclimatized to
# vast amounts of comforting output.
- parser.add_option('-v', dest='verbose', default=3,
+ parser.add_option('-v', dest='verbose', default=3, type='int',
help='Control verbosity: 0=silent, 1=progress, 3=full')
parser.add_option('-q', action='store_const', dest='verbose', const=0,
help='Be quieter (sets verbosity to 1)')
@@ -138,10 +190,9 @@
help="Chroot spec to use. Can be an absolute path to a spec file "
"or a substring of a chroot spec name (without .spec suffix)")
parser.usage = help_str
- try:
- (options, args) = parser.parse_args()
- except:
- sys.exit(1)
+
+ # Parse the arguments and separate them into top-level and subcmd arguments.
+ options, cmd_str, cmd_args = _ParseArguments(parser, sys.argv)
# Set up the cros system.
cros_env = chromite_env.ChromiteEnv()
@@ -163,13 +214,6 @@
# Already in the chroot; no need to get config...
chroot_config = None
- # Get command and arguments
- if args:
- cmd_str = args[0].lower()
- args = args[1:]
- else:
- cmd_str = ''
-
# Validate the subcmd, popping a menu if needed.
cmd_str = _FindCommand(cmd_str)
oper.Info("Running command '%s'." % cmd_str)
@@ -179,7 +223,7 @@
cmd_obj = cmd_cls()
cmd_obj.SetChromiteEnv(cros_env)
try:
- cmd_obj.Run([cmd_str] + args, chroot_config=chroot_config)
+ cmd_obj.Run([cmd_str] + cmd_args, chroot_config=chroot_config)
# Handle an error in one of the scripts: print a message and exit.
except chromite_env.ChromiteError, msg:
diff --git a/shell/main_unittest.py b/shell/main_unittest.py
index bb510f3..e18a9c8 100644
--- a/shell/main_unittest.py
+++ b/shell/main_unittest.py
@@ -7,6 +7,7 @@
"""Unit tests for main.py."""
import doctest
+import optparse
import os
import re
import unittest
@@ -399,6 +400,67 @@
'_FindSpec("%s") incorrectly returned "%s".' %
(spec_name, spec_path))
+class TestParseArguments(unittest.TestCase):
+ """Test utils.FindSpec."""
+
+ def setUp(self):
+ """Test initialization."""
+ self.parser = optparse.OptionParser()
+
+ # Verbose defaults to full for now, just to keep people acclimatized to
+ # vast amounts of comforting output.
+ self.parser.add_option('-v', dest='verbose', default=3, type='int',
+ help='Control verbosity: 0=silent, 1=progress, 3=full')
+ self.parser.add_option('-q', action='store_const', dest='verbose', const=0,
+ help='Be quieter (sets verbosity to 1)')
+
+ def testEmpty(self):
+ options, cmd, sub = main._ParseArguments(self.parser,
+ [])
+ self.assertEqual(options.verbose, 3)
+ self.assertEqual(cmd, '')
+ self.assertEqual(sub, [])
+
+ def testBadOption(self):
+ self.assertRaises(SystemExit, main._ParseArguments, self.parser,
+ ['chromite', '--bad'])
+ self.assertRaises(SystemExit, main._ParseArguments, self.parser,
+ ['chromite', '--bad', 'build'])
+
+ def testSubcmd(self):
+ options, cmd, sub = main._ParseArguments(self.parser,
+ ['chromite', 'build'])
+ self.assertEqual(options.verbose, 3)
+ self.assertEqual(cmd, 'build')
+ self.assertEqual(sub, [])
+
+ def testSubcmdQuiet(self):
+ options, cmd, sub = main._ParseArguments(self.parser,
+ ['chromite', '-q', 'build'])
+ self.assertEqual(options.verbose, 0)
+ self.assertEqual(cmd, 'build')
+ self.assertEqual(sub, [])
+
+ def testSubcmdVerbose2(self):
+ options, cmd, sub = main._ParseArguments(self.parser,
+ ['chromite', '-v2', 'build'])
+ self.assertEqual(options.verbose, 2)
+ self.assertEqual(cmd, 'build')
+ self.assertEqual(sub, [])
+
+ def testSubcmdVerbose4(self):
+ options, cmd, sub = main._ParseArguments(self.parser,
+ ['chromite', '-v', '4', 'build'])
+ self.assertEqual(options.verbose, 4)
+ self.assertEqual(cmd, 'build')
+ self.assertEqual(sub, [])
+
+ def testSubcmdArgs(self):
+ options, cmd, sub = main._ParseArguments(self.parser,
+ ['chromite', '-v', '4', 'build', 'seaboard', '--clean'])
+ self.assertEqual(options.verbose, 4)
+ self.assertEqual(cmd, 'build')
+ self.assertEqual(sub, ['seaboard', '--clean'])
if __name__ == '__main__':
doctest.testmod(main)
diff --git a/shell/subcmds/build_cmd.py b/shell/subcmds/build_cmd.py
index 9122cc4..d9750e2 100644
--- a/shell/subcmds/build_cmd.py
+++ b/shell/subcmds/build_cmd.py
@@ -64,7 +64,7 @@
return
# Put together command.
- cmd_list = [
+ arg_list = [
'--board="%s"' % build_config.get('DEFAULT', 'target'),
build_config.get('BUILD', 'setup_board_flags'),
]
@@ -156,11 +156,9 @@
usage_str = ('usage: %%prog [chromite_options] %s [options] [target]' %
raw_argv[0])
parser = optparse.OptionParser(usage=usage_str)
- # This option won't work until a later CL plumbs in optparse
- #parser.add_option('--clean', default=False, action='store_true',
- #help='Clean before building.')
+ parser.add_option('--clean', default=False, action='store_true',
+ help='Clean before building.')
(options, argv) = parser.parse_args(raw_argv[1:])
- options.clean = False
# Load the build config if needed...
if not loaded_config:
diff --git a/shell/subcmds/clean_cmd.py b/shell/subcmds/clean_cmd.py
index 6ceab88..d4f7b18 100644
--- a/shell/subcmds/clean_cmd.py
+++ b/shell/subcmds/clean_cmd.py
@@ -114,11 +114,9 @@
usage_str = ('usage: %%prog [chromite_options] %s [options] [target]' %
raw_argv[0])
parser = optparse.OptionParser(usage=usage_str)
- # This option won't work until a later CL plumbs in optparse
- #parser.add_option('-y', '--yes', default=False, action='store_true',
- #help='Answer "YES" to "are you sure?" questions.')
+ parser.add_option('-y', '--yes', default=False, action='store_true',
+ help='Answer "YES" to "are you sure?" questions.')
(options, argv) = parser.parse_args(raw_argv[1:])
- options.yes = False
# Make sure the chroot exists first, before possibly prompting for board...
# ...not really required, but nice for the user...
