Update prebuilt.py to support uploading a board tarball to Google Storage.

Update prebuilt.py to support uploading a board tarball to Google Storage.

Change-Id: Ic81562ce7b6370924e0aa1bf49ecf552720b3c75

R=scottz@chromium.org
BUG=chromium-os:8777
TEST=Upload prebuilts with and without board tarball. Ran unit tests.

Committed: http://chrome-svn/viewvc/chromeos?view=rev&revision=8fa34ea

[scottz, 2011-04-15 17:40:46]

http://codereview.chromium.org/6851021/diff/2001/buildbot/prebuilt.py
File buildbot/prebuilt.py (right):

http://codereview.chromium.org/6851021/diff/2001/buildbot/prebuilt.py#newcode520
buildbot/prebuilt.py:520: 
Args:

http://codereview.chromium.org/6851021/diff/2001/buildbot/prebuilt.py#newcode527
buildbot/prebuilt.py:527: tarfile = '/tmp/%s.tbz2' % boardname
You should use tempfile here.

I prefer not to assume we can dump gigs of files in /tmp. I would much rather
you create a temporary directory in the dir where prebuilt is executed and wipe
it if anything bad happens.

http://codereview.chromium.org/6851021/diff/2001/buildbot/prebuilt.py#newcode529
buildbot/prebuilt.py:529: for path in ('usr/lib/debug', 'usr/local/autotest',
'packages', 'tmp'):
Assigning these to a variable excluded_paths would make it a lot more clear what
you are doing here at the cost of a var added.

http://codereview.chromium.org/6851021/diff/2001/buildbot/prebuilt.py#newcode532
buildbot/prebuilt.py:532: cros_build_lib.RunCommand(cmd, cwd=cwd)
this should be in your try, if tar fails we are left with a tarball in /tmp,
this could fail easily if /tmp has no more space.

http://codereview.chromium.org/6851021/diff/2001/buildbot/prebuilt.py#newcode601
buildbot/prebuilt.py:601: if upload_board_tarball:
Please add a comment about you uploading board tarballs in the background

http://codereview.chromium.org/6851021/diff/2001/buildbot/prebuilt.py#newcode602
buildbot/prebuilt.py:602: p =
multiprocessing.Process(target=self._UploadBoardTarball,
Please consider using something other than p. tar_process?

[davidjames, 2011-04-15 19:18:08]

Feedback addressed. PTAL.

http://codereview.chromium.org/6851021/diff/2001/buildbot/prebuilt.py
File buildbot/prebuilt.py (right):

http://codereview.chromium.org/6851021/diff/2001/buildbot/prebuilt.py#newcode520
buildbot/prebuilt.py:520: 
On 2011/04/15 17:40:46, scottz wrote:
> Args:

Done.

http://codereview.chromium.org/6851021/diff/2001/buildbot/prebuilt.py#newcode527
buildbot/prebuilt.py:527: tarfile = '/tmp/%s.tbz2' % boardname
On 2011/04/15 17:40:46, scottz wrote:
> You should use tempfile here.
> 
> I prefer not to assume we can dump gigs of files in /tmp. I would much rather
> you create a temporary directory in the dir where prebuilt is executed and
wipe
> it if anything bad happens. 

Done.

Now that we switched to tempfile, it will honor the TMPDIR / TEMP / TMP
environment variables and so if the buildbot sets those variables, Python will
use one of those directories.

http://codereview.chromium.org/6851021/diff/2001/buildbot/prebuilt.py#newcode529
buildbot/prebuilt.py:529: for path in ('usr/lib/debug', 'usr/local/autotest',
'packages', 'tmp'):
On 2011/04/15 17:40:46, scottz wrote:
> Assigning these to a variable excluded_paths would make it a lot more clear
what
> you are doing here at the cost of a var added. 

Done.

http://codereview.chromium.org/6851021/diff/2001/buildbot/prebuilt.py#newcode532
buildbot/prebuilt.py:532: cros_build_lib.RunCommand(cmd, cwd=cwd)
On 2011/04/15 17:40:46, scottz wrote:
> this should be in your try, if tar fails we are left with a tarball in /tmp,
> this could fail easily if /tmp has no more space. 

Done.

http://codereview.chromium.org/6851021/diff/2001/buildbot/prebuilt.py#newcode601
buildbot/prebuilt.py:601: if upload_board_tarball:
On 2011/04/15 17:40:46, scottz wrote:
> Please add a comment about you uploading board tarballs in the background 

Done.

http://codereview.chromium.org/6851021/diff/2001/buildbot/prebuilt.py#newcode602
buildbot/prebuilt.py:602: p =
multiprocessing.Process(target=self._UploadBoardTarball,
On 2011/04/15 17:40:46, scottz wrote:
> Please consider using something other than p. tar_process?

Done.

[scottz, 2011-04-15 19:55:49]

LGTM