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
 
@@ skipping 33 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:

[epoger, 2013/11/25 19:43:47]

We didn't need this lock before, because the self._expectations_repo object had
its own lock.  But now that we're calling "gclient sync", we need to make sure
we don't have two updates running simultaneously.

+    # 1. self.results
+    # 2. the expected or actual results on local disk
+    self.results_rlock = threading.RLock()
 
   def is_exported(self):
     """ Returns true iff HTTP clients on other hosts are allowed to access
     this server. """
     return self._export
 
   def is_editable(self):
     """ Returns true iff HTTP clients are allowed to submit new baselines. """
     return self._editable
 
   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
     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.
     """
     while self._reload_seconds:
       time.sleep(self._reload_seconds)
       self.update_results()
 
   def run(self):
@@ skipping 166 matching lines @@
     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 65 matching lines @@
                       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()