bisect-builds.py: Minor bug fixes for blink bisect and better error handling.

tools/bisect-builds.py

 #!/usr/bin/env python
 # Copyright (c) 2012 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 """Snapshot Build Bisect Tool
 
 This script bisects a snapshot archive using binary search. It starts at
 a bad revision (it will try to guess HEAD) and asks for a last known-good
 revision. It will then binary search across this revision range by downloading,
@@ skipping 303 matching lines @@
       self.githash_svn_dict.update(new_dict)
     return revisions
 
   def _GetSVNRevisionFromGitHashWithoutGitCheckout(self, git_sha1, depot):
     json_url = GITHASH_TO_SVN_URL[depot] % git_sha1
     try:
       response = urllib.urlopen(json_url)
     except urllib.HTTPError as error:
       msg = 'HTTP Error %d for %s' % (error.getcode(), git_sha1)
       return None
-    data = json.loads(response.read()[4:])
+    try:
+      response_data = response.read()
+      data = json.loads(response_data[4:])
+    except ValueError:
+      print 'Response string for %s is "%s"' % (json_url, response_data)
+      raise ValueError
     if 'message' in data:
       message = data['message'].split('\n')
       message = [line for line in message if line.strip()]
       search_pattern = re.compile(SEARCH_PATTERN[depot])
       result = search_pattern.search(message[len(message)-1])
       if result:
         return result.group(1)
     print 'Failed to get svn revision number for %s' % git_sha1
     raise ValueError
 
@@ skipping 500 matching lines @@
       for f in [_GetDownloadPath(revlist[down_pivot]),
                 _GetDownloadPath(revlist[up_pivot])]:
         try:
           os.unlink(f)
         except OSError:
           pass
       sys.exit(0)
 
     rev = revlist[pivot]
 
-  return (revlist[minrev], revlist[maxrev])
+  return (revlist[minrev], revlist[maxrev], context)
 
 
 def GetBlinkDEPSRevisionForChromiumRevision(rev):
   """Returns the blink revision that was in REVISIONS file at
   chromium revision |rev|."""
   # . doesn't match newlines without re.DOTALL, so this is safe.
   blink_re = re.compile(r'webkit_revision\D*(\d+)')
   url = urllib.urlopen(DEPS_FILE % rev)
   m = blink_re.search(url.read())
   url.close()
   if m:
     return int(m.group(1))
   else:
     raise Exception('Could not get Blink revision for Chromium rev %d' % rev)
 
 
 def GetBlinkRevisionForChromiumRevision(self, rev):

[Robert Sesek, 2014/08/12 21:41:24]

Rename self -> context

[pshenoy, 2014/08/13 18:13:41]

Done.

   """Returns the blink revision that was in REVISIONS file at
   chromium revision |rev|."""
   def _IsRevisionNumber(revision):
     if isinstance(revision, int):
       return True
     else:
       return revision.isdigit()
   if str(rev) in self.githash_svn_dict:
     rev = self.githash_svn_dict[str(rev)]
   file_url = '%s/%s%s/REVISIONS' % (self.base_url,
                                     self._listing_platform_dir, rev)
   url = urllib.urlopen(file_url)
-  data = json.loads(url.read())
+  try:
+    response = url.read()
+    data = json.loads(response)
+  except ValueError:
+    print 'Response from %s: "%s"' % (file_url, response)
+    raise ValueError
   url.close()
   if 'webkit_revision' in data:
     blink_rev = data['webkit_revision']
     if not _IsRevisionNumber(blink_rev):
-      blink_rev = self.GetSVNRevisionFromGitHash(blink_rev, 'blink')
+      blink_rev = int(self.GetSVNRevisionFromGitHash(blink_rev, 'blink'))
     return blink_rev
   else:
     raise Exception('Could not get blink revision for cr rev %d' % rev)
 
 
 def FixChromiumRevForBlink(revisions_final, revisions, self, rev):
   """Returns the chromium revision that has the correct blink revision
   for blink bisect, DEPS and REVISIONS file might not match since
   blink snapshots point to tip of tree blink.
   Note: The revisions_final variable might get modified to include
@@ skipping 191 matching lines @@
     print('Number of times to run (%d) must be greater than or equal to 1.' %
           opts.times)
     parser.print_help()
     return 1
 
   if opts.asan:
     evaluator = IsGoodASANBuild
   else:
     evaluator = AskIsGoodBuild
 
-  (min_chromium_rev, max_chromium_rev) = Bisect(
+  (min_chromium_rev, max_chromium_rev, context) = Bisect(

[Robert Sesek, 2014/08/12 20:50:11]

Why do you need to return context if you're not using it?

[pshenoy, 2014/08/12 21:05:06]

'context' is passed in GetBlinkRevisionForChromiumRevision to access
githash_svn_dict. So we need the 'context' created in Bisect() to get the blink
revision from the corresponding githash value. Earlier it was using the
'context' created in main which was dummy.

[Robert Sesek, 2014/08/12 21:41:24]

Ah, makes sense. Why don't we switch from passing all these parameters to
Bisect, which merely passes them to the PathContext constructor, to creating the
context here in Main and then just passing the context to Bisect.

[pshenoy, 2014/08/13 18:13:41]

Done.

       base_url, opts.archive, opts.official_builds, opts.aura, opts.asan,
       opts.use_local_repo, good_rev, bad_rev, opts.times, opts.command,
       args, opts.profile, opts.flash_path, opts.pdf_path,
       not opts.not_interactive, evaluator)
 
   # Get corresponding blink revisions.
   try:
     min_blink_rev = GetBlinkRevisionForChromiumRevision(context,
                                                         min_chromium_rev)
     max_blink_rev = GetBlinkRevisionForChromiumRevision(context,
@@ skipping 26 matching lines @@
 
     print 'CHANGELOG URL:'
     if opts.official_builds:
       print OFFICIAL_CHANGELOG_URL % (min_chromium_rev, max_chromium_rev)
     else:
       print '  ' + CHANGELOG_URL % (min_chromium_rev, max_chromium_rev)
 
 
 if __name__ == '__main__':
   sys.exit(main())