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

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

Index: scripts/slave/recipe_modules/isolate/resources/compare_build_artifacts.py
diff --git a/scripts/slave/recipe_modules/isolate/resources/compare_build_artifacts.py b/scripts/slave/recipe_modules/isolate/resources/compare_build_artifacts.py
index 3fd28b1b2722b9a0897b98c787d0c5c5544b777c..0f8a5a0934813963f12c72a0726272863f3e3a4b 100755
--- a/scripts/slave/recipe_modules/isolate/resources/compare_build_artifacts.py
+++ b/scripts/slave/recipe_modules/isolate/resources/compare_build_artifacts.py
@@ -34,47 +34,82 @@ def get_files_to_compare(build_dir):
              check(os.path.join(build_dir, f)))
 
 
-def compare_files(first_filepath, second_filepath):
-  """Compares two binaries and return the number of differences between them.
+def diff_dict(a, b):
+  """Returns a yaml-like textural diff of two dict.
 
-  Returns None if the files are equal, a string otherwise.
+  It is currently optimized for the .isolated format.
   """
-  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
+  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)
+        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 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
+
 
-  if diffs and first_filepath.endswith('.isolated'):
-    # Unpack the files.
+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.dumps(
-          json.load(f), indent=2, sort_keys=True,
-          separators=(',', ': ')).splitlines()
+      lhs = json.load(f)
     with open(second_filepath, 'rb') as f:
-      rhs = json.dumps(
-          json.load(f), indent=2, sort_keys=True,
-          separators=(',', ': ')).splitlines()
+      rhs = json.load(f)
+    diff = diff_dict(lhs, rhs)
+    if diff:
+      return '\n' + diff
 
-    result += '\n' + '\n'.join(
-        line for line in difflib.unified_diff(lhs, rhs)
-        if not line.startswith(('---', '+++')))
-  return result
+  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)
+
+  return diff_binary(first_filepath, second_filepath, file_len)
 
 
 def compare_build_artifacts(first_dir, second_dir):