Add more error checking to preflight queue.

bin/cbuildbot.py

Index: bin/cbuildbot.py
diff --git a/bin/cbuildbot.py b/bin/cbuildbot.py
index 372ddcaf5ff646d2a966b897cffe00c95d9beea7..2168b811aabdade948e8f5e17b7793708129b4d7 100755
--- a/bin/cbuildbot.py
+++ b/bin/cbuildbot.py
@@ -181,37 +181,34 @@ def _ParseRevisionString(revision_string, repo_dictionary):
   return revisions.items()
 
 
-def _UprevFromRevisionList(buildroot, tracking_branch, revision_list, board):
+def _UprevFromRevisionList(buildroot, tracking_branch, revision_list, board,
+                           overlays):
   """Uprevs based on revision list."""
   if not revision_list:
     Info('No packages found to uprev')
     return
 
-  package_str = ''
+  packages = []
   for package, revision in revision_list:
-    package_str += package + ' '
-
-  package_str = package_str.strip()
+    packages.append(package)
 
   cwd = os.path.join(buildroot, 'src', 'scripts')
-  # TODO(davidjames): --foo="bar baz" only works here because we're using
-  # enter_chroot.
   RunCommand(['./cros_mark_as_stable',
               '--board=%s' % board,
-              '--tracking_branch="%s"' % tracking_branch,
-              '--packages="%s"' % package_str,
+              '--tracking_branch=%s' % tracking_branch,
+              '--overlays=%s' % ':'.join(overlays),
+              '--packages=%s' % ':'.join(packages),
               'commit'],
               cwd=cwd, enter_chroot=True)
 
 
-def _UprevAllPackages(buildroot, tracking_branch, board):
+def _UprevAllPackages(buildroot, tracking_branch, board, overlays):
   """Uprevs all packages that have been updated since last uprev."""
   cwd = os.path.join(buildroot, 'src', 'scripts')
-  # TODO(davidjames): --foo="bar baz" only works here because we're using
-  # enter_chroot.
   RunCommand(['./cros_mark_as_stable', '--all',
               '--board=%s' % board,
-              '--tracking_branch="%s"' % tracking_branch, 'commit'],
+              '--overlays=%s' % ':'.join(overlays),
+              '--tracking_branch=%s' % tracking_branch, 'commit'],
               cwd=cwd, enter_chroot=True)
 
 
@@ -228,12 +225,13 @@ def _GetVMConstants(buildroot):
   return (vdisk_size.strip(), statefulfs_size.strip())
 
 
-def _GitCleanup(buildroot, board, tracking_branch):
+def _GitCleanup(buildroot, board, tracking_branch, overlays):
   """Clean up git branch after previous uprev attempt."""
   cwd = os.path.join(buildroot, 'src', 'scripts')
   if os.path.exists(cwd):
     RunCommand(['./cros_mark_as_stable', '--srcroot=..',
                 '--board=%s' % board,
+                '--overlays=%s' % ':'.join(overlays),
                 '--tracking_branch=%s' % tracking_branch, 'clean'],
                cwd=cwd, error_ok=True)
 
@@ -257,9 +255,9 @@ def _WipeOldOutput(buildroot):
 # =========================== Main Commands ===================================
 
 
-def _PreFlightRinse(buildroot, board, tracking_branch):
+def _PreFlightRinse(buildroot, board, tracking_branch, overlays):
   """Cleans up any leftover state from previous runs."""
-  _GitCleanup(buildroot, board, tracking_branch)
+  _GitCleanup(buildroot, board, tracking_branch, overlays)
   _CleanUpMountPoints(buildroot)
   RunCommand(['sudo', 'killall', 'kvm'], error_ok=True)
 
@@ -347,7 +345,7 @@ def _RunSmokeSuite(buildroot, results_dir):
               ], cwd=cwd, error_ok=False)
 
 
-def _UprevPackages(buildroot, tracking_branch, revisionfile, board):
+def _UprevPackages(buildroot, tracking_branch, revisionfile, board, overlays):
   """Uprevs a package based on given revisionfile.
 
   If revisionfile is set to None or does not resolve to an actual file, this
@@ -376,26 +374,19 @@ def _UprevPackages(buildroot, tracking_branch, revisionfile, board):
   #  print >> sys.stderr, 'CBUILDBOT Revision list found %s' % revisions
   #  revision_list = _ParseRevisionString(revisions,
   #      _CreateRepoDictionary(buildroot, board))
-  #  _UprevFromRevisionList(buildroot, tracking_branch, revision_list, board)
+  #  _UprevFromRevisionList(buildroot, tracking_branch, revision_list, board,
+  #                         overlays)
   #else:
   Info('CBUILDBOT Revving all')
-  _UprevAllPackages(buildroot, tracking_branch, board)
+  _UprevAllPackages(buildroot, tracking_branch, board, overlays)
 
 
 def _UprevPush(buildroot, tracking_branch, board, overlays):
   """Pushes uprev changes to the main line."""
   cwd = os.path.join(buildroot, 'src', 'scripts')
-  public_overlay = '%s/src/third_party/chromiumos-overlay' % buildroot
-  private_overlay = '%s/src/private-overlays/chromeos-overlay' % buildroot
-  if overlays == 'private':
-    overlays = [private_overlay]
-  elif overlays == 'public':
-    overlays = [public_overlay]
-  else:
-    overlays = [public_overlay, private_overlay]
   RunCommand(['./cros_mark_as_stable', '--srcroot=..',
               '--board=%s' % board,
-              '--overlays=%s' % " ".join(overlays),
+              '--overlays=%s' % ':'.join(overlays),
               '--tracking_branch=%s' % tracking_branch,
               '--push_options=--bypass-hooks -f', 'push'],
              cwd=cwd)
@@ -466,6 +457,19 @@ def _GetConfig(config_name):
 
   return buildconfig
 
+def ResolveOverlays(overlays):
+  """Return the list of overlays to use for a given buildbot."""

[diandersAtChromium, 2010/11/03 23:53:34]

nit: Should describe overlays arg in docstring.

[davidjames, 2010/11/10 18:30:10]

Done.

+  public_overlay = '%s/src/third_party/chromiumos-overlay' % buildroot

[sosa, 2010/11/03 23:49:06]

May want to os.path.exists below.

[davidjames, 2010/11/10 18:30:10]

Done.

+  private_overlay = '%s/src/private-overlays/chromeos-overlay' % buildroot
+  if overlays == 'private':
+    return [private_overlay]
+  elif overlays == 'public':
+    return [public_overlay]
+  elif overlays == 'both':
+    return [public_overlay, private_overlay]
+  else:
+    Die('Incorrect overlay configuration: %s' % overlays)
+
 
 def main():
   # Parse options
@@ -502,8 +506,11 @@ def main():
     parser.print_usage()
     sys.exit(1)
 
+  # Calculate list of overlay directories.
+  overlays = ResolveOverlays(buildconfig['overlays'])
+
   try:
-    _PreFlightRinse(buildroot, buildconfig['board'], tracking_branch)
+    _PreFlightRinse(buildroot, buildconfig['board'], tracking_branch, overlays)
     if options.clobber or not os.path.isdir(buildroot):
       _FullCheckout(buildroot, tracking_branch, url=options.url)
     else:
@@ -519,7 +526,7 @@ def main():
 
     if buildconfig['uprev']:
       _UprevPackages(buildroot, tracking_branch, revisionfile,
-                     board=buildconfig['board'])
+                     buildconfig['board'], overlays)
 
     _EnableLocalAccount(buildroot)
     _Build(buildroot)
@@ -545,7 +552,7 @@ def main():
           # Master bot needs to check if the other slaves completed.
           if cbuildbot_comm.HaveSlavesCompleted(config):
             _UprevPush(buildroot, tracking_branch, buildconfig['board'],
-                       buildconfig['overlays'])
+                       overlays)
           else:
             Die('CBUILDBOT - One of the slaves has failed!!!')