Make build-bisect script work on win....

Make build-bisect script work on win.

Unzip using python rather than using the unzip command which might not be
available on windows.

TEST=NONE
BUG=NONE


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=43160

[Kavita Kanetkar, 2010-03-31 00:10:20]

Thanks for quickly implementing this!

http://codereview.chromium.org/1549006/diff/7001/8001
File build/build-bisect.py (right):

http://codereview.chromium.org/1549006/diff/7001/8001#newcode58
build/build-bisect.py:58: def Unzip(filename, dir):
I like informative method names. UnzipFilenameToDir()

http://codereview.chromium.org/1549006/diff/7001/8001#newcode61
build/build-bisect.py:61: # make base
Google style capitalize first letter end with .

http://codereview.chromium.org/1549006/diff/7001/8001#newcode64
build/build-bisect.py:64: os.mkdir(dir)
Things can fail. Can you wrap this in try except and just exit if unzip fails?

http://codereview.chromium.org/1549006/diff/7001/8001#newcode69
build/build-bisect.py:69: mode = info.external_attr >> 16L
Please comment.

http://codereview.chromium.org/1549006/diff/7001/8001#newcode70
build/build-bisect.py:70: if name.endswith('/'):  # dir
On windows will it  end in / ? Can you use file-separator?

http://codereview.chromium.org/1549006/diff/7001/8001#newcode80
build/build-bisect.py:80: os.chmod(name, mode)
declare mode where it's used.

http://codereview.chromium.org/1549006/diff/7001/8001#newcode152
build/build-bisect.py:152: cmd = "%s %s" % (exe, flags)
Usually through out the script, please try to keep string representations
uniform. Either use '' or "". Preferred way is ''.

[Nirnimesh, 2010-03-31 00:22:10]

new diffs up.

Thanks for the quick review.

http://codereview.chromium.org/1549006/diff/7001/8001
File build/build-bisect.py (right):

http://codereview.chromium.org/1549006/diff/7001/8001#newcode58
build/build-bisect.py:58: def Unzip(filename, dir):
On 2010/03/31 00:10:20, Kavita Kanetkar wrote:
> I like informative method names. UnzipFilenameToDir()

Done.

http://codereview.chromium.org/1549006/diff/7001/8001#newcode61
build/build-bisect.py:61: # make base
On 2010/03/31 00:10:20, Kavita Kanetkar wrote:
> Google style capitalize first letter end with .

Done.

http://codereview.chromium.org/1549006/diff/7001/8001#newcode64
build/build-bisect.py:64: os.mkdir(dir)
On 2010/03/31 00:10:20, Kavita Kanetkar wrote:
> Things can fail. Can you wrap this in try except and just exit if unzip fails?

Don't you think it's better to spit out informative stack trace in case of
failure rather than silently passing on? In the current case, if unzip fails
there isn't much sense in carrying on.

http://codereview.chromium.org/1549006/diff/7001/8001#newcode69
build/build-bisect.py:69: mode = info.external_attr >> 16L
On 2010/03/31 00:10:20, Kavita Kanetkar wrote:
> Please comment.

Done.

http://codereview.chromium.org/1549006/diff/7001/8001#newcode70
build/build-bisect.py:70: if name.endswith('/'):  # dir
On 2010/03/31 00:10:20, Kavita Kanetkar wrote:
> On windows will it  end in / ? Can you use file-separator?

This is info from the zip. It'll be '/' on all platforms.

http://codereview.chromium.org/1549006/diff/7001/8001#newcode80
build/build-bisect.py:80: os.chmod(name, mode)
On 2010/03/31 00:10:20, Kavita Kanetkar wrote:
> declare mode where it's used.

Done.

[Kavita Kanetkar, 2010-03-31 00:47:14]

LGTM

http://codereview.chromium.org/1549006/diff/7001/8001
File build/build-bisect.py (right):

http://codereview.chromium.org/1549006/diff/7001/8001#newcode64
build/build-bisect.py:64: os.mkdir(dir)
I'd like it to exit. Stack trace is fine too, so long as it doesn't continue.
You could also print the exception in except (So that it's not supressed)

On 2010/03/31 00:22:10, Nirnimesh wrote:
> On 2010/03/31 00:10:20, Kavita Kanetkar wrote:
> > Things can fail. Can you wrap this in try except and just exit if unzip
fails?
> 
> Don't you think it's better to spit out informative stack trace in case of
> failure rather than silently passing on? In the current case, if unzip fails
> there isn't much sense in carrying on.

http://codereview.chromium.org/1549006/diff/7001/8001#newcode70
build/build-bisect.py:70: if name.endswith('/'):  # dir
On 2010/03/31 00:22:10, Nirnimesh wrote:
> On 2010/03/31 00:10:20, Kavita Kanetkar wrote:
> > On windows will it  end in / ? Can you use file-separator?
> 
> This is info from the zip. It'll be '/' on all platforms.

Cool, thanks for clarifying.

http://codereview.chromium.org/1549006/diff/7001/8001#newcode152
build/build-bisect.py:152: cmd = "%s %s" % (exe, flags)
On 2010/03/31 00:10:20, Kavita Kanetkar wrote:
> Usually through out the script, please try to keep string representations
> uniform. Either use '' or "". Preferred way is ''.

Maybe put a TODO if you're not going to change it now.

[Nirnimesh, 2010-03-31 01:05:06]

http://codereview.chromium.org/1549006/diff/7001/8001
File build/build-bisect.py (right):

http://codereview.chromium.org/1549006/diff/7001/8001#newcode64
build/build-bisect.py:64: os.mkdir(dir)
On 2010/03/31 00:47:14, Kavita Kanetkar wrote:
> I'd like it to exit. Stack trace is fine too, so long as it doesn't continue.
> You could also print the exception in except (So that it's not supressed)
> 
> On 2010/03/31 00:22:10, Nirnimesh wrote:
> > On 2010/03/31 00:10:20, Kavita Kanetkar wrote:
> > > Things can fail. Can you wrap this in try except and just exit if unzip
> fails?
> > 
> > Don't you think it's better to spit out informative stack trace in case of
> > failure rather than silently passing on? In the current case, if unzip fails
> > there isn't much sense in carrying on.
> 
> 

Done.

http://codereview.chromium.org/1549006/diff/7001/8001#newcode152
build/build-bisect.py:152: cmd = "%s %s" % (exe, flags)
On 2010/03/31 00:47:14, Kavita Kanetkar wrote:
> On 2010/03/31 00:10:20, Kavita Kanetkar wrote:
> > Usually through out the script, please try to keep string representations
> > uniform. Either use '' or "". Preferred way is ''.
> 
> Maybe put a TODO if you're not going to change it now.

Done. Sorry, I missed this in the last pass.