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

chromite/chromite.py

Index: chromite/chromite.py
diff --git a/chromite/chromite.py b/chromite/chromite.py
new file mode 100755
index 0000000000000000000000000000000000000000..ded4d77456f0e32009ce71fdcd95f76eec83e4d0
--- /dev/null
+++ b/chromite/chromite.py
@@ -0,0 +1,712 @@
+#!/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 base64

[sosa, 2010/12/23 01:52:27]

Maybe sort ascii ... Capitals first, lowercase second.  Preference though

[diandersAtChromium, 2011/01/06 00:23:23]

No.  Current order matches Google python conventions: "Within each grouping,
imports should be sorted lexicographically, ignoring case, according to each
module's full package path."

+import ConfigParser
+import cPickle as pickle

[sosa, 2010/12/23 01:52:27]

it's preferred not to rename.

[diandersAtChromium, 2011/01/06 00:23:23]

Done.

+import optparse
+import os
+import StringIO
+import sys
+
+
+# Ugly chunk of code to find the Chromium OS root and Chromite root, even if
+# we're not in the chroot.
+if 'CROS_WORKON_SRCROOT' in os.environ:
+  _SRCROOT_PATH = os.environ['CROS_WORKON_SRCROOT']
+  _CHROMITE_PATH = os.path.join(_SRCROOT_PATH, 'src', 'scripts', 'chromite')
+else:
+  _CHROMITE_PATH = os.path.dirname(os.path.realpath(__file__))
+  _SRCROOT_PATH = os.path.realpath(os.path.join(_CHROMITE_PATH,

[sosa, 2010/12/23 01:52:27]

can't you always do this?  Just just use the else.

[diandersAtChromium, 2011/01/06 00:23:23]

Done.

+                                                '..', '..', '..'))
+sys.path.insert(0, os.path.join(_CHROMITE_PATH, 'lib'))

[sosa, 2010/12/23 01:52:27]

Can't you do some gooey goodness with __init__.py files?  Scottz was using this
methodology

[diandersAtChromium, 2011/01/06 00:23:23]

...or, I can just not be completely stupid and change these to things like 'from
lib import text_menu'.  Original code was looking at "../lib", which is why the
complexity was originally there.  There had also been talk about copying
chromite.py to an alternate location, but decided instead of use chromite.sh
wrapper.

+
+
+# Local library imports
+from cros_build_lib import Die
+from cros_build_lib import Info
+from cros_build_lib import RunCommand
+from cros_build_lib import RunCommandError
+import text_menu
+
+
+# Usage and description strings for OptionParser.
+_USAGE = 'usage: %prog [options] [cmd [build.spec]]'

[sosa, 2010/12/23 01:52:27]

Define these in your main

[diandersAtChromium, 2011/01/06 00:23:23]

Done.

+_DESCRIPTION = """The chromite script is a wrapper to make it easy to do
+various build things.  Current commands: %(command_list)s.  For a description
+of commands, run chromite without any options."""
+
+
+# A list of paths that we'll search through to find spec files.

[sosa, 2010/12/23 01:52:27]

Why are these globals?  You use both of these once.  Just define where they are
used.  

Also why are these arrays?  Will we ever want more than one?

[diandersAtChromium, 2011/01/06 00:23:23]

Moved to where they are used; they are lists in anticipation of the need to
support specs that live in private overlays.  Added a comment to that effect.

On 2010/12/23 01:52:27, sosa wrote:

+_BUILD_SPEC_SEARCH_PATH = [
+    os.path.join(_CHROMITE_PATH, 'specs', 'build'),
+]
+_CHROOT_SPEC_SEARCH_PATH = [
+    os.path.join(_CHROMITE_PATH, 'specs', 'chroot'),
+]
+
+
+# Spec files must end with this suffix.
+_SPEC_SUFFIX = '.spec'
+
+
+# This "file" will be fed into the SafeConfigParser to provide defaults.
+_DEFAULT_BUILD_SPEC = """

[sosa, 2010/12/23 01:52:27]

Why are these defined here vs legacy.spc etc

[diandersAtChromium, 2011/01/06 00:23:23]

Done.  Moved to specs/build/_defaults and specs/chroot/_defaults.  Since I could
put documentation in those file more cleanly, removed some of the extra cruft
from the spec files so we only specify things that we care about.

On 2010/12/23 01:52:27, sosa wrote:

+[LEGACY]
+board: %(name)s
+chroot_spec: chroot
+
+[LEGACY_BUILD]
+setup_board_flags:
+build_packages_flags:
+
+[LEGACY_IMAGE]
+build_image_flags:
+"""
+
+# This "file" will be fed into the SafeConfigParser to provide defaults.
+_DEFAULT_CHROOT_SPEC = """
+[LEGACY_CHROOT]
+path: %(name)s
+make_chroot_flags:
+enter_chroot_flags:
+"""
+
+# 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 = [
+    '_CmdBuild',
+    '_CmdClean',
+    '_CmdDistClean',
+]
+_COMMAND_STRS = [fn_str[len('_Cmd'):].lower() for fn_str in _COMMAND_HANDLERS]
+
+
+def _IsInsideChroot():

[sosa, 2010/12/23 01:52:27]

Already defined in chromite lib

[diandersAtChromium, 2011/01/06 00:23:23]

Done.

+  """Returns True if we're inside the chroot; False if not."""
+  return os.path.exists('/etc/debian_chroot')
+
+
+def _GetBoardDir(build_config):
+  """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.
+  """
+  board_name = build_config.get('LEGACY', 'board')
+
+  # 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 board_name, "Didn't expect blank board name."
+  assert len(board_name.split()) == 1, 'Board name should have no white space.'
+
+  return os.path.join('/', 'build', board_name)
+
+
+def _GetChrootAbsDir(chroot_config):
+  """Returns the absolute chroot directory the chroot config.
+
+  Args:
+    chroot_config: A SafeConfigParser representing the config for the chroot.
+  Returns:
+    The chroot directory, always absolute.
+  """
+  chroot_dir = chroot_config.get('LEGACY_CHROOT', 'path')
+  return os.path.join(_SRCROOT_PATH, chroot_dir)
+
+
+def _FindCommand(cmd_name):

[sosa, 2010/12/23 01:52:27]

Def good candidate for unit test.  Mock out TextMenu call

+  """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 substring 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 cmd_name in this_cmd:

[sosa, 2010/12/23 01:52:27]

regex is better.  "in" does any substring ... not any prefix.

[diandersAtChromium, 2011/01/06 00:23:23]

Changed to startswith().  You're right that prefix is better than substring
here.

On 2010/12/23 01:52:27, sosa wrote:

+      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".  '

[sosa, 2010/12/23 01:52:27]

I would consider throwing exceptions rather than dying here.  Exceptions are
preferred in Python

[diandersAtChromium, 2011/01/06 00:23:23]

I don't think so in this case.  Exceptions are better in weird error conditions.
 ...but in directly in response to a user action, they are confusing.  Saying
"./chromite implode' should clearly tell the user that there is no implode
command, not display an ugly stack trace.

Let me know if you feel strongly about making it an exception and we can talk.

On 2010/12/23 01:52:27, sosa wrote:

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

I can go either way.  

On 2011/01/06 00:23:23, diandersAtChromium wrote:

+        'Try running with no arguments for a menu.' %
+        cmd_name)
+
+  if len(possible_choices) == 1:
+    choice = 0
+    Info("Running command '%s'.\n" % possible_cmds[choice])
+  else:
+    choice = text_menu.TextMenu(possible_choices, 'Which chromite command:',

[sosa, 2010/12/23 01:52:27]

neat!

+                                menu_width=0)
+  return possible_cmds[choice]
+
+
+def _FindSpec(spec_name, is_chroot_spec=False, 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

[sosa, 2010/12/23 01:52:27]

I think it'd be better to combine 2 and 3 into one ... any arbitrary prefix is a
subset of an exact match.  This also cleans up your logic.  Just search for
prefix using regex.  Don't even bother trying to optimize for case 2 since it
needlessly complicates this method.

[diandersAtChromium, 2011/01/06 00:23:23]

No.  I really wanted a substring for finding the spec.  In particular, I want to
match "x86-mario" when the user passes "mario".  Going to leave it how it is.

On 2010/12/23 01:52:27, sosa wrote:

+           (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.
+    is_chroot_spec: If True, this is a chroot spec.
+    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.
+  """
+  # Get a string for the spec type (for debugging)
+  if is_chroot_spec:
+    spec_type = 'chroot'
+  else:
+    spec_type = 'build'
+
+  # 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.
+  if is_chroot_spec:
+    search_path = _CHROOT_SPEC_SEARCH_PATH
+  else:
+    search_path = _BUILD_SPEC_SEARCH_PATH
+
+  # Look for an exact match of a spec name.  An exact match will go through with
+  # no menu.
+  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:
+    for file_name in os.listdir(dir_path):
+      file_path = os.path.join(dir_path, file_name)
+      file_base_path, _ = os.path.splitext(file_path)
+
+      # Skip if this isn't a spec file.  Use samefile to check, which I think
+      # avoids any case-sensitiveness in the filesystem.
+      if not os.path.samefile(file_path, file_base_path + _SPEC_SUFFIX):
+        continue

[sosa, 2010/12/23 01:52:27]

add line after

[diandersAtChromium, 2011/01/06 00:23:23]

Done.

+      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:
+        continue
+
+      # OK, it's good.  Store the path.
+      options[this_spec_name] = file_path

[sosa, 2010/12/23 01:52:27]

You could probably merge the majority of this function and the one above into
one function ... takes array of strings + descriptions + prefix ... if 1 ->
return string with prefix o/w menu -> then return selection

[diandersAtChromium, 2011/01/06 00:23:23]

I'd rather not merge them.  It is true that there are similarities, but there
are also subtle differences that would complicate the merged function.  To me,
TextMenu() seems the right level of abstraction.

On 2010/12/23 01:52:27, sosa wrote:

+
+  # 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 len(options) == 1:
+    _, 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 build spec:')
+  return options[option_keys[choice]]
+
+
+def _ParseCommandLine(arguments):
+  """Parse the command line to figure out the build command and build spec.
+
+  Note that the command and buildspec are specified directly on the command
+  line (not using "options") because every command needs to specify them and
+  we want to make it that much easier for people to type the commands.  Also
+  note that the buildspec can be specified as any substring of the spec name
+  (and if that specifies more than one thing, we'll show a menu).
+
+  That means you can run chromite like this:
+    chromite build mario
+  ...instead of:
+    chromite --cmd=build --board=x86-mario
+
+  If the user specified a bad command line, this function will call Die().
+
+  Args:
+    arguments: The non options from the OptionParser.
+
+  Returns:
+    chromite_cmd: The chromite command that was specified (default is "build").
+    build_spec_path: The path to the build spec.
+  """
+  # Set defaults.
+  chromite_cmd = ''
+  spec_name = ''
+
+  # Make a copy of arguments so we can futz with it and caller doesn't see it
+  # change...
+  arguments = list(arguments)

[sosa, 2010/12/23 01:52:27]

[] is a shorthand for list() is it not?

[diandersAtChromium, 2011/01/06 00:23:23]

You are saying to do this instead?
  arguments = arguments[:]

That certainly does work if arguments is a list.  However, if incoming arguments
happens to be something else (tuple, numpy array, generator, ...) then it might
not do what you want.  I have gotten in the habit of copying lists with the list
constructor to be flexible in what type of sequence I take in (just as I would
be flexible if I didn't copy/futz).

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

I meant arguments = [arguments].  Are these not equivalent?

On 2011/01/06 00:23:23, diandersAtChromium wrote:

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

Not equivalent.

In [1]: l = range(10)

In [2]: list(l)
Out[2]: [0, 1, 2, 3, 4, 5, 6, 7, 8, 9]

In [3]: [l]
Out[3]: [[0, 1, 2, 3, 4, 5, 6, 7, 8, 9]]

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

+
+  # Pull the command off if it was specified.
+  if arguments:
+    chromite_cmd = arguments.pop(0).lower()
+
+  # Validate the chromite_cmd, popping a menu if needed.
+  try:
+    chromite_cmd = _FindCommand(chromite_cmd)
+  except KeyboardInterrupt:
+    Die('OK, cancelling...')

[sosa, 2010/12/23 01:52:27]

Maybe you should add a extra option that does exit rather than having Ctrl+c a
normal part of the routine

[diandersAtChromium, 2011/01/06 00:23:23]

Done.

+
+  # There should be 0 or 1 more arguments.
+  # If too many: error
+  # If 1, it's the spec name.
+  # If 0, we'll use the default spec name.
+  if len(arguments) > 1:
+    Die('Too many arguments.  Try passing --help.')
+  elif arguments:
+    spec_name = arguments[0]
+
+  # Find the spec given the name...
+  try:
+    build_spec_path = _FindSpec(spec_name)
+  except KeyboardInterrupt:
+    Die('OK, cancelling...')

[sosa, 2010/12/23 01:52:27]

same

[diandersAtChromium, 2011/01/06 00:23:23]

Done.

+
+  return chromite_cmd, build_spec_path
+
+
+def _ReadConfigs(build_spec_path):
+  """Read the build_config and chroot_config from spec files.
+
+  Args:
+    build_spec_path: The path to the build spec.
+
+  Returns:
+    build_config: A SafeConfigParser representing the config that we're
+        building.
+    build_config: A SafeConfigParser representing the config of the chroot.
+  """
+  build_spec_name, _ = os.path.splitext(os.path.basename(build_spec_path))
+
+  build_config = ConfigParser.SafeConfigParser()
+  default_build_spec = _DEFAULT_BUILD_SPEC % dict(name=build_spec_name)

[sosa, 2010/12/23 01:52:27]

{} not dict

[diandersAtChromium, 2011/01/06 00:23:23]

Done.

+  build_config.readfp(StringIO.StringIO(default_build_spec), '<defaults>')
+  build_config.read(build_spec_path)
+
+  chroot_spec = build_config.get('LEGACY', 'chroot_spec')

[sosa, 2010/12/23 01:52:27]

erm why read specified build spec but legacy chroot spec?

[diandersAtChromium, 2011/01/06 00:23:23]

This is the chroot spec specified in the LEGACY section of the build spec. 
Maybe it shouldn't belong in there.  I'm not 100% sure I understand Anush's
thoughts on how this will evolve into the final Chromite design.  He proposed
things in various LEGACY sections, and I thought this was what he meant.  I'll
check.

On 2010/12/23 01:52:27, sosa wrote:

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

ping @anush

On 2011/01/06 00:23:23, diandersAtChromium wrote:

+  chroot_spec_path = _FindSpec(chroot_spec, is_chroot_spec=True,
+                               can_show_ui=False)
+  chroot_spec_name, _ = os.path.splitext(os.path.basename(chroot_spec_path))
+
+  default_chroot_spec = _DEFAULT_CHROOT_SPEC % dict(name=chroot_spec_name)

[sosa, 2010/12/23 01:52:27]

{}

[diandersAtChromium, 2011/01/06 00:23:23]

Done.

+  chroot_config = ConfigParser.SafeConfigParser()
+  chroot_config.readfp(StringIO.StringIO(default_chroot_spec), '<defaults>')
+  chroot_config.read(chroot_spec_path)
+
+  return build_config, chroot_config
+
+
+def _DoesChrootExist(chroot_config):
+  """Returns whether the chroot already appears to exist.
+
+  Args:
+    chroot_config: A SafeConfigParser representing the config for the chroot.
+
+  Returns:
+    True if the chroot appears to exist; False if it appears not to exist.
+    Note that we're just checking for the existence of the folder--we don't
+    check whether the chroot was properly configured.
+  """
+  chroot_dir = _GetChrootAbsDir(chroot_config)
+  return os.path.isdir(chroot_dir)
+
+
+def _DoMakeChroot(chroot_config):
+  """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)

[sosa, 2010/12/23 01:52:27]

 use DoesChrootExist()

[diandersAtChromium, 2011/01/06 00:23:23]

Added ability for _GetChrootAbsDir() to check and removed _DoesChrootExist(). 
Good call.

On 2010/12/23 01:52:27, sosa wrote:

+  if os.path.isdir(chroot_dir):
+    Info('%s already exists, skipping make_chroot.' % chroot_dir)
+    return
+
+  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' % (

[sosa, 2010/12/23 01:52:27]

Why are you setting --chroot here if we always use the default in this CL?  Also
why not just put --chroot option in make_chroot_flags?

[diandersAtChromium, 2011/01/06 00:23:23]

Boards can specify alternate chroots.  The --chroot is not included in the
make_chroot_flags because it needs to be common between the make_chroot_flags
and enter_chroot_flags.

On 2010/12/23 01:52:27, sosa wrote:

+      chroot_dir,
+      chroot_config.get('LEGACY_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.  Pass any failures upward.
+  try:
+    RunCommand(cmd, shell=True, cwd=cwd, ignore_sigint=True)
+  except RunCommandError, e:
+    Die(str(e))

[sosa, 2010/12/23 01:52:27]

don't catch it

[diandersAtChromium, 2011/01/06 00:23:23]

Done.

+
+
+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, encoded, and sent through the command line.
+    - That means that args must be pickleable and not too huge.
+    - 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 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.
+  """
+  Info('ENTERING THE CHROOT')
+
+  # Encode our state in something that can be passed on the command line.
+  # TODO(dianders): What kind of limits do we have here?  64K?
+  resume_state = pickle.dumps((fn, args, kwargs), pickle.HIGHEST_PROTOCOL)
+  resume_state = base64.b64encode(resume_state)

[sosa, 2010/12/23 01:52:27]

Can you store state to a temp file?  tempfile.mkstemp.  I'm worried these
command lines might get too long

[diandersAtChromium, 2011/01/06 00:23:23]

I researched this and it looks like I have 2 megs for command line on my system
(getconf ARG_MAX); but agree that temp file is cleaner and more portable. 
Changed to use tempfile.NamedTemporaryFile().

On 2010/12/23 01:52:27, sosa wrote:

+
+  # 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') % (
+          _GetChrootAbsDir(chroot_config),
+          chroot_config.get('LEGACY_CHROOT', 'enter_chroot_flags'),
+          resume_state)
+
+  # 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)
+
+
+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)

[sosa, 2010/12/23 01:52:27]

How do you --force?

[diandersAtChromium, 2011/01/06 00:23:23]

You don't.  You do a ./chromite clean first.  This isn't quite as ideal (doesn't
background the rm -rf), but hopefully is sufficient?

On 2010/12/23 01:52:27, sosa wrote:

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

Difference between

make_chroot --replace
and setup_board --force?

There will be an outcry!

On 2011/01/06 00:23:23, diandersAtChromium wrote:

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

Done.

+  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('LEGACY', 'board'),
+      build_config.get('LEGACY_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.  Exit upon failure.
+  try:
+    RunCommand(cmd, shell=True, cwd=cwd, ignore_sigint=True)
+  except RunCommandError, e:
+    Die(str(e))

[sosa, 2010/12/23 01:52:27]

ditto on the die here

[diandersAtChromium, 2011/01/06 00:23:23]

Done.

+
+
+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('LEGACY', 'board'),
+      build_config.get('LEGACY_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.  Exit upon failure.
+  try:
+    RunCommand(cmd, shell=True, cwd=cwd, ignore_sigint=True)
+  except RunCommandError, e:
+    Die(str(e))
+
+
+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('LEGACY', 'board'),
+      build_config.get('LEGACY_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.  Exit upon failure.
+  try:
+    RunCommand(cmd, shell=True, cwd=cwd, ignore_sigint=True)
+  except RunCommandError, e:
+    Die(str(e))
+
+
+def _CmdBuild(chromite_cmd, build_config, chroot_config, options):
+  """Build the chroot, the packages, and the image for a board.
+
+  This is the main chromite command and builds an image for you.
+
+  Args:
+    chromite_cmd: The command that was called.
+    build_config: A SafeConfigParser representing the build config.
+    chroot_config: A SafeConfigParser representing the chroot config.
+    options: Options from the OptionParser
+  """
+  if not _IsInsideChroot():
+    _DoMakeChroot(chroot_config)
+    _DoEnterChroot(chroot_config, _CmdBuild, chromite_cmd, build_config,
+                   chroot_config, options)
+  else:
+    _DoSetupBoard(build_config)
+    _DoBuildPackages(build_config)
+    _DoBuildImage(build_config)
+
+
+def _CmdClean(chromite_cmd, build_config, chroot_config, options):
+  """Clean out built packages for a board.

[sosa, 2010/12/23 01:52:27]

Wait, what is this for? When would we want to do this but not do setup_board
--force?

[diandersAtChromium, 2011/01/06 00:23:23]

You would do this instead of setup_board --force.  This is similar to most other
workflows (Makefiles, VC++, etc) where clean is a separate step and not an
option to the standard build.  However, if we want to keep the existing "1 step"
process, we could probably add an option to build so you could do something like
"./chromite build mario --clean" (or --force).  What do you think?

On 2010/12/23 01:52:27, sosa wrote:

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

That is nice!

On 2011/01/06 00:23:23, diandersAtChromium wrote:

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

Done.

+
+  Args:
+    chromite_cmd: The command that was called.
+    build_config: A SafeConfigParser representing the build config.
+    chroot_config: A SafeConfigParser representing the chroot config.
+    options: Options from the OptionParser
+  """
+  # These vars are part of the standard signature, but not used.
+  _ = chromite_cmd
+
+  # We'll need the directory so we can delete stuff.
+  board_dir = _GetBoardDir(build_config)
+
+  if not _IsInsideChroot():
+    if not _DoesChrootExist(chroot_config):
+      Die("Chroot doesn't appear to exist, nothing to do.")
+
+    # We'll need to make the board directory relative to the chroot.
+    chroot_dir = _GetChrootAbsDir(chroot_config)
+    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 options.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':
+      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'
+  assert board_dir != '/', 'Board dir better not be /'
+  assert '*' not in board_dir, 'Board dir better not have any *s'
+  assert build_config.get('LEGACY', 'board'), 'Board better not be blank'
+  assert build_config.get('LEGACY', 'board') in board_dir, \
+         'Board name better be in board dir'
+
+  args = ['sudo', '--', 'rm', '-rf', board_dir]
+  RunCommand(args)
+  Info('Deleted: %s' % board_dir)
+
+
+def _CmdDistClean(chromite_cmd, build_config, chroot_config, options):
+  """Delete the chroot.
+
+  Args:
+    chromite_cmd: The command that was called.
+    build_config: A SafeConfigParser representing the build config.
+    chroot_config: A SafeConfigParser representing the chroot config.
+    options: Options from the OptionParser
+  """
+  # These vars are part of the standard signature, but not used.
+  _ = (chromite_cmd, build_config)
+
+  if _IsInsideChroot():
+    Die('Please exit the chroot before running distclean.')
+
+  chroot_dir = _GetChrootAbsDir(chroot_config)
+
+  if not os.path.isdir(chroot_dir):

[sosa, 2010/12/23 01:52:27]

This isn't necessary.  Make_chroot --delete handles this

[diandersAtChromium, 2011/01/06 00:23:23]

Yes, but it is a nicety to do this check before giving the dire warning.  I will
delete it if you want me to, though.  I suppose one option would be add the dire
warning to "make_chroot --delete --warn", then I could avoid the duplication.

On 2010/12/23 01:52:27, sosa wrote:

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

The question is fine.  I specifically meant the os.path.isdir check / Die messg

On 2011/01/06 00:23:23, diandersAtChromium wrote:

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

Done.

+    Die("Nothing to clean: the chroot doesn't exist.\n  %s" %
+        chroot_dir)
+
+  if not options.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':
+      Die("You must answer exactly 'YES' if you want to proceed.")
+
+  # Can pass args and not shell=True, since no user flags.  :)
+  args = ['./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(args, cwd=cwd)
+
+
+def main():
+  command_list = ', '.join(sorted(_COMMAND_STRS))
+  parser = optparse.OptionParser(
+      usage=_USAGE, description=_DESCRIPTION % dict(command_list=command_list)
+  )
+  parser.add_option('-y', '--yes', default=False, action='store_true',
+                    help='Answer "YES" to "are you sure?" questions.')
+  parser.add_option('--resume-state', default='',
+                    help='Base64 of state pickle (internal use only).')
+  (options, arguments) = parser.parse_args()
+
+  if options.resume_state:
+    # We've called to resume ourselves in the chroot.
+    fn, args, kwargs = pickle.loads(base64.b64decode(options.resume_state))
+    fn(*args, **kwargs)
+  else:
+    chromite_cmd, build_spec_path = _ParseCommandLine(arguments)
+    Info("Running command '%s' with spec:\n  %s" % (chromite_cmd,
+                                                    build_spec_path))
+
+    build_config, chroot_config = _ReadConfigs(build_spec_path)
+
+    cmd_fn = eval(_COMMAND_HANDLERS[_COMMAND_STRS.index(chromite_cmd)])
+    cmd_fn(chromite_cmd, build_config, chroot_config, options)
+

[sosa, 2010/12/23 01:52:27]

2 lines here

[diandersAtChromium, 2011/01/06 00:23:23]

Done.

+if __name__ == '__main__':
+  main()