Bisect official build with providing bad/good revisions.

Skip bad/good even for oficial builds.
Fixed bad > good check, it worked as string comparison when for any build type it's int or four ints.
No prompt if bad/good option ommited, just fall back to possible min/max. User always can provide them in command line. So options were redundant but increased code complexity.

BUG=none
TEST=none


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

[Vitaly Buka (NO REVIEWS), 2012-07-13 23:31:04]

Please review this one too.

[Nico, 2012-07-20 17:19:45]

From what I could tell, we used to have just the prompts, then mmoss added the
-g / -b flags in http://codereview.chromium.org/201023 . So removing the prompts
should be fine (+rsesek in case he disagrees).

https://chromiumcodereview.appspot.com/10704213/diff/1/tools/bisect-builds.py
File tools/bisect-builds.py (right):

https://chromiumcodereview.appspot.com/10704213/diff/1/tools/bisect-builds.py...
tools/bisect-builds.py:231: final_list.append(str(build_number))
This appends on every revision and then resets to an empty list on the next
loop. Isn't this simpler:

if build_number >= maxrev:
  break
if build_number >= minrev:
  final_list.append(str(build_number))

?

https://chromiumcodereview.appspot.com/10704213/diff/1/tools/bisect-builds.py...
tools/bisect-builds.py:233: break;
no semicolon

https://chromiumcodereview.appspot.com/10704213/diff/1/tools/bisect-builds.py...
tools/bisect-builds.py:575: nh = urllib.urlopen(url)
No need for a nh variable

https://chromiumcodereview.appspot.com/10704213/diff/1/tools/bisect-builds.py...
tools/bisect-builds.py:578: print('Could not determine latest revision. This
could be bad...')
Why catch this? Just let it propagate, no?

…ah, this is just moved up from below. Feel free to ignore this comment and the
one above then.

[Vitaly Buka (NO REVIEWS), 2012-07-20 17:37:01]

https://chromiumcodereview.appspot.com/10704213/diff/1/tools/bisect-builds.py
File tools/bisect-builds.py (right):

https://chromiumcodereview.appspot.com/10704213/diff/1/tools/bisect-builds.py...
tools/bisect-builds.py:231: final_list.append(str(build_number))
It's not what I tried to do.
Example: 
we have revissions [0 1 4 5 7 9]
and asked -g 3 -b 6
I'd like to bisect [1 4 5 7]
You code will bisect [4 5 7]


On 2012/07/20 17:19:45, Nico wrote:
> This appends on every revision and then resets to an empty list on the next
> loop. Isn't this simpler:
> 
> if build_number >= maxrev:
>   break
> if build_number >= minrev:
>   final_list.append(str(build_number))
> 
> ?

https://chromiumcodereview.appspot.com/10704213/diff/1/tools/bisect-builds.py...
tools/bisect-builds.py:233: break;
On 2012/07/20 17:19:45, Nico wrote:
> no semicolon

Done.

https://chromiumcodereview.appspot.com/10704213/diff/1/tools/bisect-builds.py...
tools/bisect-builds.py:575: nh = urllib.urlopen(url)
On 2012/07/20 17:19:45, Nico wrote:
> No need for a nh variable

Done.

https://chromiumcodereview.appspot.com/10704213/diff/1/tools/bisect-builds.py...
tools/bisect-builds.py:578: print('Could not determine latest revision. This
could be bad...')
On 2012/07/20 17:19:45, Nico wrote:
> Why catch this? Just let it propagate, no?
> 
> …ah, this is just moved up from below. Feel free to ignore this comment and
the
> one above then.

Done.

[Nico, 2012-07-20 17:39:11]

https://chromiumcodereview.appspot.com/10704213/diff/1/tools/bisect-builds.py
File tools/bisect-builds.py (right):

https://chromiumcodereview.appspot.com/10704213/diff/1/tools/bisect-builds.py...
tools/bisect-builds.py:231: final_list.append(str(build_number))
On 2012/07/20 17:37:01, Vitaly Buka wrote:
> It's not what I tried to do.
> Example: 
> we have revissions [0 1 4 5 7 9]
> and asked -g 3 -b 6
> I'd like to bisect [1 4 5 7]
> You code will bisect [4 5 7]

I see. The code on the left did what my code did though, right? Why are you
changing this?

> 
> 
> On 2012/07/20 17:19:45, Nico wrote:
> > This appends on every revision and then resets to an empty list on the next
> > loop. Isn't this simpler:
> > 
> > if build_number >= maxrev:
> >   break
> > if build_number >= minrev:
> >   final_list.append(str(build_number))
> > 
> > ?
>

[Vitaly Buka (NO REVIEWS), 2012-07-20 17:43:37]

> > I'd like to bisect [1 4 5 7]
> > You code will bisect [4 5 7]
Actually your code will bisect [4 5]

[Vitaly Buka (NO REVIEWS), 2012-07-20 18:01:04]

On 2012/07/20 17:39:11, Nico wrote:
> https://chromiumcodereview.appspot.com/10704213/diff/1/tools/bisect-builds.py
> File tools/bisect-builds.py (right):
> 
>
https://chromiumcodereview.appspot.com/10704213/diff/1/tools/bisect-builds.py...
> tools/bisect-builds.py:231: final_list.append(str(build_number))
> On 2012/07/20 17:37:01, Vitaly Buka wrote:
> > It's not what I tried to do.
> > Example: 
> > we have revissions [0 1 4 5 7 9]
> > and asked -g 3 -b 6
> > I'd like to bisect [1 4 5 7]
> > You code will bisect [4 5 7]
> 
> I see. The code on the left did what my code did though, right? Why are you
> changing this?
> 

Agree. I have no good reason to change this. 

> > if build_number >= maxrev:
I still change to if build_number > maxrev:
Because it's how bisect works for unofficial.

[Nico, 2012-07-20 18:04:01]

lgtm

Wait 2h or so with landing, to see if rsesek wants to chime in

https://chromiumcodereview.appspot.com/10704213/diff/9002/tools/bisect-builds.py
File tools/bisect-builds.py (right):

https://chromiumcodereview.appspot.com/10704213/diff/9002/tools/bisect-builds...
tools/bisect-builds.py:228: # Keep only one revision before and after if no
exact match found.
Now the comment is no longer true, right?

[Vitaly Buka (NO REVIEWS), 2012-07-20 18:11:24]

https://chromiumcodereview.appspot.com/10704213/diff/9002/tools/bisect-builds.py
File tools/bisect-builds.py (right):

https://chromiumcodereview.appspot.com/10704213/diff/9002/tools/bisect-builds...
tools/bisect-builds.py:228: # Keep only one revision before and after if no
exact match found.
On 2012/07/20 18:04:02, Nico wrote:
> Now the comment is no longer true, right?

Done.

[Robert Sesek, 2012-07-20 18:12:27]

LGTM otherwise

https://chromiumcodereview.appspot.com/10704213/diff/9002/tools/bisect-builds.py
File tools/bisect-builds.py (right):

https://chromiumcodereview.appspot.com/10704213/diff/9002/tools/bisect-builds...
tools/bisect-builds.py:598: help = 'The bad revision to bisect to.')
Update the help to say it defaults to HEAD.

[Vitaly Buka (NO REVIEWS), 2012-07-20 18:20:12]

https://chromiumcodereview.appspot.com/10704213/diff/9002/tools/bisect-builds.py
File tools/bisect-builds.py (right):

https://chromiumcodereview.appspot.com/10704213/diff/9002/tools/bisect-builds...
tools/bisect-builds.py:598: help = 'The bad revision to bisect to.')
On 2012/07/20 18:12:27, rsesek wrote:
> Update the help to say it defaults to HEAD.

Done.

[commit-bot: I haz the power, 2012-07-20 18:21:38]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/vitalybuka@chromium.org/10704213/7005

[commit-bot: I haz the power, 2012-07-20 20:13:57]

Change committed as 147707