Add show_gm_changes script: show change revisions and hashes for a GM

gm/show_gm_changes.py

+#!/usr/bin/python
+# Copyright (c) 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.
+
+
+"""Find and display recent changes in the given GM."""

[epoger, 2014/04/01 17:39:02]

Please paste your example run command line within this docstring.

[borenet, 2014/04/01 18:11:38]

Done.

+
+
+import argparse
+import json
+import os
+import re
+import subprocess
+import sys
+
+
+def _get_hash_and_last_change(gm_name, filepath):
+  """Find the current hash for the given GM and the last-changed revision.
+
+  This function runs "svn blame", which is slow.
+
+  Args:
+      gm_name: string; name of the GM in question.
+      filepath: string: path to the actual-results.json file.

[epoger, 2014/04/01 17:39:02]

maybe semicolon after "string" just to be consistent

[borenet, 2014/04/01 18:11:38]

Whoops. Done.

+  Returns:
+      tuple of the form (last_changed_rev, hash), where last_changed_rev is an
+      int and hash is a string, or (None, None) if the file does not exist, the
+      GM is not found in the file, or some other problem occurs. 
+  """
+  if not os.path.isfile(filepath):
+    # If the file doesn't exist, we may have synced to before it was created.
+    return (None, None)
+  output = subprocess.check_output(['svn', 'blame', '--force', filepath])
+  pattern = (r'^\s+\d+\s+.+\s+"%s.png" : {\s*\n\s+\d+\s+.+\s+"allowed-digests" '
+             ': \[\s*\n\s+(\d+)\s+.+\s+\[ "bitmap-64bitMD5",\s+\n*(\d+)')

[borenet, 2014/04/01 13:49:57]

This isn't quite right, since it will only find changes in the first hash. 
AFAIK we don't have any GMs with multiple supported hashes, but this could be a
problem in the future.

[epoger, 2014/04/01 17:39:02]

That's not a problem; see https://code.google.com/p/skia/issues/detail?id=1787
('proposal: stop pretending to support multiple image results per GM')

I am a bit worried that this pattern may prove to be fragile in some other way,
because it is so dependent on the exact formatting of the line, but I think we
can cross that bridge when we come to it.

[borenet, 2014/04/01 18:11:38]

Agreed.  I guess another way to do it would be to go through the output of "svn
blame" line-by-line looking for the GM name and then look for the line of the
next hash.  I think it's going to be fragile either way though, unfortunately.

+  match = re.search(pattern % gm_name, output, re.MULTILINE)
+  if match:
+    try:
+      return (int(match.groups()[0]), match.groups()[1])
+    except Exception:
+      # If there are any problems with the above (incorrect number of matches,
+      # inability to parse an integer), just return None.
+      return (None, None)
+  return (None, None)
+
+
+def find_changes(builder_name, gm_name, autogen_path):
+  """Find and return recent changes in the given GM.
+
+  This function runs "svn blame" and "svn update" numerous times and is
+  therefore very slow.
+
+  Args:
+      builder_name: string; name of the builder.
+      gm_name: string; name of the GM.
+      autogen_path: string; path to skia-autogen checkout.
+  Yields:
+      tuples of the form: (autogen_revision, hash)
+  """
+  actuals_path = os.path.join(autogen_path, builder_name, 'actual-results.json')
+
+  # Capture the initial state of the skia-autogen checkout so that we can return
+  # to the same state later.
+  orig_rev = subprocess.check_output(['svnversion', '.'],
+                                     cwd=autogen_path).rstrip()
+
+  try:
+    last_change_rev, hash = _get_hash_and_last_change(gm_name, actuals_path)
+    while last_change_rev:
+      yield (str(last_change_rev), hash)
+      # Sync to the revision just *before* the last change
+      subprocess.check_call(['svn', 'update', '-r', str(last_change_rev - 1)],
+                            cwd=autogen_path,
+                            stdout=subprocess.PIPE,
+                            stderr=subprocess.PIPE)
+      last_change_rev, hash = _get_hash_and_last_change(gm_name, actuals_path)
+  finally:
+    # Return the repository to its initial state.
+    subprocess.check_call(['svn', 'update', '-r', orig_rev],
+                          cwd=autogen_path,
+                          stdout=subprocess.PIPE,
+                          stderr=subprocess.PIPE)
+
+
+def main():
+  """Find and display recent changes in the given GM."""
+  parser = argparse.ArgumentParser(description=sys.modules[__name__].__doc__)
+  parser.add_argument('builder_name', help='Name of the builder.')
+  parser.add_argument('gm_name', help='Name of the GM.')
+  parser.add_argument('--autogen-path', default=os.curdir,
+                      help=('Path to a skia-autogen checkout. This checkout '
+                            'will be modified but the script will attempt to '
+                            'restore it to its original state. Default: '
+                            '"%(default)s"'))
+  args = parser.parse_args()
+
+  print 'Rev\tHash'
+  for change in find_changes(args.builder_name, args.gm_name,
+                             args.autogen_path):
+    print '\t'.join(change)
+  
+
+if __name__ == '__main__':
+  sys.exit(main())

[epoger, 2014/04/01 17:39:02]

Does this mean main() should return an int?  Because it seems like it doesn't...

[borenet, 2014/04/01 18:11:38]

This is a convention I've seen in a number of places and just sort of followed
for no particular reason.  Since main() returns None (there's no return
statement), it's equivalent to exiting with code 0.  I guess the arguable
benefit is that main() could "return 1" instead of sys.exit(1)?

[epoger, 2014/04/01 18:18:55]

I would buy that.