From e704edb5e2365beb3d35d6d3a552cd42e3d22007 Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Wed, 15 Feb 2017 08:50:23 -0800 Subject: [PATCH] Refactor Repository to be more functional --- pre_commit/repository.py | 180 ++++++++++++++++++--------------------- tests/repository_test.py | 34 ++++---- 2 files changed, 99 insertions(+), 115 deletions(-) diff --git a/pre_commit/repository.py b/pre_commit/repository.py index 8ec6302b..4fc970b3 100644 --- a/pre_commit/repository.py +++ b/pre_commit/repository.py @@ -31,6 +31,70 @@ _pre_commit_version = pkg_resources.parse_version( INSTALLED_STATE_VERSION = '1' +def _state(additional_deps): + return {'additional_dependencies': sorted(additional_deps)} + + +def _state_filename(cmd_runner, venv): + return cmd_runner.path(venv, '.install_state_v' + INSTALLED_STATE_VERSION) + + +def _read_installed_state(cmd_runner, venv): + filename = _state_filename(cmd_runner, venv) + if not os.path.exists(filename): + return None + else: + return json.loads(io.open(filename).read()) + + +def _write_installed_state(cmd_runner, venv, state): + state_filename = _state_filename(cmd_runner, venv) + staging = state_filename + 'staging' + with io.open(staging, 'w') as state_file: + state_file.write(five.to_text(json.dumps(state))) + # Move the file into place atomically to indicate we've installed + os.rename(staging, state_filename) + + +def _installed(cmd_runner, language_name, language_version, additional_deps): + language = languages[language_name] + venv = environment_dir(language.ENVIRONMENT_DIR, language_version) + return ( + venv is None or + _read_installed_state(cmd_runner, venv) == _state(additional_deps) + ) + + +def _install_all(venvs, repo_url): + """Tuple of (cmd_runner, language, version, deps)""" + need_installed = tuple( + (cmd_runner, language_name, version, deps) + for cmd_runner, language_name, version, deps in venvs + if not _installed(cmd_runner, language_name, version, deps) + ) + + if need_installed: + logger.info( + 'Installing environment for {}.'.format(repo_url) + ) + logger.info('Once installed this environment will be reused.') + logger.info('This may take a few minutes...') + + for cmd_runner, language_name, version, deps in need_installed: + language = languages[language_name] + venv = environment_dir(language.ENVIRONMENT_DIR, version) + + # There's potentially incomplete cleanup from previous runs + # Clean it up! + if cmd_runner.exists(venv): + shutil.rmtree(cmd_runner.path(venv)) + + language.install_environment(cmd_runner, version, deps) + # Write our state to indicate we're installed + state = _state(deps) + _write_installed_state(cmd_runner, venv, state) + + class Repository(object): def __init__(self, repo_config, repo_path_getter): self.repo_config = repo_config @@ -48,24 +112,24 @@ class Repository(object): return cls(config, repo_path_getter) @cached_property - def repo_url(self): - return self.repo_config['repo'] + def _cmd_runner(self): + return PrefixedCommandRunner(self.repo_path_getter.repo_path) @cached_property - def languages(self): - return { - (hook['language'], hook['language_version']) - for _, hook in self.hooks - } - - @cached_property - def additional_dependencies(self): - dep_dict = defaultdict(lambda: defaultdict(_UniqueList)) + def _venvs(self): + deps_dict = defaultdict(_UniqueList) for _, hook in self.hooks: - dep_dict[hook['language']][hook['language_version']].update( + deps_dict[(hook['language'], hook['language_version'])].update( hook.get('additional_dependencies', []), ) - return dep_dict + ret = [] + for (language, version), deps in deps_dict.items(): + ret.append((self._cmd_runner, language, version, deps)) + return tuple(ret) + + @cached_property + def manifest(self): + return Manifest(self.repo_path_getter, self.repo_config['repo']) @cached_property def hooks(self): @@ -96,92 +160,10 @@ class Repository(object): for hook in self.repo_config['hooks'] ) - @cached_property - def manifest(self): - return Manifest(self.repo_path_getter, self.repo_url) - - @cached_property - def cmd_runner(self): - return PrefixedCommandRunner(self.repo_path_getter.repo_path) - def require_installed(self): - if self.__installed: - return - - self.install() - self.__installed = True - - def install(self): - """Install the hook repository.""" - def state(language_name, language_version): - return { - 'additional_dependencies': sorted( - self.additional_dependencies[ - language_name - ][language_version], - ) - } - - def state_filename(venv, suffix=''): - return self.cmd_runner.path( - venv, '.install_state_v' + INSTALLED_STATE_VERSION + suffix, - ) - - def read_state(venv): - if not os.path.exists(state_filename(venv)): - return None - else: - return json.loads(io.open(state_filename(venv)).read()) - - def write_state(venv, language_name, language_version): - with io.open( - state_filename(venv, suffix='staging'), 'w', - ) as state_file: - state_file.write(five.to_text(json.dumps( - state(language_name, language_version), - ))) - # Move the file into place atomically to indicate we've installed - os.rename( - state_filename(venv, suffix='staging'), - state_filename(venv), - ) - - def language_is_installed(language_name, language_version): - language = languages[language_name] - venv = environment_dir(language.ENVIRONMENT_DIR, language_version) - return ( - venv is None or - read_state(venv) == state(language_name, language_version) - ) - - if not all( - language_is_installed(language_name, language_version) - for language_name, language_version in self.languages - ): - logger.info( - 'Installing environment for {}.'.format(self.repo_url) - ) - logger.info('Once installed this environment will be reused.') - logger.info('This may take a few minutes...') - - for language_name, language_version in self.languages: - if language_is_installed(language_name, language_version): - continue - - language = languages[language_name] - venv = environment_dir(language.ENVIRONMENT_DIR, language_version) - - # There's potentially incomplete cleanup from previous runs - # Clean it up! - if self.cmd_runner.exists(venv): - shutil.rmtree(self.cmd_runner.path(venv)) - - language.install_environment( - self.cmd_runner, language_version, - self.additional_dependencies[language_name][language_version], - ) - # Write our state to indicate we're installed - write_state(venv, language_name, language_version) + if not self.__installed: + _install_all(self._venvs, self.repo_config['repo']) + self.__installed = True def run_hook(self, hook, file_args): """Run a hook. @@ -192,7 +174,7 @@ class Repository(object): """ self.require_installed() return languages[hook['language']].run_hook( - self.cmd_runner, hook, file_args, + self._cmd_runner, hook, file_args, ) diff --git a/tests/repository_test.py b/tests/repository_test.py index 984973f5..472407ab 100644 --- a/tests/repository_test.py +++ b/tests/repository_test.py @@ -459,11 +459,12 @@ def test_repo_url(mock_repo_config): @pytest.mark.integration -def test_languages(tempdir_factory, store): +def test_venvs(tempdir_factory, store): path = make_repo(tempdir_factory, 'python_hooks_repo') config = make_config_from_repo(path) repo = Repository.create(config, store) - assert repo.languages == {('python', 'default')} + venv, = repo._venvs + assert venv == (mock.ANY, 'python', 'default', []) @pytest.mark.integration @@ -472,7 +473,8 @@ def test_additional_dependencies(tempdir_factory, store): config = make_config_from_repo(path) config['hooks'][0]['additional_dependencies'] = ['pep8'] repo = Repository.create(config, store) - assert repo.additional_dependencies['python']['default'] == ['pep8'] + venv, = repo._venvs + assert venv == (mock.ANY, 'python', 'default', ['pep8']) @pytest.mark.integration @@ -481,11 +483,11 @@ def test_additional_dependencies_duplicated( ): path = make_repo(tempdir_factory, 'ruby_hooks_repo') config = make_config_from_repo(path) - config['hooks'][0]['additional_dependencies'] = [ - 'thread_safe', 'tins', 'thread_safe'] + deps = ['thread_safe', 'tins', 'thread_safe'] + config['hooks'][0]['additional_dependencies'] = deps repo = Repository.create(config, store) - assert repo.additional_dependencies['ruby']['default'] == [ - 'thread_safe', 'tins'] + venv, = repo._venvs + assert venv == (mock.ANY, 'ruby', 'default', ['thread_safe', 'tins']) @pytest.mark.integration @@ -495,7 +497,7 @@ def test_additional_python_dependencies_installed(tempdir_factory, store): config['hooks'][0]['additional_dependencies'] = ['mccabe'] repo = Repository.create(config, store) repo.require_installed() - with python.in_env(repo.cmd_runner, 'default'): + with python.in_env(repo._cmd_runner, 'default'): output = cmd_output('pip', 'freeze', '-l')[1] assert 'mccabe' in output @@ -512,7 +514,7 @@ def test_additional_dependencies_roll_forward(tempdir_factory, store): repo = Repository.create(config, store) repo.require_installed() # We should see our additional dependency installed - with python.in_env(repo.cmd_runner, 'default'): + with python.in_env(repo._cmd_runner, 'default'): output = cmd_output('pip', 'freeze', '-l')[1] assert 'mccabe' in output @@ -528,7 +530,7 @@ def test_additional_ruby_dependencies_installed( config['hooks'][0]['additional_dependencies'] = ['thread_safe', 'tins'] repo = Repository.create(config, store) repo.require_installed() - with ruby.in_env(repo.cmd_runner, 'default'): + with ruby.in_env(repo._cmd_runner, 'default'): output = cmd_output('gem', 'list', '--local')[1] assert 'thread_safe' in output assert 'tins' in output @@ -546,7 +548,7 @@ def test_additional_node_dependencies_installed( config['hooks'][0]['additional_dependencies'] = ['lodash'] repo = Repository.create(config, store) repo.require_installed() - with node.in_env(repo.cmd_runner, 'default'): + with node.in_env(repo._cmd_runner, 'default'): cmd_output('npm', 'config', 'set', 'global', 'true') output = cmd_output('npm', 'ls')[1] assert 'lodash' in output @@ -563,7 +565,7 @@ def test_additional_golang_dependencies_installed( config['hooks'][0]['additional_dependencies'] = deps repo = Repository.create(config, store) repo.require_installed() - binaries = os.listdir(repo.cmd_runner.path( + binaries = os.listdir(repo._cmd_runner.path( helpers.environment_dir(golang.ENVIRONMENT_DIR, 'default'), 'bin', )) # normalize for windows @@ -611,7 +613,7 @@ def test_control_c_control_c_on_install(tempdir_factory, store): repo.run_hook(hook, []) # Should have made an environment, however this environment is broken! - assert os.path.exists(repo.cmd_runner.path('py_env-default')) + assert os.path.exists(repo._cmd_runner.path('py_env-default')) # However, it should be perfectly runnable (reinstall after botched # install) @@ -699,7 +701,7 @@ def test_hook_id_not_present(tempdir_factory, store, fake_log_handler): config['hooks'][0]['id'] = 'i-dont-exist' repo = Repository.create(config, store) with pytest.raises(SystemExit): - repo.install() + repo.require_installed() assert fake_log_handler.handle.call_args[0][0].msg == ( '`i-dont-exist` is not present in repository {}. ' 'Typo? Perhaps it is introduced in a newer version? ' @@ -714,7 +716,7 @@ def test_too_new_version(tempdir_factory, store, fake_log_handler): config = make_config_from_repo(path) repo = Repository.create(config, store) with pytest.raises(SystemExit): - repo.install() + repo.require_installed() msg = fake_log_handler.handle.call_args[0][0].msg assert re.match( r'^The hook `bash_hook` requires pre-commit version 999\.0\.0 but ' @@ -734,4 +736,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).require_installed()