From 02b3e4f3d52cd8182219c07df56d4030d41d3958 Mon Sep 17 00:00:00 2001 From: David James Date: Fri, 12 Nov 2010 14:47:57 -0800 Subject: [PATCH] Revert "Add more error checking to preflight queue." This reverts commit 795bd30f069bc64da6f68d8c3e23ed15fa62e072. BUG= TEST= Review URL: http://codereview.chromium.org/4904003 Change-Id: I9148026ecd0df75c253cfcf0c67af3d26771dd21 --- bin/cbuildbot.py | 93 ++++++++++++++------------------------- bin/cbuildbot_config.py | 39 ++++++++++++---- bin/cbuildbot_unittest.py | 17 ++----- cros_mark_as_stable.py | 31 ++++--------- 4 files changed, 76 insertions(+), 104 deletions(-) diff --git a/bin/cbuildbot.py b/bin/cbuildbot.py index 58d5c3e2c3..372ddcaf5f 100755 --- a/bin/cbuildbot.py +++ b/bin/cbuildbot.py @@ -18,8 +18,7 @@ import cbuildbot_comm from cbuildbot_config import config sys.path.append(os.path.join(os.path.dirname(__file__), '../lib')) -from cros_build_lib import (Die, Info, ReinterpretPathForChroot, RunCommand, - Warning) +from cros_build_lib import Die, Info, RunCommand, Warning _DEFAULT_RETRIES = 3 ARCHIVE_BASE = '/var/www/archive' @@ -182,38 +181,37 @@ def _ParseRevisionString(revision_string, repo_dictionary): return revisions.items() -def _UprevFromRevisionList(buildroot, tracking_branch, revision_list, board, - overlays): +def _UprevFromRevisionList(buildroot, tracking_branch, revision_list, board): """Uprevs based on revision list.""" if not revision_list: Info('No packages found to uprev') return - packages = [] + package_str = '' for package, revision in revision_list: - assert ':' not in package, 'Invalid package name: %s' % package - packages.append(package) + package_str += package + ' ' - chroot_overlays = [ReinterpretPathForChroot(path) for path in overlays] + package_str = package_str.strip() 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, - '--overlays=%s' % ':'.join(chroot_overlays), - '--packages=%s' % ':'.join(packages), + '--tracking_branch="%s"' % tracking_branch, + '--packages="%s"' % package_str, 'commit'], - cwd=cwd, enter_chroot=True) + cwd=cwd, enter_chroot=True) -def _UprevAllPackages(buildroot, tracking_branch, board, overlays): +def _UprevAllPackages(buildroot, tracking_branch, board): """Uprevs all packages that have been updated since last uprev.""" cwd = os.path.join(buildroot, 'src', 'scripts') - chroot_overlays = [ReinterpretPathForChroot(path) for path in overlays] + # TODO(davidjames): --foo="bar baz" only works here because we're using + # enter_chroot. RunCommand(['./cros_mark_as_stable', '--all', '--board=%s' % board, - '--overlays=%s' % ':'.join(chroot_overlays), - '--tracking_branch=%s' % tracking_branch, 'commit'], + '--tracking_branch="%s"' % tracking_branch, 'commit'], cwd=cwd, enter_chroot=True) @@ -230,13 +228,12 @@ def _GetVMConstants(buildroot): return (vdisk_size.strip(), statefulfs_size.strip()) -def _GitCleanup(buildroot, board, tracking_branch, overlays): +def _GitCleanup(buildroot, board, tracking_branch): """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) @@ -260,9 +257,9 @@ def _WipeOldOutput(buildroot): # =========================== Main Commands =================================== -def _PreFlightRinse(buildroot, board, tracking_branch, overlays): +def _PreFlightRinse(buildroot, board, tracking_branch): """Cleans up any leftover state from previous runs.""" - _GitCleanup(buildroot, board, tracking_branch, overlays) + _GitCleanup(buildroot, board, tracking_branch) _CleanUpMountPoints(buildroot) RunCommand(['sudo', 'killall', 'kvm'], error_ok=True) @@ -350,7 +347,7 @@ def _RunSmokeSuite(buildroot, results_dir): ], cwd=cwd, error_ok=False) -def _UprevPackages(buildroot, tracking_branch, revisionfile, board, overlays): +def _UprevPackages(buildroot, tracking_branch, revisionfile, board): """Uprevs a package based on given revisionfile. If revisionfile is set to None or does not resolve to an actual file, this @@ -379,19 +376,26 @@ def _UprevPackages(buildroot, tracking_branch, revisionfile, board, overlays): # print >> sys.stderr, 'CBUILDBOT Revision list found %s' % revisions # revision_list = _ParseRevisionString(revisions, # _CreateRepoDictionary(buildroot, board)) - # _UprevFromRevisionList(buildroot, tracking_branch, revision_list, board, - # overlays) + # _UprevFromRevisionList(buildroot, tracking_branch, revision_list, board) #else: Info('CBUILDBOT Revving all') - _UprevAllPackages(buildroot, tracking_branch, board, overlays) + _UprevAllPackages(buildroot, tracking_branch, board) 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) @@ -463,34 +467,6 @@ def _GetConfig(config_name): return buildconfig -def _ResolveOverlays(buildroot, overlays): - """Return the list of overlays to use for a given buildbot. - - Args: - buildroot: The root directory where the build occurs. Must be an absolute - path. - 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. - """ - public_overlay = '%s/src/third_party/chromiumos-overlay' % buildroot - private_overlay = '%s/src/private-overlays/chromeos-overlay' % buildroot - if overlays == 'private': - paths = [private_overlay] - elif overlays == 'public': - paths = [public_overlay] - elif overlays == 'both': - paths = [public_overlay, private_overlay] - else: - Die('Incorrect overlay configuration: %s' % overlays) - for path in paths: - assert ':' not in path, 'Overlay must not contain colons: %s' % path - if not os.path.isdir(path): - Die('Missing overlay: %s' % path) - return paths - - def main(): # Parse options usage = "usage: %prog [options] cbuildbot_config" @@ -515,7 +491,7 @@ def main(): (options, args) = parser.parse_args() - buildroot = os.path.abspath(options.buildroot) + buildroot = options.buildroot revisionfile = options.revisionfile tracking_branch = options.tracking_branch @@ -526,11 +502,8 @@ def main(): parser.print_usage() sys.exit(1) - # Calculate list of overlay directories. - overlays = _ResolveOverlays(buildroot, buildconfig['overlays']) - try: - _PreFlightRinse(buildroot, buildconfig['board'], tracking_branch, overlays) + _PreFlightRinse(buildroot, buildconfig['board'], tracking_branch) if options.clobber or not os.path.isdir(buildroot): _FullCheckout(buildroot, tracking_branch, url=options.url) else: @@ -546,7 +519,7 @@ def main(): if buildconfig['uprev']: _UprevPackages(buildroot, tracking_branch, revisionfile, - buildconfig['board'], overlays) + board=buildconfig['board']) _EnableLocalAccount(buildroot) _Build(buildroot) @@ -572,7 +545,7 @@ def main(): # Master bot needs to check if the other slaves completed. if cbuildbot_comm.HaveSlavesCompleted(config): _UprevPush(buildroot, tracking_branch, buildconfig['board'], - overlays) + buildconfig['overlays']) else: Die('CBUILDBOT - One of the slaves has failed!!!') diff --git a/bin/cbuildbot_config.py b/bin/cbuildbot_config.py index 718cd36a70..831ef06bd1 100644 --- a/bin/cbuildbot_config.py +++ b/bin/cbuildbot_config.py @@ -19,9 +19,9 @@ hostname -- Needed for 'important' slaves. The hostname of the bot. Should match hostname in slaves.cfg in buildbot checkout. unittests -- Runs unittests for packages. smoke_bvt -- Runs the test smoke suite in a qemu-based VM using KVM. -overlays -- Select what overlays to look at. This can be 'public', 'private' - or 'both'. There should only be one master bot pushing changes to - each overlay per branch. +overlays -- If this bot is a master bot, select what overlays to push changes + to. This can be 'public', 'private', or 'both'. There should only + be one bot pushing changes to each overlay. """ @@ -33,7 +33,6 @@ config['default'] = { 'important' : False, 'unittests' : False, 'smoke_bvt' : False, - 'overlays': 'public', } config['x86-generic-pre-flight-queue'] = { 'board' : 'x86-generic', @@ -70,7 +69,6 @@ config['x86_agz_bin'] = { 'important' : False, 'unittests' : True, 'smoke_bvt' : True, - 'overlays': 'private', } config['x86_dogfood_bin'] = { 'board' : 'x86-dogfood', @@ -79,29 +77,52 @@ config['x86_dogfood_bin'] = { 'important' : False, 'unittests' : True, 'smoke_bvt' : True, - 'overlays': 'private', } config['x86_pineview_bin'] = { 'board' : 'x86-pineview', 'uprev' : True, 'master' : False, 'important' : False, + 'hostname' : 'codf200.jail', 'unittests': True, - 'overlays': 'public', } config['arm_tegra2_bin'] = { 'board' : 'tegra2', 'uprev' : True, 'master' : False, 'important' : False, + 'hostname' : 'codg172.jail', + 'unittests' : False, +} +config['arm_voguev210_bin'] = { + 'board' : 'voguev210', + 'uprev' : True, + 'master' : False, + 'important' : False, + 'hostname' : 'codf196.jail', + 'unittests' : False, +} +config['arm_beagleboard_bin'] = { + 'board' : 'beagleboard', + 'master' : False, + 'uprev' : True, + 'important' : False, + 'hostname' : 'codf202.jail', + 'unittests' : False, +} +config['arm_st1q_bin'] = { + 'board' : 'st1q', + 'uprev' : True, + 'master' : False, + 'important' : False, + 'hostname' : 'codg158.jail', 'unittests' : False, - 'overlays': 'public', } config['arm_generic_bin'] = { 'board' : 'arm-generic', 'uprev' : True, 'master' : False, 'important' : False, + 'hostname' : 'codg175.jail', 'unittests' : False, - 'overlays': 'public', } diff --git a/bin/cbuildbot_unittest.py b/bin/cbuildbot_unittest.py index 979cda55d4..bbbc56c6f7 100755 --- a/bin/cbuildbot_unittest.py +++ b/bin/cbuildbot_unittest.py @@ -16,7 +16,6 @@ import unittest # Fixes circular dependency error. import cbuildbot_comm import cbuildbot -from cros_build_lib import ReinterpretPathForChroot class CBuildBotTest(mox.MoxTestBase): @@ -43,10 +42,6 @@ class CBuildBotTest(mox.MoxTestBase): ['dev-util/perf', '12345test'], ['chromos-base/libcros', '12345test'] ] - self._overlays = ['%s/src/third_party/chromiumos-overlay' % self._buildroot] - self._chroot_overlays = [ - ReinterpretPathForChroot(p) for p in self._overlays - ] def testParseRevisionString(self): """Test whether _ParseRevisionString parses string correctly.""" @@ -173,15 +168,13 @@ class CBuildBotTest(mox.MoxTestBase): cbuildbot.RunCommand(['./cros_mark_as_stable', '--all', '--board=%s' % self._test_board, - '--overlays=%s' % ':'.join(self._chroot_overlays), - '--tracking_branch=cros/master', 'commit'], + '--tracking_branch="cros/master"', 'commit'], cwd='%s/src/scripts' % self._buildroot, enter_chroot=True) self.mox.ReplayAll() cbuildbot._UprevPackages(self._buildroot, self.tracking_branch, - self._revision_file, self._test_board, - self._overlays) + self._revision_file, self._test_board) self.mox.VerifyAll() def testUprevAllPackages(self): @@ -196,15 +189,13 @@ class CBuildBotTest(mox.MoxTestBase): cbuildbot.RunCommand(['./cros_mark_as_stable', '--all', '--board=%s' % self._test_board, - '--overlays=%s' % ':'.join(self._chroot_overlays), - '--tracking_branch=cros/master', 'commit'], + '--tracking_branch="cros/master"', 'commit'], cwd='%s/src/scripts' % self._buildroot, enter_chroot=True) self.mox.ReplayAll() cbuildbot._UprevPackages(self._buildroot, self.tracking_branch, - self._revision_file, self._test_board, - self._overlays) + self._revision_file, self._test_board) self.mox.VerifyAll() diff --git a/cros_mark_as_stable.py b/cros_mark_as_stable.py index a6888439ed..56a589c682 100755 --- a/cros_mark_as_stable.py +++ b/cros_mark_as_stable.py @@ -22,10 +22,10 @@ from cros_build_lib import Info, RunCommand, Warning, Die gflags.DEFINE_string('board', '', 'Board for which the package belongs.', short_name='b') gflags.DEFINE_string('overlays', '', - 'Colon-separated list of overlays to modify.', + 'Space separated list of overlays to modify.', short_name='o') gflags.DEFINE_string('packages', '', - 'Colon-separated list of packages to mark as stable.', + 'Space separated list of packages to mark as stable.', short_name='p') gflags.DEFINE_string('push_options', '', 'Options to use with git-cl push using push command.') @@ -245,11 +245,7 @@ def _SimpleRunCommand(command): """Runs a shell command and returns stdout back to caller.""" _Print(' + %s' % command) proc_handle = subprocess.Popen(command, stdout=subprocess.PIPE, shell=True) - stdout = proc_handle.communicate()[0] - retcode = proc_handle.wait() - if retcode != 0: - raise subprocess.CalledProcessError(retcode, command, output=stdout) - return stdout + return proc_handle.communicate()[0] # ======================= End Global Helper Functions ======================== @@ -327,7 +323,7 @@ class _EBuild(object): # Grab and evaluate CROS_WORKON variables from this ebuild. unstable_ebuild = '%s-9999.ebuild' % self.ebuild_path_no_version - cmd = ('export CROS_WORKON_LOCALNAME="%s" CROS_WORKON_PROJECT="%s"; ' + cmd = ('CROS_WORKON_LOCALNAME="%s" CROS_WORKON_PROJECT="%s" ' 'eval $(grep -E "^CROS_WORKON" %s) && ' 'echo $CROS_WORKON_PROJECT ' '$CROS_WORKON_LOCALNAME/$CROS_WORKON_SUBDIR' @@ -345,10 +341,10 @@ class _EBuild(object): # TODO(anush): This hack is only necessary because the kernel ebuild has # 'if' statements, so we can't grab the CROS_WORKON_LOCALNAME properly. # We should clean up the kernel ebuild and remove this hack. - if not os.path.isdir(srcdir) and subdir == 'kernel/': + if not os.path.exists(srcdir) and subdir == 'kernel/': srcdir = os.path.join(srcroot, 'third_party/kernel/files') - if not os.path.isdir(srcdir): + if not os.path.exists(srcdir): Die('Cannot find commit id for %s' % self.ebuild_path) # Verify that we're grabbing the commit id from the right project name. @@ -491,16 +487,11 @@ def main(argv): except gflags.FlagsError, e : _PrintUsageAndDie(str(e)) - package_list = gflags.FLAGS.packages.split(':') + package_list = gflags.FLAGS.packages.split() _CheckSaneArguments(package_list, command) if gflags.FLAGS.overlays: - overlays = {} - for path in gflags.FLAGS.overlays.split(':'): - if not os.path.isdir(path): - Die('Cannot find overlay: %s' % path) - overlays[path] = [] + overlays = dict((path, []) for path in gflags.FLAGS.overlays.split()) else: - Warning('Missing --overlays argument') overlays = { '%s/private-overlays/chromeos-overlay' % gflags.FLAGS.srcroot: [], '%s/third_party/chromiumos-overlay' % gflags.FLAGS.srcroot: [] @@ -510,13 +501,9 @@ def main(argv): _BuildEBuildDictionary(overlays, gflags.FLAGS.all, package_list) for overlay, ebuilds in overlays.items(): - if not os.path.isdir(overlay): + if not os.path.exists(overlay): Warning("Skipping %s" % overlay) continue - - # TODO(davidjames): Currently, all code that interacts with git depends on - # the cwd being set to the overlay directory. We should instead pass in - # this parameter so that we don't need to modify the cwd globally. os.chdir(overlay) if command == 'clean':