Plumb in subcommand options

shell/main.py

 #!/usr/bin/python
 #
 # Copyright (c) 2011 The Chromium OS Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 """Main file for the chromite shell."""
 
 # Python imports
 import optparse
@@ skipping 110 matching lines @@
       """%(prog)s [chromite_options] [cmd [args]]\n"""
       """\n"""
       """The chromite script is a wrapper to make it easy to do various\n"""
       """build tasks.  For a list of commands, run without any arguments."""
   ) % {'prog': os.path.basename(sys.argv[0])}
 
   parser = optparse.OptionParser()
 
   # 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)')
   if not cros_lib.IsInsideChroot():
     parser.add_option('--chroot', action='store', type='string',
         dest='chroot_name', default='chroot',
         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)
+
+  # We want to parse only the options that we understand at the top level of

[diandersAtChromium, 2011/04/08 00:43:46]

Maybe put this whole chunk of parsing code in a helper function?

[sjg, 2011/04/08 20:25:26]

done - added a few tests also.

+  # 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/
+  our_args = []   # Arguments that we will parse here.
+  cmd_args = []   # Arguments that we will pass down to the command level.
+  positional = 0
+  for arg in sys.argv:

[diandersAtChromium, 2011/04/08 00:43:46]

This loop keeps confusing me the way it's written.  Perhaps:

cmd_args = list(sys.argv)
while (positional < 2) and cmd_args:
  arg = cmd_args.pop(0)
  if not arg.startswith('-'):
    positional += 1
  our_args.append(arg)

...I think that makes things roughly like the below "pull a few more off" case,
too, which is a bonus.  Maybe you can just always do a single 'while not
cmd_str' loop?

[sjg, 2011/04/08 20:25:26]

Yes, and in fact this inspired me to simplify it a bit more. The turd is getting
pretty shiny now.

+
+    # Option arguments are collected as is, but we only allow two positional
+    # arguments: argv[0] is the program name, the next is the command name.
+    if positional == 2:
+      cmd_args = sys.argv[len(our_args):]
+      break
+    if arg[0] != '-':
+      positional += 1
+    our_args.append(arg)
+
+  # Loop until we have enough arguments to get a command.
+  cmd_str = ''
+  while not cmd_str:
+    (options, args) = parser.parse_args(our_args)
+
+    # Get the command from the resulting arg list.
+    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[0]

[diandersAtChromium, 2011/04/08 00:43:46]

arg = cmd_args.pop(0)

[sjg, 2011/04/08 20:25:26]

done

+        our_args.append(arg)
+        cmd_args = cmd_args[1:]
+        if arg[0] != '-':

[diandersAtChromium, 2011/04/08 00:43:46]

startswith

[sjg, 2011/04/08 20:25:26]

done

+          break
+    else:
+      # No more cmd_args to consume.
+      break
 
   # Set up the cros system.
   cros_env = chromite_env.ChromiteEnv()
 
   # Configure the operation setup.
   oper = cros_env.GetOperation()
   oper.verbose = options.verbose >= 3
   oper.progress = options.verbose >= 1
 
   # Look for special "--chroot" argument to allow for alternate chroots
   if not cros_lib.IsInsideChroot():
     chroot_spec_path = utils.FindSpec(options.chroot_name,
                                       spec_type=utils.CHROOT_SPEC_TYPE)
 
     oper.Info('Using chroot "%s"' % os.path.relpath(chroot_spec_path))
 
     chroot_config = utils.ReadConfig(chroot_spec_path)
   else:
     # 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)
 
   # Finally, call the function w/ standard argv.
   cmd_cls = _COMMAND_HANDLERS[_COMMAND_STRS.index(cmd_str)]
   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:
     sys.exit(1)
 
 if __name__ == '__main__':
   main()