Merge pull request #846 from pre-commit/performance

Improve performance by factoring out pkg_resources

.coveragerc

@@ -8,6 +8,7 @@ omit =
     # Don't complain if non-runnable code isn't run
     */__main__.py
     pre_commit/color_windows.py
+    pre_commit/resources/*
 
 [report]
 show_missing = True

pre_commit/commands/install_uninstall.py

@@ -12,7 +12,7 @@
 from pre_commit.util import cmd_output
 from pre_commit.util import make_executable
 from pre_commit.util import mkdirp
-from pre_commit.util import resource_filename
+from pre_commit.util import resource_text
 
 
 logger = logging.getLogger(__name__)
@@ -80,8 +80,7 @@ def install(
     }
 
     with io.open(hook_path, 'w') as hook_file:
-        with io.open(resource_filename('hook-tmpl')) as f:
-            contents = f.read()
+        contents = resource_text('hook-tmpl')
         before, rest = contents.split(TEMPLATE_START)
         to_template, after = rest.split(TEMPLATE_END)
 

pre_commit/constants.py

@@ -1,7 +1,7 @@
 from __future__ import absolute_import
 from __future__ import unicode_literals
 
-import pkg_resources
+import importlib_metadata  # TODO: importlib.metadata py38?
 
 CONFIG_FILE = '.pre-commit-config.yaml'
 MANIFEST_FILE = '.pre-commit-hooks.yaml'
@@ -18,8 +18,7 @@
 # Bump when modifying `empty_template`
 LOCAL_REPO_VERSION = '1'
 
-VERSION = pkg_resources.get_distribution('pre-commit').version
-VERSION_PARSED = pkg_resources.parse_version(VERSION)
+VERSION = importlib_metadata.version('pre_commit')
 
 # `manual` is not invoked by any installed git hook.  See #719
 STAGES = ('commit', 'commit-msg', 'manual', 'push')

pre_commit/languages/ruby.py

@@ -11,7 +11,7 @@
 from pre_commit.languages import helpers
 from pre_commit.util import CalledProcessError
 from pre_commit.util import clean_path_on_failure
-from pre_commit.util import resource_filename
+from pre_commit.util import resource_bytesio
 from pre_commit.xargs import xargs
 
 
@@ -47,22 +47,23 @@ def in_env(prefix, language_version):  # pragma: windows no cover
         yield
 
 
+def _extract_resource(filename, dest):
+    with resource_bytesio(filename) as bio:
+        with tarfile.open(fileobj=bio) as tf:
+            tf.extractall(dest)
+
+
 def _install_rbenv(prefix, version='default'):  # pragma: windows no cover
     directory = helpers.environment_dir(ENVIRONMENT_DIR, version)
 
-    with tarfile.open(resource_filename('rbenv.tar.gz')) as tf:
-        tf.extractall(prefix.path('.'))
+    _extract_resource('rbenv.tar.gz', prefix.path('.'))
     shutil.move(prefix.path('rbenv'), prefix.path(directory))
 
     # Only install ruby-build if the version is specified
     if version != 'default':
-        # ruby-download
-        with tarfile.open(resource_filename('ruby-download.tar.gz')) as tf:
-            tf.extractall(prefix.path(directory, 'plugins'))
-
-        # ruby-build
-        with tarfile.open(resource_filename('ruby-build.tar.gz')) as tf:
-            tf.extractall(prefix.path(directory, 'plugins'))
+        plugins_dir = prefix.path(directory, 'plugins')
+        _extract_resource('ruby-download.tar.gz', plugins_dir)
+        _extract_resource('ruby-build.tar.gz', plugins_dir)
 
     activate_path = prefix.path(directory, 'bin', 'activate')
     with io.open(activate_path, 'w') as activate_file:

pre_commit/make_archives.py

@@ -8,7 +8,6 @@
 
 from pre_commit import output
 from pre_commit.util import cmd_output
-from pre_commit.util import resource_filename
 from pre_commit.util import rmtree
 from pre_commit.util import tmpdir
 
@@ -56,7 +55,7 @@ def make_archive(name, repo, ref, destdir):
 
 def main(argv=None):
     parser = argparse.ArgumentParser()
-    parser.add_argument('--dest', default=resource_filename())
+    parser.add_argument('--dest', default='pre_commit/resources')
     args = parser.parse_args(argv)
     for archive_name, repo, ref in REPOS:
         output.write_line('Making {}.tar.gz for {}@{}'.format(

pre_commit/repository.py

@@ -8,7 +8,6 @@
 import shutil
 import sys
 
-import pkg_resources
 from cached_property import cached_property
 from cfgv import apply_defaults
 from cfgv import validate
@@ -23,6 +22,7 @@
 from pre_commit.languages.all import languages
 from pre_commit.languages.helpers import environment_dir
 from pre_commit.prefix import Prefix
+from pre_commit.util import parse_version
 
 
 logger = logging.getLogger('pre_commit')
@@ -110,13 +110,13 @@ def _hook(*hook_dicts):
     for dct in rest:
         ret.update(dct)
 
-    version = pkg_resources.parse_version(ret['minimum_pre_commit_version'])
-    if version > C.VERSION_PARSED:
+    version = ret['minimum_pre_commit_version']
+    if parse_version(version) > parse_version(C.VERSION):
         logger.error(
             'The hook `{}` requires pre-commit version {} but version {} '
             'is installed.  '
             'Perhaps run `pip install --upgrade pre-commit`.'.format(
-                ret['id'], version, C.VERSION_PARSED,
+                ret['id'], version, C.VERSION,
             ),
         )
         exit(1)

pre_commit/store.py

@@ -11,9 +11,8 @@
 from pre_commit import file_lock
 from pre_commit.util import clean_path_on_failure
 from pre_commit.util import cmd_output
-from pre_commit.util import copy_tree_to_path
 from pre_commit.util import no_git_env
-from pre_commit.util import resource_filename
+from pre_commit.util import resource_text
 
 
 logger = logging.getLogger('pre_commit')
@@ -149,9 +148,17 @@ def _git_cmd(*args):
 
         return self._new_repo(repo, ref, deps, clone_strategy)
 
+    LOCAL_RESOURCES = (
+        'Cargo.toml', 'main.go', 'main.rs', '.npmignore', 'package.json',
+        'pre_commit_dummy_package.gemspec', 'setup.py',
+    )
+
     def make_local(self, deps):
         def make_local_strategy(directory):
-            copy_tree_to_path(resource_filename('empty_template'), directory)
+            for resource in self.LOCAL_RESOURCES:
+                contents = resource_text('empty_template_{}'.format(resource))
+                with io.open(os.path.join(directory, resource), 'w') as f:
+                    f.write(contents)
 
             env = no_git_env()
             name, email = 'pre-commit', 'asottile+pre-commit@umich.edu'

pre_commit/util.py

@@ -7,14 +7,21 @@
 import shutil
 import stat
 import subprocess
+import sys
 import tempfile
 
-import pkg_resources
 import six
 
 from pre_commit import five
 from pre_commit import parse_shebang
 
+if sys.version_info >= (3, 7):  # pragma: no cover (PY37+)
+    from importlib.resources import open_binary
+    from importlib.resources import read_text
+else:  # pragma: no cover (<PY37)
+    from importlib_resources import open_binary
+    from importlib_resources import read_text
+
 
 def mkdirp(path):
     try:
@@ -84,10 +91,12 @@ def tmpdir():
         rmtree(tempdir)
 
 
-def resource_filename(*segments):
-    return pkg_resources.resource_filename(
-        'pre_commit', os.path.join('resources', *segments),
-    )
+def resource_bytesio(filename):
+    return open_binary('pre_commit.resources', filename)
+
+
+def resource_text(filename):
+    return read_text('pre_commit.resources', filename)
 
 
 def make_executable(filename):
@@ -195,19 +204,6 @@ def handle_remove_readonly(func, path, exc):  # pragma: no cover (windows)
     shutil.rmtree(path, ignore_errors=False, onerror=handle_remove_readonly)
 
 
-def copy_tree_to_path(src_dir, dest_dir):
-    """Copies all of the things inside src_dir to an already existing dest_dir.
-
-    This looks eerily similar to shutil.copytree, but copytree has no option
-    for not creating dest_dir.
-    """
-    names = os.listdir(src_dir)
-
-    for name in names:
-        srcname = os.path.join(src_dir, name)
-        destname = os.path.join(dest_dir, name)
-
-        if os.path.isdir(srcname):
-            shutil.copytree(srcname, destname)
-        else:
-            shutil.copy(srcname, destname)
+def parse_version(s):
+    """poor man's version comparison"""
+    return tuple(int(p) for p in s.split('.'))

setup.py

@@ -29,23 +29,25 @@
     packages=find_packages(exclude=('tests*', 'testing*')),
     package_data={
         'pre_commit': [
-            'resources/*-tmpl',
+            'resources/hook-tmpl',
             'resources/*.tar.gz',
-            'resources/empty_template/*',
-            'resources/empty_template/.npmignore',
+            'resources/empty_template_*',
         ],
     },
     install_requires=[
         'aspy.yaml',
         'cached-property',
         'cfgv>=1.0.0',
         'identify>=1.0.0',
+        # if this makes it into python3.8 move to extras_require
+        'importlib-metadata',
         'nodeenv>=0.11.1',
         'pyyaml',
         'six',
         'toml',
         'virtualenv',
     ],
+    extras_require={':python_version<"3.7"': ['importlib-resources']},
     entry_points={
         'console_scripts': [
             'pre-commit = pre_commit.main:main',

testing/fixtures.py

@@ -4,6 +4,7 @@
 import contextlib
 import io
 import os.path
+import shutil
 from collections import OrderedDict
 
 from aspy.yaml import ordered_dump
@@ -16,10 +17,27 @@
 from pre_commit.clientlib import CONFIG_SCHEMA
 from pre_commit.clientlib import load_manifest
 from pre_commit.util import cmd_output
-from pre_commit.util import copy_tree_to_path
 from testing.util import get_resource_path
 
 
+def copy_tree_to_path(src_dir, dest_dir):
+    """Copies all of the things inside src_dir to an already existing dest_dir.
+
+    This looks eerily similar to shutil.copytree, but copytree has no option
+    for not creating dest_dir.
+    """
+    names = os.listdir(src_dir)
+
+    for name in names:
+        srcname = os.path.join(src_dir, name)
+        destname = os.path.join(dest_dir, name)
+
+        if os.path.isdir(srcname):
+            shutil.copytree(srcname, destname)
+        else:
+            shutil.copy(srcname, destname)
+
+
 def git_dir(tempdir_factory):
     path = tempdir_factory.get()
     cmd_output('git', 'init', path)

tests/commands/install_uninstall_test.py

@@ -22,7 +22,7 @@
 from pre_commit.util import cmd_output
 from pre_commit.util import make_executable
 from pre_commit.util import mkdirp
-from pre_commit.util import resource_filename
+from pre_commit.util import resource_text
 from testing.fixtures import git_dir
 from testing.fixtures import make_consuming_repo
 from testing.fixtures import remove_config_from_repo
@@ -36,7 +36,7 @@ def test_is_not_script():
 
 
 def test_is_script():
-    assert is_our_script(resource_filename('hook-tmpl'))
+    assert is_our_script('pre_commit/resources/hook-tmpl')
 
 
 def test_is_previous_pre_commit(tmpdir):
@@ -415,8 +415,7 @@ def test_replace_old_commit_script(tempdir_factory, store):
         runner = Runner(path, C.CONFIG_FILE)
 
         # Install a script that looks like our old script
-        with io.open(resource_filename('hook-tmpl')) as f:
-            pre_commit_contents = f.read()
+        pre_commit_contents = resource_text('hook-tmpl')
         new_contents = pre_commit_contents.replace(
             CURRENT_HASH, PRIOR_HASHES[-1],
         )

tests/store_test.py

@@ -153,3 +153,12 @@ def test_require_created_when_directory_exists_but_not_db(store):
     os.makedirs(store.directory)
     store.require_created()
     assert os.path.exists(store.db_path)
+
+
+def test_local_resources_reflects_reality():
+    on_disk = {
+        res[len('empty_template_'):]
+        for res in os.listdir('pre_commit/resources')
+        if res.startswith('empty_template_')
+    }
+    assert on_disk == set(Store.LOCAL_RESOURCES)

tests/util_test.py

@@ -9,6 +9,7 @@
 from pre_commit.util import clean_path_on_failure
 from pre_commit.util import cmd_output
 from pre_commit.util import memoize_by_cwd
+from pre_commit.util import parse_version
 from pre_commit.util import tmpdir
 from testing.util import cwd
 
@@ -117,3 +118,9 @@ def test_cmd_output_exe_not_found():
     ret, out, _ = cmd_output('i-dont-exist', retcode=None)
     assert ret == 1
     assert out == 'Executable `i-dont-exist` not found'
+
+
+def test_parse_version():
+    assert parse_version('0.0') == parse_version('0.0')
+    assert parse_version('0.1') > parse_version('0.0')
+    assert parse_version('2.1') >= parse_version('2')