Add more error checking to preflight queue.

bin/cbuildbot.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.
 
 """CBuildbot is wrapper around the build process used by the pre-flight queue"""
 
 import errno
 import heapq
@@ skipping 163 matching lines @@
     repo_name = revision_tuple[0].replace('.git', '')
     # Might not have entry if no matching ebuild.
     if repo_dictionary.has_key(repo_name):
       # May be many corresponding packages to a given git repo e.g. kernel).
       for package in repo_dictionary[repo_name]:
         revisions[package] = revision_tuple[1]
 
   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()
+    assert ':' not in package, 'Invalid package name: %s' % package

[scottz-goog, 2010/11/10 23:48:14]

Why is this being added? Have their been cases where this has caused a problem?

[davidjames, 2010/11/11 02:23:24]

Nope. Dianders just suggested I add the check.

+    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)
 
 
 def _GetVMConstants(buildroot):
   """Returns minimum (vdisk_size, statefulfs_size) recommended for VM's."""
   cwd = os.path.join(buildroot, 'src', 'scripts', 'lib')
   source_cmd = 'source %s/cros_vm_constants.sh' % cwd
   vdisk_size = RunCommand([
       '/bin/bash', '-c', '%s && echo $MIN_VDISK_SIZE_FULL' % source_cmd],
        redirect_stdout=True)
   statefulfs_size = RunCommand([
       '/bin/bash', '-c', '%s && echo $MIN_STATEFUL_FS_SIZE_FULL' % source_cmd],
        redirect_stdout=True)
   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)
 
 
 def _CleanUpMountPoints(buildroot):
   """Cleans up any stale mount points from previous runs."""
   mount_output = RunCommand(['mount'], redirect_stdout=True)
   mount_pts_in_buildroot = RunCommand(['grep', buildroot], input=mount_output,
                                       redirect_stdout=True, error_ok=True)
 
   for mount_pt_str in mount_pts_in_buildroot.splitlines():
     mount_pt = mount_pt_str.rpartition(' type ')[0].partition(' on ')[2]
     RunCommand(['sudo', 'umount', '-l', mount_pt], error_ok=True)
 
 
 def _WipeOldOutput(buildroot):
   """Wipes out build output directories."""
   RunCommand(['rm', '-rf', 'src/build/images'], cwd=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)
 
 
 def _FullCheckout(buildroot, tracking_branch, rw_checkout=True,
                   retries=_DEFAULT_RETRIES,
                   url='http://git.chromium.org/git/manifest'):
   """Performs a full checkout and clobbers any previous checkouts."""
   RunCommand(['sudo', 'rm', '-rf', buildroot])
   MakeDir(buildroot, parents=True)
@@ skipping 67 matching lines @@
     shutil.rmtree(results_dir_in_chroot)
 
   cwd = os.path.join(buildroot, 'src', 'scripts')
   RunCommand(['bin/cros_run_vm_test',
               '--no_graphics',
               '--test_case=suite_Smoke',
               '--results_dir_root=%s' % 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
   function will uprev all packages.
 
   Keyword arguments:
   revisionfile -- string specifying a file that contains a list of revisions to
       uprev.
   """
   # Purposefully set to None as it means Force Build was pressed.
   revisions = 'None'
   if (revisionfile):
     try:
       rev_file = open(revisionfile)
       revisions = rev_file.read()
       rev_file.close()
     except Exception, e:
       Warning('Error reading %s, revving all' % revisionfile)
       revisions = 'None'
 
   revisions = revisions.strip()
 
   # TODO(sosa): Un-comment once we close individual trees.
   # revisions == "None" indicates a Force Build.
   #if revisions != 'None':
   #  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)
 
 
 def _ArchiveTestResults(buildroot, board, archive_dir, test_results_dir):
   """Archives the test results into the www dir for later use.
 
   Takes the results from the test_results_dir and dumps them into the archive
   dir specified.  This also archives the last qemu image.
@@ skipping 51 matching lines @@
 
   buildconfig = config[config_name]
 
   for key in default.iterkeys():
     if not buildconfig.has_key(key):
       buildconfig[key] = default[key]
 
   return buildconfig
 
 
+def ResolveOverlays(overlays):
+  """Return the list of overlays to use for a given buildbot.
+

[scottz-goog, 2010/11/10 23:48:14]

I know this file doesn't have proper docstrings anywhere but lets start please.

Arguments:
    overlays: .....

Returns:
     a list of overlays to use for a given buildbot

[davidjames, 2010/11/11 02:23:24]

Done.

+  overlays: A string describing which overlays you want.
+              'private': Just the private overlay.
+              'public': Just the public overlay.
+              'both': Both the public and private overlays.
+"""

[scottz-goog, 2010/11/10 23:48:14]

Align indent.

[davidjames, 2010/11/11 02:23:24]

Done.

+  public_overlay = '%s/src/third_party/chromiumos-overlay' % buildroot
+  private_overlay = '%s/src/private-overlays/chromeos-overlay' % buildroot
+  if overlays == 'private':
+    dirs = [private_overlay]
+  elif overlays == 'public':
+    dirs = [public_overlay]
+  elif overlays == 'both':
+    dirs = [public_overlay, private_overlay]
+  else:
+    Die('Incorrect overlay configuration: %s' % overlays)
+  for dir in dirs:
+    assert ':' not in dir, 'Overlay must not contain colons: %s' % dir

[scottz-goog, 2010/11/10 23:48:14]

Why again are we doing this asssertion? Can you please give me a little
explanation of this addition?

[davidjames, 2010/11/11 02:23:24]

It's very unlikely that it'll contain colons. Just in case.

+    if not os.path.exists(dir):
+      Die('Missing overlay: %s' % dir)
+  return dirs
+
+
 def main():
   # Parse options
   usage = "usage: %prog [options] cbuildbot_config"
   parser = optparse.OptionParser(usage=usage)
   parser.add_option('-r', '--buildroot',
                     help='root directory where build occurs', default=".")
   parser.add_option('-n', '--buildnumber',
                     help='build number', type='int', default=0)
   parser.add_option('-f', '--revisionfile',
                     help='file where new revisions are stored')
@@ skipping 15 matching lines @@
   revisionfile = options.revisionfile
   tracking_branch = options.tracking_branch
 
   if len(args) >= 1:
     buildconfig = _GetConfig(args[-1])
   else:
     Warning('Missing configuration description')
     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:
       _IncrementalCheckout(buildroot)
 
     chroot_path = os.path.join(buildroot, 'chroot')
     if not os.path.isdir(chroot_path):
       _MakeChroot(buildroot)
 
     boardpath = os.path.join(chroot_path, 'build', buildconfig['board'])
     if not os.path.isdir(boardpath):
       _SetupBoard(buildroot, board=buildconfig['board'])
 
     if buildconfig['uprev']:
       _UprevPackages(buildroot, tracking_branch, revisionfile,
-                     board=buildconfig['board'])
+                     buildconfig['board'], overlays)
 
     _EnableLocalAccount(buildroot)
     _Build(buildroot)
     if buildconfig['unittests']:
       _RunUnitTests(buildroot)
 
     _BuildImage(buildroot)
 
     if buildconfig['smoke_bvt']:
       _BuildVMImageForTesting(buildroot)
       test_results_dir = '/tmp/run_remote_tests.%s' % options.buildnumber
       try:
         _RunSmokeSuite(buildroot, test_results_dir)
       finally:
         _ArchiveTestResults(buildroot, buildconfig['board'],
                             archive_dir=options.buildnumber,
                             test_results_dir=test_results_dir)
 
     if buildconfig['uprev']:
       # Don't push changes for developers.
       if not options.debug:
         if buildconfig['master']:
           # 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!!!')
 
         else:
           # Publish my status to the master if its expecting it.
           if buildconfig['important']:
             cbuildbot_comm.PublishStatus(cbuildbot_comm.STATUS_BUILD_COMPLETE)
 
   except:
     # Send failure to master bot.
     if not buildconfig['master'] and buildconfig['important']:
       cbuildbot_comm.PublishStatus(cbuildbot_comm.STATUS_BUILD_FAILED)
 
     raise
 
 
 if __name__ == '__main__':
     main()