WIP Chromite supporting "LEGACY" mode, as requested by Anush.

chromite/chromite.py

+#!/usr/bin/python
+# Copyright (c) 2010 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.
+
+"""Chromite."""
+
+# Python imports
+import ConfigParser
+import cPickle as cPickle

[sosa, 2011/01/10 18:38:27]

redundant?

[diandersAtChromium, 2011/01/12 00:04:45]

Done.

+import optparse
+import os
+import sys
+import tempfile
+
+
+# Local library imports
+from lib import text_menu
+from lib.cros_build_lib import Die
+from lib.cros_build_lib import Info
+from lib.cros_build_lib import IsInsideChroot
+from lib.cros_build_lib import RunCommand
+from lib.cros_build_lib import Warning as Warn
+
+
+# Find the Chromium OS root and Chromite root...
+_CHROMITE_PATH = os.path.dirname(os.path.realpath(__file__))
+_SRCROOT_PATH = os.path.realpath(os.path.join(_CHROMITE_PATH, '..', '..', '..'))
+
+
+# Commands can take one of these two types of specs.  Note that if a command
+# takes a build spec, we will find the associated chroot spec.  This should be
+# a human-readable string.  It is printed and also is the name of the spec
+# directory.
+_BUILD_SPEC_TYPE = 'build'
+_CHROOT_SPEC_TYPE = 'chroot'
+
+
+# This is a special target that indicates that you want to do something just
+# to the host.  This means different things to different commands.
+# TODO(dianders): Good idea or bad idea?
+_HOST_TARGET = 'HOST'
+
+
+# Define command handlers and command strings.  We define them in this way
+# so that someone searching for what calls _CmdXyz can find it easy with a grep.
+#
+# ORDER MATTERS here when we show the menu.
+_COMMAND_HANDLERS = [

[sosa, 2011/01/11 04:44:22]

can be overriden in unittest i.e.

chromite._COMMAND_HANDLERS = [teststuffffff]

[diandersAtChromium, 2011/01/12 00:04:45]

Good point!  Hmmm, at the moment though, I am going to leave the existing tests
the way they are, though (since I actually do want to test that a build command
exists and that sh maps to the shell command).  When I add a few more unit
tests, I will add one that does this.

Added a TODO in chromite_unittest.py.

On 2011/01/11 04:44:22, sosa wrote:

+    '_CmdBuild',
+    '_CmdClean',
+    '_CmdDistClean',
+    '_CmdEbuild',
+    '_CmdEmerge',
+    '_CmdEquery',
+    '_CmdPortageq',
+    '_CmdShell',
+    '_CmdWorkon',
+]
+_COMMAND_STRS = [fn_str[len('_Cmd'):].lower() for fn_str in _COMMAND_HANDLERS]
+
+
+def _GetBoardDir(build_config):

[sosa, 2011/01/11 04:44:22]

May wanna consider making a Class for commands and baking some common logic like
vars in there.  I think it might be good to use classes for commands like the
buildbots so that you can do some fun inheritance stuff, etc. It'll also make it
easier to change their implementations later.

Also this file is getting big.  May wanna consider breaking out commands into
their own module.

[diandersAtChromium, 2011/01/12 00:04:45]

Added TODO for class for commands.

Splitting things into multiple files will wait until chromite moves to its own
repo, which will happen after this pushes in a separate CL.

On 2011/01/11 04:44:22, sosa wrote:

+  """Returns the board directory (inside the chroot) given the name.
+
+  Args:
+    build_config: A SafeConfigParser representing the config that we're
+        building.
+
+  Returns:
+    The absolute path to the board.
+  """
+  target_name = build_config.get('BUILD', 'target')
+
+  # Extra checks on these, since we sometimes might do a rm -f on the board
+  # directory and these could cause some really bad behavior.
+  assert target_name, "Didn't expect blank target name."
+  assert len(target_name.split()) == 1, 'Target name should have no whitespace.'

[sosa, 2011/01/11 04:44:22]

is this a strict requirement?

[diandersAtChromium, 2011/01/12 00:04:45]

I haven't tested with targets with whitespace.  I could imagine that passing
--board=blah might not work with whitespace.  I added a TODO to check this in a
better way.

On 2011/01/11 04:44:22, sosa wrote:

+
+  return os.path.join('/', 'build', target_name)
+
+
+def _GetChrootAbsDir(chroot_config, check_exists=False):
+  """Returns the absolute chroot directory the chroot config.
+
+  Args:
+    chroot_config: A SafeConfigParser representing the config for the chroot.
+    check_exists: If True, we'll return None if the chroot dir doesn't already

[sosa, 2011/01/11 04:44:22]

Seems weird to me to be combining this functionality here.

[diandersAtChromium, 2011/01/12 00:04:45]

Done.

+        exist.
+  Returns:
+    The chroot directory, always absolute; if check_exists, will return None
+    if the os.path.isdir() returned False for this directory.
+  """
+  chroot_dir = chroot_config.get('CHROOT', 'path')
+  chroot_dir = os.path.join(_SRCROOT_PATH, chroot_dir)
+
+  if check_exists and not os.path.isdir(chroot_dir):
+    return None
+
+  return chroot_dir
+
+
+def _FindCommand(cmd_name):
+  """Find the command that matches the given command name.
+
+  This tries to be smart.  See the cmd_name parameter for details.
+
+  Args:
+    cmd_name: Can be any of the following:
+        1. The full name of a command.  This is checked first so that if one
+           command name is a substring of another, you can still specify
+           the shorter spec name and know you won't get a menu (the exact
+           match prevents the menu).
+        2. A _prefix_ that will be used to pare-down a menu of commands
+           Can be the empty string to show a menu of all commands
+
+  Returns:
+    The command name.
+  """
+  # Always make cmd_name lower.  Commands are case-insensitive.
+  cmd_name = cmd_name.lower()
+
+  # If we're an exact match, we're done!
+  if cmd_name in _COMMAND_STRS:
+    return cmd_name
+
+  # Find ones that match and put them in a menu...
+  possible_cmds = []
+  possible_choices = []
+  for cmd_num, this_cmd in enumerate(_COMMAND_STRS):
+    if this_cmd.startswith(cmd_name):
+      handler = eval(_COMMAND_HANDLERS[cmd_num])
+      assert hasattr(handler, '__doc__'), \
+             ('All handlers must have docstrings: %s' % cmd_name)
+      desc = handler.__doc__.splitlines()[0]
+
+      possible_cmds.append(this_cmd)
+      possible_choices.append('%s - %s' % (this_cmd, desc))
+
+  if not possible_choices:
+    Die('No commands matched: "%s".  '
+        'Try running with no arguments for a menu.' %
+        cmd_name)
+
+  if cmd_name and len(possible_choices) == 1:
+    choice = 0
+    Info("Running command '%s'." % possible_cmds[choice])
+  else:
+    choice = text_menu.TextMenu(possible_choices, 'Which chromite command',
+                                menu_width=0)
+
+  if choice is None:

[sosa, 2011/01/11 04:44:22]

reverse ... if/else...just do if choice.  Good to avoid "is None"

[diandersAtChromium, 2011/01/12 00:04:45]

No.  Better habit is to test for None if you know that object is specified to be
None in a certain case.  

From PEP-8: Also, beware of writing "if x" when you really mean "if x is not
None"

Google style guide does not contradict as far as I know.

I believe "is None" has the side effect of being slightly faster (not that it
matters in this case).

On 2011/01/11 04:44:22, sosa wrote:

+    return None
+  else:
+    return possible_cmds[choice]
+
+
+def _FindSpec(spec_name, spec_type=_BUILD_SPEC_TYPE, can_show_ui=True):
+  """Find the spec with the given name.
+
+  This tries to be smart about helping the user to find the right spec.  See
+  the spec_name parameter for details.
+
+  Args:
+    spec_name: Can be any of the following:
+        1. A full path to a spec file (including the .spec suffix).  This is
+           checked first (i.e. if os.path.isfile(spec_name), we assume we've
+           got this case).
+        2. The full name of a spec file somewhere in the spec search path
+           (not including the .spec suffix).  This is checked second.  Putting
+           this check second means that if one spec name is a substring of
+           another, you can still specify the shorter spec name and know you
+           won't get a menu (the exact match prevents the menu).
+        3. A substring that will be used to pare-down a menu of spec files
+           found in the spec search path.  Can be the empty string to show a
+           menu of all spec files in the spec path.  NOTE: Only possible if
+           can_show_ui is True.
+    spec_type: The type of spec this is: 'chroot' or 'build'.
+    can_show_ui: If True, enables the spec name to be a substring since we can
+        then show a menu if the substring matches more than one thing.
+
+  Returns:
+    A path to the spec file.
+  """
+  spec_suffix = '.spec'
+
+  # Handle 'HOST' for spec name w/ no searching, so it counts as an exact match.
+  if spec_type == _BUILD_SPEC_TYPE and spec_name == _HOST_TARGET.lower():
+    return _HOST_TARGET
+
+  # If we have an exact path name, that's it.  No searching.
+  if os.path.isfile(spec_name):
+    return spec_name
+
+  # Figure out what our search path should be.
+  # ...make these lists in anticipation of the need to support specs that live
+  # in private overlays.
+  search_path = [

[sosa, 2011/01/11 04:44:22]

...maybe may wanna make plural in anticipation as well :)

[diandersAtChromium, 2011/01/12 00:04:45]

No.  Search path is used in the singular as far as I know.  According to the
google: http://www.google.com/search?sclient=psy&hl=en&q=define:search+path

On 2011/01/11 04:44:22, sosa wrote:

+      os.path.join(_CHROMITE_PATH, 'specs', spec_type),
+  ]
+
+  # Look for an exact match of a spec name.  An exact match will go through with
+  # no menu.
+  if spec_name:
+    for dir_path in search_path:
+      spec_path = os.path.join(dir_path, spec_name + spec_suffix)
+      if os.path.isfile(spec_path):
+        return spec_path
+
+  # Die right away if we can't show UI and didn't have an exact match.
+  if not can_show_ui:
+    Die("Couldn't find %s spec: %s" % (spec_type, spec_name))
+
+  # No full path and no exact match.  Move onto a menu.
+  # First step is to construct the options.  We'll store in a dict keyed by
+  # spec name.
+  options = {}
+  for dir_path in search_path:

[sosa, 2011/01/11 04:44:22]

What should happen with subtrees?

[diandersAtChromium, 2011/01/12 00:04:45]

They are not searched.  Only the directory itself.  Is there a compelling reason
that subtrees should be searched?

On 2011/01/11 04:44:22, sosa wrote:

[sosa, 2011/01/13 19:07:04]

Nothing except if someone adds a directory that ends in .spec something weird
might happen.

On 2011/01/12 00:04:45, diandersAtChromium wrote:

[diandersAtChromium, 2011/01/13 21:26:50]

Ah, good point!  Skipping all things where os.path.isfile() returns False.

On 2011/01/13 19:07:04, sosa wrote:

+    for file_name in os.listdir(dir_path):
+      # Find any files that end with ".spec" in a case-insensitive manner.
+      if not file_name.lower().endswith(spec_suffix):
+        continue
+
+      this_spec_name, _ = os.path.splitext(file_name)
+
+      # Skip if this spec file doesn't contain our substring.  We are _always_
+      # case insensitive here to be helpful to the user.
+      if spec_name.lower() not in this_spec_name.lower():
+        continue
+
+      # Skip if we've already seen this spec.  This means that if the same spec
+      # name is in multiple parts of the path, we'll only match the first one.
+      if this_spec_name in options:

[sosa, 2011/01/11 04:44:22]

This seems bad.  Maybe disallow.

[diandersAtChromium, 2011/01/12 00:04:45]

Done.

+        continue
+
+      # OK, it's good.  Store the path.
+      options[this_spec_name] = os.path.join(dir_path, file_name)
+
+  # Add 'HOST'.  All caps so it sorts first.
+  if (spec_type == _BUILD_SPEC_TYPE and
+      spec_name.lower() in _HOST_TARGET.lower()):
+    options[_HOST_TARGET] = _HOST_TARGET
+
+  # If no match, die.
+  if not options:
+    Die("Couldn't find any matching %s specs for: %s" % (spec_type, spec_name))
+
+  # If exactly one match, we're done.
+  if spec_name and len(options) == 1:

[sosa, 2011/01/11 04:44:22]

This will almost never happen because of host special case.  Not even sure if
it's worth optimizing.  

[diandersAtChromium, 2011/01/12 00:04:45]

No.  This is an extremely common case.  I use it all the time:  chromite build
mario

On 2011/01/11 04:44:22, sosa wrote:

[sosa, 2011/01/13 19:07:04]

We're talking about milliseconds where something is interactive already.  I
often choose the slightly slower code when it comes to interactive code if it
makes the code easier to read.  It's up to you though.

On 2011/01/12 00:04:45, diandersAtChromium wrote:

[diandersAtChromium, 2011/01/13 21:26:50]

Not sure I understand.  This code path avoids presenting the user with a menu of
1 item.  It's not about speed, but avoiding the weird UI.

Added a clearer comment.

On 2011/01/13 19:07:04, sosa wrote:

+    _, spec_path = options.popitem()
+    return spec_path
+
+  # If more than one, show a menu...
+  option_keys = sorted(options.iterkeys())
+  choice = text_menu.TextMenu(option_keys, 'Choose a %s spec' % spec_type)
+
+  if choice is None:

[sosa, 2011/01/11 04:44:22]

Same as before.

[diandersAtChromium, 2011/01/12 00:04:45]

No.

On 2011/01/11 04:44:22, sosa wrote:

[sosa, 2011/01/13 19:07:04]

Hater :).

On 2011/01/12 00:04:45, diandersAtChromium wrote:

+    return None
+  else:
+    return options[option_keys[choice]]
+
+
+def _ReadConfig(spec_path):
+  """Read the a build config or chroot config from a spec file.
+
+  This includes adding thue proper _default stuff in.
+
+  Args:
+    spec_path: The path to the build or chroot spec.
+
+  Returns:
+    config: A SafeConfigParser representing the config.
+  """
+  spec_name, _ = os.path.splitext(os.path.basename(spec_path))
+  spec_dir = os.path.dirname(spec_path)
+
+  config = ConfigParser.SafeConfigParser({'name': spec_name})
+  config.read(os.path.join(spec_dir, '_defaults'))
+  config.read(spec_path)
+
+  return config
+
+
+def _GetBuildConfigFromArgs(argv):
+  """Helper for commands that take a build config in the arg list.
+
+  This function can Die() in some instances.
+
+  Args:
+    argv: A list of command line arguments.  If non-empty, [0] should be the
+        build spec.  These will not be modified.
+
+  Returns:
+    argv: The argv with the build spec stripped off.  This might be argv[1:] or
+        just argv.  Not guaranteed to be new memory.
+    build_config: The SafeConfigParser for the build config; might be None if
+        this is a host config.  TODO(dianders): Should there be a build spec for
+        the host?
+  """
+  # The spec name is optional.  If no arguments, we'll show a menu...
+  # Note that if there are arguments, but the first argument is a flag, we'll
+  # assume that we got called before OptionParser.  In that case, they might
+  # have specified options but still want the board menu.
+  if argv and not argv[0].startswith('-'):
+    spec_name = argv[0]
+    argv = argv[1:]
+  else:
+    spec_name = ''
+
+  spec_path = _FindSpec(spec_name)
+  if not spec_path:

[sosa, 2011/01/11 04:44:22]

Sort of weird to assume it was a cancel here.  Either switch to exceptions or
move this Die into _FindSpec

[diandersAtChromium, 2011/01/12 00:04:45]

Done.

+    Die('OK, cancelling...')
+
+  if spec_path == _HOST_TARGET:
+    return argv, None
+
+  build_config = _ReadConfig(spec_path)
+
+  return argv, build_config
+
+
+def _SplitEnvFromArgs(argv):
+  """Split environment settings from arguments.
+
+  This function will just loop over the arguments, looking for ones with an '='
+  character in them.  As long as it finds them, it takes them out of the arg
+  list adds them to a dictionary.  As soon as it finds the first argument

[sosa, 2011/01/11 04:44:22]

Why the last part?

[diandersAtChromium, 2011/01/12 00:04:45]

Why what last part?  See doctests below for usage.

On 2011/01/11 04:44:22, sosa wrote:

+  without an '=', it stops looping.
+
+  NOTE: Some doctests below test for equality with ==, since dicts with more
+        than one item may be arbitrarily ordered.
+
+  >>> result = _SplitEnvFromArgs(['abc=1', 'def=two', 'three'])
+  >>> result == ({'abc': '1', 'def': 'two'}, ['three'])
+  True
+
+  >>> _SplitEnvFromArgs(['one', 'two', 'three'])
+  ({}, ['one', 'two', 'three'])
+
+  >>> _SplitEnvFromArgs(['abc=1', 'three', 'def=two'])
+  ({'abc': '1'}, ['three', 'def=two'])
+
+  >>> result = _SplitEnvFromArgs(['abc=1', 'ghi=4 4', 'def=two'])
+  >>> result == ({'abc': '1', 'ghi': '4 4', 'def': 'two'}, [])
+  True
+
+  >>> _SplitEnvFromArgs(['abc=1', 'abc=2', 'three'])
+  ({'abc': '2'}, ['three'])
+
+  >>> _SplitEnvFromArgs([])
+  ({}, [])
+
+  Args:
+    argv: The arguments to parse; this list is not modified.  Should
+        not include "argv[0]"
+  Returns:
+    env: A dict containing key=value paris.
+    argv: A new list containing anything left after.
+  """
+  # Copy the list so we don't screw with caller...
+  argv = list(argv)
+
+  env = {}
+  while argv:
+    if '=' in argv[0]:
+      key, val = argv.pop(0).split('=', 2)
+      env[key] = val
+    else:
+      break
+
+  return env, argv
+
+
+def _DoMakeChroot(chroot_config):

[sosa, 2011/01/11 04:44:22]

Again might be nice to class / objectize commands.  Have a pre-start /
precondtion stage ... have some NAME that you print out, before running,
cleanup, etc.  Commonize src/scripts etc.

[diandersAtChromium, 2011/01/12 00:04:45]

Added TODO.

On 2011/01/11 04:44:22, sosa wrote:

+  """Build the chroot, if needed.
+
+  Args:
+    chroot_config: A SafeConfigParser representing the config for the chroot.
+  """
+  # Skip this whole command if things already exist.
+  # TODO(dianders): Theoretically, calling make_chroot a second time is OK
+  # and "fixes up" the chroot.  ...but build_packages will do the fixups
+  # anyway (I think), so this isn't that important.
+  chroot_dir = _GetChrootAbsDir(chroot_config, check_exists=True)
+  if chroot_dir is not None:
+    Info('%s already exists, skipping make_chroot.' % chroot_dir)
+    return
+
+  chroot_dir = _GetChrootAbsDir(chroot_config)
+
+  Info('MAKING THE CHROOT')
+
+  # Put together command.  We're going to force the shell to do all of the
+  # splitting of arguments, since we're throwing all of the flags from the
+  # config file in there.
+  cmd = './make_chroot --chroot="%s" %s' % (
+      chroot_dir,
+      chroot_config.get('CHROOT', 'make_chroot_flags')
+  )
+
+  # We'll put CWD as src/scripts when running the command.  Since everyone
+  # running by hand has their cwd there, it is probably the safest.
+  cwd = os.path.join(_SRCROOT_PATH, 'src', 'scripts')
+
+  # Run it.  Exceptions will cause the program to exit.
+  RunCommand(cmd, shell=True, cwd=cwd, ignore_sigint=True)
+
+
+def _DoEnterChroot(chroot_config, fn, *args, **kwargs):
+  """Re-run the given function inside the chroot.
+
+  When the function is run, it will be run in a SEPARATE INSTANCE of chromite,
+  which will be run in the chroot.  This is a little weird.  Specifically:
+  - When the callee executes, it will be a separate python instance.
+    - Globals will be reset back to defaults.
+    - A different version of python (with different modules) may end up running
+      the script in the chroot.
+  - All arguments are pickled up into a temp file, whose path is passed on the
+    command line.
+    - That means that args must be pickleable.
+    - It also means that modifications to the parameters by the callee are not
+      visible to the caller.
+    - Even the function is "pickled".  The way the pickle works, I belive, is it
+      just passes the name of the function.  If this name somehow resolves
+      differently in the chroot, you may get weirdness.
+  - Since we're in the chroot, obviously files may have different paths.  It's
+    up to you to convert parameters if you need to.
+  - The stdin, stdout, and stderr aren't touched.
+
+  Args:
+    chroot_config: A SafeConfigParser representing the config for the chroot.
+    fn: The function to call.
+    args: All other arguments will be passed to the function as is.
+    kwargs: All other arguments will be passed to the function as is.
+  """
+  # Make sure that the chroot exists...
+  chroot_dir = _GetChrootAbsDir(chroot_config, check_exists=True)
+  if chroot_dir is None:
+    Die('Chroot dir does not exist; try the "build" command.\n  %s.' %

[sosa, 2011/01/11 04:44:22]

build host?

[diandersAtChromium, 2011/01/12 00:04:45]

Done.

+        _GetChrootAbsDir(chroot_config))
+
+  Info('ENTERING THE CHROOT')
+
+  # Save state to a temp file (inside the chroot!) using pickle.
+  tmp_dir = os.path.join(chroot_dir, 'tmp')
+  state_file = tempfile.NamedTemporaryFile(prefix='chromite', dir=tmp_dir)
+  try:
+    cPickle.dump((fn, args, kwargs), state_file, cPickle.HIGHEST_PROTOCOL)
+    state_file.flush()
+
+    # Translate temp file name into a chroot path...
+    chroot_state_path = os.path.join('/tmp', os.path.basename(state_file.name))
+
+    # Put together command.  We're going to force the shell to do all of the
+    # splitting of arguments, since we're throwing all of the flags from the
+    # config file in there.
+    # TODO(dianders): Once chromite is in the path inside the chroot, we should
+    #                 change it from 'chromite/chromite' to just 'chromite'.
+    cmd = (
+        './enter_chroot.sh --chroot="%s" %s --'
+        ' chromite/chromite --resume-state %s') % (
+            chroot_dir,
+            chroot_config.get('CHROOT', 'enter_chroot_flags'),
+            chroot_state_path)
+
+    # We'll put CWD as src/scripts when running the command.  Since everyone
+    # running by hand has their cwd there, it is probably the safest.
+    cwd = os.path.join(_SRCROOT_PATH, 'src', 'scripts')
+
+    # Run it.  We allow "error" so we don't print a confusing error message
+    # filled with out resume-state garbage on control-C.
+    cmd_result = RunCommand(cmd, shell=True, cwd=cwd, print_cmd=False,
+                            exit_code=True, error_ok=True, ignore_sigint=True)
+
+    if cmd_result.returncode:
+      Die('Chroot exited with error code %d' % cmd_result.returncode)
+  finally:
+    # Make sure things get closed (and deleted), even upon exception.
+    state_file.close()

[sosa, 2011/01/11 04:44:22]

Shouldn't you close the state_file before using it?

[diandersAtChromium, 2011/01/12 00:04:45]

No.  That will delete it.

On 2011/01/11 04:44:22, sosa wrote:

+
+
+def _DoSetupBoard(build_config):
+  """Setup the board, if needed.
+
+  This just runs the setup_board command with the proper args, if needed.
+
+  Args:
+    build_config: A SafeConfigParser representing the build config.
+  """
+  # Skip this whole command if things already exist.
+  board_dir = _GetBoardDir(build_config)
+  if os.path.isdir(board_dir):
+    Info('%s already exists, skipping setup_board.' % board_dir)
+    return
+
+  Info('SETTING UP THE BOARD')
+
+  # Put together command.  We're going to force the shell to do all of the
+  # splitting of arguments, since we're throwing all of the flags from the
+  # config file in there.
+  cmd = './setup_board --board="%s" %s' % (
+      build_config.get('BUILD', 'target'),
+      build_config.get('BUILD', 'setup_board_flags')
+  )
+
+  # We'll put CWD as src/scripts when running the command.  Since everyone
+  # running by hand has their cwd there, it is probably the safest.
+  cwd = os.path.join(_SRCROOT_PATH, 'src', 'scripts')
+
+  # Run it.  Exceptions will cause the program to exit.
+  RunCommand(cmd, shell=True, cwd=cwd, ignore_sigint=True)
+
+
+def _DoBuildPackages(build_config):
+  """Build packages.
+
+  This just runs the build_packages command with the proper args.
+
+  Args:
+    build_config: A SafeConfigParser representing the build config.
+  """
+  Info('BUILDING PACKAGES')
+
+  # Put together command.  We're going to force the shell to do all of the
+  # splitting of arguments, since we're throwing all of the flags from the
+  # config file in there.
+  cmd = './build_packages --board="%s" %s' % (
+      build_config.get('BUILD', 'target'),
+      build_config.get('BUILD', 'build_packages_flags')
+  )
+
+  # We'll put CWD as src/scripts when running the command.  Since everyone
+  # running by hand has their cwd there, it is probably the safest.
+  cwd = os.path.join(_SRCROOT_PATH, 'src', 'scripts')
+
+  # Run it.  Exceptions will cause the program to exit.
+  RunCommand(cmd, shell=True, cwd=cwd, ignore_sigint=True)
+
+
+def _DoBuildImage(build_config):
+  """Build an image.
+
+  This just runs the build_image command with the proper args.
+
+  Args:
+    build_config: A SafeConfigParser representing the build config.
+  """
+  Info('BUILDING THE IMAGE')
+
+  # Put together command.  We're going to force the shell to do all of the
+  # splitting of arguments, since we're throwing all of the flags from the
+  # config file in there.
+  cmd = './build_image --board="%s" %s' % (
+      build_config.get('BUILD', 'target'),
+      build_config.get('IMAGE', 'build_image_flags')
+  )
+
+  # We'll put CWD as src/scripts when running the command.  Since everyone
+  # running by hand has their cwd there, it is probably the safest.
+  cwd = os.path.join(_SRCROOT_PATH, 'src', 'scripts')
+
+  # Run it.  Exceptions will cause the program to exit.
+  RunCommand(cmd, shell=True, cwd=cwd, ignore_sigint=True)
+
+
+def _DoClean(chroot_config, build_config, want_force_yes):
+  """Clean a target.
+
+  Args:
+    chroot_config: A SafeConfigParser representing the config for the chroot.
+    build_config: A SafeConfigParser representing the build config.
+    want_force_yes: If True, we won't ask any questions--we'll just assume
+        that the user really wants to kill the directory.  If False, we'll
+        show UI asking the user to confirm.
+  """
+  # We'll need the directory so we can delete stuff; this is a chroot path.
+  board_dir = _GetBoardDir(build_config)
+
+  # If not in the chroot, convert board_dir into a non-chroot path...
+  if not IsInsideChroot():
+    chroot_dir = _GetChrootAbsDir(chroot_config, check_exists=True)
+
+    # We'll need to make the board directory relative to the chroot.
+    assert board_dir.startswith('/'), 'Expected unix-style, absolute path.'
+    board_dir = board_dir.lstrip('/')
+    board_dir = os.path.join(chroot_dir, board_dir)
+
+  if not os.path.isdir(board_dir):
+    Die("Nothing to clean: the board directory doesn't exist.\n  %s" %
+        board_dir)
+
+  if not want_force_yes:
+    sys.stderr.write('\n'
+                     'Board dir is at: %s\n'
+                     'Are you sure you want to delete it (YES/NO)? ' %
+                     board_dir)
+    answer = raw_input()
+    if answer != 'YES':

[sosa, 2011/01/11 04:44:22]

you have to type in caps?  Generally it's lowercase and any prefix i.e.
y,n,ye,no,yes and uppercase

[diandersAtChromium, 2011/01/12 00:04:45]

Requiring exactly 'YES' was on purpose.  Maybe that's a little too up-tight. 
Changing to allow any capitalization of 'yes', but not just 'y'.  If people get
too annoyed by it, we will make it looser.

On 2011/01/11 04:44:22, sosa wrote:

[sosa, 2011/01/13 19:07:04]

As a note, you are mimic'ing behavior already existing.  Usually it's best to
conform to the old style if it exists, but I'm ok I guess with making it more
restrictive.

On 2011/01/12 00:04:45, diandersAtChromium wrote:

[diandersAtChromium, 2011/01/13 21:26:50]

OK, will mimic other behavior.  Done.

On 2011/01/13 19:07:04, sosa wrote:

+      Die("You must answer exactly 'YES' if you want to proceed.")
+
+  # Since we're about to do a sudo rm -rf, these are just extra precautions.
+  # This shouldn't be the only place testing these (assert fails are ugly and
+  # can be turned off), but better safe than sorry.
+  # Note that the restriction on '*' is a bit unnecessary, since no shell
+  # expansion should happen.  ...but again, I'd rather be safe.
+  assert os.path.isabs(board_dir), 'Board dir better be absolute'

[sosa, 2011/01/11 04:44:22]

it'll be weird if any of these asserts happen.  it'll be unclear that the dev
has a bad config.

Maybe all these sorta checks should go into some CONFIG checker class or
something rather than as part of the cmds.

[diandersAtChromium, 2011/01/12 00:04:45]

Generally I try to use assert fails for program errors or for errors that are
never going to come up in real life (but I'd still rather have an assert fail
than something bad happen).  The comment above talks about this, though make
it's not clear enough.

Many of these asserts indicate that someone has mucked with _GetBoardDir(),
which will always return a non-blank absolute path with the target name in it. 
That is a program errors.

Only two of these could be triggered by a bad user config: blank target name and
target name with a '*' in it.  

The '*' case is unlikely to happen (who would put a * in their target name???)
and really won't cause any badness.  Maybe I should just remove it.

The blank target case probably should be checked elsewhere.  Added a TODO for
now.

On 2011/01/11 04:44:22, sosa wrote:

+  assert board_dir != '/', 'Board dir better not be /'
+  assert '*' not in board_dir, 'Board dir better not have any *s'
+  assert build_config.get('BUILD', 'target'), 'Target better not be blank'
+  assert build_config.get('BUILD', 'target') in board_dir, \
+         'Target name better be in board dir'
+
+  argv = ['sudo', '--', 'rm', '-rf', board_dir]
+  RunCommand(argv)
+  Info('Deleted: %s' % board_dir)
+
+
+def _DoDistClean(chroot_config, want_force_yes):
+  """Remove the whole chroot.
+
+  Args:
+    chroot_config: A SafeConfigParser representing the config for the chroot.
+    want_force_yes: If True, we won't ask any questions--we'll just assume
+        that the user really wants to kill the directory.  If False, we'll
+        show UI asking the user to confirm.
+  """
+  if IsInsideChroot():
+    Die('Please exit the chroot before trying to delete it.')
+
+  chroot_dir = _GetChrootAbsDir(chroot_config, check_exists=True)
+  if chroot_dir is None:
+    Die("Nothing to clean: the chroot doesn't exist.\n  %s" %
+        _GetChrootAbsDir(chroot_config))
+
+  if not want_force_yes:
+    sys.stderr.write('\n'
+                     'Chroot is at: %s\n'
+                     'Are you sure you want to delete it (YES/NO)? ' %
+                     chroot_dir)
+    answer = raw_input()
+    if answer != 'YES':

[sosa, 2011/01/11 04:44:22]

again with the YES

[diandersAtChromium, 2011/01/12 00:04:45]

Done.

+      Die("You must answer exactly 'YES' if you want to proceed.")
+
+  # Can pass argv and not shell=True, since no user flags.  :)
+  argv = ['./make_chroot', '--chroot=%s' % chroot_dir, '--delete']
+
+  # We'll put CWD as src/scripts when running the command.  Since everyone
+  # running by hand has their cwd there, it is probably the safest.
+  cwd = os.path.join(_SRCROOT_PATH, 'src', 'scripts')
+
+  # Run it.  Pass any failures upward.
+  RunCommand(argv, cwd=cwd)
+
+
+def _DoWrappedChrootCommand(target_cmd, host_cmd, raw_argv, need_args=False,
+                            chroot_config=None, argv=None, build_config=None):
+  """Helper function for any command that is simply wrapped by chromite.
+
+  These are commands where:
+  - We parse the command line only enough to figure out what board they
+    want.  All othe command line parsing is handled by the wrapped command.
+    Because of this, the board name _needs_ to be specified first.
+  - Everything else (arg parsing, help, etc) is handled by the wrapped command.
+    The usage string will be a little messed up, but hopefully that's OK.
+
+  Args:
+    target_cmd: We'll put this at the start of argv when calling a target
+        command.  We'll substiture %s with the target.
+        Like - ['my_command-%s'] or ['my_command', '--board=%s']
+    host_cmd: We'll put this at the start of argv when calling a host command.
+        Like - ['my_command']
+    raw_argv: Command line arguments, including this command's name, but not
+        the chromite command name or chromite options.
+    need_args: If True, we'll prompt for arguments if they weren't specified.
+        This makes the most sense when someone runs chromite with no arguments
+        and then walks through the menus.  It's not ideal, but less sucky than
+        just quitting.
+    chroot_config: A SafeConfigParser for the chroot config; or None chromite
+        was called from within the chroot.
+    argv: None when called normally, but contains argv with board stripped off
+        if we call ourselves with _DoEnterChroot().
+    build_config: None when called normally, but contains the SafeConfigParser
+        for the build config if we call ourselves with _DoEnterChroot().  Note
+        that even when called through _DoEnterChroot(), could still be None
+        if user chose 'HOST' as the target.
+  """
+  # If we didn't get called through EnterChroot, we need to read the build
+  # config.
+  if argv is None:
+    # We look for the build config without calling OptionParser.  This means
+    # that the board _needs_ to be first (if it's specified) and all options
+    # will be passed straight to our subcommand.
+    argv, build_config = _GetBuildConfigFromArgs(raw_argv[1:])
+
+  # Enter the chroot if needed...
+  if not IsInsideChroot():
+    _DoEnterChroot(chroot_config, _DoWrappedChrootCommand, target_cmd, host_cmd,
+                   raw_argv, need_args=need_args, argv=argv,
+                   build_config=build_config)
+  else:
+    # We'll put CWD as src/scripts when running the command.  Since everyone
+    # running by hand has their cwd there, it is probably the safest.
+    cwd = os.path.join(_SRCROOT_PATH, 'src', 'scripts')
+
+    # Get command to call.  If build_config is None, it means host.
+    if build_config is None:
+      argv_prefix = host_cmd
+    else:
+      # Make argv_prefix w/ target.
+      target_name = build_config.get('BUILD', 'target')
+      argv_prefix = [arg % target_name for arg in target_cmd]
+
+    # Not a great way to to specify arguments, but works for now...  Wrapped
+    # commands are not wonderful interfaces anyway...
+    if need_args and not argv:
+      while True:
+        sys.stderr.write('arg %d (blank to exit): ' % (len(argv)+1))
+        arg = raw_input()
+        if not arg:
+          break
+        argv.append(arg)
+
+    # Add the prefix...
+    argv = argv_prefix + argv
+
+    # Run, ignoring errors since some comamnds (ahem, cros_workon) seem to

[sosa, 2011/01/11 04:44:22]

typo

[diandersAtChromium, 2011/01/12 00:04:45]

Done.

+    # return errors from things like --help.
+    RunCommand(argv, cwd=cwd, ignore_sigint=True, error_ok=True)
+
+
+def _CmdBuild(raw_argv, chroot_config=None, loaded_config=False,
+              build_config=None):
+  """Build the chroot (if needed), the packages for a target, and the image.
+
+  Args:
+    raw_argv: Command line arguments, including this command's name, but not
+        the chromite command name or chromite options.
+    chroot_config: A SafeConfigParser for the chroot config; or None chromite
+        was called from within the chroot.
+    loaded_config: If True, we've already loaded the config.
+    build_config: None when called normally, but contains the SafeConfigParser
+        for the build config if we call ourselves with _DoEnterChroot().  Note
+        that even when called through _DoEnterChroot(), could still be None
+        if user chose 'HOST' as the target.
+  """
+  # Parse options for command...
+  usage_str = ('usage: %%prog [chromite_options] %s [options] [target]' %
+               raw_argv[0])
+  parser = optparse.OptionParser(usage=usage_str)
+  (options, argv) = parser.parse_args(raw_argv[1:])

[sosa, 2011/01/11 04:44:22]

Why are you skipping 1?

[diandersAtChromium, 2011/01/12 00:04:45]

I'm skipping argument 0.  From raw_argv desc, this list includes the command's
name.  That shouldn't be passed to the parse_args function.

On 2011/01/11 04:44:22, sosa wrote:

+
+  # We don't use any options...
+  _ = options

[sosa, 2011/01/11 04:44:22]

?

[diandersAtChromium, 2011/01/12 00:04:45]

Canonical way to make pylint and other automated checkers happy.  From pylint
docs, you can see that pylint will not (by default) complain about the variables
"_" and "dummy" being unused.  Thus, a good way to document that you have a
variable that is unused is to assign it to _.  See "dummy-variables-rgx" in
pylint.

This is probably more common in lua, where the standard language docs encourage
it.  An example from the lua docs: http://www.lua.org/pil/20.3.html

...however, I believe it is also the convension in python.  This stackoverflow
question might also be interesting:
  http://stackoverflow.com/questions/1739514/as-variable-name-in-python

On 2011/01/11 04:44:22, sosa wrote:

[sosa, 2011/01/13 19:07:04]

Um, why not just do (_, argv) = parser blah then?

On 2011/01/12 00:04:45, diandersAtChromium wrote:

[diandersAtChromium, 2011/01/13 21:26:50]

Could do that.  Somehow, I thought it was clearer in this case, but I guess not.
 I will change to your suggested syntax.

On 2011/01/13 19:07:04, sosa wrote:

+
+  # Load the build config if needed...
+  if not loaded_config:
+    argv, build_config = _GetBuildConfigFromArgs(argv)
+    if argv:
+      Die('Unknown arguments: %s' % ' '.join(argv))
+
+  if not IsInsideChroot():
+    _DoMakeChroot(chroot_config)
+
+    if build_config is not None:
+      _DoEnterChroot(chroot_config, _CmdBuild, raw_argv,
+                     build_config=build_config, loaded_config=True)
+
+    Info('Done building.')
+  else:
+    if build_config is None:
+      Die("You can't build the host chroot from within the chroot.")
+
+    _DoSetupBoard(build_config)
+    _DoBuildPackages(build_config)
+    _DoBuildImage(build_config)
+
+
+def _CmdWorkon(raw_argv, *args, **kwargs):
+  """Run cros_workon.

[sosa, 2011/01/11 04:44:22]

Why oh why are we wrapping these things????  I don't know if agree with this.

[diandersAtChromium, 2011/01/12 00:04:45]

See discussion at _CmdShell().

On 2011/01/11 04:44:22, sosa wrote:

+
+  This is just a wrapped command.
+
+  Args:
+    raw_argv: Command line arguments, including this command's name, but not
+        the chromite command name or chromite options.
+    args: The rest of the positional arguments.  See _DoWrappedChrootCommand.
+    kwargs: The keyword arguments.  See _DoWrappedChrootCommand.
+  """
+  # Slight optimization, just since I do this all the time...
+  if len(raw_argv) >= 2:
+    if raw_argv[1] in ('start', 'stop', 'list', 'list-all', 'iterate'):
+      Warn('OOPS, looks like you forgot a board name. Pick one.')
+      raw_argv = raw_argv[:1] + [''] + raw_argv[1:]
+
+  # Note that host version uses "./", since it's in src/scripts and not in the
+  # path...
+  _DoWrappedChrootCommand(['cros_workon-%s'], ['./cros_workon', '--host'],
+                          raw_argv, need_args=True, *args, **kwargs)
+
+
+def _CmdEbuild(raw_argv, *args, **kwargs):
+  """Run ebuild.
+
+  This is just a wrapped command.
+
+  Args:
+    raw_argv: Command line arguments, including this command's name, but not
+        the chromite command name or chromite options.
+    args: The rest of the positional arguments.  See _DoWrappedChrootCommand.
+    kwargs: The keyword arguments.  See _DoWrappedChrootCommand.
+  """
+  _DoWrappedChrootCommand(['ebuild-%s'], ['ebuild'],
+                          raw_argv, need_args=True, *args, **kwargs)
+
+
+def _CmdEmerge(raw_argv, *args, **kwargs):
+  """Run emerge.
+
+  This is just a wrapped command.
+
+  Args:
+    raw_argv: Command line arguments, including this command's name, but not
+        the chromite command name or chromite options.
+    args: The rest of the positional arguments.  See _DoWrappedChrootCommand.
+    kwargs: The keyword arguments.  See _DoWrappedChrootCommand.
+  """
+  _DoWrappedChrootCommand(['emerge-%s'], ['emerge'],
+                          raw_argv, need_args=True, *args, **kwargs)
+
+
+def _CmdEquery(raw_argv, *args, **kwargs):
+  """Run equery.
+
+  This is just a wrapped command.
+
+  Args:
+    raw_argv: Command line arguments, including this command's name, but not
+        the chromite command name or chromite options.
+    args: The rest of the positional arguments.  See _DoWrappedChrootCommand.
+    kwargs: The keyword arguments.  See _DoWrappedChrootCommand.
+  """
+  _DoWrappedChrootCommand(['equery-%s'], ['equery'],
+                          raw_argv, need_args=True, *args, **kwargs)
+
+
+def _CmdPortageq(raw_argv, *args, **kwargs):
+  """Run portageq.
+
+  This is just a wrapped command.
+
+  Args:
+    raw_argv: Command line arguments, including this command's name, but not
+        the chromite command name or chromite options.
+    args: The rest of the positional arguments.  See _DoWrappedChrootCommand.
+    kwargs: The keyword arguments.  See _DoWrappedChrootCommand.
+  """
+  _DoWrappedChrootCommand(['portageq-%s'], ['portageq'],
+                          raw_argv, need_args=True, *args, **kwargs)
+
+
+def _CmdShell(raw_argv, chroot_config=None):
+  """Start a shell in the chroot.

[sosa, 2011/01/11 04:44:22]

Why not use this rather than randomly have some explicit other ones?

[diandersAtChromium, 2011/01/12 00:04:45]

I think what you mean here is why do we have things like "emerge" as a special
chromite command.  Why not just let user get at emerge using the generic "shell"
syntax.

Sounds like you worked this out with Anush (from your recent comment in this
code review).

I think Chris's point was that the emerge, portage, etc commands don't seem to
"fit" the rest of chromite.  Also: using them really requires knowledge of the
chroot (chroot paths, etc), so maybe don't belong in chromite.

Only advantage I can see of wrapping commands like emerge and portage is that
they can refer to build specs.

On 2011/01/11 04:44:22, sosa wrote:

+
+  This can either just start a simple interactive shell, it can be used to
+  run an arbirtary command inside the chroot and then exit.
+
+  Args:
+    raw_argv: Command line arguments, including this command's name, but not
+        the chromite command name or chromite options.
+    chroot_config: A SafeConfigParser for the chroot config; or None chromite
+        was called from within the chroot.
+  """
+  # Parse options for command...
+  # ...note that OptionParser will eat the '--' if it's there, which is what
+  # we want..
+  usage_str = ('usage: %%prog [chromite_options] %s [options] [VAR=value] '
+               '[-- command [arg1] [arg2] ...]') % raw_argv[0]
+  parser = optparse.OptionParser(usage=usage_str)
+  (options, argv) = parser.parse_args(raw_argv[1:])
+
+  # We don't use any options...
+  _ = options
+
+  # Enter the chroot if needed...
+  if not IsInsideChroot():
+    _DoEnterChroot(chroot_config, _CmdShell, raw_argv)
+  else:
+    # We'll put CWD as src/scripts when running the command.  Since everyone
+    # running by hand has their cwd there, it is probably the safest.
+    cwd = os.path.join(_SRCROOT_PATH, 'src', 'scripts')
+
+    # By default, no special environment...
+    env = None
+
+    if not argv:
+      # If no arguments, we'll just start bash.
+      argv = ['bash']
+    else:
+      # Parse the command line, looking at the beginning for VAR=value type
+      # statements.  I couldn't figure out a way to get bash to do this for me.
+      user_env, argv = _SplitEnvFromArgs(argv)
+      if not argv:
+        Die('No command specified')
+
+      # If there was some environment, use it to override the standard
+      # environment.
+      if user_env:
+        env = dict(os.environ)
+        env.update(user_env)
+
+    # Don't show anything special for errors; we'll let the shell report them.
+    RunCommand(argv, cwd=cwd, env=env, error_ok=True, ignore_sigint=True)
+
+
+def _CmdClean(raw_argv, chroot_config=None):
+  """Clean out built packages for a target; if target is host, deletes chroot.
+
+  Args:
+    raw_argv: Command line arguments, including this command's name, but not
+        the chromite command name or chromite options.
+    chroot_config: A SafeConfigParser for the chroot config; or None chromite
+        was called from within the chroot.
+  """
+  # Parse options for command...
+  usage_str = ('usage: %%prog [chromite_options] %s [options] [target]' %
+               raw_argv[0])
+  parser = optparse.OptionParser(usage=usage_str)
+  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:])
+
+  # Make sure the chroot exists first, before possibly prompting for board...
+  # ...not really required, but nice for the user...
+  if not IsInsideChroot():
+    chroot_dir = _GetChrootAbsDir(chroot_config, check_exists=True)
+    if chroot_dir is None:
+      Die("Nothing to clean: the chroot doesn't exist.\n  %s" %
+          _GetChrootAbsDir(chroot_config))
+
+  # Load the build config...
+  argv, build_config = _GetBuildConfigFromArgs(argv)
+  if argv:
+    Die('Unknown arguments: %s' % ' '.join(argv))
+
+  # If they do clean host, we'll treat it as a distclean.  Does that mean
+  # We should get rid of distclean?
+  if build_config is None:
+    _DoDistClean(chroot_config, options.yes)

[sosa, 2011/01/13 19:07:04]

I think we should have one way to do it.

+  else:
+    _DoClean(chroot_config, build_config, options.yes)
+
+
+def _CmdDistClean(raw_argv, chroot_config=None):

[sosa, 2011/01/11 04:44:22]

There's two ways to do this???

[diandersAtChromium, 2011/01/12 00:04:45]

Yes, at the moment.  Distclean was introduced before 'host' was a target.  Once
host was a target, it seemed to make sense to have 'clean host' do distclean,
but I'm not sure.  There is a comment above asking "Does that mean we should get
rid of distclean"?

I would love your opinion on which of the three things we should do:
1. Make 'clean host' an error condition

2. Remove 'distclean'.  This assumes 'clean host' is discoverable enough.

3. Allow either 'clean host' or 'distclean'.


I am fine with any of the three and would love to get outside opinions.

On 2011/01/11 04:44:22, sosa wrote:

+  """Delete the chroot.
+
+  Args:
+    raw_argv: Command line arguments, including this command's name, but not
+        the chromite command name or chromite options.
+    chroot_config: A SafeConfigParser for the chroot config; or None chromite
+        was called from within the chroot.
+  """
+  # Parse options for command...
+  usage_str = 'usage: %%prog [chromite_options] %s [options]' % raw_argv[0]
+  parser = optparse.OptionParser(usage=usage_str)
+  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:])
+
+  if argv:
+    Die('Unknown arguments: %s' % ' '.join(argv))
+
+  # Now that we've parsed arguments, really do the cleaning.
+  _DoDistClean(chroot_config, options.yes)
+
+
+def main():
+  help_str = (
+      """Usage: %(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.\n"""
+      """\n"""
+      """Options:\n"""
+      """  -h, --help            show this help message and exit\n"""
+  ) % {'prog': os.path.basename(sys.argv[0])}
+  if not IsInsideChroot():
+    help_str += (
+        """  --chroot=CHROOT_NAME  Chroot spec to use. Can be an absolute\n"""
+        """                        path to a spec file or a substring of a\n"""
+        """                        chroot spec name (without .spec suffix)\n"""

[sosa, 2011/01/11 04:44:22]

I think you should be able to do some better stuff with options parser to make
the help more useful. It doesn't seem that using --help will ever be useful.

[diandersAtChromium, 2011/01/12 00:04:45]

Added a TODO for this.

On 2011/01/11 04:44:22, sosa wrote:

+    )
+
+  # We don't use OptionParser here, since options for different subcommands are
+  # so different.  We just look for the chromite options here...
+  if sys.argv[1:2] == ['--help']:

[sosa, 2011/01/11 04:44:22]

sys.argv[1]?

[diandersAtChromium, 2011/01/12 00:04:45]

This is a shorthand for:
  if len(sys.argv) >= 1 and sys.argv[1] == '--help'.

...if you think it's too "clever" and less readable, I will change the syntax.

On 2011/01/11 04:44:22, sosa wrote:

+    print help_str
+    sys.exit(0)
+  elif sys.argv[1:2] == ['--resume-state']:
+    # Internal mechanism (not documented to users) to resume in the chroot.
+    # ...actual resume state file is passed in sys.argv[2] for simplicity...
+    assert len(sys.argv) == 3, 'Resume State not passed properly.'
+    fn, args, kwargs = cPickle.load(open(sys.argv[2], 'rb'))
+    fn(*args, **kwargs)
+  else:
+    # Start by skipping argv[0]
+    argv = sys.argv[1:]
+
+    # Look for special "--chroot" argument to allow for alternate chroots
+    if not IsInsideChroot():
+      # Default chroot name...
+      chroot_name = 'chroot'
+
+      # Get chroot spec name if specified; trim argv down if needed...
+      if argv:
+        if argv[0].startswith('--chroot='):

[sosa, 2011/01/11 04:44:22]

Why is there some strict ordering here :(

[diandersAtChromium, 2011/01/12 00:04:45]

Talked verbally about this.  Hard to have arbitrary ordering with subcommands.

On 2011/01/11 04:44:22, sosa wrote:

+          _, chroot_name = argv[0].split('=', 2)
+          argv = argv[1:]
+        elif argv[0] == '--chroot':
+          if len(argv) < 2:
+            Die('Chroot not specified.')
+
+          chroot_name = argv[1]
+          argv = argv[2:]
+
+      chroot_spec_path = _FindSpec(chroot_name, spec_type=_CHROOT_SPEC_TYPE)
+      if chroot_spec_path is None:
+        Die('OK, cancelling...')
+
+      Info('Using chroot "%s"' % os.path.relpath(chroot_spec_path))
+
+      chroot_config = _ReadConfig(chroot_spec_path)
+    else:
+      # Already in the chroot; no need to get config...
+      chroot_config = None
+
+    # Get command and arguments
+    if argv:
+      chromite_cmd = argv[0].lower()
+      argv = argv[1:]
+    else:
+      chromite_cmd = ''
+
+    # Validate the chromite_cmd, popping a menu if needed.
+    chromite_cmd = _FindCommand(chromite_cmd)
+    if chromite_cmd is None:
+      Die('OK, cancelling...')
+
+    # Finally, call the function w/ standard argv.
+    cmd_fn = eval(_COMMAND_HANDLERS[_COMMAND_STRS.index(chromite_cmd)])
+    cmd_fn([chromite_cmd] + argv, chroot_config=chroot_config)
+
+
+if __name__ == '__main__':
+  main()