Resolves cwd problem

.travis.yml

@@ -5,7 +5,7 @@ python:
     - 2.7
 
 install: pip install virtualenv
-script: make
+script: make coverage
 
 
 before_install:

pre_commit/languages/all.py

@@ -5,14 +5,22 @@ from pre_commit.languages import ruby
 
 # A language implements the following two functions in its module:
 #
-# def install_environment():
+# def install_environment(repo_cmd_runner):
 #     """Installs a repository in the given repository.  Note that the current
 #     working directory will already be inside the repository.
+#
+#     Args:
+#         repo_cmd_runner - `PrefixedCommandRunner` bound to the repository.
 #     """
 #
-# def run_hook(hook, file_args):
+# def run_hook(repo_cmd_runner, hook, file_args):
 #     """Runs a hook and returns the returncode and output of running that hook.
 #
+#     Args:
+#         repo_cmd_runner - `PrefixedCommandRunner` bound to the repository.
+#         hook - Hook dictionary
+#         file_args - The files to be run
+#
 #     Returns:
 #         (returncode, stdout, stderr)
 #     """

pre_commit/languages/helpers.py

@@ -1,15 +1,17 @@
 
-import subprocess
-
 
 def run_hook(env, hook, file_args):
     return env.run(
         ' '.join(['xargs', hook['entry']] + hook.get('args', [])),
         stdin='\n'.join(list(file_args) + ['']),
+        retcode=None,
     )
 
 
 class Environment(object):
+    def __init__(self, repo_cmd_runner):
+        self.repo_cmd_runner = repo_cmd_runner
+
     @property
     def env_prefix(self):
         """env_prefix is a value that is prefixed to the command that is run.
@@ -24,14 +26,8 @@ class Environment(object):
         """
         raise NotImplementedError
 
-    def run(self, cmd, stdin=None):
+    def run(self, cmd, **kwargs):
         """Returns (returncode, stdout, stderr)."""
-        proc = subprocess.Popen(
+        return self.repo_cmd_runner.run(
-            ['bash', '-c', ' '.join([self.env_prefix, cmd])],
+            ['bash', '-c', ' '.join([self.env_prefix, cmd])], **kwargs
-            stdin=subprocess.PIPE,
-            stdout=subprocess.PIPE,
-            stderr=subprocess.PIPE,
         )
-        stdout, stderr = proc.communicate(stdin)
-
-        return proc.returncode, stdout, stderr

pre_commit/languages/node.py

@@ -1,5 +1,5 @@
 import contextlib
-from plumbum import local
+import subprocess
 
 from pre_commit.languages import helpers
 from pre_commit.languages import python
@@ -12,37 +12,44 @@ class NodeEnv(python.PythonEnv):
     @property
     def env_prefix(self):
         base = super(NodeEnv, self).env_prefix
-        return ' '.join([base, '. {0}/bin/activate &&'.format(NODE_ENV)])
+        return ' '.join([
+            base,
+            '. {{prefix}}{0}/bin/activate &&'.format(NODE_ENV)]
+        )
 
 
 @contextlib.contextmanager
-def in_env():
+def in_env(repo_cmd_runner):
-    yield NodeEnv()
+    yield NodeEnv(repo_cmd_runner)
 
 
-def install_environment():
+def install_environment(repo_cmd_runner):
-    assert local.path('package.json').exists()
+    assert repo_cmd_runner.exists('package.json')
 
-    if local.path(NODE_ENV).exists():
+    # Return immediately if we already have a virtualenv
+    if repo_cmd_runner.exists(NODE_ENV):
         return
 
-    local['virtualenv'][python.PY_ENV]()
+    repo_cmd_runner.run(['virtualenv', '{{prefix}}{0}'.format(python.PY_ENV)])
 
-    with python.in_env() as python_env:
+    with python.in_env(repo_cmd_runner) as python_env:
         python_env.run('pip install nodeenv')
 
         # Try and use the system level node executable first
-        retcode, _, _ = python_env.run('nodeenv -n system {0}'.format(NODE_ENV))
+        try:
-        # TODO: log failure here
+            python_env.run('nodeenv -n system {{prefix}}{0}'.format(NODE_ENV))
-        # cleanup
+        except subprocess.CalledProcessError:
-        if retcode:
+            # TODO: log failure here
-            local.path(NODE_ENV).delete()
+            # cleanup
-            python_env.run('nodeenv --jobs 4 {0}'.format(NODE_ENV))
+            # TODO: local.path(NODE_ENV).delete()
+            python_env.run('nodeenv --jobs 4 {{prefix}}{0}'.format(NODE_ENV))
 
-        with in_env() as node_env:
+        with in_env(repo_cmd_runner) as node_env:
-            node_env.run('npm install -g')
+            node_env.run(
+                'cd {0} && npm install -g'.format(repo_cmd_runner.prefix_dir),
+            )
 
 
-def run_hook(hook, file_args):
+def run_hook(repo_cmd_runner, hook, file_args):
-    with in_env() as node_env:
+    with in_env(repo_cmd_runner) as env:
-        return helpers.run_hook(node_env, hook, file_args)
+        return helpers.run_hook(env, hook, file_args)

pre_commit/languages/python.py

@@ -1,34 +1,35 @@
 
 import contextlib
-from plumbum import local
+
 from pre_commit.languages import helpers
 
+
 PY_ENV = 'py_env'
 
 
 class PythonEnv(helpers.Environment):
     @property
     def env_prefix(self):
-        return '. {0}/bin/activate &&'.format(PY_ENV)
+        return '. {{prefix}}{0}/bin/activate &&'.format(PY_ENV)
 
 
 @contextlib.contextmanager
-def in_env():
+def in_env(repo_cmd_runner):
-    yield PythonEnv()
+    yield PythonEnv(repo_cmd_runner)
 
 
-def install_environment():
+def install_environment(repo_cmd_runner):
-    assert local.path('setup.py').exists()
+    assert repo_cmd_runner.exists('setup.py')
     # Return immediately if we already have a virtualenv
-    if local.path(PY_ENV).exists():
+    if repo_cmd_runner.exists(PY_ENV):
         return
 
     # Install a virtualenv
-    local['virtualenv'][PY_ENV]()
+    repo_cmd_runner.run(['virtualenv', '{{prefix}}{0}'.format(PY_ENV)])
-    with in_env() as env:
+    with in_env(repo_cmd_runner) as env:
-        env.run('pip install .')
+        env.run('cd {0} && pip install .'.format(repo_cmd_runner.prefix_dir))
 
 
-def run_hook(hook, file_args):
+def run_hook(repo_cmd_runner, hook, file_args):
-    with in_env() as env:
+    with in_env(repo_cmd_runner) as env:
         return helpers.run_hook(env, hook, file_args)

pre_commit/languages/ruby.py

@@ -1,6 +1,5 @@
 
 import contextlib
-from plumbum import local
 
 from pre_commit.languages import helpers
 
@@ -8,29 +7,27 @@ from pre_commit.languages import helpers
 RVM_ENV = 'rvm_env'
 
 
-class RubyEnv(object):
+class RubyEnv(helpers.Environment):
-    def __init__(self):
+    @property
-        self.env_prefix = '. {0}/.rvm/scripts/rvm'.format(RVM_ENV)
+    def env_prefix(self):
-
+        return '. {{prefix}}{0}/bin/activate &&'.format(RVM_ENV)
-    def run(self, cmd, **kwargs):
-        return local['bash']['-c', ' '.join([self.env_prefix, cmd])].run(**kwargs)
 
 
 @contextlib.contextmanager
-def in_env():
+def in_env(repo_cmd_runner):
-    yield RubyEnv()
+    yield RubyEnv(repo_cmd_runner)
 
 
-def install_environment():
+def install_environment(repo_cmd_runner):
     # Return immediately if we already have a virtualenv
-    if local.path(RVM_ENV).exists():
+    if repo_cmd_runner.exists(RVM_ENV):
         return
 
-    local['__rvm-env.sh'][RVM_ENV]()
+    repo_cmd_runner.run(['__rvm-env.sh', '{{prefix}}{0}'.format(RVM_ENV)])
-    with in_env() as env:
+    with in_env(repo_cmd_runner) as env:
-        env.run('bundle install')
+        env.run('bundle install', cwd=repo_cmd_runner.prefix_dir)
 
 
-def run_hook(hook, file_args):
+def run_hook(repo_cmd_runner, hook, file_args):
-    with in_env() as env:
+    with in_env(repo_cmd_runner) as env:
         return helpers.run_hook(env, hook, file_args)

pre_commit/prefixed_command_runner.py

@@ -17,26 +17,42 @@ class PrefixedCommandRunner(object):
 
     will run ['/tmpl/foo/foo.sh', 'bar', 'baz']
     """
-    def __init__(self, prefix_dir, popen=subprocess.Popen):
+    def __init__(self, prefix_dir, popen=subprocess.Popen, makedirs=os.makedirs):
         self.prefix_dir = prefix_dir.rstrip(os.sep) + os.sep
         self.__popen = popen
+        self.__makedirs = makedirs
 
-    def run(self, cmd, stdin=None):
+    def _create_path_if_not_exists(self):
+        if not os.path.exists(self.prefix_dir):
+            self.__makedirs(self.prefix_dir)
+
+    def run(self, cmd, retcode=0, stdin=None, **kwargs):
+        self._create_path_if_not_exists()
         replaced_cmd = _replace_cmd(cmd, prefix=self.prefix_dir)
         proc = self.__popen(
             replaced_cmd,
             stdin=subprocess.PIPE,
             stdout=subprocess.PIPE,
             stderr=subprocess.PIPE,
+            **kwargs
         )
         stdout, stderr = proc.communicate(stdin)
+        returncode = proc.returncode
+
+        if retcode is not None and retcode != returncode:
+            raise subprocess.CalledProcessError(
+                returncode, replaced_cmd, output=(stdout, stderr),
+            )
 
         return proc.returncode, stdout, stderr
 
-    def path(self, path_end):
+    def path(self, *parts):
-        path = os.path.join(self.prefix_dir, path_end)
+        path = os.path.join(self.prefix_dir, *parts)
         return os.path.normpath(path)
 
+    def exists(self, *parts):
+        return os.path.exists(self.path(*parts))
+
     @classmethod
     def from_command_runner(cls, command_runner, path_end):
         """Constructs a new command runner from an existing one by appending

pre_commit/repository.py

@@ -7,12 +7,15 @@ from pre_commit.clientlib.validate_manifest import load_manifest
 from pre_commit.hooks_workspace import in_hooks_workspace
 from pre_commit.languages.all import languages
 from pre_commit.ordereddict import OrderedDict
+from pre_commit.prefixed_command_runner import PrefixedCommandRunner
 from pre_commit.util import cached_property
 
 
 class Repository(object):
     def __init__(self, repo_config):
         self.repo_config = repo_config
+        self.__created = False
+        self.__installed = False
 
     @cached_property
     def repo_url(self):
@@ -43,13 +46,17 @@ class Repository(object):
                 for hook in load_manifest(C.MANIFEST_FILE)
             )
 
-    @contextlib.contextmanager
+    def get_cmd_runner(self, hooks_cmd_runner):
-    def in_checkout(self):
+        return PrefixedCommandRunner.from_command_runner(
-        with in_hooks_workspace():
+            hooks_cmd_runner, self.sha,
-            # SMELL:
+        )
-            self.create()
+
-            with local.cwd(self.sha):
+    def require_created(self):
-                yield
+        if self.__created:
+            return
+
+        self.create()
+        self.__created = True
 
     def create(self):
         with in_hooks_workspace():
@@ -61,13 +68,42 @@ class Repository(object):
             with self.in_checkout():
                 local['git']['checkout', self.sha]()
 
-    def install(self):
+    def require_installed(self, cmd_runner):
-        with self.in_checkout():
+        if self.__installed:
-            for language in C.SUPPORTED_LANGUAGES:
+            return
-                if language in self.languages:
-                    languages[language].install_environment()
 
-    def run_hook(self, hook_id, file_args):
+        self.install(cmd_runner)
-        with self.in_checkout():
+
-            hook = self.hooks[hook_id]
+    def install(self, cmd_runner):
-            return languages[hook['language']].run_hook(hook, file_args)
+        """Install the hook repository.
+
+        Args:
+            cmd_runner - A `PrefixedCommandRunner` bound to the hooks workspace
+        """
+        self.require_created()
+        repo_cmd_runner = self.get_cmd_runner(cmd_runner)
+        for language in C.SUPPORTED_LANGUAGES:
+            if language in self.languages:
+                languages[language].install_environment(repo_cmd_runner)
+
+    @contextlib.contextmanager
+    def in_checkout(self):
+        self.require_created()
+        with in_hooks_workspace():
+            with local.cwd(self.sha):
+                yield
+
+    def run_hook(self, cmd_runner, 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)
+        hook = self.hooks[hook_id]
+        return languages[hook['language']].run_hook(
+            repo_cmd_runner, hook, file_args,
+        )

pre_commit/run.py

@@ -18,9 +18,7 @@ COLS = int(subprocess.Popen(['tput', 'cols'], stdout=subprocess.PIPE).communicat
 PASS_FAIL_LENGTH = 6
 
 
-def _run_single_hook(repository, hook_id, all_files=False):
+def _run_single_hook(runner, repository, hook_id, all_files=False):
-    repository.install()
-
     if all_files:
         get_filenames = git.get_all_files_matching
     else:
@@ -36,6 +34,7 @@ def _run_single_hook(repository, hook_id, all_files=False):
     ),
 
     retcode, stdout, stderr = repository.run_hook(
+        runner.cmd_runner,
         hook_id,
         map(os.path.abspath, get_filenames(hook['files'])),
     )
@@ -69,6 +68,7 @@ def run_hooks(runner, all_files=False):
     for repo in runner.repositories:
         for hook_id in repo.hooks:
             retval |= _run_single_hook(
+                runner,
                 repo,
                 hook_id,
                 all_files=all_files,
@@ -81,6 +81,7 @@ def run_single_hook(runner, hook_id, all_files=False):
     for repo in runner.repositories:
         if hook_id in repo.hooks:
             return _run_single_hook(
+                runner,
                 repo,
                 hook_id,
                 all_files=all_files,

pre_commit/runner.py

@@ -47,5 +47,5 @@ class Runner(object):
         return os.path.join(self.git_root, '.git/hooks/pre-commit')
 
     @cached_property
-    def workspace_runner(self):
+    def cmd_runner(self):
-        return PrefixedCommandRunner(C.HOOKS_WORKSPACE)
+        return PrefixedCommandRunner(self.hooks_workspace_path)

tests/prefixed_command_runner_test.py

@@ -3,6 +3,7 @@ import os
 import mock
 import pytest
 import subprocess
+from plumbum import local
 
 from pre_commit.prefixed_command_runner import _replace_cmd
 from pre_commit.prefixed_command_runner import PrefixedCommandRunner
@@ -10,11 +11,16 @@ from pre_commit.prefixed_command_runner import PrefixedCommandRunner
 
 @pytest.fixture
 def popen_mock():
-    popen = mock.Mock()
+    popen = mock.Mock(spec=subprocess.Popen)
     popen.return_value.communicate.return_value = (mock.Mock(), mock.Mock())
     return popen
 
 
+@pytest.fixture
+def makedirs_mock():
+    return mock.Mock(spec=os.makedirs)
+
+
 @pytest.mark.parametrize(('input', 'kwargs', 'expected_output'), (
     ([], {}, []),
     (['foo'], {}, ['foo']),
@@ -40,9 +46,9 @@ def test_init_normalizes_path_endings(input, expected_prefix):
     assert instance.prefix_dir == expected_prefix
 
 
-def test_run_substitutes_prefix(popen_mock):
+def test_run_substitutes_prefix(popen_mock, makedirs_mock):
-    instance = PrefixedCommandRunner('prefix', popen=popen_mock)
+    instance = PrefixedCommandRunner('prefix', popen=popen_mock, makedirs=makedirs_mock)
-    ret = instance.run(['{prefix}bar', 'baz'])
+    ret = instance.run(['{prefix}bar', 'baz'], retcode=None)
     popen_mock.assert_called_once_with(
         ['prefix/bar', 'baz'],
         stdin=subprocess.PIPE,
@@ -72,6 +78,12 @@ def test_path(prefix, path_end, expected_output):
     assert ret == expected_output
 
 
+def test_path_multiple_args():
+    instance = PrefixedCommandRunner('foo')
+    ret = instance.path('bar', 'baz')
+    assert ret == 'foo/bar/baz'
+
+
 @pytest.mark.parametrize(('prefix', 'path_end', 'expected_output'),
     tuple(
         (prefix, path_end, expected_output + os.sep)
@@ -84,13 +96,41 @@ def test_from_command_runner(prefix, path_end, expected_output):
     assert second.prefix_dir == expected_output
 
 
-def test_from_command_runner_preserves_popen(popen_mock):
+def test_from_command_runner_preserves_popen(popen_mock, makedirs_mock):
-    first = PrefixedCommandRunner('foo', popen=popen_mock)
+    first = PrefixedCommandRunner('foo', popen=popen_mock, makedirs=makedirs_mock)
     second = PrefixedCommandRunner.from_command_runner(first, 'bar')
-    second.run(['foo/bar/baz'])
+    second.run(['foo/bar/baz'], retcode=None)
     popen_mock.assert_called_once_with(
         ['foo/bar/baz'],
         stdin=subprocess.PIPE,
         stdout=subprocess.PIPE,
         stderr=subprocess.PIPE,
     )
+
+
+def test_create_path_if_not_exists(tmpdir):
+    with local.cwd(tmpdir.strpath):
+        instance = PrefixedCommandRunner('foo')
+        assert not os.path.exists('foo')
+        instance._create_path_if_not_exists()
+        assert os.path.exists('foo')
+
+
+def test_exists_does_not_exist(tmpdir):
+    with local.cwd(tmpdir.strpath):
+        assert not PrefixedCommandRunner('.').exists('foo')
+
+
+def test_exists_does_exist(tmpdir):
+    with local.cwd(tmpdir.strpath):
+        os.mkdir('foo')
+        assert PrefixedCommandRunner('.').exists('foo')
+
+
+def test_raises_on_error(popen_mock, makedirs_mock):
+    popen_mock.return_value.returncode = 1
+    with pytest.raises(subprocess.CalledProcessError):
+        instance = PrefixedCommandRunner(
+            '.', popen=popen_mock, makedirs=makedirs_mock,
+        )
+        instance.run(['foo'])

tests/repository_test.py

@@ -6,6 +6,7 @@ import pre_commit.constants as C
 from pre_commit import git
 from pre_commit.clientlib.validate_config import CONFIG_JSON_SCHEMA
 from pre_commit.clientlib.validate_config import validate_config_extra
+from pre_commit.prefixed_command_runner import PrefixedCommandRunner
 from pre_commit.repository import Repository
 
 
@@ -31,7 +32,7 @@ def test_create_repo_in_env(dummy_repo_config, dummy_git_repo):
 @pytest.mark.integration
 def test_install_python_repo_in_env(config_for_python_hooks_repo):
     repo = Repository(config_for_python_hooks_repo)
-    repo.install()
+    repo.install(PrefixedCommandRunner(C.HOOKS_WORKSPACE))
 
     assert os.path.exists(
         os.path.join(
@@ -43,11 +44,13 @@ def test_install_python_repo_in_env(config_for_python_hooks_repo):
     )
 
 
+@pytest.mark.herpderp
 @pytest.mark.integration
 def test_run_a_python_hook(config_for_python_hooks_repo):
     repo = Repository(config_for_python_hooks_repo)
-    repo.install()
+    ret = repo.run_hook(
-    ret = repo.run_hook('foo', ['/dev/null'])
+        PrefixedCommandRunner(C.HOOKS_WORKSPACE), 'foo', ['/dev/null'],
+    )
 
     assert ret[0] == 0
     assert ret[1] == "['/dev/null']\nHello World\n"
@@ -56,18 +59,19 @@ def test_run_a_python_hook(config_for_python_hooks_repo):
 @pytest.mark.integration
 def test_run_a_hook_lots_of_files(config_for_python_hooks_repo):
     repo = Repository(config_for_python_hooks_repo)
-    repo.install()
+    ret = repo.run_hook(
-    ret = repo.run_hook('foo', ['/dev/null'] * 15000)
+        PrefixedCommandRunner(C.HOOKS_WORKSPACE), 'foo', ['/dev/null'] * 15000,
+    )
 
     assert ret[0] == 0
 
 
-@pytest.mark.xfail
 @pytest.mark.integration
 def test_cwd_of_hook(config_for_prints_cwd_repo):
     repo = Repository(config_for_prints_cwd_repo)
-    repo.install()
+    ret = repo.run_hook(
-    ret = repo.run_hook('prints_cwd', [])
+        PrefixedCommandRunner(C.HOOKS_WORKSPACE), 'prints_cwd', [],
+    )
 
     assert ret[0] == 0
     assert ret[1] == '{0}\n'.format(repo.repo_url)
@@ -80,8 +84,7 @@ def test_cwd_of_hook(config_for_prints_cwd_repo):
 @pytest.mark.integration
 def test_run_a_node_hook(config_for_node_hooks_repo):
     repo = Repository(config_for_node_hooks_repo)
-    repo.install()
+    ret = repo.run_hook(PrefixedCommandRunner(C.HOOKS_WORKSPACE), 'foo', [])
-    ret = repo.run_hook('foo', [])
 
     assert ret[0] == 0
     assert ret[1] == 'Hello World\n'

tests/runner_test.py

@@ -61,7 +61,7 @@ def test_pre_commit_path():
     assert runner.pre_commit_path == expected_path
 
 
-def test_workspace_runneR():
+def test_cmd_runner():
     runner = Runner('foo/bar')
-    ret = runner.workspace_runner
+    ret = runner.cmd_runner
-    assert ret.prefix_dir == C.HOOKS_WORKSPACE + '/'
+    assert ret.prefix_dir == os.path.join('foo/bar', C.HOOKS_WORKSPACE) + '/'