From f000241dcb1957e13dbb4da40a262df5f0c2940d Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Wed, 15 Feb 2017 12:47:13 -0800 Subject: [PATCH] Local repositories clone a blank repo --- pre_commit/commands/autoupdate.py | 2 +- pre_commit/manifest.py | 9 +- pre_commit/repository.py | 90 ++++++++++++------- .../resources/empty_template/.npmignore | 1 + pre_commit/resources/empty_template/main.go | 3 + .../resources/empty_template/package.json | 4 + .../pre_commit_dummy_package.gemspec | 5 ++ pre_commit/resources/empty_template/setup.py | 4 + pre_commit/store.py | 62 ++++++------- pre_commit/util.py | 18 ++++ setup.py | 4 +- testing/fixtures.py | 2 +- testing/util.py | 19 ---- tests/manifest_test.py | 9 +- tests/repository_test.py | 45 +++++----- 15 files changed, 156 insertions(+), 121 deletions(-) create mode 100644 pre_commit/resources/empty_template/.npmignore create mode 100644 pre_commit/resources/empty_template/main.go create mode 100644 pre_commit/resources/empty_template/package.json create mode 100644 pre_commit/resources/empty_template/pre_commit_dummy_package.gemspec create mode 100644 pre_commit/resources/empty_template/setup.py diff --git a/pre_commit/commands/autoupdate.py b/pre_commit/commands/autoupdate.py index 293c81fc..01a361c8 100644 --- a/pre_commit/commands/autoupdate.py +++ b/pre_commit/commands/autoupdate.py @@ -32,7 +32,7 @@ def _update_repo(repo_config, runner, tags_only): """ repo = Repository.create(repo_config, runner.store) - with cwd(repo.repo_path_getter.repo_path): + with cwd(repo._repo_path): cmd_output('git', 'fetch') tag_cmd = ('git', 'describe', 'origin/master', '--tags') if tags_only: diff --git a/pre_commit/manifest.py b/pre_commit/manifest.py index 55f7c1ae..2b4614c7 100644 --- a/pre_commit/manifest.py +++ b/pre_commit/manifest.py @@ -13,15 +13,14 @@ logger = logging.getLogger('pre_commit') class Manifest(object): - def __init__(self, repo_path_getter, repo_url): - self.repo_path_getter = repo_path_getter + def __init__(self, repo_path, repo_url): + self.repo_path = repo_path self.repo_url = repo_url @cached_property def manifest_contents(self): - repo_path = self.repo_path_getter.repo_path - default_path = os.path.join(repo_path, C.MANIFEST_FILE) - legacy_path = os.path.join(repo_path, C.MANIFEST_FILE_LEGACY) + default_path = os.path.join(self.repo_path, C.MANIFEST_FILE) + legacy_path = os.path.join(self.repo_path, C.MANIFEST_FILE_LEGACY) if os.path.exists(legacy_path) and not os.path.exists(default_path): logger.warning( '{} uses legacy {} to provide hooks.\n' diff --git a/pre_commit/repository.py b/pre_commit/repository.py index 4fc970b3..c5091683 100644 --- a/pre_commit/repository.py +++ b/pre_commit/repository.py @@ -96,40 +96,34 @@ def _install_all(venvs, repo_url): class Repository(object): - def __init__(self, repo_config, repo_path_getter): + def __init__(self, repo_config, store): self.repo_config = repo_config - self.repo_path_getter = repo_path_getter + self.store = store self.__installed = False @classmethod def create(cls, config, store): if is_local_hooks(config): - return LocalRepository(config) + return LocalRepository(config, store) else: - repo_path_getter = store.get_repo_path_getter( - config['repo'], config['sha'] - ) - return cls(config, repo_path_getter) + return cls(config, store) + + @cached_property + def _repo_path(self): + return self.store.clone( + self.repo_config['repo'], self.repo_config['sha'], + ) @cached_property def _cmd_runner(self): - return PrefixedCommandRunner(self.repo_path_getter.repo_path) + return PrefixedCommandRunner(self._repo_path) - @cached_property - def _venvs(self): - deps_dict = defaultdict(_UniqueList) - for _, hook in self.hooks: - deps_dict[(hook['language'], hook['language_version'])].update( - hook.get('additional_dependencies', []), - ) - ret = [] - for (language, version), deps in deps_dict.items(): - ret.append((self._cmd_runner, language, version, deps)) - return tuple(ret) + def _cmd_runner_from_deps(self, language_name, deps): + return self._cmd_runner @cached_property def manifest(self): - return Manifest(self.repo_path_getter, self.repo_config['repo']) + return Manifest(self._repo_path, self.repo_config['repo']) @cached_property def hooks(self): @@ -160,6 +154,18 @@ class Repository(object): for hook in self.repo_config['hooks'] ) + @cached_property + def _venvs(self): + deps_dict = defaultdict(_UniqueList) + for _, hook in self.hooks: + deps_dict[(hook['language'], hook['language_version'])].update( + hook.get('additional_dependencies', []), + ) + ret = [] + for (language, version), deps in deps_dict.items(): + ret.append((self._cmd_runner, language, version, deps)) + return tuple(ret) + def require_installed(self): if not self.__installed: _install_all(self._venvs, self.repo_config['repo']) @@ -168,19 +174,30 @@ class Repository(object): def run_hook(self, hook, file_args): """Run a hook. - Args: - hook - Hook dictionary - file_args - List of files to run + :param dict hook: + :param tuple file_args: all the files to run the hook on """ self.require_installed() - return languages[hook['language']].run_hook( - self._cmd_runner, hook, file_args, - ) + language_name = hook['language'] + deps = hook.get('additional_dependencies', []) + cmd_runner = self._cmd_runner_from_deps(language_name, deps) + return languages[language_name].run_hook(cmd_runner, hook, file_args) class LocalRepository(Repository): - def __init__(self, repo_config): - super(LocalRepository, self).__init__(repo_config, None) + def _cmd_runner_from_deps(self, language_name, deps): + """local repositories have a cmd runner per hook""" + language = languages[language_name] + # pcre / script / system do not have environments so they work out + # of the current directory + if language.ENVIRONMENT_DIR is None: + return PrefixedCommandRunner(git.get_root()) + else: + return PrefixedCommandRunner(self.store.make_local(deps)) + + @cached_property + def manifest(self): + raise NotImplementedError @cached_property def hooks(self): @@ -190,12 +207,17 @@ class LocalRepository(Repository): ) @cached_property - def cmd_runner(self): - return PrefixedCommandRunner(git.get_root()) - - @cached_property - def manifest(self): - raise NotImplementedError + def _venvs(self): + ret = [] + for _, hook in self.hooks: + language = hook['language'] + version = hook['language_version'] + deps = hook.get('additional_dependencies', []) + ret.append(( + self._cmd_runner_from_deps(language, deps), + language, version, deps, + )) + return tuple(ret) class _UniqueList(list): diff --git a/pre_commit/resources/empty_template/.npmignore b/pre_commit/resources/empty_template/.npmignore new file mode 100644 index 00000000..72e8ffc0 --- /dev/null +++ b/pre_commit/resources/empty_template/.npmignore @@ -0,0 +1 @@ +* diff --git a/pre_commit/resources/empty_template/main.go b/pre_commit/resources/empty_template/main.go new file mode 100644 index 00000000..38dd16da --- /dev/null +++ b/pre_commit/resources/empty_template/main.go @@ -0,0 +1,3 @@ +package main + +func main() {} diff --git a/pre_commit/resources/empty_template/package.json b/pre_commit/resources/empty_template/package.json new file mode 100644 index 00000000..ac7b7259 --- /dev/null +++ b/pre_commit/resources/empty_template/package.json @@ -0,0 +1,4 @@ +{ + "name": "pre_commit_dummy_package", + "version": "0.0.0" +} diff --git a/pre_commit/resources/empty_template/pre_commit_dummy_package.gemspec b/pre_commit/resources/empty_template/pre_commit_dummy_package.gemspec new file mode 100644 index 00000000..5c361d5c --- /dev/null +++ b/pre_commit/resources/empty_template/pre_commit_dummy_package.gemspec @@ -0,0 +1,5 @@ +Gem::Specification.new do |s| + s.name = 'pre_commit_dummy_package' + s.version = '0.0.0' + s.authors = ['Anthony Sottile'] +end diff --git a/pre_commit/resources/empty_template/setup.py b/pre_commit/resources/empty_template/setup.py new file mode 100644 index 00000000..68860648 --- /dev/null +++ b/pre_commit/resources/empty_template/setup.py @@ -0,0 +1,4 @@ +from setuptools import setup + + +setup(name='pre-commit-dummy-package', version='0.0.0') diff --git a/pre_commit/store.py b/pre_commit/store.py index aecc7dc1..3f24d0b0 100644 --- a/pre_commit/store.py +++ b/pre_commit/store.py @@ -12,8 +12,10 @@ from cached_property import cached_property from pre_commit.prefixed_command_runner import PrefixedCommandRunner 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 cwd from pre_commit.util import no_git_env +from pre_commit.util import resource_filename logger = logging.getLogger('pre_commit') @@ -35,16 +37,6 @@ def _get_default_directory(): class Store(object): get_default_directory = staticmethod(_get_default_directory) - class RepoPathGetter(object): - def __init__(self, repo, ref, store): - self._repo = repo - self._ref = ref - self._store = store - - @cached_property - def repo_path(self): - return self._store.clone(self._repo, self._ref) - def __init__(self, directory=None): if directory is None: directory = self.get_default_directory() @@ -91,45 +83,55 @@ class Store(object): def require_created(self): """Require the pre-commit file store to be created.""" - if self.__created: - return + if not self.__created: + self._create() + self.__created = True - self._create() - self.__created = True - - def clone(self, url, ref): - """Clone the given url and checkout the specific ref.""" + def _new_repo(self, repo, ref, make_strategy): self.require_created() # Check if we already exist with sqlite3.connect(self.db_path) as db: result = db.execute( 'SELECT path FROM repos WHERE repo = ? AND ref = ?', - [url, ref], + [repo, ref], ).fetchone() if result: return result[0] - logger.info('Initializing environment for {}.'.format(url)) + logger.info('Initializing environment for {}.'.format(repo)) - dir = tempfile.mkdtemp(prefix='repo', dir=self.directory) - with clean_path_on_failure(dir): - cmd_output( - 'git', 'clone', '--no-checkout', url, dir, env=no_git_env(), - ) - with cwd(dir): - cmd_output('git', 'reset', ref, '--hard', env=no_git_env()) + directory = tempfile.mkdtemp(prefix='repo', dir=self.directory) + with clean_path_on_failure(directory): + make_strategy(directory) # Update our db with the created repo with sqlite3.connect(self.db_path) as db: db.execute( 'INSERT INTO repos (repo, ref, path) VALUES (?, ?, ?)', - [url, ref, dir], + [repo, ref, directory], ) - return dir + return directory - def get_repo_path_getter(self, repo, ref): - return self.RepoPathGetter(repo, ref, self) + def clone(self, repo, ref): + """Clone the given url and checkout the specific ref.""" + def clone_strategy(directory): + cmd_output( + 'git', 'clone', '--no-checkout', repo, directory, + env=no_git_env(), + ) + with cwd(directory): + cmd_output('git', 'reset', ref, '--hard', env=no_git_env()) + + return self._new_repo(repo, ref, clone_strategy) + + def make_local(self, deps): + def make_local_strategy(directory): + copy_tree_to_path(resource_filename('empty_template'), directory) + return self._new_repo( + 'local:{}'.format(','.join(sorted(deps))), 'N/A', + make_local_strategy, + ) @cached_property def cmd_runner(self): diff --git a/pre_commit/util.py b/pre_commit/util.py index 9cf4c164..73719d1b 100644 --- a/pre_commit/util.py +++ b/pre_commit/util.py @@ -204,3 +204,21 @@ def rmtree(path): else: raise 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) diff --git a/setup.py b/setup.py index 25ac40f2..6f7baf25 100644 --- a/setup.py +++ b/setup.py @@ -34,7 +34,9 @@ setup( 'resources/rbenv.tar.gz', 'resources/ruby-build.tar.gz', 'resources/ruby-download.tar.gz', - ] + 'resources/empty_template/*', + 'resources/empty_template/.npmignore', + ], }, install_requires=[ 'aspy.yaml', diff --git a/testing/fixtures.py b/testing/fixtures.py index aaa4203d..16cda572 100644 --- a/testing/fixtures.py +++ b/testing/fixtures.py @@ -15,8 +15,8 @@ from pre_commit.clientlib.validate_config import validate_config_extra from pre_commit.clientlib.validate_manifest import load_manifest from pre_commit.jsonschema_extensions import apply_defaults from pre_commit.util import cmd_output +from pre_commit.util import copy_tree_to_path from pre_commit.util import cwd -from testing.util import copy_tree_to_path from testing.util import get_head_sha from testing.util import get_resource_path diff --git a/testing/util.py b/testing/util.py index 311376a6..4cb6f0d8 100644 --- a/testing/util.py +++ b/testing/util.py @@ -1,7 +1,6 @@ from __future__ import unicode_literals import os.path -import shutil import jsonschema import pytest @@ -20,24 +19,6 @@ def get_resource_path(path): return os.path.join(TESTING_DIR, 'resources', 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 get_head_sha(dir): with cwd(dir): return cmd_output('git', 'rev-parse', 'HEAD')[1].strip() diff --git a/tests/manifest_test.py b/tests/manifest_test.py index e9e39dd4..58242214 100644 --- a/tests/manifest_test.py +++ b/tests/manifest_test.py @@ -12,8 +12,8 @@ from testing.util import get_head_sha def manifest(store, tempdir_factory): path = make_repo(tempdir_factory, 'script_hooks_repo') head_sha = get_head_sha(path) - repo_path_getter = store.get_repo_path_getter(path, head_sha) - yield Manifest(repo_path_getter, path) + repo_path = store.clone(path, head_sha) + yield Manifest(repo_path, path) def test_manifest_contents(manifest): @@ -54,9 +54,8 @@ def test_hooks(manifest): def test_legacy_manifest_warn(store, tempdir_factory, log_warning_mock): path = make_repo(tempdir_factory, 'legacy_hooks_yaml_repo') head_sha = get_head_sha(path) - repo_path_getter = store.get_repo_path_getter(path, head_sha) - - Manifest(repo_path_getter, path).manifest_contents + repo_path = store.clone(path, head_sha) + Manifest(repo_path, path).manifest_contents # Should have printed a warning assert log_warning_mock.call_args_list[0][0][0] == ( diff --git a/tests/repository_test.py b/tests/repository_test.py index 472407ab..bfe4517b 100644 --- a/tests/repository_test.py +++ b/tests/repository_test.py @@ -11,11 +11,10 @@ import mock import pkg_resources import pytest +from pre_commit import constants as C from pre_commit import five from pre_commit import parse_shebang -from pre_commit.clientlib.validate_config import CONFIG_JSON_SCHEMA -from pre_commit.clientlib.validate_config import validate_config_extra -from pre_commit.jsonschema_extensions import apply_defaults +from pre_commit.clientlib.validate_manifest import load_manifest from pre_commit.languages import golang from pre_commit.languages import helpers from pre_commit.languages import node @@ -30,6 +29,7 @@ from testing.fixtures import git_dir from testing.fixtures import make_config_from_repo from testing.fixtures import make_repo from testing.fixtures import modify_manifest +from testing.util import get_resource_path from testing.util import skipif_cant_run_docker from testing.util import skipif_cant_run_swift from testing.util import skipif_slowtests_false @@ -51,9 +51,9 @@ def _test_hook_repo( path = make_repo(tempdir_factory, repo_path) config = make_config_from_repo(path, **(config_kwargs or {})) repo = Repository.create(config, store) - hook_dict = [ + hook_dict, = [ hook for repo_hook_id, hook in repo.hooks if repo_hook_id == hook_id - ][0] + ] ret = repo.run_hook(hook_dict, args) assert ret[0] == expected_return_code assert ret[1].replace(b'\r\n', b'\n') == expected @@ -438,26 +438,6 @@ def test_lots_of_files(tempdir_factory, store): ) -@pytest.fixture -def mock_repo_config(): - config = { - 'repo': 'git@github.com:pre-commit/pre-commit-hooks', - 'sha': '5e713f8878b7d100c0e059f8cc34be4fc2e8f897', - 'hooks': [{ - 'id': 'pyflakes', - 'files': '\\.py$', - }], - } - config_wrapped = apply_defaults([config], CONFIG_JSON_SCHEMA) - validate_config_extra(config_wrapped) - return config_wrapped[0] - - -def test_repo_url(mock_repo_config): - repo = Repository(mock_repo_config, None) - assert repo.repo_url == 'git@github.com:pre-commit/pre-commit-hooks' - - @pytest.mark.integration def test_venvs(tempdir_factory, store): path = make_repo(tempdir_factory, 'python_hooks_repo') @@ -686,6 +666,21 @@ def test_local_repository(): assert len(local_repo.hooks) == 1 +def test_local_python_repo(store): + # Make a "local" hooks repo that just installs our other hooks repo + repo_path = get_resource_path('python_hooks_repo') + manifest = load_manifest(os.path.join(repo_path, C.MANIFEST_FILE)) + hooks = [ + dict(hook, additional_dependencies=[repo_path]) for hook in manifest + ] + config = {'repo': 'local', 'hooks': hooks} + repo = Repository.create(config, store) + (_, hook), = repo.hooks + ret = repo.run_hook(hook, ('filename',)) + assert ret[0] == 0 + assert ret[1].replace(b'\r\n', b'\n') == b"['filename']\nHello World\n" + + @pytest.yield_fixture def fake_log_handler(): handler = mock.Mock(level=logging.INFO)