diff --git a/.travis.yml b/.travis.yml index 976fde03..f5f7a807 100644 --- a/.travis.yml +++ b/.travis.yml @@ -5,7 +5,7 @@ python: - 2.7 install: pip install virtualenv -script: make +script: make coverage before_install: diff --git a/pre_commit/languages/all.py b/pre_commit/languages/all.py index 84f30771..858af7e5 100644 --- a/pre_commit/languages/all.py +++ b/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) # """ diff --git a/pre_commit/languages/helpers.py b/pre_commit/languages/helpers.py index 1aaf80a8..4c05f3e7 100644 --- a/pre_commit/languages/helpers.py +++ b/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( - ['bash', '-c', ' '.join([self.env_prefix, cmd])], - stdin=subprocess.PIPE, - stdout=subprocess.PIPE, - stderr=subprocess.PIPE, + return self.repo_cmd_runner.run( + ['bash', '-c', ' '.join([self.env_prefix, cmd])], **kwargs ) - stdout, stderr = proc.communicate(stdin) - - return proc.returncode, stdout, stderr diff --git a/pre_commit/languages/node.py b/pre_commit/languages/node.py index 0776d75e..ec9da388 100644 --- a/pre_commit/languages/node.py +++ b/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(): - yield NodeEnv() +def in_env(repo_cmd_runner): + yield NodeEnv(repo_cmd_runner) -def install_environment(): - assert local.path('package.json').exists() +def install_environment(repo_cmd_runner): + 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)) - # TODO: log failure here - # cleanup - if retcode: - local.path(NODE_ENV).delete() - python_env.run('nodeenv --jobs 4 {0}'.format(NODE_ENV)) + try: + python_env.run('nodeenv -n system {{prefix}}{0}'.format(NODE_ENV)) + except subprocess.CalledProcessError: + # TODO: log failure here + # cleanup + # TODO: local.path(NODE_ENV).delete() + python_env.run('nodeenv --jobs 4 {{prefix}}{0}'.format(NODE_ENV)) - with in_env() as node_env: - node_env.run('npm install -g') + with in_env(repo_cmd_runner) as node_env: + node_env.run( + 'cd {0} && npm install -g'.format(repo_cmd_runner.prefix_dir), + ) -def run_hook(hook, file_args): - with in_env() as node_env: - return helpers.run_hook(node_env, hook, file_args) +def run_hook(repo_cmd_runner, hook, file_args): + with in_env(repo_cmd_runner) as env: + return helpers.run_hook(env, hook, file_args) diff --git a/pre_commit/languages/python.py b/pre_commit/languages/python.py index 6be6c93b..91d43b2e 100644 --- a/pre_commit/languages/python.py +++ b/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(): - yield PythonEnv() +def in_env(repo_cmd_runner): + yield PythonEnv(repo_cmd_runner) -def install_environment(): - assert local.path('setup.py').exists() +def install_environment(repo_cmd_runner): + 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]() - with in_env() as env: - env.run('pip install .') + repo_cmd_runner.run(['virtualenv', '{{prefix}}{0}'.format(PY_ENV)]) + with in_env(repo_cmd_runner) as env: + env.run('cd {0} && pip install .'.format(repo_cmd_runner.prefix_dir)) -def run_hook(hook, file_args): - with in_env() as env: +def run_hook(repo_cmd_runner, hook, file_args): + with in_env(repo_cmd_runner) as env: return helpers.run_hook(env, hook, file_args) diff --git a/pre_commit/languages/ruby.py b/pre_commit/languages/ruby.py index 219f74d7..fcd3ad42 100644 --- a/pre_commit/languages/ruby.py +++ b/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): - def __init__(self): - self.env_prefix = '. {0}/.rvm/scripts/rvm'.format(RVM_ENV) - - def run(self, cmd, **kwargs): - return local['bash']['-c', ' '.join([self.env_prefix, cmd])].run(**kwargs) +class RubyEnv(helpers.Environment): + @property + def env_prefix(self): + return '. {{prefix}}{0}/bin/activate &&'.format(RVM_ENV) @contextlib.contextmanager -def in_env(): - yield RubyEnv() +def in_env(repo_cmd_runner): + 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]() - with in_env() as env: - env.run('bundle install') + repo_cmd_runner.run(['__rvm-env.sh', '{{prefix}}{0}'.format(RVM_ENV)]) + with in_env(repo_cmd_runner) as env: + env.run('bundle install', cwd=repo_cmd_runner.prefix_dir) -def run_hook(hook, file_args): - with in_env() as env: +def run_hook(repo_cmd_runner, hook, file_args): + with in_env(repo_cmd_runner) as env: return helpers.run_hook(env, hook, file_args) diff --git a/pre_commit/prefixed_command_runner.py b/pre_commit/prefixed_command_runner.py index 0b3012c1..b064b1ef 100644 --- a/pre_commit/prefixed_command_runner.py +++ b/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): - path = os.path.join(self.prefix_dir, path_end) + def path(self, *parts): + 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 diff --git a/pre_commit/repository.py b/pre_commit/repository.py index abb9044b..7e710aaa 100644 --- a/pre_commit/repository.py +++ b/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 in_checkout(self): - with in_hooks_workspace(): - # SMELL: - self.create() - with local.cwd(self.sha): - yield + def get_cmd_runner(self, hooks_cmd_runner): + return PrefixedCommandRunner.from_command_runner( + hooks_cmd_runner, self.sha, + ) + + def require_created(self): + 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): - with self.in_checkout(): - for language in C.SUPPORTED_LANGUAGES: - if language in self.languages: - languages[language].install_environment() + def require_installed(self, cmd_runner): + if self.__installed: + return - def run_hook(self, hook_id, file_args): - with self.in_checkout(): - hook = self.hooks[hook_id] - return languages[hook['language']].run_hook(hook, file_args) + self.install(cmd_runner) + + def install(self, cmd_runner): + """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, + ) diff --git a/pre_commit/run.py b/pre_commit/run.py index f83d629f..1fd9d5c0 100644 --- a/pre_commit/run.py +++ b/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): - repository.install() - +def _run_single_hook(runner, repository, hook_id, all_files=False): 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, diff --git a/pre_commit/runner.py b/pre_commit/runner.py index b7263356..93507f22 100644 --- a/pre_commit/runner.py +++ b/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): - return PrefixedCommandRunner(C.HOOKS_WORKSPACE) + def cmd_runner(self): + return PrefixedCommandRunner(self.hooks_workspace_path) diff --git a/tests/prefixed_command_runner_test.py b/tests/prefixed_command_runner_test.py index 630d8a4a..22b9e6de 100644 --- a/tests/prefixed_command_runner_test.py +++ b/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): - instance = PrefixedCommandRunner('prefix', popen=popen_mock) - ret = instance.run(['{prefix}bar', 'baz']) +def test_run_substitutes_prefix(popen_mock, makedirs_mock): + instance = PrefixedCommandRunner('prefix', popen=popen_mock, makedirs=makedirs_mock) + 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): - first = PrefixedCommandRunner('foo', popen=popen_mock) +def test_from_command_runner_preserves_popen(popen_mock, makedirs_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']) diff --git a/tests/repository_test.py b/tests/repository_test.py index 35550b3b..cf68bf9c 100644 --- a/tests/repository_test.py +++ b/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('foo', ['/dev/null']) + ret = repo.run_hook( + 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('foo', ['/dev/null'] * 15000) + ret = repo.run_hook( + 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('prints_cwd', []) + ret = repo.run_hook( + 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('foo', []) + ret = repo.run_hook(PrefixedCommandRunner(C.HOOKS_WORKSPACE), 'foo', []) assert ret[0] == 0 assert ret[1] == 'Hello World\n' diff --git a/tests/runner_test.py b/tests/runner_test.py index beac918e..e6138df1 100644 --- a/tests/runner_test.py +++ b/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 - assert ret.prefix_dir == C.HOOKS_WORKSPACE + '/' + ret = runner.cmd_runner + assert ret.prefix_dir == os.path.join('foo/bar', C.HOOKS_WORKSPACE) + '/'