rebaseline_server: make --reload work in git checkout

gm/rebaseline_server/server.py

 #!/usr/bin/python
 
 """
 Copyright 2013 Google Inc.
 
 Use of this source code is governed by a BSD-style license that can be
 found in the LICENSE file.
 
 HTTP server for our HTML rebaseline viewer.
 """
 
 # System-level imports
 import argparse
 import BaseHTTPServer
 import json
 import logging
 import os
 import posixpath
 import re
 import shutil
 import socket
+import subprocess
 import sys
 import thread
+import threading
 import time
 import urlparse
 
 # Imports from within Skia
 #
 # We need to add the 'tools' directory, so that we can import svn.py within
 # that directory.
 # Make sure that the 'tools' dir is in the PYTHONPATH, but add it at the *end*
 # so any dirs that are already in the PYTHONPATH will be preferred.
 PARENT_DIRECTORY = os.path.dirname(os.path.realpath(__file__))
 TRUNK_DIRECTORY = os.path.dirname(os.path.dirname(PARENT_DIRECTORY))
 TOOLS_DIRECTORY = os.path.join(TRUNK_DIRECTORY, 'tools')
 if TOOLS_DIRECTORY not in sys.path:
   sys.path.append(TOOLS_DIRECTORY)
 import svn
 
 # Imports from local dir
 import results
 
 ACTUALS_SVN_REPO = 'http://skia-autogen.googlecode.com/svn/gm-actual'
 PATHSPLIT_RE = re.compile('/([^/]+)/(.+)')
-TRUNK_DIRECTORY = os.path.dirname(os.path.dirname(os.path.dirname(
-    os.path.realpath(__file__))))
 EXPECTATIONS_DIR = os.path.join(TRUNK_DIRECTORY, 'expectations', 'gm')
 GENERATED_IMAGES_ROOT = os.path.join(PARENT_DIRECTORY, 'static',
                                      'generated-images')
 
 # A simple dictionary of file name extensions to MIME types. The empty string
 # entry is used as the default when no extension was given or if the extension
 # has no entry in this dictionary.
 MIME_TYPE_MAP = {'': 'application/octet-stream',
                  'html': 'text/html',
                  'css': 'text/css',
                  'png': 'image/png',
                  'js': 'application/javascript',
                  'json': 'application/json'
                  }
 
 DEFAULT_ACTUALS_DIR = '.gm-actuals'
 DEFAULT_PORT = 8888
 
 _HTTP_HEADER_CONTENT_LENGTH = 'Content-Length'
 _HTTP_HEADER_CONTENT_TYPE = 'Content-Type'
 
 _SERVER = None   # This gets filled in by main()
 
+
+def _run_command(args, directory):
+  """Runs a command and returns stdout as a single string.
+
+  Args:
+    args: the command to run, as a list of arguments
+    directory: directory within which to run the command
+
+  Returns: stdout, as a string
+
+  Raises an Exception if the command failed (exited with nonzero return code).
+  """
+  logging.debug('_run_command: %s in directory %s' % (args, directory))
+  proc = subprocess.Popen(args, cwd=directory,
+                          stdout=subprocess.PIPE,
+                          stderr=subprocess.PIPE)
+  (stdout, stderr) = proc.communicate()
+  if proc.returncode is not 0:
+    raise Exception('command "%s" failed in dir "%s": %s' %
+                    (args, directory, stderr))
+  return stdout
+
+
 def _get_routable_ip_address():
   """Returns routable IP address of this host (the IP address of its network
      interface that would be used for most traffic, not its localhost
      interface).  See http://stackoverflow.com/a/166589 """
   sock = socket.socket(socket.AF_INET, socket.SOCK_DGRAM)
   sock.connect(('8.8.8.8', 80))
   host = sock.getsockname()[0]
   sock.close()
   return host
 
+
 def _create_svn_checkout(dir_path, repo_url):
   """Creates local checkout of an SVN repository at the specified directory
   path, returning an svn.Svn object referring to the local checkout.
 
   Args:
     dir_path: path to the local checkout; if this directory does not yet exist,
               it will be created and the repo will be checked out into it
     repo_url: URL of SVN repo to check out into dir_path (unless the local
               checkout already exists)
   Returns: an svn.Svn object referring to the local checkout.
@@ skipping 23 matching lines @@
                       if 0, don't check for new results at all
     """
     self._actuals_dir = actuals_dir
     self._port = port
     self._export = export
     self._editable = editable
     self._reload_seconds = reload_seconds
     self._actuals_repo = _create_svn_checkout(
         dir_path=actuals_dir, repo_url=ACTUALS_SVN_REPO)
 
-    # We only update the expectations dir if the server was run with a
-    # nonzero --reload argument; otherwise, we expect the user to maintain
-    # her own expectations as she sees fit.
-    #
-    # TODO(epoger): Use git instead of svn to update the expectations dir, since
-    # the Skia repo is moving to git.
-    # When we make that change, we will have to update the entire workspace,
-    # not just the expectations dir, because git only operates on the repo
-    # as a whole.
-    # And since Skia uses depot_tools to manage its dependencies, we will have
-    # to run "gclient sync" rather than a raw "git pull".
-    if reload_seconds:
-      self._expectations_repo = svn.Svn(EXPECTATIONS_DIR)
-    else:
-      self._expectations_repo = None
+    # Reentrant lock that must be held whenever updating EITHER of:
+    # 1. self._results
+    # 2. the expected or actual results on local disk
+    self.results_rlock = threading.RLock()
+    # self._results will be filled in by calls to update_results()
+    self._results = None
 
+  @property
+  def results(self):
+    """ Returns the most recently generated results, or None if update_results()
+    has not been called yet. """
+    return self._results
+
+  @property
   def is_exported(self):
     """ Returns true iff HTTP clients on other hosts are allowed to access
     this server. """
     return self._export
 
+  @property
   def is_editable(self):
     """ Returns true iff HTTP clients are allowed to submit new baselines. """
     return self._editable
 
+  @property
   def reload_seconds(self):
     """ Returns the result reload period in seconds, or 0 if we don't reload
     results. """
     return self._reload_seconds
 
   def update_results(self):
-    """ Create or update self.results, based on the expectations in
+    """ Create or update self._results, based on the expectations in
     EXPECTATIONS_DIR and the latest actuals from skia-autogen.
+
+    We hold self.results_rlock while we do this, to guarantee that no other
+    thread attempts to update either self._results or the underlying files at
+    the same time.
     """
-    logging.info('Updating actual GM results in %s from SVN repo %s ...' % (
-        self._actuals_dir, ACTUALS_SVN_REPO))
-    self._actuals_repo.Update('.')
+    with self.results_rlock:
+      logging.info('Updating actual GM results in %s from SVN repo %s ...' % (
+          self._actuals_dir, ACTUALS_SVN_REPO))
+      self._actuals_repo.Update('.')
 
-    if self._expectations_repo:
+      # We only update the expectations dir if the server was run with a
+      # nonzero --reload argument; otherwise, we expect the user to maintain
+      # her own expectations as she sees fit.
+      #
+      # Because the Skia repo is moving from SVN to git, and git does not
+      # support updating a single directory tree, we have to update the entire
+      # repo checkout.
+      #
+      # Because Skia uses depot_tools, we have to update using "gclient sync"
+      # instead of raw git (or SVN) update.  Happily, this will work whether
+      # the checkout was created using git or SVN.
+      if self._reload_seconds:
+        logging.info(
+            'Updating expected GM results in %s by syncing Skia repo ...' %
+            EXPECTATIONS_DIR)
+        _run_command(['gclient', 'sync'], TRUNK_DIRECTORY)
+
       logging.info(
-          'Updating expected GM results in %s ...' % EXPECTATIONS_DIR)
-      self._expectations_repo.Update('.')
-
-    logging.info(
           ('Parsing results from actuals in %s and expectations in %s, '
-          + 'and generating pixel diffs (may take a while) ...') % (
-          self._actuals_dir, EXPECTATIONS_DIR))
-    self.results = results.Results(
-        actuals_root=self._actuals_dir,
-        expected_root=EXPECTATIONS_DIR,
-        generated_images_root=GENERATED_IMAGES_ROOT)
+           + 'and generating pixel diffs (may take a while) ...') % (
+               self._actuals_dir, EXPECTATIONS_DIR))
+      self._results = results.Results(
+          actuals_root=self._actuals_dir,
+          expected_root=EXPECTATIONS_DIR,
+          generated_images_root=GENERATED_IMAGES_ROOT)
 
   def _result_reloader(self):
-    """ If --reload argument was specified, reload results at the appropriate
-    interval.
+    """ Reload results at the appropriate interval.  This never exits, so it
+    should be run in its own thread.
     """
-    while self._reload_seconds:
+    while True:
       time.sleep(self._reload_seconds)
       self.update_results()
 
   def run(self):
     self.update_results()
-    thread.start_new_thread(self._result_reloader, ())
+    if self._reload_seconds:
+      thread.start_new_thread(self._result_reloader, ())
 
     if self._export:
       server_address = ('', self._port)
       host = _get_routable_ip_address()
       if self._editable:
         logging.warning('Running with combination of "export" and "editable" '
                         'flags.  Users on other machines will '
                         'be able to modify your GM expectations!')
     else:
       host = '127.0.0.1'
@@ skipping 53 matching lines @@
       time_updated = results_obj.get_timestamp()
 
       response_dict['header'] = {
         # Timestamps:
         # 1. when this data was last updated
         # 2. when the caller should check back for new data (if ever)
         #
         # We only return these timestamps if the --reload argument was passed;
         # otherwise, we have no idea when the expectations were last updated
         # (we allow the user to maintain her own expectations as she sees fit).
-        'timeUpdated': time_updated if _SERVER.reload_seconds() else None,
+        'timeUpdated': time_updated if _SERVER.reload_seconds else None,
         'timeNextUpdateAvailable': (
-            (time_updated+_SERVER.reload_seconds()) if _SERVER.reload_seconds()
+            (time_updated+_SERVER.reload_seconds) if _SERVER.reload_seconds
             else None),
 
         # The type we passed to get_results_of_type()
         'type': type,
 
         # Hash of testData, which the client must return with any edits--
         # this ensures that the edits were made to a particular dataset.
         'dataHash': str(hash(repr(response_dict['testData']))),
 
         # Whether the server will accept edits back.
-        'isEditable': _SERVER.is_editable(),
+        'isEditable': _SERVER.is_editable,
 
         # Whether the service is accessible from other hosts.
-        'isExported': _SERVER.is_exported(),
+        'isExported': _SERVER.is_exported,
       }
       self.send_json_dict(response_dict)
     except:
       self.send_error(404)
       raise
 
   def do_GET_static(self, path):
     """ Handle a GET request for a file under the 'static' directory.
     Only allow serving of files within the 'static' directory that is a
     filesystem sibling of this script.
@@ skipping 50 matching lines @@
           'config': 'gpu',
           'expectedHashType': 'bitmap-64bitMD5',
           'expectedHashDigest': '1707359671708613629',
         },
         ...
       ],
     }
 
     Raises an Exception if there were any problems.
     """
-    if not _SERVER.is_editable():
+    if not _SERVER.is_editable:
       raise Exception('this server is not running in --editable mode')
 
     content_type = self.headers[_HTTP_HEADER_CONTENT_TYPE]
     if content_type != 'application/json;charset=UTF-8':
       raise Exception('unsupported %s [%s]' % (
           _HTTP_HEADER_CONTENT_TYPE, content_type))
 
     content_length = int(self.headers[_HTTP_HEADER_CONTENT_LENGTH])
     json_data = self.rfile.read(content_length)
     data = json.loads(json_data)
     logging.debug('do_POST_edits: received new GM expectations data [%s]' %
                   data)
 
-    # Since we must make multiple calls to the Results object, grab a
-    # reference to it in case it is updated to point at a new Results
-    # object within another thread.
-    results_obj = _SERVER.results
-    oldResultsType = data['oldResultsType']
-    oldResults = results_obj.get_results_of_type(oldResultsType)
-    oldResultsHash = str(hash(repr(oldResults['testData'])))
-    if oldResultsHash != data['oldResultsHash']:
-      raise Exception('results of type "%s" changed while the client was '
-                      'making modifications. The client should reload the '
-                      'results and submit the modifications again.' %
-                      oldResultsType)
-    results_obj.edit_expectations(data['modifications'])
-
-    # Now that the edits have been committed, update results to reflect them.
-    _SERVER.update_results()
+    # Update the results on disk with the information we received from the
+    # client.
+    # We must hold _SERVER.results_rlock while we do this, to guarantee that
+    # no other thread updates expectations (from the Skia repo) while we are
+    # updating them (using the info we received from the client).
+    with _SERVER.results_rlock:
+      oldResultsType = data['oldResultsType']
+      oldResults = _SERVER.results.get_results_of_type(oldResultsType)
+      oldResultsHash = str(hash(repr(oldResults['testData'])))
+      if oldResultsHash != data['oldResultsHash']:
+        raise Exception('results of type "%s" changed while the client was '
+                        'making modifications. The client should reload the '
+                        'results and submit the modifications again.' %
+                        oldResultsType)
+      _SERVER.results.edit_expectations(data['modifications'])
+      # Read the updated results back from disk.
+      _SERVER.update_results()
 
   def redirect_to(self, url):
     """ Redirect the HTTP client to a different url.
 
     Args:
       url: URL to redirect the HTTP client to
     """
     self.send_response(301)
     self.send_header('Location', url)
     self.end_headers()
@@ skipping 52 matching lines @@
                             'to access this server.  WARNING: doing so will '
                             'allow users on other hosts to modify your '
                             'GM expectations, if combined with --editable.'))
   parser.add_argument('--port', type=int,
                       help=('Which TCP port to listen on for HTTP requests; '
                             'defaults to %(default)s'),
                       default=DEFAULT_PORT)
   parser.add_argument('--reload', type=int,
                       help=('How often (a period in seconds) to update the '
                             'results.  If specified, both expected and actual '
-                            'results will be updated.  '
+                            'results will be updated by running "gclient sync" '
+                            'on your Skia checkout as a whole.  '
                             'By default, we do not reload at all, and you '
                             'must restart the server to pick up new data.'),
                       default=0)
   args = parser.parse_args()
   global _SERVER
   _SERVER = Server(actuals_dir=args.actuals_dir,
                    port=args.port, export=args.export, editable=args.editable,
                    reload_seconds=args.reload)
   _SERVER.run()
 
+
 if __name__ == '__main__':
   main()