Factor out bash and activate files

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

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.

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):
+def run_hook(cmd_args, file_args):
-    quoted_args = [pipes.quote(arg) for arg in hook['args']]
+    return cmd_output(
-    return env.run(
         # 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
     )

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):
+def get_env_patch(venv):
-    @property
+    return (
-    def env_prefix(self):
+        ('NODE_VIRTUAL_ENV', venv),
-        return ". '{{prefix}}{0}/bin/activate' &&".format(
+        ('NPM_CONFIG_PREFIX', venv),
-            helpers.environment_dir(ENVIRONMENT_DIR, self.language_version),
+        ('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:
+        with in_env(repo_cmd_runner, version):
-            node_env.run("cd '{prefix}' && npm install -g", encoding=None)
+            helpers.run_setup_cmd(
-            if additional_dependencies:
+                repo_cmd_runner,
-                node_env.run(
+                ('npm', 'install', '-g', '.') + additional_dependencies,
-                    "cd '{prefix}' && npm install -g " +
-                    ' '.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:
+    with in_env(repo_cmd_runner, hook['language_version']):
-        return helpers.run_hook(env, hook, file_args)
+        return helpers.run_hook(
+            (hook['entry'],) + tuple(hook['args']), file_args,
+        )

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'
+    grep_command = '{0} -H -n -P'.format(
-    if platform == 'darwin':  # pragma: no cover (osx)
+        'ggrep' if platform == 'darwin' else 'grep',
-        grep_command = 'ggrep -H -n -P'
+    )
 
     # For PCRE the entry is the regular expression to match
-    return repo_cmd_runner.run(
+    return helpers.run_hook(
-        [
+        (
-            'xargs', '-0', 'sh', '-c',
+            '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),
+        file_args,
-        retcode=None,
-        encoding=None,
     )

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)
+    bin_part = 'Scripts' if os.name == 'nt' else 'bin'
-        return os.path.join(venv, 'Scripts')
+    return os.path.join(venv, bin_part)
-    else:
-        return os.path.join(venv, 'bin')
 
 
-class PythonEnv(helpers.Environment):
+def get_env_patch(venv):
-    @property
+    return (
-    def env_prefix(self):
+        ('PYTHONHOME', UNSET),
-        return ". '{{prefix}}{0}{1}activate' &&".format(
+        ('VIRTUAL_ENV', venv),
-            bin_dir(
+        ('PATH', (bin_dir(venv), os.pathsep, Var('PATH'))),
-                helpers.environment_dir(ENVIRONMENT_DIR, self.language_version)
-            ),
-            os.sep,
     )
 
 
 @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:
+        with in_env(repo_cmd_runner, version):
-            env.run("cd '{prefix}' && pip install .", encoding=None)
+            helpers.run_setup_cmd(
-            if additional_dependencies:
+                repo_cmd_runner,
-                env.run(
+                ('pip', 'install', '.') + additional_dependencies,
-                    "cd '{prefix}' && pip install " +
-                    ' '.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:
+    with in_env(repo_cmd_runner, hook['language_version']):
-        return helpers.run_hook(env, hook, file_args)
+        return helpers.run_hook(
+            (hook['entry'],) + tuple(hook['args']), file_args,
+        )

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):
+def get_env_patch(venv, language_version):
-    @property
+    return (
-    def env_prefix(self):
+        ('GEM_HOME', os.path.join(venv, 'gems')),
-        return '. {{prefix}}{0}/bin/activate &&'.format(
+        ('RBENV_ROOT', venv),
-            helpers.environment_dir(ENVIRONMENT_DIR, self.language_version)
+        ('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)
+                _install_ruby(repo_cmd_runner, version)
-            ruby_env.run(
+            # Need to call this after installing to set up the shims
-                'cd {prefix} && gem build *.gemspec && '
+            helpers.run_setup_cmd(repo_cmd_runner, ('rbenv', 'rehash'))
-                'gem install --no-ri --no-rdoc *.gem',
+            helpers.run_setup_cmd(
-                encoding=None,
+                repo_cmd_runner,
+                ('gem', 'build') + repo_cmd_runner.star('.gemspec'),
             )
-            if additional_dependencies:
+            helpers.run_setup_cmd(
-                ruby_env.run(
+                repo_cmd_runner,
-                    'cd {prefix} && gem install --no-ri --no-rdoc ' +
+                (
-                    ' '.join(
+                    ('gem', 'install', '--no-ri', '--no-rdoc') +
-                        shell_escape(dep) for dep in additional_dependencies
+                    repo_cmd_runner.star('.gem') + additional_dependencies
                 ),
-                    encoding=None,
             )
 
 
 def run_hook(repo_cmd_runner, hook, file_args):
-    with in_env(repo_cmd_runner, hook['language_version']) as env:
+    with in_env(repo_cmd_runner, hook['language_version']):
-        return helpers.run_hook(env, hook, file_args)
+        return helpers.run_hook(
+            (hook['entry'],) + tuple(hook['args']), file_args,
+        )

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(
+    return helpers.run_hook(
-        ['xargs', '-0', '{{prefix}}{0}'.format(hook['entry'])] + hook['args'],
+        (repo_cmd_runner.prefix_dir + hook['entry'],) + tuple(hook['args']),
-        stdin=file_args_to_stdin(file_args),
+        file_args,
-        retcode=None,
-        encoding=None,
     )

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(
+    return helpers.run_hook(
-        ['xargs', '-0'] + shlex.split(hook['entry']) + hook['args'],
+        tuple(shlex.split(hook['entry'])) + tuple(hook['args']), file_args,
-        stdin=file_args_to_stdin(file_args),
-        retcode=None,
-        encoding=None,
     )

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 star(self, end):
-    def from_command_runner(cls, command_runner, path_end):
+        return tuple(
-        """Constructs a new command runner from an existing one by appending
+            path for path in os.listdir(self.prefix_dir) if path.endswith(end)
-        `path_end` to the command runner's prefix directory.
-        """
-        return cls(
-            command_runner.path(path_end),
-            popen=command_runner.__popen,
-            makedirs=command_runner.__makedirs,
         )

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

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')

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:
+    with python.in_env(repo.cmd_runner, 'default'):
-        output = env.run('pip freeze -l')[1]
+        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:
+    with python.in_env(repo.cmd_runner, 'default'):
-        output = env.run('pip freeze -l')[1]
+        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:
+    with ruby.in_env(repo.cmd_runner, 'default'):
-        output = env.run('gem list --local')[1]
+        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:
+    with node.in_env(repo.cmd_runner, 'default'):
-        env.run('npm config set global true')
+        cmd_output('npm', 'config', 'set', 'global', 'true')
-        output = env.run(('npm ls'))[1]
+        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,