Refactor Repository to be more functional

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):
+    def _cmd_runner(self):
-        return self.repo_config['repo']
+        return PrefixedCommandRunner(self.repo_path_getter.repo_path)
 
     @cached_property
-    def languages(self):
+    def _venvs(self):
-        return {
+        deps_dict = defaultdict(_UniqueList)
-            (hook['language'], hook['language_version'])
-            for _, hook in self.hooks
-        }
-
-    @cached_property
-    def additional_dependencies(self):
-        dep_dict = defaultdict(lambda: 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:
+        if not self.__installed:
-            return
+            _install_all(self._venvs, self.repo_config['repo'])
-
+            self.__installed = True
-        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)
 
     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,
         )
 
 

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'] = [
+    deps = ['thread_safe', 'tins', 'thread_safe']
-        'thread_safe', 'tins', 'thread_safe']
+    config['hooks'][0]['additional_dependencies'] = deps
     repo = Repository.create(config, store)
-    assert repo.additional_dependencies['ruby']['default'] == [
+    venv, = repo._venvs
-        'thread_safe', 'tins']
+    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()