From 133931f4abdafa5bb0bdea3a02af1e5a70d5ac98 Mon Sep 17 00:00:00 2001 From: epoger Date: Fri, 11 Jul 2014 08:52:35 -0700 Subject: [PATCH] roll "common" DEPS, and replace tools/pyutils with it BUG=skia:2682 R=borenet@google.com Author: epoger@google.com Review URL: https://codereview.chromium.org/385783002 --- DEPS | 23 +++--- gm/rebaseline_server/compare_configs.py | 16 ++-- gm/rebaseline_server/compare_rendered_pictures.py | 25 +++--- gm/rebaseline_server/compare_to_expectations.py | 6 +- gm/rebaseline_server/download_actuals.py | 8 +- gm/rebaseline_server/download_actuals_test.py | 9 +-- gm/rebaseline_server/fix_pythonpath.py | 15 ++-- gm/rebaseline_server/results.py | 4 +- gm/rebaseline_server/server.py | 6 +- tools/pyutils/__init__.py | 0 tools/pyutils/gs_utils.py | 97 ----------------------- tools/pyutils/url_utils.py | 63 --------------- tools/pyutils/url_utils_test.py | 61 -------------- 13 files changed, 59 insertions(+), 274 deletions(-) delete mode 100644 tools/pyutils/__init__.py delete mode 100755 tools/pyutils/gs_utils.py delete mode 100755 tools/pyutils/url_utils.py delete mode 100755 tools/pyutils/url_utils_test.py diff --git a/DEPS b/DEPS index e004a2f..920eb32 100644 --- a/DEPS +++ b/DEPS @@ -3,7 +3,20 @@ use_relative_paths = True # Dependencies on outside packages. # deps = { - "common": "https://skia.googlesource.com/common.git@767bf85299d7a822796b57252e7de1e84e6b2e61", + "common": "https://skia.googlesource.com/common.git@0ad95c13aaab3bbb89a97952b83c915d470681df", + # TODO(epoger): Whenever you update the hash of the "common" repo above, + # update the following transitive dependencies as well... + # (I tried to use From() to import the revision numbers from + # common's DEPS file, but that caused "gclient sync" to block forever.) + # + # Once we fix http://crbug.com/393000 ('Gclient recursedeps feature + # doesn't seem to respect use_relative_paths'), we can import them + # recursively, like so: + # recursedeps = {"common"} + "common/third_party/externals/google-api-python-client" : "https://github.com/google/google-api-python-client.git@ecc64a0a2baa4a77f35dec83ad05c6c9ba2d2841", + "common/third_party/externals/httplib2" : "https://github.com/jcgregorio/httplib2.git@7d1b88a3cf34774242bf4c0578c09c0092bb05d8", + "common/third_party/externals/oauth2client" : "https://github.com/google/oauth2client.git@d02b317af0313dcf66755844f5421651af5eb356", + "common/third_party/externals/uritemplate-py" : "https://github.com/uri-templates/uritemplate-py.git@1e780a49412cdbb273e9421974cb91845c124f3f", # DEPS using https://chromium.googlesource.com are pulled from chromium @ r205199 # (see https://chromium.googlesource.com/chromium/chromium/+/c59bfa8ef877f45bfa859669053859857af1d279) @@ -17,14 +30,6 @@ deps = { "third_party/externals/libwebp" : "https://chromium.googlesource.com/webm/libwebp.git@3fe91635df8734b23f3c1b9d1f0c4fa8cfaf4e39", # "third_party/externals/v8" : "git://github.com/v8/v8.git@d15b0f0f2099dbd72867f3df70e9aaf5b8afbd2c", "third_party/externals/nanomsg": "git://github.com/nanomsg/nanomsg.git@0.4-beta", - - # TODO(epoger): These dependencies are needed specifically for google-api-python-client (see http://skbug.com/2641 ). - # Once we get them working, we will want to move them all into the "common" repo - # (see http://skbug.com/2682 and https://codereview.chromium.org/377323002/ ) - "third_party/externals/google-api-python-client" : "https://github.com/google/google-api-python-client.git@ecc64a0a2baa4a77f35dec83ad05c6c9ba2d2841", - "third_party/externals/httplib2" : "https://github.com/jcgregorio/httplib2.git@7d1b88a3cf34774242bf4c0578c09c0092bb05d8", - "third_party/externals/oauth2client" : "https://github.com/google/oauth2client.git@d02b317af0313dcf66755844f5421651af5eb356", - "third_party/externals/uritemplate-py" : "https://github.com/uri-templates/uritemplate-py.git@1e780a49412cdbb273e9421974cb91845c124f3f", } deps_os = { diff --git a/gm/rebaseline_server/compare_configs.py b/gm/rebaseline_server/compare_configs.py index 4075da4..cdc19dd 100755 --- a/gm/rebaseline_server/compare_configs.py +++ b/gm/rebaseline_server/compare_configs.py @@ -11,15 +11,14 @@ Compare GM results for two configs, across all builders. # System-level imports import argparse -import fnmatch -import json import logging -import re import time +# Must fix up PYTHONPATH before importing from within Skia +import fix_pythonpath # pylint: disable=W0611 + # Imports from within Skia -import fix_pythonpath # must do this first -from pyutils import url_utils +from py.utils import url_utils import gm_json import imagediffdb import imagepair @@ -112,7 +111,7 @@ class ConfigComparisons(results.BaseComparisons): tests_found = set() for image_name in sorted(results_of_this_type.keys()): - (test, config) = results.IMAGE_FILENAME_RE.match(image_name).groups() + (test, _) = results.IMAGE_FILENAME_RE.match(image_name).groups() tests_found.add(test) for test in tests_found: @@ -160,9 +159,10 @@ class ConfigComparisons(results.BaseComparisons): failing_image_pairs.add_image_pair(image_pair) except (KeyError, TypeError): logging.exception( - 'got exception while creating ImagePair for image_name ' - '"%s", builder "%s"' % (image_name, builder)) + 'got exception while creating ImagePair for test ' + '"%s", builder "%s"' % (test, builder)) + # pylint: disable=W0201 self._results = { results.KEY__HEADER__RESULTS_ALL: all_image_pairs.as_dict(), results.KEY__HEADER__RESULTS_FAILURES: failing_image_pairs.as_dict(), diff --git a/gm/rebaseline_server/compare_rendered_pictures.py b/gm/rebaseline_server/compare_rendered_pictures.py index 73d0627..a48d1c5 100755 --- a/gm/rebaseline_server/compare_rendered_pictures.py +++ b/gm/rebaseline_server/compare_rendered_pictures.py @@ -12,12 +12,13 @@ Compare results of two render_pictures runs. # System-level imports import logging import os -import re import time +# Must fix up PYTHONPATH before importing from within Skia +import fix_pythonpath # pylint: disable=W0611 + # Imports from within Skia -import fix_pythonpath # must do this first -from pyutils import url_utils +from py.utils import url_utils import gm_json import imagediffdb import imagepair @@ -25,9 +26,14 @@ import imagepairset import results # URL under which all render_pictures images can be found in Google Storage. +# +# pylint: disable=C0301 # TODO(epoger): Move this default value into # https://skia.googlesource.com/buildbot/+/master/site_config/global_variables.json -DEFAULT_IMAGE_BASE_URL = 'http://chromium-skia-gm.commondatastorage.googleapis.com/render_pictures/images' +# pylint: enable=C0301 +DEFAULT_IMAGE_BASE_URL = ( + 'http://chromium-skia-gm.commondatastorage.googleapis.com/' + 'render_pictures/images') class RenderedPicturesComparisons(results.BaseComparisons): @@ -145,14 +151,15 @@ class RenderedPicturesComparisons(results.BaseComparisons): image_dict_A=tiled_images_A[tile_num], image_dict_B=tiled_images_B[tile_num])) - for imagepair in imagepairs_for_this_skp: - if imagepair: - all_image_pairs.add_image_pair(imagepair) - result_type = imagepair.extra_columns_dict\ + for one_imagepair in imagepairs_for_this_skp: + if one_imagepair: + all_image_pairs.add_image_pair(one_imagepair) + result_type = one_imagepair.extra_columns_dict\ [results.KEY__EXTRACOLUMNS__RESULT_TYPE] if result_type != results.KEY__RESULT_TYPE__SUCCEEDED: - failing_image_pairs.add_image_pair(imagepair) + failing_image_pairs.add_image_pair(one_imagepair) + # pylint: disable=W0201 self._results = { results.KEY__HEADER__RESULTS_ALL: all_image_pairs.as_dict(), results.KEY__HEADER__RESULTS_FAILURES: failing_image_pairs.as_dict(), diff --git a/gm/rebaseline_server/compare_to_expectations.py b/gm/rebaseline_server/compare_to_expectations.py index dad206d..cdd03be 100755 --- a/gm/rebaseline_server/compare_to_expectations.py +++ b/gm/rebaseline_server/compare_to_expectations.py @@ -17,12 +17,10 @@ import os import time # Must fix up PYTHONPATH before importing from within Skia -# pylint: disable=W0611 -import fix_pythonpath -# pylint: enable=W0611 +import fix_pythonpath # pylint: disable=W0611 # Imports from within Skia -from pyutils import url_utils +from py.utils import url_utils import column import gm_json import imagediffdb diff --git a/gm/rebaseline_server/download_actuals.py b/gm/rebaseline_server/download_actuals.py index 2f92898..a619f71 100755 --- a/gm/rebaseline_server/download_actuals.py +++ b/gm/rebaseline_server/download_actuals.py @@ -16,10 +16,12 @@ import posixpath import re import urllib2 +# Must fix up PYTHONPATH before importing from within Skia +import fix_pythonpath # pylint: disable=W0611 + # Imports from within Skia -import fix_pythonpath # must do this first -from pyutils import gs_utils -from pyutils import url_utils +from py.utils import gs_utils +from py.utils import url_utils import buildbot_globals import gm_json diff --git a/gm/rebaseline_server/download_actuals_test.py b/gm/rebaseline_server/download_actuals_test.py index c405a3c..a74389f 100755 --- a/gm/rebaseline_server/download_actuals_test.py +++ b/gm/rebaseline_server/download_actuals_test.py @@ -20,13 +20,12 @@ within self._output_dir_expected, which wouldn't be good... # System-level imports import os -import shutil -import tempfile -import urllib + +# Must fix up PYTHONPATH before importing from within Skia +import fix_pythonpath # pylint: disable=W0611 # Imports from within Skia -import fix_pythonpath # must do this first -from pyutils import url_utils +from py.utils import url_utils import base_unittest import download_actuals diff --git a/gm/rebaseline_server/fix_pythonpath.py b/gm/rebaseline_server/fix_pythonpath.py index ed578ce..cc32f4a 100755 --- a/gm/rebaseline_server/fix_pythonpath.py +++ b/gm/rebaseline_server/fix_pythonpath.py @@ -6,16 +6,15 @@ Copyright 2014 Google Inc. Use of this source code is governed by a BSD-style license that can be found in the LICENSE file. -Adds [trunk]/gm and [trunk]/tools to PYTHONPATH, if they aren't already there. +Adds possibly-needed directories to PYTHONPATH, if they aren't already there. """ import os import sys -TRUNK_DIRECTORY = os.path.dirname(os.path.dirname(os.path.dirname(__file__))) -GM_DIRECTORY = os.path.join(TRUNK_DIRECTORY, 'gm') -TOOLS_DIRECTORY = os.path.join(TRUNK_DIRECTORY, 'tools') -if GM_DIRECTORY not in sys.path: - sys.path.append(GM_DIRECTORY) -if TOOLS_DIRECTORY not in sys.path: - sys.path.append(TOOLS_DIRECTORY) +TRUNK_DIRECTORY = os.path.abspath(os.path.join( + os.path.dirname(__file__), os.pardir, os.pardir)) +for subdir in ['common', 'gm', 'tools']: + fullpath = os.path.join(TRUNK_DIRECTORY, subdir) + if fullpath not in sys.path: + sys.path.append(fullpath) diff --git a/gm/rebaseline_server/results.py b/gm/rebaseline_server/results.py index 8e6bc43..da970af 100755 --- a/gm/rebaseline_server/results.py +++ b/gm/rebaseline_server/results.py @@ -15,9 +15,7 @@ import os import re # Must fix up PYTHONPATH before importing from within Skia -# pylint: disable=W0611 -import fix_pythonpath -# pylint: enable=W0611 +import fix_pythonpath # pylint: disable=W0611 # Imports from within Skia import gm_json diff --git a/gm/rebaseline_server/server.py b/gm/rebaseline_server/server.py index e82ecf2..418f5dd 100755 --- a/gm/rebaseline_server/server.py +++ b/gm/rebaseline_server/server.py @@ -26,12 +26,10 @@ import time import urlparse # Must fix up PYTHONPATH before importing from within Skia -# pylint: disable=W0611 -import fix_pythonpath -# pylint: enable=W0611 +import fix_pythonpath # pylint: disable=W0611 # Imports from within Skia -from pyutils import gs_utils +from py.utils import gs_utils import gm_json # Imports from local dir diff --git a/tools/pyutils/__init__.py b/tools/pyutils/__init__.py deleted file mode 100644 index e69de29..0000000 diff --git a/tools/pyutils/gs_utils.py b/tools/pyutils/gs_utils.py deleted file mode 100755 index fdf16ad..0000000 --- a/tools/pyutils/gs_utils.py +++ /dev/null @@ -1,97 +0,0 @@ -#!/usr/bin/python - -""" -Copyright 2014 Google Inc. - -Use of this source code is governed by a BSD-style license that can be -found in the LICENSE file. - -Utilities for accessing Google Cloud Storage. -""" - -# System-level imports -import os -import posixpath -import sys - -# Imports from third-party code -TRUNK_DIRECTORY = os.path.dirname(os.path.dirname(os.path.dirname(__file__))) -for import_subdir in ['google-api-python-client', 'httplib2', 'oauth2client', - 'uritemplate-py']: - import_dirpath = os.path.join( - TRUNK_DIRECTORY, 'third_party', 'externals', import_subdir) - if import_dirpath not in sys.path: - # We need to insert at the beginning of the path, to make sure that our - # imported versions are favored over others that might be in the path. - # Also, the google-api-python-client checkout contains an empty - # oauth2client directory, which will confuse things unless we insert - # our checked-out oauth2client in front of it in the path. - sys.path.insert(0, import_dirpath) -try: - from googleapiclient.discovery import build as build_service -except ImportError: - # TODO(epoger): We are moving toward not needing any dependencies to be - # installed at a system level, but in the meanwhile, if developers run into - # trouble they can install those system-level dependencies to get unblocked. - print ('Missing dependencies of google-api-python-client. Please install ' - 'google-api-python-client to get those dependencies; directions ' - 'can be found at https://developers.google.com/api-client-library/' - 'python/start/installation . More details in http://skbug.com/2641 ') - raise - -# Local imports -import url_utils - - -def download_file(source_bucket, source_path, dest_path, - create_subdirs_if_needed=False): - """ Downloads a single file from Google Cloud Storage to local disk. - - Args: - source_bucket: GCS bucket to download the file from - source_path: full path (Posix-style) within that bucket - dest_path: full path (local-OS-style) on local disk to copy the file to - create_subdirs_if_needed: boolean; whether to create subdirectories as - needed to create dest_path - """ - source_http_url = posixpath.join( - 'http://storage.googleapis.com', source_bucket, source_path) - url_utils.copy_contents(source_url=source_http_url, dest_path=dest_path, - create_subdirs_if_needed=create_subdirs_if_needed) - - -def list_bucket_contents(bucket, subdir=None): - """ Returns files in the Google Cloud Storage bucket as a (dirs, files) tuple. - - Uses the API documented at - https://developers.google.com/storage/docs/json_api/v1/objects/list - - Args: - bucket: name of the Google Storage bucket - subdir: directory within the bucket to list, or None for root directory - """ - # The GCS command relies on the subdir name (if any) ending with a slash. - if subdir and not subdir.endswith('/'): - subdir += '/' - subdir_length = len(subdir) if subdir else 0 - - storage = build_service('storage', 'v1') - command = storage.objects().list( - bucket=bucket, delimiter='/', fields='items(name),prefixes', - prefix=subdir) - results = command.execute() - - # The GCS command returned two subdicts: - # prefixes: the full path of every directory within subdir, with trailing '/' - # items: property dict for each file object within subdir - # (including 'name', which is full path of the object) - dirs = [] - for dir_fullpath in results.get('prefixes', []): - dir_basename = dir_fullpath[subdir_length:] - dirs.append(dir_basename[:-1]) # strip trailing slash - files = [] - for file_properties in results.get('items', []): - file_fullpath = file_properties['name'] - file_basename = file_fullpath[subdir_length:] - files.append(file_basename) - return (dirs, files) diff --git a/tools/pyutils/url_utils.py b/tools/pyutils/url_utils.py deleted file mode 100755 index b107f56..0000000 --- a/tools/pyutils/url_utils.py +++ /dev/null @@ -1,63 +0,0 @@ -#!/usr/bin/python - -""" -Copyright 2014 Google Inc. - -Use of this source code is governed by a BSD-style license that can be -found in the LICENSE file. - -Utilities for working with URLs. - -TODO(epoger): move this into tools/utils for broader use? -""" - -# System-level imports -import contextlib -import os -import shutil -import urllib -import urlparse - - -def create_filepath_url(filepath): - """ Returns a file:/// URL pointing at the given filepath on local disk. - - Args: - filepath: string; path to a file on local disk (may be absolute or relative, - and the file does not need to exist) - - Returns: - A file:/// URL pointing at the file. Regardless of whether filepath was - specified as a relative or absolute path, the URL will contain an - absolute path to the file. - - Raises: - An Exception, if filepath is already a URL. - """ - if urlparse.urlparse(filepath).scheme: - raise Exception('"%s" is already a URL' % filepath) - return urlparse.urljoin( - 'file:', urllib.pathname2url(os.path.abspath(filepath))) - - -def copy_contents(source_url, dest_path, create_subdirs_if_needed=False): - """ Copies the full contents of the URL 'source_url' into - filepath 'dest_path'. - - Args: - source_url: string; complete URL to read from - dest_path: string; complete filepath to write to (may be absolute or - relative) - create_subdirs_if_needed: boolean; whether to create subdirectories as - needed to create dest_path - - Raises: - Some subclass of Exception if unable to read source_url or write dest_path. - """ - if create_subdirs_if_needed: - dest_dir = os.path.dirname(dest_path) - if not os.path.exists(dest_dir): - os.makedirs(dest_dir) - with contextlib.closing(urllib.urlopen(source_url)) as source_handle: - with open(dest_path, 'wb') as dest_handle: - shutil.copyfileobj(fsrc=source_handle, fdst=dest_handle) diff --git a/tools/pyutils/url_utils_test.py b/tools/pyutils/url_utils_test.py deleted file mode 100755 index ef3d8c8..0000000 --- a/tools/pyutils/url_utils_test.py +++ /dev/null @@ -1,61 +0,0 @@ -#!/usr/bin/python - -""" -Copyright 2014 Google Inc. - -Use of this source code is governed by a BSD-style license that can be -found in the LICENSE file. - -Test url_utils.py -""" - -# System-level imports -import os -import shutil -import tempfile -import unittest -import urllib - -# Imports from within Skia -import url_utils - - -class UrlUtilsTest(unittest.TestCase): - - def test_create_filepath_url(self): - """Tests create_filepath_url(). """ - with self.assertRaises(Exception): - url_utils.create_filepath_url('http://1.2.3.4/path') - # Pass absolute filepath. - self.assertEquals( - url_utils.create_filepath_url( - '%sdir%sfile' % (os.path.sep, os.path.sep)), - 'file:///dir/file') - # Pass relative filepath. - self.assertEquals( - url_utils.create_filepath_url(os.path.join('dir', 'file')), - 'file://%s/dir/file' % urllib.pathname2url(os.getcwd())) - - def test_copy_contents(self): - """Tests copy_contents(). """ - contents = 'these are the contents' - tempdir_path = tempfile.mkdtemp() - try: - source_path = os.path.join(tempdir_path, 'source') - source_url = url_utils.create_filepath_url(source_path) - with open(source_path, 'w') as source_handle: - source_handle.write(contents) - dest_path = os.path.join(tempdir_path, 'new_subdir', 'dest') - # Destination subdir does not exist, so copy_contents() should fail - # if create_subdirs_if_needed is False. - with self.assertRaises(Exception): - url_utils.copy_contents(source_url=source_url, - dest_path=dest_path, - create_subdirs_if_needed=False) - # If create_subdirs_if_needed is True, it should work. - url_utils.copy_contents(source_url=source_url, - dest_path=dest_path, - create_subdirs_if_needed=True) - self.assertEquals(open(dest_path).read(), contents) - finally: - shutil.rmtree(tempdir_path) -- 2.7.4