Update cbuildbot.py to upload prebuilts from preflight buildbot.

chromite/lib/binpkg.py

+# 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.
+#
+# Adapted from portage/getbinpkg.py -- Portage binary-package helper functions
+# Copyright 2003-2004 Gentoo Foundation
+# Distributed under the terms of the GNU General Public License v2
+
+import operator
+import os
+import time
+import urllib2
+
+class PackageIndex(object):

[dianders, 2010/11/23 21:47:39]

Docstring?

...would be nice to include a spec for what a package file should look like.

[davidjames, 2010/11/23 22:53:14]

Thanks for all the feedback! Will address your feedback when I resubmit this
part of the CL.

[davidjames, 2010/11/29 21:18:55]

Done. (See http://codereview.chromium.org/5344002/ )

+
+  def __init__(self):
+    """Constructor."""

[dianders, 2010/11/23 21:47:39]

All of these members are public?  Can you document what they are supposed to
contain?

[davidjames, 2010/11/29 21:18:55]

Done.

+    self.header = {}
+    self.packages = []
+    self.modified = False
+
+  def _ReadPkgIndex(self, pkgfile):
+    """Read entry from packages file.

[dianders, 2010/11/23 21:47:39]

Read entries (plural).

[davidjames, 2010/11/29 21:18:55]

Done.

+

[dianders, 2010/11/23 21:47:39]

Docstring: 

This will read entries that look like:

key: value

...until a blank line is encountered.  

Any lines without a colon in them are ignored.

[davidjames, 2010/11/29 21:18:55]

Done.

+    Args:
+      pkgfile: A python file object.

[dianders, 2010/11/23 21:47:39]

Returns: ?

[davidjames, 2010/11/29 21:18:55]

Done.

+    """
+    d = {}
+    for line in pkgfile:
+      line = line.rstrip('\n')
+      if not line:
+        break
+      line = line.split(': ', 1)
+      if len(line) == 2:
+        k, v = line
+        d[k] = v
+    return d
+
+  def _WritePkgIndex(self, pkgfile, items):
+    """Write entry to packages file.
+

[dianders, 2010/11/23 21:47:39]

This will terminate the list of items with a blank line.

[davidjames, 2010/11/29 21:18:55]

Done.

+    Args:
+      pkgfile: A python file object.
+      items: A list of tuples containing the entries to write.

[dianders, 2010/11/23 21:47:39]

...containing (key, value) pairs to write.

[davidjames, 2010/11/29 21:18:55]

Done.

+    """
+    for k, v in items:

[dianders, 2010/11/23 21:47:39]

assert ':' not in k

[davidjames, 2010/11/29 21:18:55]

Skipped this one.

+      pkgfile.write('%s: %s\n' % (k, v))
+    pkgfile.write('\n')
+
+  def Read(self, pkgfile):
+    """Read entire packages file.
+

[dianders, 2010/11/23 21:47:39]

This is a shortcut for calling ReadHeader() and ReadBody().

...should be noted, since ReadHeader() and ReadBody() are public.  ...or should
ReadHeader() and ReadBody() not be public?

[davidjames, 2010/11/29 21:18:55]

Made those two functions private.

+    Args:
+      pkgfile: A python file object.
+    """
+    self.ReadHeader(pkgfile)
+    self.ReadBody(pkgfile)
+
+  def ReadHeader(self, pkgfile):
+    """Read header of packages file.
+
+    Args:
+      pkgfile: A python file object.
+    """
+    self.header.update(self._ReadPkgIndex(pkgfile))
+
+  def ReadBody(self, pkgfile):
+    """Read body of packages file.
+
+    Args:
+      pkgfile: A python file object.

[dianders, 2010/11/23 21:47:39]

The header and the blank line following the header should have already been read
from the file (using ReadHeader()).

+    """

[dianders, 2010/11/23 21:47:39]

Comment: Read all the sections in the body by looping until we get to the end of
the file, or get two blank lines in a row.

+    while True:
+      d = self._ReadPkgIndex(pkgfile)
+      if not d:
+        break

[dianders, 2010/11/23 21:47:39]

In order to be a valid body section, there must be a CPV key in the section.

+      if d.get('CPV'):

[dianders, 2010/11/23 21:47:39]

if 'CPV' in d:

+        self.packages.append(d)
+
+  def Write(self, pkgfile):
+    """Write a packages file to disk.
+

[dianders, 2010/11/23 21:47:39]

Document: This has a side effect of updating the TIMESTAMP and PACKAGES header
entries if the modified bit is set.

Why use PACKAGES and not NUM_PACKAGES?  

Document: does not write out header entries with no value.

[davidjames, 2010/11/23 22:53:14]

That'd be a better name, but Portage calls the variable "PACKAGES" so we need to
use the same name for compatibility (we create the file -- Portage reads it and
expects it to be called "PACKAGES")

+    Args:
+      pkgfile: A python file object.
+    """
+    if self.modified:
+      self.header['TIMESTAMP'] = str(long(time.time()))
+      self.header['PACKAGES'] = str(len(self.packages))

[dianders, 2010/11/23 21:47:39]

Should we be clearing .modified?

+    keys = list(self.header)
+    keys.sort()
+    self._WritePkgIndex(pkgfile, [(k, self.header[k]) \
+      for k in keys if self.header[k]])

[dianders, 2010/11/23 21:47:39]

AKA:

self._WritePkgIndex(pkgfile, sorted((k, v) for (k, v) in self.header.iteritems()
if v))

+    for metadata in sorted(self.packages, key=operator.itemgetter('CPV')):

[dianders, 2010/11/23 21:47:39]

Comment: Will write each item in self.packages (is this one package?) to the
file.  Order by the CPV (what is the CPV?).

+      cpv = metadata['CPV']
+      keys = list(metadata)
+      keys.sort()
+      self._WritePkgIndex(pkgfile,
+        [(k, metadata[k]) for k in keys if metadata[k]])
+
+
+def GrabRemotePackageIndex(binhost_url):

[dianders, 2010/11/23 21:47:39]

Technically, this could be a class method on PackageIndex (works sorta like an
alternate constructor).  That might be overkill, though, so feel free to ignore.

+  """Grab the latest binary package database from the specified URL.
+
+  Args:
+    binhost_url: Base URL of remote packages (PORTAGE_BINHOST).
+
+  Returns:
+    A PackageIndex object

[dianders, 2010/11/23 21:47:39]

COmment: Might return None in certain error conditions.

+  """
+
+  def _RetryUrlOpen(url, tries=3):

[dianders, 2010/11/23 21:47:39]

Why does this need to be an encapsulated function?  Move to top level.

+    """Open the specified url, retrying if we run into temporary errors.
+
+    We retry for both network errors and 5xx Server Errors. We do not retry
+    for HTTP errors with a non-5xx code.

[dianders, 2010/11/23 21:47:39]

Does this timeout, or can it ever get stuck forever in bad network conditions?

[dianders, 2010/11/23 21:47:39]

Comment: We will sleep for 10 seconds before retrying.

+
+    Args:
+      url: The specified url.
+      tries: The number of times to try.
+
+    Returns:
+      The result of urllib2.urlopen(url).

[dianders, 2010/11/23 21:47:39]

This is a file-like object.

+    """
+    for i in range(tries):
+      try:
+        return urllib2.urlopen(url)
+      except urllib2.HTTPError as e:
+        if i + 1 >= tries or e.code < 500:
+          raise
+        else:
+          print 'Cannot GET %s: %s' % (url, str(e))
+      except urllib2.URLError as e:
+        if i + 1 >= tries:
+          raise
+        else:
+          print 'Cannot GET %s: %s' % (url, str(e))
+      print "Sleeping for 10 seconds before retrying..."
+      time.sleep(10)
+
+  url = os.path.join(binhost_url, 'Packages')

[dianders, 2010/11/23 21:47:39]

No, not os.path.join.  This will fail on windows or anything with a path
separator other than '/'.  

Try urlparse.urljoin().

+  try:
+    f = _RetryUrlOpen(url)
+  except urllib2.HTTPError as e:
+    if e.code == 404:
+      return None
+    raise
+
+  pkgindex = PackageIndex()
+  pkgindex.Read(f)
+  f.close()
+  return pkgindex
+
+