From 9a8fb17070a6a935a99502788146c702dfb73e2d Mon Sep 17 00:00:00 2001 From: Thierry Deo Date: Tue, 14 Feb 2017 23:47:02 +0100 Subject: [PATCH] Install local hooks in the ~/.pre-commit folder --- pre_commit/clientlib/validate_config.py | 1 + pre_commit/commands/autoupdate.py | 4 +- pre_commit/languages/docker.py | 2 +- pre_commit/languages/golang.py | 2 +- pre_commit/languages/node.py | 2 +- pre_commit/languages/pcre.py | 2 +- pre_commit/languages/python.py | 4 +- pre_commit/languages/ruby.py | 2 +- pre_commit/languages/script.py | 2 +- pre_commit/languages/swift.py | 2 +- pre_commit/languages/system.py | 2 +- pre_commit/repository.py | 22 ++------ pre_commit/runner.py | 4 +- pre_commit/store.py | 68 ++++++++++++++++++++----- tests/repository_test.py | 59 ++++++++++----------- 15 files changed, 103 insertions(+), 75 deletions(-) diff --git a/pre_commit/clientlib/validate_config.py b/pre_commit/clientlib/validate_config.py index 3624e62b..4f2caf1b 100644 --- a/pre_commit/clientlib/validate_config.py +++ b/pre_commit/clientlib/validate_config.py @@ -70,6 +70,7 @@ def validate_config_extra(config): raise InvalidConfigError( '"sha" property provided for local hooks' ) + repo['sha'] = 'local' elif 'sha' not in repo: raise InvalidConfigError( 'Missing "sha" field for repository {}'.format(repo['repo']) diff --git a/pre_commit/commands/autoupdate.py b/pre_commit/commands/autoupdate.py index 293c81fc..502799d9 100644 --- a/pre_commit/commands/autoupdate.py +++ b/pre_commit/commands/autoupdate.py @@ -30,7 +30,7 @@ def _update_repo(repo_config, runner, tags_only): Args: repo_config - A config for a repository """ - repo = Repository.create(repo_config, runner.store) + repo = Repository.create(repo_config, runner.store, runner.git_root) with cwd(repo.repo_path_getter.repo_path): cmd_output('git', 'fetch') @@ -51,7 +51,7 @@ def _update_repo(repo_config, runner, tags_only): # Construct a new config with the head sha new_config = OrderedDict(repo_config) new_config['sha'] = rev - new_repo = Repository.create(new_config, runner.store) + new_repo = Repository.create(new_config, runner.store, runner.git_root) # See if any of our hooks were deleted with the new commits hooks = {hook['id'] for hook in repo.repo_config['hooks']} diff --git a/pre_commit/languages/docker.py b/pre_commit/languages/docker.py index c4406648..00954577 100644 --- a/pre_commit/languages/docker.py +++ b/pre_commit/languages/docker.py @@ -58,7 +58,7 @@ def install_environment( repo_cmd_runner, version='default', additional_dependencies=(), - is_local_hook = False, + is_local_hook=False, ): # pragma: windows no cover assert repo_cmd_runner.exists('Dockerfile'), ( 'No Dockerfile was found in the hook repository' diff --git a/pre_commit/languages/golang.py b/pre_commit/languages/golang.py index d5924536..459b453f 100644 --- a/pre_commit/languages/golang.py +++ b/pre_commit/languages/golang.py @@ -48,7 +48,7 @@ def install_environment( repo_cmd_runner, version='default', additional_dependencies=(), - is_local_hook = False, + is_local_hook=False, ): helpers.assert_version_default('golang', version) directory = repo_cmd_runner.path( diff --git a/pre_commit/languages/node.py b/pre_commit/languages/node.py index 6896b358..8297213d 100644 --- a/pre_commit/languages/node.py +++ b/pre_commit/languages/node.py @@ -37,7 +37,7 @@ def install_environment( repo_cmd_runner, version='default', additional_dependencies=(), - is_local_hook = False, + is_local_hook=False, ): # pragma: windows no cover additional_dependencies = tuple(additional_dependencies) assert repo_cmd_runner.exists('package.json') diff --git a/pre_commit/languages/pcre.py b/pre_commit/languages/pcre.py index 39741da0..b1b9d7cb 100644 --- a/pre_commit/languages/pcre.py +++ b/pre_commit/languages/pcre.py @@ -13,7 +13,7 @@ def install_environment( repo_cmd_runner, version='default', additional_dependencies=(), - is_local_hook = False, + is_local_hook=False, ): """Installation for pcre type is a noop.""" raise AssertionError('Cannot install pcre repo.') diff --git a/pre_commit/languages/python.py b/pre_commit/languages/python.py index d3215045..bfeb983d 100644 --- a/pre_commit/languages/python.py +++ b/pre_commit/languages/python.py @@ -58,7 +58,7 @@ def install_environment( repo_cmd_runner, version='default', additional_dependencies=(), - is_local_hook = False, + is_local_hook=False, ): additional_dependencies = tuple(additional_dependencies) directory = helpers.environment_dir(ENVIRONMENT_DIR, version) @@ -74,7 +74,7 @@ def install_environment( else: venv_cmd.extend(['-p', os.path.realpath(sys.executable)]) repo_cmd_runner.run(venv_cmd, cwd='/') - to_install = () if is_local_hook else ('.') + to_install = () if is_local_hook else ('.',) to_install += additional_dependencies with in_env(repo_cmd_runner, version): helpers.run_setup_cmd( diff --git a/pre_commit/languages/ruby.py b/pre_commit/languages/ruby.py index 3c2b36ab..54dafcdf 100644 --- a/pre_commit/languages/ruby.py +++ b/pre_commit/languages/ruby.py @@ -100,7 +100,7 @@ def install_environment( repo_cmd_runner, version='default', additional_dependencies=(), - is_local_hook = False, + is_local_hook=False, ): # pragma: windows no cover additional_dependencies = tuple(additional_dependencies) directory = helpers.environment_dir(ENVIRONMENT_DIR, version) diff --git a/pre_commit/languages/script.py b/pre_commit/languages/script.py index 151bf5df..2fef2704 100644 --- a/pre_commit/languages/script.py +++ b/pre_commit/languages/script.py @@ -11,7 +11,7 @@ def install_environment( repo_cmd_runner, version='default', additional_dependencies=(), - is_local_hook = False, + is_local_hook=False, ): """Installation for script type is a noop.""" raise AssertionError('Cannot install script repo.') diff --git a/pre_commit/languages/swift.py b/pre_commit/languages/swift.py index e418a5e6..eafa3a1c 100644 --- a/pre_commit/languages/swift.py +++ b/pre_commit/languages/swift.py @@ -32,7 +32,7 @@ def install_environment( repo_cmd_runner, version='default', additional_dependencies=(), - is_local_hook = False, + is_local_hook=False, ): # pragma: windows no cover helpers.assert_version_default('swift', version) helpers.assert_no_additional_deps('swift', additional_dependencies) diff --git a/pre_commit/languages/system.py b/pre_commit/languages/system.py index 424ba4b5..c698ad51 100644 --- a/pre_commit/languages/system.py +++ b/pre_commit/languages/system.py @@ -11,7 +11,7 @@ def install_environment( repo_cmd_runner, version='default', additional_dependencies=(), - is_local_hook = False, + is_local_hook=False, ): """Installation for system type is a noop.""" raise AssertionError('Cannot install system repo.') diff --git a/pre_commit/repository.py b/pre_commit/repository.py index 56a53c86..db384c14 100644 --- a/pre_commit/repository.py +++ b/pre_commit/repository.py @@ -11,7 +11,6 @@ import pkg_resources from cached_property import cached_property from pre_commit import five -from pre_commit import git from pre_commit.clientlib.validate_config import is_local_hooks from pre_commit.clientlib.validate_manifest import MANIFEST_JSON_SCHEMA from pre_commit.jsonschema_extensions import apply_defaults @@ -38,13 +37,13 @@ class Repository(object): self.__installed = False @classmethod - def create(cls, config, store): + def create(cls, config, store, owner): + repo_path_getter = store.get_repo_path_getter( + config['repo'], config['sha'], owner + ) if is_local_hooks(config): - return LocalRepository(config) + return LocalRepository(config, repo_path_getter) else: - repo_path_getter = store.get_repo_path_getter( - config['repo'], config['sha'] - ) return cls(config, repo_path_getter) @cached_property @@ -198,9 +197,6 @@ class Repository(object): class LocalRepository(Repository): - def __init__(self, repo_config): - super(LocalRepository, self).__init__(repo_config, None) - @cached_property def hooks(self): return tuple( @@ -208,14 +204,6 @@ class LocalRepository(Repository): for hook in self.repo_config['hooks'] ) - @cached_property - def cmd_runner(self): - return PrefixedCommandRunner(git.get_root()) - - @cached_property - def manifest(self): - raise NotImplementedError - class _UniqueList(list): def __init__(self): diff --git a/pre_commit/runner.py b/pre_commit/runner.py index 985e6456..53545812 100644 --- a/pre_commit/runner.py +++ b/pre_commit/runner.py @@ -41,7 +41,9 @@ class Runner(object): def repositories(self): """Returns a tuple of the configured repositories.""" config = load_config(self.config_file_path) - repositories = tuple(Repository.create(x, self.store) for x in config) + repositories = tuple( + Repository.create(x, self.store, self.git_root) for x in config + ) for repository in repositories: repository.require_installed() return repositories diff --git a/pre_commit/store.py b/pre_commit/store.py index aecc7dc1..ec948310 100644 --- a/pre_commit/store.py +++ b/pre_commit/store.py @@ -9,6 +9,7 @@ import tempfile from cached_property import cached_property +from pre_commit.clientlib.validate_config import _LOCAL_HOOKS_MAGIC_REPO_STRING from pre_commit.prefixed_command_runner import PrefixedCommandRunner from pre_commit.util import clean_path_on_failure from pre_commit.util import cmd_output @@ -45,6 +46,17 @@ class Store(object): def repo_path(self): return self._store.clone(self._repo, self._ref) + class LocalRepoPathGetter(RepoPathGetter): + def __init__(self, repo, sha, store, owner): + super(Store.LocalRepoPathGetter, self).__init__( + repo, sha, store, + ) + self._owner = owner + + @cached_property + def repo_path(self): + return self._store.initialize_local_repo(self._owner) + def __init__(self, directory=None): if directory is None: directory = self.get_default_directory() @@ -97,19 +109,31 @@ class Store(object): self._create() self.__created = True - def clone(self, url, ref): - """Clone the given url and checkout the specific ref.""" - self.require_created() - - # Check if we already exist + def find_local_path(self, repo, ref): 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] + def store_path(self, repo, ref, path): + with sqlite3.connect(self.db_path) as db: + db.execute( + 'INSERT INTO repos (repo, ref, path) VALUES (?, ?, ?)', + [repo, ref, path], + ) + + def clone(self, url, sha): + """Clone the given url and checkout the specific sha.""" + self.require_created() + + # Check if we already exist + local_path = self.find_local_path(url, sha) + if local_path: + return local_path + logger.info('Initializing environment for {}.'.format(url)) dir = tempfile.mkdtemp(prefix='repo', dir=self.directory) @@ -121,15 +145,33 @@ class Store(object): cmd_output('git', 'reset', ref, '--hard', env=no_git_env()) # 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], - ) + self.store_path(url, sha, dir) return dir - def get_repo_path_getter(self, repo, ref): - return self.RepoPathGetter(repo, ref, self) + def initialize_local_repo(self, owner): + """Initializes a repo in the default repository for the local repo""" + self.require_created() + local_path = self.find_local_path( + _LOCAL_HOOKS_MAGIC_REPO_STRING, owner, + ) + if local_path: + return local_path + + logger.info('Initializing environment for {}.'.format( + _LOCAL_HOOKS_MAGIC_REPO_STRING, + )) + + dir = tempfile.mkdtemp(prefix='repo', dir=self.directory) + + # Update our db with the created repo + self.store_path(_LOCAL_HOOKS_MAGIC_REPO_STRING, owner, dir) + return dir + + def get_repo_path_getter(self, repo, sha, owner=None): + if sha == _LOCAL_HOOKS_MAGIC_REPO_STRING: + return self.LocalRepoPathGetter(repo, sha, self, owner) + else: + return self.RepoPathGetter(repo, sha, self) @cached_property def cmd_runner(self): diff --git a/tests/repository_test.py b/tests/repository_test.py index 663fca61..3ac4cf70 100644 --- a/tests/repository_test.py +++ b/tests/repository_test.py @@ -50,11 +50,12 @@ 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) + repo = Repository.create(config, store, path) 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) + print ret assert ret[0] == expected_return_code assert ret[1].replace(b'\r\n', b'\n') == expected @@ -108,7 +109,7 @@ def test_switch_language_versions_doesnt_clobber(tempdir_factory, store): config = make_config_from_repo( path, hooks=[{'id': 'python3-hook', 'language_version': version}], ) - repo = Repository.create(config, store) + repo = Repository.create(config, store, path) hook_dict, = [ hook for repo_hook_id, hook in repo.hooks @@ -462,7 +463,7 @@ def test_repo_url(mock_repo_config): def test_languages(tempdir_factory, store): path = make_repo(tempdir_factory, 'python_hooks_repo') config = make_config_from_repo(path) - repo = Repository.create(config, store) + repo = Repository.create(config, store, path) assert repo.languages == {('python', 'default')} @@ -471,7 +472,7 @@ def test_additional_dependencies(tempdir_factory, store): path = make_repo(tempdir_factory, 'python_hooks_repo') config = make_config_from_repo(path) config['hooks'][0]['additional_dependencies'] = ['pep8'] - repo = Repository.create(config, store) + repo = Repository.create(config, store, path) assert repo.additional_dependencies['python']['default'] == ['pep8'] @@ -483,7 +484,7 @@ def test_additional_dependencies_duplicated( config = make_config_from_repo(path) config['hooks'][0]['additional_dependencies'] = [ 'thread_safe', 'tins', 'thread_safe'] - repo = Repository.create(config, store) + repo = Repository.create(config, store, path) assert repo.additional_dependencies['ruby']['default'] == [ 'thread_safe', 'tins'] @@ -493,7 +494,7 @@ def test_additional_python_dependencies_installed(tempdir_factory, store): path = make_repo(tempdir_factory, 'python_hooks_repo') config = make_config_from_repo(path) config['hooks'][0]['additional_dependencies'] = ['mccabe'] - repo = Repository.create(config, store) + repo = Repository.create(config, store, path) repo.require_installed() with python.in_env(repo.cmd_runner, 'default'): output = cmd_output('pip', 'freeze', '-l')[1] @@ -505,11 +506,11 @@ def test_additional_dependencies_roll_forward(tempdir_factory, store): path = make_repo(tempdir_factory, 'python_hooks_repo') config = make_config_from_repo(path) # Run the repo once without additional_dependencies - repo = Repository.create(config, store) + repo = Repository.create(config, store, path) repo.require_installed() # Now run it with additional_dependencies config['hooks'][0]['additional_dependencies'] = ['mccabe'] - repo = Repository.create(config, store) + repo = Repository.create(config, store, path) repo.require_installed() # We should see our additional dependency installed with python.in_env(repo.cmd_runner, 'default'): @@ -526,7 +527,7 @@ def test_additional_ruby_dependencies_installed( path = make_repo(tempdir_factory, 'ruby_hooks_repo') config = make_config_from_repo(path) config['hooks'][0]['additional_dependencies'] = ['thread_safe', 'tins'] - repo = Repository.create(config, store) + repo = Repository.create(config, store, path) repo.require_installed() with ruby.in_env(repo.cmd_runner, 'default'): output = cmd_output('gem', 'list', '--local')[1] @@ -544,7 +545,7 @@ def test_additional_node_dependencies_installed( config = make_config_from_repo(path) # Careful to choose a small package that's not depped by npm config['hooks'][0]['additional_dependencies'] = ['lodash'] - repo = Repository.create(config, store) + repo = Repository.create(config, store, path) repo.require_installed() with node.in_env(repo.cmd_runner, 'default'): cmd_output('npm', 'config', 'set', 'global', 'true') @@ -561,7 +562,7 @@ def test_additional_golang_dependencies_installed( # A small go package deps = ['github.com/golang/example/hello'] config['hooks'][0]['additional_dependencies'] = deps - repo = Repository.create(config, store) + repo = Repository.create(config, store, path) repo.require_installed() binaries = os.listdir(repo.cmd_runner.path( helpers.environment_dir(golang.ENVIRONMENT_DIR, 'default'), 'bin', @@ -579,7 +580,8 @@ def test_install_local_ruby_hook( ): # pragma: no cover (non-windows) config = config_with_local_hooks('ruby') config['hooks'][0]['additional_dependencies'] = ['thread_safe'] - repo = Repository.create(config, store) + validate_config_extra([config]) + repo = Repository.create(config, store, '/path/to/repo/') repo.require_installed() with ruby.in_env(repo.cmd_runner, 'default'): output = cmd_output('gem', 'list', '--local')[1] @@ -592,7 +594,8 @@ def test_install_local_python_hook( ): # pragma: no cover (non-windows) config = config_with_local_hooks('python') config['hooks'][0]['additional_dependencies'] = ['mccabe'] - repo = Repository.create(config, store) + validate_config_extra([config]) + repo = Repository.create(config, store, '/path/to/repo/') repo.require_installed() with python.in_env(repo.cmd_runner, 'default'): output = cmd_output('pip', 'freeze', '-l')[1] @@ -602,7 +605,7 @@ def test_install_local_python_hook( def test_reinstall(tempdir_factory, store, log_info_mock): path = make_repo(tempdir_factory, 'python_hooks_repo') config = make_config_from_repo(path) - repo = Repository.create(config, store) + repo = Repository.create(config, store, path) repo.require_installed() # We print some logging during clone (1) + install (3) assert log_info_mock.call_count == 4 @@ -611,7 +614,7 @@ def test_reinstall(tempdir_factory, store, log_info_mock): repo.require_installed() assert log_info_mock.call_count == 0 # Reinstall on another run should not trigger another install - repo = Repository.create(config, store) + repo = Repository.create(config, store, path) repo.require_installed() assert log_info_mock.call_count == 0 @@ -620,7 +623,7 @@ def test_control_c_control_c_on_install(tempdir_factory, store): """Regression test for #186.""" path = make_repo(tempdir_factory, 'python_hooks_repo') config = make_config_from_repo(path) - repo = Repository.create(config, store) + repo = Repository.create(config, store, path) hook = repo.hooks[0][1] class MyKeyboardInterrupt(KeyboardInterrupt): @@ -657,7 +660,7 @@ def test_really_long_file_paths(tempdir_factory, store): config = make_config_from_repo(path) with cwd(really_long_path): - repo = Repository.create(config, store) + repo = Repository.create(config, store, path) repo.require_installed() @@ -666,11 +669,11 @@ def test_config_overrides_repo_specifics(tempdir_factory, store): path = make_repo(tempdir_factory, 'script_hooks_repo') config = make_config_from_repo(path) - repo = Repository.create(config, store) + repo = Repository.create(config, store, path) assert repo.hooks[0][1]['files'] == '' # Set the file regex to something else config['hooks'][0]['files'] = '\\.sh$' - repo = Repository.create(config, store) + repo = Repository.create(config, store, path) assert repo.hooks[0][1]['files'] == '\\.sh$' @@ -690,28 +693,20 @@ def test_tags_on_repositories(in_tmpdir, tempdir_factory, store): ) repo_1 = Repository.create( - make_config_from_repo(git_dir_1, sha=tag), store, + make_config_from_repo(git_dir_1, sha=tag), store, git_dir_1 ) ret = repo_1.run_hook(repo_1.hooks[0][1], ['-L']) assert ret[0] == 0 assert ret[1].strip() == _norm_pwd(in_tmpdir) repo_2 = Repository.create( - make_config_from_repo(git_dir_2, sha=tag), store, + make_config_from_repo(git_dir_2, sha=tag), store, git_dir_2 ) ret = repo_2.run_hook(repo_2.hooks[0][1], ['bar']) assert ret[0] == 0 assert ret[1] == b'bar\nHello World\n' -def test_local_repository(): - config = config_with_local_hooks() - local_repo = Repository.create(config, 'dummy') - with pytest.raises(NotImplementedError): - local_repo.manifest - assert len(local_repo.hooks) == 1 - - @pytest.yield_fixture def fake_log_handler(): handler = mock.Mock(level=logging.INFO) @@ -725,7 +720,7 @@ def test_hook_id_not_present(tempdir_factory, store, fake_log_handler): path = make_repo(tempdir_factory, 'script_hooks_repo') config = make_config_from_repo(path) config['hooks'][0]['id'] = 'i-dont-exist' - repo = Repository.create(config, store) + repo = Repository.create(config, store, path) with pytest.raises(SystemExit): repo.install() assert fake_log_handler.handle.call_args[0][0].msg == ( @@ -740,7 +735,7 @@ def test_too_new_version(tempdir_factory, store, fake_log_handler): with modify_manifest(path) as manifest: manifest[0]['minimum_pre_commit_version'] = '999.0.0' config = make_config_from_repo(path) - repo = Repository.create(config, store) + repo = Repository.create(config, store, path) with pytest.raises(SystemExit): repo.install() msg = fake_log_handler.handle.call_args[0][0].msg @@ -762,4 +757,4 @@ def test_versions_ok(tempdir_factory, store, version): manifest[0]['minimum_pre_commit_version'] = version config = make_config_from_repo(path) # Should succeed - Repository.create(config, store).install() + Repository.create(config, store, path).install()