Merge pull request #92 from pre-commit/simplify_repo_cmd_runner

Simplify Repository.cmd_runner

pre_commit/commands.py

@@ -195,11 +195,7 @@ def _run_single_hook(runner, repository, hook_id, args, write, skips=set()):
     write(get_hook_message(_hook_msg_start(hook, args.verbose), end_len=6))
     sys.stdout.flush()
 
-    retcode, stdout, stderr = repository.run_hook(
-        runner.cmd_runner,
-        hook_id,
-        filenames,
-    )
+    retcode, stdout, stderr = repository.run_hook(hook_id, filenames)
 
     if retcode != repository.hooks[hook_id]['expected_return_value']:
         retcode = 1

pre_commit/prefixed_command_runner.py

@@ -40,7 +40,7 @@ class PrefixedCommandRunner(object):
     For instance:
         PrefixedCommandRunner('/tmp/foo').run(['{prefix}foo.sh', 'bar', 'baz'])
 
-    will run ['/tmpl/foo/foo.sh', 'bar', 'baz']
+    will run ['/tmp/foo/foo.sh', 'bar', 'baz']
     """
     def __init__(self, prefix_dir, popen=subprocess.Popen, makedirs=os.makedirs):
         self.prefix_dir = prefix_dir.rstrip(os.sep) + os.sep

pre_commit/repository.py

@@ -43,47 +43,38 @@ class Repository(object):
     def manifest(self):
         return Manifest(self.repo_path_getter)
 
-    def get_cmd_runner(self, hooks_cmd_runner):
-        # TODO: this effectively throws away the original cmd runner
-        return PrefixedCommandRunner.from_command_runner(
-            hooks_cmd_runner, self.repo_path_getter.repo_path,
-        )
+    @cached_property
+    def cmd_runner(self):
+        return PrefixedCommandRunner(self.repo_path_getter.repo_path)
 
-    def require_installed(self, cmd_runner):
+    def require_installed(self):
         if self.__installed:
             return
 
-        self.install(cmd_runner)
+        self.install()
         self.__installed = True
 
-    def install(self, cmd_runner):
-        """Install the hook repository.
-
-        Args:
-            cmd_runner - A `PrefixedCommandRunner` bound to the hooks workspace
-        """
-        repo_cmd_runner = self.get_cmd_runner(cmd_runner)
+    def install(self):
+        """Install the hook repository."""
         for language_name in self.languages:
             language = languages[language_name]
             if (
                 language.ENVIRONMENT_DIR is None or
-                repo_cmd_runner.exists(language.ENVIRONMENT_DIR)
+                self.cmd_runner.exists(language.ENVIRONMENT_DIR)
             ):
                 # The language is already installed
                 continue
-            language.install_environment(repo_cmd_runner)
+            language.install_environment(self.cmd_runner)
 
-    def run_hook(self, cmd_runner, hook_id, file_args):
+    def run_hook(self, hook_id, file_args):
         """Run a hook.
 
         Args:
-            cmd_runner - A `PrefixedCommandRunner` bound to the hooks workspace
             hook_id - Id of the hook
             file_args - List of files to run
         """
-        self.require_installed(cmd_runner)
-        repo_cmd_runner = self.get_cmd_runner(cmd_runner)
+        self.require_installed()
         hook = self.hooks[hook_id]
         return languages[hook['language']].run_hook(
-            repo_cmd_runner, hook, file_args,
+            self.cmd_runner, hook, file_args,
         )

tests/repository_test.py

@@ -5,23 +5,20 @@ from plumbum import local
 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.prefixed_command_runner import PrefixedCommandRunner
 from pre_commit.repository import Repository
 
 
 @pytest.mark.integration
 def test_install_python_repo_in_env(config_for_python_hooks_repo, store):
     repo = Repository.create(config_for_python_hooks_repo, store)
-    repo.install(PrefixedCommandRunner(store.directory))
+    repo.install()
     assert os.path.exists(os.path.join(store.directory, repo.sha, 'py_env'))
 
 
 @pytest.mark.integration
 def test_run_a_python_hook(config_for_python_hooks_repo, store):
     repo = Repository.create(config_for_python_hooks_repo, store)
-    ret = repo.run_hook(
-        PrefixedCommandRunner(store.directory), 'foo', ['/dev/null'],
-    )
+    ret = repo.run_hook('foo', ['/dev/null'])
 
     assert ret[0] == 0
     assert ret[1] == "['/dev/null']\nHello World\n"
@@ -30,9 +27,7 @@ def test_run_a_python_hook(config_for_python_hooks_repo, store):
 @pytest.mark.integration
 def test_lots_of_files(config_for_python_hooks_repo, store):
     repo = Repository.create(config_for_python_hooks_repo, store)
-    ret = repo.run_hook(
-        PrefixedCommandRunner(store.directory), 'foo', ['/dev/null'] * 15000,
-    )
+    ret = repo.run_hook('foo', ['/dev/null'] * 15000)
 
     assert ret[0] == 0
 
@@ -41,9 +36,7 @@ def test_lots_of_files(config_for_python_hooks_repo, store):
 def test_cwd_of_hook(config_for_prints_cwd_repo, store):
     # Note: this doubles as a test for `system` hooks
     repo = Repository.create(config_for_prints_cwd_repo, store)
-    ret = repo.run_hook(
-        PrefixedCommandRunner(store.directory), 'prints_cwd', [],
-    )
+    ret = repo.run_hook('prints_cwd', [])
 
     assert ret[0] == 0
     assert ret[1] == repo.repo_url + '\n'
@@ -56,7 +49,7 @@ def test_cwd_of_hook(config_for_prints_cwd_repo, store):
 @pytest.mark.integration
 def test_run_a_node_hook(config_for_node_hooks_repo, store):
     repo = Repository.create(config_for_node_hooks_repo, store)
-    ret = repo.run_hook(PrefixedCommandRunner(store.directory), 'foo', [])
+    ret = repo.run_hook('foo', [])
 
     assert ret[0] == 0
     assert ret[1] == 'Hello World\n'
@@ -65,9 +58,7 @@ def test_run_a_node_hook(config_for_node_hooks_repo, store):
 @pytest.mark.integration
 def test_run_a_script_hook(config_for_script_hooks_repo, store):
     repo = Repository.create(config_for_script_hooks_repo, store)
-    ret = repo.run_hook(
-        PrefixedCommandRunner(store.directory), 'bash_hook', ['bar'],
-    )
+    ret = repo.run_hook('bash_hook', ['bar'])
 
     assert ret[0] == 0
     assert ret[1] == 'bar\nHello World\n'
@@ -106,14 +97,14 @@ def test_languages(config_for_python_hooks_repo, store):
 
 def test_reinstall(config_for_python_hooks_repo, store):
     repo = Repository.create(config_for_python_hooks_repo, store)
-    repo.require_installed(PrefixedCommandRunner(store.directory))
+    repo.require_installed()
     # Reinstall with same repo should not trigger another install
     # TODO: how to assert this?
-    repo.require_installed(PrefixedCommandRunner(store.directory))
+    repo.require_installed()
     # Reinstall on another run should not trigger another install
     # TODO: how to assert this?
     repo = Repository.create(config_for_python_hooks_repo, store)
-    repo.require_installed(PrefixedCommandRunner(store.directory))
+    repo.require_installed()
 
 
 @pytest.mark.integration
@@ -122,4 +113,4 @@ def test_really_long_file_paths(config_for_python_hooks_repo, store):
     local['git']['init', path]()
     with local.cwd(path):
         repo = Repository.create(config_for_python_hooks_repo, store)
-        repo.require_installed(PrefixedCommandRunner(store.directory))
+        repo.require_installed()