Optimize compare_build_artifacts.py to have an even more compact output.

scripts/slave/recipe_modules/isolate/resources/compare_build_artifacts.py

 #!/usr/bin/env python
 # Copyright 2014 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.
 
 """Compare the artifacts from two builds."""
 
 import difflib
 import json
 import optparse
@@ skipping 16 matching lines @@
       return False
     ext = os.path.splitext(f)[1]
     if ext == '.isolated':
       return True
     return ext in allowed and os.access(f, os.X_OK)
 
   return set(f for f in os.listdir(build_dir) if
              check(os.path.join(build_dir, f)))
 
 
+def diff_dict(a, b):
+  """Returns a yaml-like textural diff of two dict.
+
+  It is currently optimized for the .isolated format.
+  """
+  out = ''
+  for key in set(a) | set(b):
+    va = a.get(key)
+    vb = b.get(key)
+    if va.__class__ != vb.__class__:
+      out += '- %s:\n    %r != %r\n' % (key, va, vb)
+    elif isinstance(va, dict):
+      c = diff_dict(va, vb)
+      if c:
+        out += '- %s:\n%s\n' % (
+            key, '\n'.join('    ' + l for l in c.splitlines()))
+    elif va != vb:
+      out += '- %s:\n    %s != %s\n' % (key, va, vb)
+  return out.rstrip()
+
+
+def diff_binary(first_filepath, second_filepath, file_len):
+  """Returns a compact binary diff if the diff is small enough."""
+  CHUNK_SIZE = 32
+  MAX_STREAMS = 10
+  diffs = 0
+  streams = []
+  offset = 0
+  with open(first_filepath, 'rb') as lhs:
+    with open(second_filepath, 'rb') as rhs:
+      while True:
+        lhs_data = lhs.read(CHUNK_SIZE)
+        rhs_data = rhs.read(CHUNK_SIZE)
+        if not lhs_data:
+          break
+        if lhs_data != rhs_data:
+          diffs += sum(l != r for l, r in zip(lhs_data, rhs_data))
+          if streams is not None:
+            if len(streams) < MAX_STREAMS:
+              streams.append((offset, lhs_data, rhs_data))
+            else:
+              streams = None
+        offset += len(lhs_data)
+        del lhs_data
+        del rhs_data
+  if not diffs:
+    return None
+  result = '%d out of %d bytes are different (%.2f%%)' % (
+        diffs, file_len, 100.0 * diffs / file_len)
+  if streams:
+    result += ''.join(

[Sébastien Marchand, 2014/11/04 20:29:57]

Should we also print the ascii representation of those bytes ? (with a  filter
to only print the values that correspond to a valid char),

something like:
46 72 61 6D 65 72 32 39 42 75 69 6C 64 56 65 72 | Framer29BuildVer

+        '\n%9d: %s\n           %s' % (a, b.encode('hex'), c.encode('hex'))
+        for a, b, c in streams)
+  return result
+
+
 def compare_files(first_filepath, second_filepath):
   """Compares two binaries and return the number of differences between them.
 
   Returns None if the files are equal, a string otherwise.
   """
+  if first_filepath.endswith('.isolated'):
+    with open(first_filepath, 'rb') as f:
+      lhs = json.load(f)
+    with open(second_filepath, 'rb') as f:
+      rhs = json.load(f)
+    diff = diff_dict(lhs, rhs)
+    if diff:
+      return '\n' + diff
+
   file_len = os.stat(first_filepath).st_size
   if file_len != os.stat(second_filepath).st_size:
     return 'different size: %d != %d' % (
         file_len, os.stat(second_filepath).st_size)
 
-  chunk_size = 1024 * 1024
-  diffs = 0
-  with open(first_filepath, 'rb') as lhs:
-    with open(second_filepath, 'rb') as rhs:
-      while True:
-        lhs_data = lhs.read(chunk_size)
-        rhs_data = rhs.read(chunk_size)
-        if not lhs_data:
-          break
-        diffs += sum(l != r for l, r in zip(lhs_data, rhs_data))
-  if not diffs:
-    return None
-
-  result = '%d out of %d bytes are different (%.2f%%)' % (
-        diffs, file_len, 100.0 * diffs / file_len)
-
-  if diffs and first_filepath.endswith('.isolated'):
-    # Unpack the files.
-    with open(first_filepath, 'rb') as f:
-      lhs = json.dumps(
-          json.load(f), indent=2, sort_keys=True,
-          separators=(',', ': ')).splitlines()
-    with open(second_filepath, 'rb') as f:
-      rhs = json.dumps(
-          json.load(f), indent=2, sort_keys=True,
-          separators=(',', ': ')).splitlines()
-
-    result += '\n' + '\n'.join(
-        line for line in difflib.unified_diff(lhs, rhs)
-        if not line.startswith(('---', '+++')))
-  return result
+  return diff_binary(first_filepath, second_filepath, file_len)
 
 
 def compare_build_artifacts(first_dir, second_dir):
   """Compare the artifacts from two distinct builds."""
   if not os.path.isdir(first_dir):
     print >> sys.stderr, '%s isn\'t a valid directory.' % first_dir
     return 1
   if not os.path.isdir(second_dir):
     print >> sys.stderr, '%s isn\'t a valid directory.' % second_dir
     return 1
@@ skipping 41 matching lines @@
     parser.error('--first-build-dir is required')
   if not options.second_build_dir:
     parser.error('--second-build-dir is required')
 
   return compare_build_artifacts(options.first_build_dir,
                                  options.second_build_dir)
 
 
 if __name__ == '__main__':
   sys.exit(main())