Bisect official build with providing bad/good revisions.

tools/bisect-builds.py

Index: tools/bisect-builds.py
diff --git a/tools/bisect-builds.py b/tools/bisect-builds.py
index 307a7b99f3a1ecf82e3ad9893d66a656617beb3c..23ac88843cd08fea99731dd12d78ea6415eb844b 100755
--- a/tools/bisect-builds.py
+++ b/tools/bisect-builds.py
@@ -216,10 +216,7 @@ class PathContext(object):
     handle.close()
     build_numbers = re.findall(r'<a href="([0-9][0-9].*)/">', dirindex)
     final_list = []
-    start_index = 0
-    end_index = 0
     i = 0
-
     parsed_build_numbers = [LooseVersion(x) for x in build_numbers]
     for build_number in sorted(parsed_build_numbers):
       path = OFFICIAL_BASE_URL + '/' + str(build_number) + '/' + \
@@ -228,14 +225,15 @@ class PathContext(object):
       try:
         connection = urllib.urlopen(path)
         connection.close()
+        # Keep only one revision before and after if no exact match found.
+        if build_number <= minrev:
+          final_list = []
         final_list.append(str(build_number))

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

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:37:01]

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:

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

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

-        if str(build_number) == minrev:
-          start_index = i
-        if str(build_number) == maxrev:
-          end_index = i
+        if build_number >= maxrev:
+          break;

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

no semicolon

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

Done.

       except urllib.HTTPError, e:
         pass
-    return final_list[start_index:end_index]
+    return final_list
 
 def UnzipFilenameToDir(filename, dir):
   """Unzip |filename| to directory |dir|."""
@@ -444,6 +442,8 @@ def Bisect(platform,
     msg = 'We don\'t have enough builds to bisect. revlist: %s' % revlist
     raise RuntimeError(msg)
 
+  print 'Bisecting range [%s, %s].' % (revlist[0], revlist[-1])
+
   # Figure out our bookends and first pivot point; fetch the pivot revision.
   good = 0
   bad = len(revlist) - 1
@@ -568,6 +568,17 @@ def GetWebKitRevisionForChromiumRevision(rev):
     raise Exception('Could not get webkit revision for cr rev %d' % rev)
 
 
+def GetChromiumRevision(url):
+  """Returns the chromium revision read from given URL."""
+  try:
+    # Location of the latest build revision number
+    nh = urllib.urlopen(url)

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

No need for a nh variable

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

Done.

+    return int(nh.read())
+  except Exception, e:
+    print('Could not determine latest revision. This could be bad...')

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

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]

Done.

+    return 0
+
+
 def main():
   usage = ('%prog [options] [-- chromium-options]\n'
            'Perform binary search on the snapshot builds.\n'
@@ -604,47 +615,34 @@ def main():
     parser.print_help()
     return 1
 
-  if opts.bad and opts.good and (opts.good > opts.bad):
-    print ('The good revision (%d) must precede the bad revision (%d).\n' %
-           (opts.good, opts.bad))
-    parser.print_help()
-    return 1
-
   # Create the context. Initialize 0 for the revisions as they are set below.
   context = PathContext(opts.archive, 0, 0, opts.official_builds)
-
-  if opts.official_builds and opts.bad is None:
-    print >>sys.stderr, 'Bisecting official builds requires a bad build number.'
-    parser.print_help()
-    return 1
-
   # Pick a starting point, try to get HEAD for this.
   if opts.bad:
     bad_rev = opts.bad
   else:
-    bad_rev = 0
-    try:
-      # Location of the latest build revision number
-      nh = urllib.urlopen(context.GetLastChangeURL())
-      latest = int(nh.read())
-      nh.close()
-      bad_rev = raw_input('Bad revision [HEAD:%d]: ' % latest)
-      if (bad_rev == ''):
-        bad_rev = latest
-      bad_rev = int(bad_rev)
-    except Exception, e:
-      print('Could not determine latest revision. This could be bad...')
-      bad_rev = int(raw_input('Bad revision: '))
+    bad_rev = '999.0.0.0'
+    if not opts.official_builds:
+      bad_rev = GetChromiumRevision(context.GetLastChangeURL())
 
   # Find out when we were good.
   if opts.good:
     good_rev = opts.good
   else:
-    good_rev = 0
-    try:
-      good_rev = int(raw_input('Last known good [0]: '))
-    except Exception, e:
-      pass
+    good_rev = '0.0.0.0' if opts.official_builds else 0
+
+  if opts.official_builds:
+    good_rev = LooseVersion(good_rev)
+    bad_rev = LooseVersion(bad_rev)
+  else:
+    good_rev = int(good_rev)
+    bad_rev = int(bad_rev)
+
+  if good_rev > bad_rev:
+    print ('The good revision (%s) must precede the bad revision (%s).\n' %
+           (good_rev, bad_rev))
+    parser.print_help()
+    return 1
 
   if opts.times < 1:
     print('Number of times to run (%d) must be greater than or equal to 1.' %