diff --git a/Makefile b/Makefile index 75415fc4..16868f1e 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,3 @@ - REBUILD_FLAG = .PHONY: all @@ -21,7 +20,7 @@ test: .venv.touch .PHONY: clean clean: - find . -iname '*.pyc' | xargs rm -f + find . -name '*.pyc' -delete rm -rf .tox rm -rf ./venv-* rm -f .venv.touch diff --git a/pre_commit/languages/all.py b/pre_commit/languages/all.py index 63c1d514..40c23131 100644 --- a/pre_commit/languages/all.py +++ b/pre_commit/languages/all.py @@ -15,7 +15,7 @@ from pre_commit.languages import system # def install_environment( # repo_cmd_runner, # version='default', -# additional_dependencies=None, +# additional_dependencies=(), # ): # """Installs a repository in the given repository. Note that the current # working directory will already be inside the repository. diff --git a/pre_commit/languages/helpers.py b/pre_commit/languages/helpers.py index b0add575..5887d3e2 100644 --- a/pre_commit/languages/helpers.py +++ b/pre_commit/languages/helpers.py @@ -1,6 +1,10 @@ from __future__ import unicode_literals -import pipes +from pre_commit.util import cmd_output + + +def run_setup_cmd(runner, cmd): + cmd_output(*cmd, cwd=runner.prefix_dir, encoding=None) def environment_dir(ENVIRONMENT_DIR, language_version): @@ -14,39 +18,12 @@ def file_args_to_stdin(file_args): return '\0'.join(list(file_args) + ['']) -def run_hook(env, hook, file_args): - quoted_args = [pipes.quote(arg) for arg in hook['args']] - return env.run( +def run_hook(cmd_args, file_args): + return cmd_output( # Use -s 4000 (slightly less than posix mandated minimum) # This is to prevent "xargs: ... Bad file number" on windows - ' '.join(['xargs', '-0', '-s4000', hook['entry']] + quoted_args), + 'xargs', '-0', '-s4000', *cmd_args, stdin=file_args_to_stdin(file_args), retcode=None, - encoding=None, + encoding=None ) - - -class Environment(object): - def __init__(self, repo_cmd_runner, language_version): - self.repo_cmd_runner = repo_cmd_runner - self.language_version = language_version - - @property - def env_prefix(self): - """env_prefix is a value that is prefixed to the command that is run. - - Usually this is to source a virtualenv, etc. - - Commands basically end up looking like: - - bash -c '{env_prefix} {cmd}' - - so you'll often want to end your prefix with && - """ - raise NotImplementedError - - def run(self, cmd, **kwargs): - """Returns (returncode, stdout, stderr).""" - return self.repo_cmd_runner.run( - ['bash', '-c', ' '.join([self.env_prefix, cmd])], **kwargs - ) diff --git a/pre_commit/languages/node.py b/pre_commit/languages/node.py index 99b46df7..1c1108ac 100644 --- a/pre_commit/languages/node.py +++ b/pre_commit/languages/node.py @@ -1,34 +1,44 @@ from __future__ import unicode_literals import contextlib +import os import sys +from pre_commit.envcontext import envcontext +from pre_commit.envcontext import Var from pre_commit.languages import helpers from pre_commit.util import clean_path_on_failure -from pre_commit.util import shell_escape ENVIRONMENT_DIR = 'node_env' -class NodeEnv(helpers.Environment): - @property - def env_prefix(self): - return ". '{{prefix}}{0}/bin/activate' &&".format( - helpers.environment_dir(ENVIRONMENT_DIR, self.language_version), - ) +def get_env_patch(venv): + return ( + ('NODE_VIRTUAL_ENV', venv), + ('NPM_CONFIG_PREFIX', venv), + ('npm_config_prefix', venv), + ('NODE_PATH', os.path.join(venv, 'lib', 'node_modules')), + ('PATH', (os.path.join(venv, 'bin'), os.pathsep, Var('PATH'))), + ) @contextlib.contextmanager def in_env(repo_cmd_runner, language_version): - yield NodeEnv(repo_cmd_runner, language_version) + envdir = os.path.join( + repo_cmd_runner.prefix_dir, + helpers.environment_dir(ENVIRONMENT_DIR, language_version), + ) + with envcontext(get_env_patch(envdir)): + yield def install_environment( repo_cmd_runner, version='default', - additional_dependencies=None, + additional_dependencies=(), ): + additional_dependencies = tuple(additional_dependencies) assert repo_cmd_runner.exists('package.json') directory = helpers.environment_dir(ENVIRONMENT_DIR, version) @@ -44,18 +54,15 @@ def install_environment( repo_cmd_runner.run(cmd) - with in_env(repo_cmd_runner, version) as node_env: - node_env.run("cd '{prefix}' && npm install -g", encoding=None) - if additional_dependencies: - node_env.run( - "cd '{prefix}' && npm install -g " + - ' '.join( - shell_escape(dep) for dep in additional_dependencies - ), - encoding=None, - ) + with in_env(repo_cmd_runner, version): + helpers.run_setup_cmd( + repo_cmd_runner, + ('npm', 'install', '-g', '.') + additional_dependencies, + ) def run_hook(repo_cmd_runner, hook, file_args): - with in_env(repo_cmd_runner, hook['language_version']) as env: - return helpers.run_hook(env, hook, file_args) + with in_env(repo_cmd_runner, hook['language_version']): + return helpers.run_hook( + (hook['entry'],) + tuple(hook['args']), file_args, + ) diff --git a/pre_commit/languages/pcre.py b/pre_commit/languages/pcre.py index 823e55cb..d8b7fd3e 100644 --- a/pre_commit/languages/pcre.py +++ b/pre_commit/languages/pcre.py @@ -2,7 +2,7 @@ from __future__ import unicode_literals from sys import platform -from pre_commit.languages.helpers import file_args_to_stdin +from pre_commit.languages import helpers from pre_commit.util import shell_escape @@ -12,29 +12,28 @@ ENVIRONMENT_DIR = None def install_environment( repo_cmd_runner, version='default', - additional_dependencies=None, + additional_dependencies=(), ): """Installation for pcre type is a noop.""" raise AssertionError('Cannot install pcre repo.') def run_hook(repo_cmd_runner, hook, file_args): - grep_command = 'grep -H -n -P' - if platform == 'darwin': # pragma: no cover (osx) - grep_command = 'ggrep -H -n -P' + grep_command = '{0} -H -n -P'.format( + 'ggrep' if platform == 'darwin' else 'grep', + ) # For PCRE the entry is the regular expression to match - return repo_cmd_runner.run( - [ - 'xargs', '-0', 'sh', '-c', + return helpers.run_hook( + ( + 'sh', '-c', # Grep usually returns 0 for matches, and nonzero for non-matches # so we flip it here. '! {0} {1} {2} $@'.format( grep_command, ' '.join(hook['args']), - shell_escape(hook['entry'])), + shell_escape(hook['entry']), + ), '--', - ], - stdin=file_args_to_stdin(file_args), - retcode=None, - encoding=None, + ), + file_args, ) diff --git a/pre_commit/languages/python.py b/pre_commit/languages/python.py index 4c463874..8bfc83b3 100644 --- a/pre_commit/languages/python.py +++ b/pre_commit/languages/python.py @@ -5,9 +5,11 @@ import distutils.spawn import os import sys +from pre_commit.envcontext import envcontext +from pre_commit.envcontext import UNSET +from pre_commit.envcontext import Var from pre_commit.languages import helpers from pre_commit.util import clean_path_on_failure -from pre_commit.util import shell_escape ENVIRONMENT_DIR = 'py_env' @@ -15,26 +17,26 @@ ENVIRONMENT_DIR = 'py_env' def bin_dir(venv): """On windows there's a different directory for the virtualenv""" - if os.name == 'nt': # pragma: no cover (windows) - return os.path.join(venv, 'Scripts') - else: - return os.path.join(venv, 'bin') + bin_part = 'Scripts' if os.name == 'nt' else 'bin' + return os.path.join(venv, bin_part) -class PythonEnv(helpers.Environment): - @property - def env_prefix(self): - return ". '{{prefix}}{0}{1}activate' &&".format( - bin_dir( - helpers.environment_dir(ENVIRONMENT_DIR, self.language_version) - ), - os.sep, - ) +def get_env_patch(venv): + return ( + ('PYTHONHOME', UNSET), + ('VIRTUAL_ENV', venv), + ('PATH', (bin_dir(venv), os.pathsep, Var('PATH'))), + ) @contextlib.contextmanager def in_env(repo_cmd_runner, language_version): - yield PythonEnv(repo_cmd_runner, language_version) + envdir = os.path.join( + repo_cmd_runner.prefix_dir, + helpers.environment_dir(ENVIRONMENT_DIR, language_version), + ) + with envcontext(get_env_patch(envdir)): + yield def norm_version(version): @@ -55,9 +57,9 @@ def norm_version(version): def install_environment( repo_cmd_runner, version='default', - additional_dependencies=None, + additional_dependencies=(), ): - assert repo_cmd_runner.exists('setup.py') + additional_dependencies = tuple(additional_dependencies) directory = helpers.environment_dir(ENVIRONMENT_DIR, version) # Install a virtualenv @@ -69,18 +71,15 @@ def install_environment( if version != 'default': venv_cmd.extend(['-p', norm_version(version)]) repo_cmd_runner.run(venv_cmd) - with in_env(repo_cmd_runner, version) as env: - env.run("cd '{prefix}' && pip install .", encoding=None) - if additional_dependencies: - env.run( - "cd '{prefix}' && pip install " + - ' '.join( - shell_escape(dep) for dep in additional_dependencies - ), - encoding=None, - ) + with in_env(repo_cmd_runner, version): + helpers.run_setup_cmd( + repo_cmd_runner, + ('pip', 'install', '.') + additional_dependencies, + ) def run_hook(repo_cmd_runner, hook, file_args): - with in_env(repo_cmd_runner, hook['language_version']) as env: - return helpers.run_hook(env, hook, file_args) + with in_env(repo_cmd_runner, hook['language_version']): + return helpers.run_hook( + (hook['entry'],) + tuple(hook['args']), file_args, + ) diff --git a/pre_commit/languages/ruby.py b/pre_commit/languages/ruby.py index a23df5d3..ed494c24 100644 --- a/pre_commit/languages/ruby.py +++ b/pre_commit/languages/ruby.py @@ -2,30 +2,42 @@ from __future__ import unicode_literals import contextlib import io +import os.path import shutil +from pre_commit.envcontext import envcontext +from pre_commit.envcontext import Var from pre_commit.languages import helpers from pre_commit.util import CalledProcessError from pre_commit.util import clean_path_on_failure from pre_commit.util import resource_filename -from pre_commit.util import shell_escape from pre_commit.util import tarfile_open ENVIRONMENT_DIR = 'rbenv' -class RubyEnv(helpers.Environment): - @property - def env_prefix(self): - return '. {{prefix}}{0}/bin/activate &&'.format( - helpers.environment_dir(ENVIRONMENT_DIR, self.language_version) - ) +def get_env_patch(venv, language_version): + return ( + ('GEM_HOME', os.path.join(venv, 'gems')), + ('RBENV_ROOT', venv), + ('RBENV_VERSION', language_version), + ('PATH', ( + os.path.join(venv, 'gems', 'bin'), os.pathsep, + os.path.join(venv, 'shims'), os.pathsep, + os.path.join(venv, 'bin'), os.pathsep, Var('PATH'), + )), + ) @contextlib.contextmanager def in_env(repo_cmd_runner, language_version): - yield RubyEnv(repo_cmd_runner, language_version) + envdir = os.path.join( + repo_cmd_runner.prefix_dir, + helpers.environment_dir(ENVIRONMENT_DIR, language_version), + ) + with envcontext(get_env_patch(envdir, language_version)): + yield def _install_rbenv(repo_cmd_runner, version='default'): @@ -71,42 +83,48 @@ def _install_rbenv(repo_cmd_runner, version='default'): activate_file.write('export RBENV_VERSION="{0}"\n'.format(version)) -def _install_ruby(environment, version): +def _install_ruby(runner, version): try: - environment.run('rbenv download {0}'.format(version)) + helpers.run_setup_cmd(runner, ('rbenv', 'download', version)) except CalledProcessError: # pragma: no cover (usually find with download) # Failed to download from mirror for some reason, build it instead - environment.run('rbenv install {0}'.format(version)) + helpers.run_setup_cmd(runner, ('rbenv', 'install', version)) def install_environment( repo_cmd_runner, version='default', - additional_dependencies=None, + additional_dependencies=(), ): + additional_dependencies = tuple(additional_dependencies) directory = helpers.environment_dir(ENVIRONMENT_DIR, version) with clean_path_on_failure(repo_cmd_runner.path(directory)): # TODO: this currently will fail if there's no version specified and # there's no system ruby installed. Is this ok? _install_rbenv(repo_cmd_runner, version=version) - with in_env(repo_cmd_runner, version) as ruby_env: + with in_env(repo_cmd_runner, version): + # Need to call this before installing so rbenv's directories are + # set up + helpers.run_setup_cmd(repo_cmd_runner, ('rbenv', 'init', '-')) if version != 'default': - _install_ruby(ruby_env, version) - ruby_env.run( - 'cd {prefix} && gem build *.gemspec && ' - 'gem install --no-ri --no-rdoc *.gem', - encoding=None, + _install_ruby(repo_cmd_runner, version) + # Need to call this after installing to set up the shims + helpers.run_setup_cmd(repo_cmd_runner, ('rbenv', 'rehash')) + helpers.run_setup_cmd( + repo_cmd_runner, + ('gem', 'build') + repo_cmd_runner.star('.gemspec'), + ) + helpers.run_setup_cmd( + repo_cmd_runner, + ( + ('gem', 'install', '--no-ri', '--no-rdoc') + + repo_cmd_runner.star('.gem') + additional_dependencies + ), ) - if additional_dependencies: - ruby_env.run( - 'cd {prefix} && gem install --no-ri --no-rdoc ' + - ' '.join( - shell_escape(dep) for dep in additional_dependencies - ), - encoding=None, - ) def run_hook(repo_cmd_runner, hook, file_args): - with in_env(repo_cmd_runner, hook['language_version']) as env: - return helpers.run_hook(env, hook, file_args) + with in_env(repo_cmd_runner, hook['language_version']): + return helpers.run_hook( + (hook['entry'],) + tuple(hook['args']), file_args, + ) diff --git a/pre_commit/languages/script.py b/pre_commit/languages/script.py index 5ba871ea..7c413c36 100644 --- a/pre_commit/languages/script.py +++ b/pre_commit/languages/script.py @@ -1,6 +1,6 @@ from __future__ import unicode_literals -from pre_commit.languages.helpers import file_args_to_stdin +from pre_commit.languages import helpers ENVIRONMENT_DIR = None @@ -9,16 +9,14 @@ ENVIRONMENT_DIR = None def install_environment( repo_cmd_runner, version='default', - additional_dependencies=None, + additional_dependencies=(), ): """Installation for script type is a noop.""" raise AssertionError('Cannot install script repo.') def run_hook(repo_cmd_runner, hook, file_args): - return repo_cmd_runner.run( - ['xargs', '-0', '{{prefix}}{0}'.format(hook['entry'])] + hook['args'], - stdin=file_args_to_stdin(file_args), - retcode=None, - encoding=None, + return helpers.run_hook( + (repo_cmd_runner.prefix_dir + hook['entry'],) + tuple(hook['args']), + file_args, ) diff --git a/pre_commit/languages/system.py b/pre_commit/languages/system.py index 3930422b..340f4feb 100644 --- a/pre_commit/languages/system.py +++ b/pre_commit/languages/system.py @@ -2,7 +2,7 @@ from __future__ import unicode_literals import shlex -from pre_commit.languages.helpers import file_args_to_stdin +from pre_commit.languages import helpers ENVIRONMENT_DIR = None @@ -11,16 +11,13 @@ ENVIRONMENT_DIR = None def install_environment( repo_cmd_runner, version='default', - additional_dependencies=None, + additional_dependencies=(), ): """Installation for system type is a noop.""" raise AssertionError('Cannot install system repo.') def run_hook(repo_cmd_runner, hook, file_args): - return repo_cmd_runner.run( - ['xargs', '-0'] + shlex.split(hook['entry']) + hook['args'], - stdin=file_args_to_stdin(file_args), - retcode=None, - encoding=None, + return helpers.run_hook( + tuple(shlex.split(hook['entry'])) + tuple(hook['args']), file_args, ) diff --git a/pre_commit/prefixed_command_runner.py b/pre_commit/prefixed_command_runner.py index 2b1212a2..fc4a3198 100644 --- a/pre_commit/prefixed_command_runner.py +++ b/pre_commit/prefixed_command_runner.py @@ -45,13 +45,7 @@ class PrefixedCommandRunner(object): 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 - `path_end` to the command runner's prefix directory. - """ - return cls( - command_runner.path(path_end), - popen=command_runner.__popen, - makedirs=command_runner.__makedirs, + def star(self, end): + return tuple( + path for path in os.listdir(self.prefix_dir) if path.endswith(end) ) diff --git a/tests/languages/all_test.py b/tests/languages/all_test.py index 2254e388..73b89cb5 100644 --- a/tests/languages/all_test.py +++ b/tests/languages/all_test.py @@ -14,7 +14,7 @@ def test_install_environment_argspec(language): args=['repo_cmd_runner', 'version', 'additional_dependencies'], varargs=None, keywords=None, - defaults=('default', None), + defaults=('default', ()), ) argspec = inspect.getargspec(languages[language].install_environment) assert argspec == expected_argspec diff --git a/tests/prefixed_command_runner_test.py b/tests/prefixed_command_runner_test.py index bafcae72..3f691b4b 100644 --- a/tests/prefixed_command_runner_test.py +++ b/tests/prefixed_command_runner_test.py @@ -110,35 +110,6 @@ def test_path_multiple_args(): assert ret == os.path.join('foo', 'bar', 'baz') -@pytest.mark.parametrize( - ('prefix', 'path_end', 'expected_output'), - tuple( - (prefix, path_end, expected_output + os.sep) - for prefix, path_end, expected_output in PATH_TESTS - ), -) -def test_from_command_runner(prefix, path_end, expected_output): - first = PrefixedCommandRunner(prefix) - second = PrefixedCommandRunner.from_command_runner(first, path_end) - assert second.prefix_dir == expected_output - - -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'], retcode=None) - popen_mock.assert_called_once_with( - [five.n('foo/bar/baz')], - env=None, - stdin=subprocess.PIPE, - stdout=subprocess.PIPE, - stderr=subprocess.PIPE, - ) - makedirs_mock.assert_called_once_with(os.path.join('foo', 'bar') + os.sep) - - def test_create_path_if_not_exists(in_tmpdir): instance = PrefixedCommandRunner('foo') assert not os.path.exists('foo') diff --git a/tests/repository_test.py b/tests/repository_test.py index ef870b53..9642b4c5 100644 --- a/tests/repository_test.py +++ b/tests/repository_test.py @@ -16,6 +16,7 @@ from pre_commit import five 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.languages import helpers from pre_commit.languages import node from pre_commit.languages import python from pre_commit.languages import ruby @@ -355,8 +356,8 @@ def test_additional_python_dependencies_installed(tempdir_factory, store): config['hooks'][0]['additional_dependencies'] = ['mccabe'] repo = Repository.create(config, store) repo.run_hook(repo.hooks[0][1], []) - with python.in_env(repo.cmd_runner, 'default') as env: - output = env.run('pip freeze -l')[1] + with python.in_env(repo.cmd_runner, 'default'): + output = cmd_output('pip', 'freeze', '-l')[1] assert 'mccabe' in output @@ -372,8 +373,8 @@ def test_additional_dependencies_roll_forward(tempdir_factory, store): repo = Repository.create(config, store) repo.run_hook(repo.hooks[0][1], []) # We should see our additional dependency installed - with python.in_env(repo.cmd_runner, 'default') as env: - output = env.run('pip freeze -l')[1] + with python.in_env(repo.cmd_runner, 'default'): + output = cmd_output('pip', 'freeze', '-l')[1] assert 'mccabe' in output @@ -388,8 +389,8 @@ def test_additional_ruby_dependencies_installed( config['hooks'][0]['additional_dependencies'] = ['thread_safe'] repo = Repository.create(config, store) repo.run_hook(repo.hooks[0][1], []) - with ruby.in_env(repo.cmd_runner, 'default') as env: - output = env.run('gem list --local')[1] + with ruby.in_env(repo.cmd_runner, 'default'): + output = cmd_output('gem', 'list', '--local')[1] assert 'thread_safe' in output @@ -405,9 +406,9 @@ def test_additional_node_dependencies_installed( config['hooks'][0]['additional_dependencies'] = ['lodash'] repo = Repository.create(config, store) repo.run_hook(repo.hooks[0][1], []) - with node.in_env(repo.cmd_runner, 'default') as env: - env.run('npm config set global true') - output = env.run(('npm ls'))[1] + 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 @@ -443,7 +444,7 @@ def test_control_c_control_c_on_install(tempdir_factory, store): # raise as well. with pytest.raises(MyKeyboardInterrupt): with mock.patch.object( - python.PythonEnv, 'run', side_effect=MyKeyboardInterrupt, + helpers, 'run_setup_cmd', side_effect=MyKeyboardInterrupt, ): with mock.patch.object( shutil, 'rmtree', side_effect=MyKeyboardInterrupt,