Parse Git hash for dependency repositories from DEPS file.

tools/bisect-perf-regression.py

 #!/usr/bin/env python
 # Copyright (c) 2013 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.
 
 """Performance Test Bisect Tool
 
 This script bisects a series of changelists using binary search. It starts at
 a bad revision where a performance metric has regressed, and asks for a last
 known-good revision. It will then binary search across this revision range by
@@ skipping 1409 matching lines @@
     # angle.git@[a-fA-F0-9]{40}$ and replace git hash.
     if depot == 'angle':
       return _UpdateDEPSForAngle(revision, depot, deps_file)
 
     try:
       deps_contents = ReadStringFromFile(deps_file)
       # Check whether the depot and revision pattern in DEPS file vars
       # e.g. for webkit the format is "webkit_revision": "12345".
       deps_revision = re.compile(r'(?<="%s": ")([0-9]+)(?=")' % deps_var,
                                  re.MULTILINE)
-      match = re.search(deps_revision, deps_contents)
-      if match:
+      new_data = None
+      if re.search(deps_revision, deps_contents):
         svn_revision = self.source_control.SVNFindRev(
             revision, self._GetDepotDirectory(depot))
         if not svn_revision:
           print 'Could not determine SVN revision for %s' % revision
           return False
         # Update the revision information for the given depot
         new_data = re.sub(deps_revision, str(svn_revision), deps_contents)
+      else:
+      # Check whether the depot and revision pattern in DEPS file vars
+      # e.g. for webkit the format is "webkit_revision": "559a6d4ab7a84c539..".
+        deps_revision = re.compile(
+            r'(?<=[",\']%s[",\']: [",\'])([a-fA-F0-9]{40})(?=[",\'])' %

[qyearsley, 2014/09/03 01:40:36]

1. The character class [",\'] in this regex matches commas as well:

=>>> re.match(r'[",\']12345[",\']', ',12345,').group(0)
=> ',12345,'

2. The lookahead (?=...) and lookbehind (?<=...) assertions aren't necessary; as
long as the main part that we want to capture is in a group, then we can capture
it.

=>>> re.match(r'["\']([0-9]{5})["\']', '"12345"').group(1)
=> '12345'

So, I think the regex maybe should be:
r'["\']%s["\']: [",\']([a-fA-F0-9]{40})["\']' %  deps_var

Also, this regex doesn't guarantee that the quotes match.

=>>> re.match(r'["\']12345["\']', '"12345\'').group(0)
=> '"12345\''

But, well, that's OK.

[prasadv, 2014/09/03 22:10:14]

Done.

+            deps_var,
+            re.MULTILINE)
+        if re.search(deps_revision, deps_contents):
+          new_data = re.sub(deps_revision, revision, deps_contents)
 
+      if new_data:
         # For v8_bleeding_edge revisions change V8 branch in order
         # to fetch bleeding edge revision.
         if depot == 'v8_bleeding_edge':
           new_data = _UpdateV8Branch(new_data)
           if not new_data:
             return False

[qyearsley, 2014/09/03 01:40:36]

Refactoring suggestion:

Everything in between line 1425 (reading from the file) and line 1456 (writing
back to the file) can be extracted out to a helper function which just takes an
original deps file contents and returns modified contents. This method could be
called something like _UpdateDepsContents, and take a deps_contents, revision,
and depot.

Then, it would be relatively easy to add a test for just this helper method.

[prasadv, 2014/09/03 22:10:14]

Done.

         # Write changes to DEPS file
         WriteStringToFile(new_data, deps_file)
         return True
     except IOError, e:
       print 'Something went wrong while updating DEPS file. [%s]' % e
     return False
 
   def CreateDEPSPatch(self, depot, revision):
     """Modifies DEPS and returns diff as text.
 
@@ skipping 1005 matching lines @@
           if min_revision_data['passed'] == '?':
             next_revision_index = min_revision
           elif max_revision_data['passed'] == '?':
             next_revision_index = max_revision
           elif current_depot in ['android-chrome', 'cros', 'chromium', 'v8']:
             previous_revision = revision_list[min_revision]
             # If there were changes to any of the external libraries we track,
             # should bisect the changes there as well.
             external_depot = self._FindNextDepotToBisect(
                 current_depot, min_revision_data, max_revision_data)
-
             # If there was no change in any of the external depots, the search
             # is over.
             if not external_depot:
               if current_depot == 'v8':
                 self.warnings.append('Unfortunately, V8 bisection couldn\'t '
                     'continue any further. The script can only bisect into '
                     'V8\'s bleeding_edge repository if both the current and '
                     'previous revisions in trunk map directly to revisions in '
                     'bleeding_edge.')
               break
@@ skipping 877 matching lines @@
       # bugs. If you change this, please update the perf dashboard as well.
       bisect_utils.OutputAnnotationStepStart('Results')
     print 'Error: %s' % e.message
     if opts.output_buildbot_annotations:
       bisect_utils.OutputAnnotationStepClosed()
   return 1
 
 
 if __name__ == '__main__':
   sys.exit(main())