Add ability to rev and build a chrome from cbuildbot

bin/cros_mark_chrome_as_stable.py

Index: bin/cros_mark_chrome_as_stable.py
diff --git a/bin/cros_mark_chrome_as_stable.py b/bin/cros_mark_chrome_as_stable.py
index 3f8802ad3a3b84b47d0a53ab04088bfc1d36f6a4..bfed1e690a323be688fc4c8d3fb27f370150ecad 100755
--- a/bin/cros_mark_chrome_as_stable.py
+++ b/bin/cros_mark_chrome_as_stable.py
@@ -1,4 +1,4 @@
-#!/usr/bin/python2.4
+#!/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
@@ -32,9 +32,6 @@ _NON_STICKY_REGEX = '%s[(_rc.*)|(_alpha.*)]+' % _CHROME_VERSION_REGEX
 _CHROME_OVERLAY_DIR = ('%(srcroot)s/third_party/chromiumos-overlay'
                        '/chromeos-base/chromeos-chrome')
 
-# Different than cros_mark so devs don't have local collisions.
-_STABLE_BRANCH_NAME = 'chrome_stabilizing_branch'
-
 _GIT_COMMIT_MESSAGE = ('Marking %(chrome_rev)s for chrome ebuild with version '
                        '%(chrome_version)s as stable.')
 
@@ -235,6 +232,8 @@ def MarkChromeEBuildAsStable(stable_candidate, unstable_ebuild, chrome_rev,
     chrome_version:  The \d.\d.\d.\d version of Chrome.
     commit:  Used with TIP_OF_TRUNK.  The svn revision of chrome.
     overlay_dir:  Path to the chromeos-chrome package dir.
+  Returns:
+    Full portage version atom (including rc's, etc) that was revved.
   """
   base_path = os.path.join(overlay_dir, 'chromeos-chrome-%s' % chrome_version)
   # Case where we have the last stable candidate with same version just rev.
@@ -260,38 +259,26 @@ def MarkChromeEBuildAsStable(stable_candidate, unstable_ebuild, chrome_rev,
       _GIT_COMMIT_MESSAGE % {'chrome_rev': chrome_rev,
                              'chrome_version': chrome_version})
 
+  new_ebuild = ChromeEBuild(new_ebuild_path)
+  return '%s-%s' % (new_ebuild.package, new_ebuild.version)
+
 
 def main(argv):
-  usage = '%s OPTIONS commit|clean|push'
+  usage = '%s OPTIONS [%s]' % ((__file__), '|'.join(CHROME_REV))

[scottz, 2010/11/23 02:44:40]

__file__ withut the parens? 

[sosa, 2010/11/23 21:09:19]

Done.

   parser = optparse.OptionParser(usage)
-  parser.add_option('-c', '--chrome_rev', default=None,
-                    help='One of %s' % CHROME_REV)
-  parser.add_option('-s', '--srcroot', default='.',
+  parser.add_option('-s', '--srcroot', default=('%s/trunk/src' % 

[scottz, 2010/11/23 02:44:40]

os.path.join(os.environ['HOME'], ....)

[sosa, 2010/11/23 21:09:19]

Done.

+                                                os.environ['HOME']),
                     help='Path to the src directory')
   parser.add_option('-t', '--tracking_branch', default='cros/master',
                     help='Branch we are tracking changes against')
   (options, argv) = parser.parse_args(argv)

[scottz, 2010/11/23 02:44:40]

I know this isn't your code but this is bad. We are taking sys.argv as passed in
below and then passing it to parser which isn't necessary and then redefining
argv to be something different.

Can we just change this to parser.parse_args()? Rename argv to args in the tuple
and not pass sys.argv in to main at all? 

I got to this from line 281 so it sort of relates to your change :) 

[sosa, 2010/11/23 21:09:19]

Done.

 
-  if len(argv) != 2 or argv[1] not in (
-      cros_mark_as_stable.COMMAND_DICTIONARY.keys()):
-    parser.error('Arguments are invalid, see usage.')
+  if len(argv) != 2 or argv[1] not in CHROME_REV:
+    parser.error('Commit requires arg set to one of %s.' % CHROME_REV)
 
-  command = argv[1]
   overlay_dir = os.path.abspath(_CHROME_OVERLAY_DIR %
                                 {'srcroot': options.srcroot})
-
-  os.chdir(overlay_dir)
-  if command == 'clean':
-    cros_mark_as_stable.Clean(options.tracking_branch)
-    return
-  elif command == 'push':
-    cros_mark_as_stable.PushChange(_STABLE_BRANCH_NAME, options.tracking_branch)
-    return
-
-  if not options.chrome_rev or options.chrome_rev not in CHROME_REV:
-    parser.error('Commit requires type set to one of %s.' % CHROME_REV)
-
-  chrome_rev = options.chrome_rev
+  chrome_rev = argv[1]
   version_to_uprev = None
   commit_to_use = None
 
@@ -315,12 +302,16 @@ def main(argv):
     Info('Found nothing to do for chrome_rev %s with version %s.' % (
         chrome_rev, version_to_uprev))
   else:
+    os.chdir(overlay_dir)
     work_branch = cros_mark_as_stable.GitBranch(
-        _STABLE_BRANCH_NAME, options.tracking_branch)
+        cros_mark_as_stable.STABLE_BRANCH_NAME, options.tracking_branch)
     work_branch.CreateBranch()
     try:
-      MarkChromeEBuildAsStable(stable_candidate, unstable_ebuild, chrome_rev,
-                               version_to_uprev, commit_to_use, overlay_dir)
+      portage_version_atom = MarkChromeEBuildAsStable(
+          stable_candidate, unstable_ebuild, chrome_rev, version_to_uprev,
+          commit_to_use, overlay_dir)
+      # Explicit print to communicate to caller.

[scottz, 2010/11/23 02:44:40]

perhaps I do not understand all that is being done here but are we explicitly
printing this so the caller can scrape this? If so maybe adding a bit more about
this or to the file in general about how if prints are added it could break
things?  We may want to print something like PORTAGE_VERSION_ATOM=XXXXX 

then whoever is scraping it can key in on that but i need more information about
what is going on here.

[sosa, 2010/11/23 21:09:19]

Good idea.  Even though cbuildbot is the only expected caller it does make sense
to make it as general as possible.

Sorry this was committed in a previous commit and is new.  I broke this into 4
commits to make it easier to review.


On 2010/11/23 02:44:40, scottz wrote:

+      print portage_version_atom
     except:
       work_branch.Delete()
       raise