Merge pull request #100 from pre-commit/language_version

Allow an optional language_version property in hooks.yaml

.pre-commit-config.yaml

@@ -2,9 +2,9 @@
     sha: ca93f6834f2afc8a8f7de46c0e02076419077c7a
     hooks:
     -   id: trailing-whitespace
-        files: \.(py|sh|yaml)$
+        files: \.(js|py|sh|yaml)$
     -   id: end-of-file-fixer
-        files: \.(py|sh|yaml)$
+        files: \.(js|py|sh|yaml)$
     -   id: check-yaml
         files: \.(yaml|yml)$
     -   id: debug-statements

pre_commit/clientlib/validate_manifest.py

@@ -20,6 +20,7 @@ MANIFEST_JSON_SCHEMA = {
             'description': {'type': 'string', 'default': ''},
             'entry': {'type': 'string'},
             'language': {'type': 'string'},
+            'language_version': {'type': 'string', 'default': 'default'},
             'expected_return_value': {'type': 'number', 'default': 0},
         },
         'required': ['id', 'name', 'entry', 'language'],
@@ -31,9 +32,9 @@ def additional_manifest_check(obj):
     for hook_config in obj:
         language = hook_config['language']
 
-        if not any(language.startswith(lang) for lang in all_languages):
+        if language not in all_languages:
             raise InvalidManifestError(
-                'Expected language {0} for {1} to start with one of {2!r}'.format(
+                'Expected language {0} for {1} to be one of {2!r}'.format(
                     hook_config['id'],
                     hook_config['language'],
                     all_languages,

pre_commit/languages/node.py

@@ -1,7 +1,6 @@
 import contextlib
 
 from pre_commit.languages import helpers
-from pre_commit.languages import python
 from pre_commit.prefixed_command_runner import CalledProcessError
 from pre_commit.util import clean_path_on_failure
 
@@ -9,14 +8,10 @@ from pre_commit.util import clean_path_on_failure
 ENVIRONMENT_DIR = 'node_env'
 
 
-class NodeEnv(python.PythonEnv):
+class NodeEnv(helpers.Environment):
     @property
     def env_prefix(self):
-        base = super(NodeEnv, self).env_prefix
-        return ' '.join([
-            base,
-            '. {{prefix}}{0}/bin/activate &&'.format(ENVIRONMENT_DIR)]
-        )
+        return '. {{prefix}}{0}/bin/activate &&'.format(ENVIRONMENT_DIR)
 
 
 @contextlib.contextmanager
@@ -24,32 +19,34 @@ def in_env(repo_cmd_runner):
     yield NodeEnv(repo_cmd_runner)
 
 
-def install_environment(repo_cmd_runner):
+def install_environment(repo_cmd_runner, version='default'):
     assert repo_cmd_runner.exists('package.json')
 
-    with clean_path_on_failure(repo_cmd_runner.path(python.ENVIRONMENT_DIR)):
-        repo_cmd_runner.run(
-            ['virtualenv', '{{prefix}}{0}'.format(python.ENVIRONMENT_DIR)],
-        )
+    env_dir = repo_cmd_runner.path(ENVIRONMENT_DIR)
+    with clean_path_on_failure(env_dir):
+        if version == 'default':
+            # In the default case we attempt to install system node and if that
+            # doesn't work we use --prebuilt
+            try:
+                with clean_path_on_failure(env_dir):
+                    repo_cmd_runner.run([
+                        'nodeenv', '-n', 'system',
+                        '{{prefix}}{0}'.format(ENVIRONMENT_DIR),
+                    ])
+            except CalledProcessError:
+                # TODO: log failure here
+                repo_cmd_runner.run([
+                    'nodeenv', '--prebuilt',
+                    '{{prefix}}{0}'.format(ENVIRONMENT_DIR)
+                ])
+        else:
+            repo_cmd_runner.run([
+                'nodeenv', '--prebuilt', '-n', version,
+                '{{prefix}}{0}'.format(ENVIRONMENT_DIR)
+            ])
 
-        with python.in_env(repo_cmd_runner) as python_env:
-            python_env.run('pip install nodeenv')
-
-            with clean_path_on_failure(repo_cmd_runner.path(ENVIRONMENT_DIR)):
-                # Try and use the system level node executable first
-                try:
-                    with clean_path_on_failure(repo_cmd_runner.path(ENVIRONMENT_DIR)):
-                        python_env.run(
-                            'nodeenv -n system {{prefix}}{0}'.format(ENVIRONMENT_DIR),
-                        )
-                except CalledProcessError:
-                    # TODO: log failure here
-                    python_env.run(
-                        'nodeenv --jobs 4 {{prefix}}{0}'.format(ENVIRONMENT_DIR),
-                    )
-
-                with in_env(repo_cmd_runner) as node_env:
-                    node_env.run('cd {prefix} && npm install -g')
+        with in_env(repo_cmd_runner) as node_env:
+            node_env.run('cd {prefix} && npm install -g')
 
 
 def run_hook(repo_cmd_runner, hook, file_args):

pre_commit/languages/python.py

@@ -18,12 +18,15 @@ def in_env(repo_cmd_runner):
     yield PythonEnv(repo_cmd_runner)
 
 
-def install_environment(repo_cmd_runner):
+def install_environment(repo_cmd_runner, version='default'):
     assert repo_cmd_runner.exists('setup.py')
 
     # Install a virtualenv
     with clean_path_on_failure(repo_cmd_runner.path(ENVIRONMENT_DIR)):
-        repo_cmd_runner.run(['virtualenv', '{{prefix}}{0}'.format(ENVIRONMENT_DIR)])
+        venv_cmd = ['virtualenv', '{{prefix}}{0}'.format(ENVIRONMENT_DIR)]
+        if version != 'default':
+            venv_cmd.extend(['-p', version])
+        repo_cmd_runner.run(venv_cmd)
         with in_env(repo_cmd_runner) as env:
             env.run('cd {prefix} && pip install .')
 

pre_commit/languages/ruby.py

@@ -2,7 +2,6 @@ from __future__ import unicode_literals
 
 import contextlib
 import io
-import os
 
 from pre_commit.languages import helpers
 from pre_commit.util import clean_path_on_failure
@@ -16,25 +15,23 @@ class RubyEnv(helpers.Environment):
     def env_prefix(self):
         return '. {{prefix}}{0}/bin/activate &&'.format(ENVIRONMENT_DIR)
 
-    def run(self, *args, **kwargs):
-        # TODO: hardcoded version smell
-        env = dict(os.environ, RBENV_VERSION='1.9.3-p547')
-        return super(RubyEnv, self).run(*args, env=env, **kwargs)
-
 
 @contextlib.contextmanager
 def in_env(repo_cmd_runner):
     yield RubyEnv(repo_cmd_runner)
 
 
-def _install_rbenv(repo_cmd_runner):
+def _install_rbenv(repo_cmd_runner, version='default'):
     repo_cmd_runner.run([
         'git', 'clone', 'git://github.com/sstephenson/rbenv', '{prefix}rbenv',
     ])
-    repo_cmd_runner.run([
-        'git', 'clone', 'git://github.com/sstephenson/ruby-build',
-        '{prefix}rbenv/plugins/ruby-build',
-    ])
+
+    # Only install ruby-build if the version is specified
+    if version != 'default':
+        repo_cmd_runner.run([
+            'git', 'clone', 'git://github.com/sstephenson/ruby-build',
+            '{prefix}rbenv/plugins/ruby-build',
+        ])
 
     activate_path = repo_cmd_runner.path('rbenv', 'bin', 'activate')
     with io.open(activate_path, 'w') as activate_file:
@@ -55,13 +52,19 @@ def _install_rbenv(repo_cmd_runner):
             '\n'.format(repo_cmd_runner.path('rbenv'))
         )
 
+        # If we aren't using the system ruby, add a version here
+        if version != 'default':
+            activate_file.write('export RBENV_VERSION="{0}"\n'.format(version))
 
-def install_environment(repo_cmd_runner):
+
+def install_environment(repo_cmd_runner, version='default'):
     with clean_path_on_failure(repo_cmd_runner.path('rbenv')):
-        _install_rbenv(repo_cmd_runner)
+        # 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) as ruby_env:
-            # TODO: hardcoded version smell
-            ruby_env.run('rbenv install 1.9.3-p547')
+            if version != 'default':
+                ruby_env.run('rbenv install {0}'.format(version))
             ruby_env.run(
                 'cd {prefix} && gem build *.gemspec && gem install *.gem',
             )

pre_commit/languages/script.py

@@ -1,7 +1,7 @@
 ENVIRONMENT_DIR = None
 
 
-def install_environment(repo_cmd_runner):
+def install_environment(repo_cmd_runner, version='default'):
     """Installation for script type is a noop."""
 
 

pre_commit/languages/system.py

@@ -4,7 +4,7 @@ import shlex
 ENVIRONMENT_DIR = None
 
 
-def install_environment(repo_cmd_runner):
+def install_environment(repo_cmd_runner, version='default'):
     """Installation for system type is a noop."""
 
 

pre_commit/repository.py

@@ -29,7 +29,10 @@ class Repository(object):
 
     @cached_property
     def languages(self):
-        return set(hook['language'] for hook in self.hooks.values())
+        return set(
+            (hook['language'], hook['language_version'])
+            for hook in self.hooks.values()
+        )
 
     @cached_property
     def hooks(self):
@@ -56,7 +59,7 @@ class Repository(object):
 
     def install(self):
         """Install the hook repository."""
-        for language_name in self.languages:
+        for language_name, language_version in self.languages:
             language = languages[language_name]
             if (
                 language.ENVIRONMENT_DIR is None or
@@ -64,7 +67,7 @@ class Repository(object):
             ):
                 # The language is already installed
                 continue
-            language.install_environment(self.cmd_runner)
+            language.install_environment(self.cmd_runner, language_version)
 
     def run_hook(self, hook_id, file_args):
         """Run a hook.

setup.py

@@ -33,9 +33,11 @@ setup(
         'asottile.ordereddict',
         'asottile.yaml',
         'jsonschema',
+        'nodeenv>=0.9.4',
         'plumbum',
         'pyyaml',
         'simplejson',
+        'virtualenv',
     ],
     entry_points={
         'console_scripts': [

testing/resources/node_0_11_8_hooks_repo/bin/main.js

@@ -0,0 +1,4 @@
+#!/usr/bin/env node
+
+console.log(process.version);
+console.log('Hello World');

testing/resources/node_0_11_8_hooks_repo/hooks.yaml

@@ -0,0 +1,5 @@
+-   id: node-11-8-hook
+    name: Node 0.11.8 hook
+    entry: node-11-8-hook
+    language: node
+    language_version: 0.11.8

testing/resources/node_0_11_8_hooks_repo/package.json

@@ -0,0 +1,5 @@
+{
+    "name": "node-11-8-hook",
+    "version": "0.0.1",
+    "bin": {"node-11-8-hook": "./bin/main.js"}
+}

testing/resources/python3_hooks_repo/hooks.yaml

@@ -0,0 +1,5 @@
+-   id: python3-hook
+    name: Python 3 Hook
+    entry: python3-hook
+    language: python
+    language_version: python3.3

testing/resources/python3_hooks_repo/python3_hook/main.py

@@ -0,0 +1,9 @@
+from __future__ import print_function
+import sys
+
+
+def func():
+    print('{0}.{1}'.format(*sys.version_info[:2]))
+    print(repr(sys.argv[1:]))
+    print('Hello World')
+    return 0

testing/resources/python3_hooks_repo/setup.py

@@ -0,0 +1,11 @@
+from setuptools import find_packages
+from setuptools import setup
+
+setup(
+    name='python3_hook',
+    version='0.0.0',
+    packages=find_packages('.'),
+    entry_points={
+        'console_scripts': ['python3-hook = python3_hook.main:func'],
+    },
+)

testing/resources/ruby_1_9_3_p547_hooks_repo/.gitignore

@@ -0,0 +1 @@
+*.gem

testing/resources/ruby_1_9_3_p547_hooks_repo/bin/ruby_hook

@@ -0,0 +1,5 @@
+#!/usr/bin/env ruby
+
+puts RUBY_VERSION
+puts RUBY_PATCHLEVEL
+puts 'Hello world from a ruby hook'

testing/resources/ruby_1_9_3_p547_hooks_repo/hooks.yaml

@@ -0,0 +1,5 @@
+-   id: ruby_hook
+    name: Ruby Hook
+    entry: ruby_hook
+    language: ruby
+    language_version: 1.9.3-p547

testing/resources/ruby_1_9_3_p547_hooks_repo/ruby_hook.gemspec

@@ -0,0 +1,9 @@
+Gem::Specification.new do |s|
+    s.name = 'ruby_hook'
+    s.version = '0.1.0'
+    s.authors = ['Anthony Sottile']
+    s.summary = 'A ruby hook!'
+    s.description = 'A ruby hook!'
+    s.files = ['bin/ruby_hook']
+    s.executables = ['ruby_hook']
+end

tests/clientlib/validate_manifest_test.py

@@ -27,14 +27,25 @@ def test_additional_manifest_check_raises_for_bad_language():
         additional_manifest_check([{'id': 'foo', 'language': 'not valid'}])
 
 
-@pytest.mark.parametrize(('obj'), (
-    [{'language': 'python'}],
-    [{'language': 'ruby'}],
-))
+@pytest.mark.parametrize(
+    'obj', ([{'language': 'python'}], [{'language': 'ruby'}]),
+)
 def test_additional_manifest_check_languages(obj):
     additional_manifest_check(obj)
 
 
+@pytest.mark.parametrize(
+    'obj',
+    (
+        [{'id': 'a', 'language': 'not a language'}],
+        [{'id': 'a', 'language': 'python3'}],
+    ),
+)
+def test_additional_manifest_check_languages_failing(obj):
+    with pytest.raises(InvalidManifestError):
+        additional_manifest_check(obj)
+
+
 @pytest.mark.parametrize(('manifest_obj', 'expected'), (
     ([], False),
     ([{'id': 'a', 'name': 'b', 'entry': 'c', 'language': 'python'}], True),

tests/conftest.py

@@ -72,16 +72,31 @@ def python_hooks_repo(dummy_git_repo):
     yield _make_repo(dummy_git_repo, 'python_hooks_repo')
 
 
+@pytest.yield_fixture
+def python3_hooks_repo(dummy_git_repo):
+    yield _make_repo(dummy_git_repo, 'python3_hooks_repo')
+
+
 @pytest.yield_fixture
 def node_hooks_repo(dummy_git_repo):
     yield _make_repo(dummy_git_repo, 'node_hooks_repo')
 
 
+@pytest.yield_fixture
+def node_0_11_8_hooks_repo(dummy_git_repo):
+    yield _make_repo(dummy_git_repo, 'node_0_11_8_hooks_repo')
+
+
 @pytest.yield_fixture
 def ruby_hooks_repo(dummy_git_repo):
     yield _make_repo(dummy_git_repo, 'ruby_hooks_repo')
 
 
+@pytest.yield_fixture
+def ruby_1_9_3_p547_hooks_repo(dummy_git_repo):
+    yield _make_repo(dummy_git_repo, 'ruby_1_9_3_p547_hooks_repo')
+
+
 @pytest.yield_fixture
 def consumer_repo(dummy_git_repo):
     yield _make_repo(dummy_git_repo, 'consumer_repo')
@@ -123,16 +138,31 @@ def config_for_node_hooks_repo(node_hooks_repo):
     yield _make_config(node_hooks_repo, 'foo', '\\.js$')
 
 
+@pytest.yield_fixture
+def config_for_node_0_11_8_hooks_repo(node_0_11_8_hooks_repo):
+    yield _make_config(node_0_11_8_hooks_repo, 'node-11-8-hook', '\\.js$')
+
+
 @pytest.yield_fixture
 def config_for_ruby_hooks_repo(ruby_hooks_repo):
     yield _make_config(ruby_hooks_repo, 'ruby_hook', '\\.rb$')
 
 
+@pytest.yield_fixture
+def config_for_ruby_1_9_3_p547_hooks_repo(ruby_1_9_3_p547_hooks_repo):
+    yield _make_config(ruby_1_9_3_p547_hooks_repo, 'ruby_hook', '\\.rb$')
+
+
 @pytest.yield_fixture
 def config_for_python_hooks_repo(python_hooks_repo):
     yield _make_config(python_hooks_repo, 'foo', '\\.py$')
 
 
+@pytest.yield_fixture
+def config_for_python3_hooks_repo(python3_hooks_repo):
+    yield _make_config(python3_hooks_repo, 'python3-hook', '\\.py$')
+
+
 @pytest.yield_fixture
 def config_for_prints_cwd_repo(prints_cwd_repo):
     yield _make_config(prints_cwd_repo, 'prints_cwd', '^$')

tests/languages/ruby_test.py

@@ -1,8 +1,10 @@
 import os.path
 
 from pre_commit.languages.ruby import _install_rbenv
+from testing.util import skipif_slowtests_false
 
 
+@skipif_slowtests_false
 def test_install_rbenv(cmd_runner):
     _install_rbenv(cmd_runner)
     # Should have created rbenv directory
@@ -13,7 +15,21 @@ def test_install_rbenv(cmd_runner):
     activate_path = cmd_runner.path('rbenv', 'bin', 'activate')
     assert os.path.exists(activate_path)
 
-    # Should be able to activate using our script and access the install method
+    # Should be able to activate using our script and access rbenv
+    cmd_runner.run(
+        [
+            'bash',
+            '-c',
+            '. {prefix}/rbenv/bin/activate && rbenv --help',
+        ],
+    )
+
+
+@skipif_slowtests_false
+def test_install_rbenv_with_version(cmd_runner):
+    _install_rbenv(cmd_runner, version='1.9.3p547')
+
+    # Should be able to activate and use rbenv install
     cmd_runner.run(
         [
             'bash',

tests/manifest_test.py

@@ -19,6 +19,7 @@ def test_manifest_contents(manifest):
         'expected_return_value': 0,
         'id': 'bash_hook',
         'language': 'script',
+        'language_version': 'default',
         'name': 'Bash hook',
     }]
 
@@ -30,5 +31,6 @@ def test_hooks(manifest):
         'expected_return_value': 0,
         'id': 'bash_hook',
         'language': 'script',
+        'language_version': 'default',
         'name': 'Bash hook',
     }

tests/repository_test.py

@@ -24,6 +24,32 @@ def test_run_a_python_hook(config_for_python_hooks_repo, store):
     assert ret[1] == "['/dev/null']\nHello World\n"
 
 
+@pytest.mark.integration
+def test_run_versioned_hook(config_for_python3_hooks_repo, store):
+    repo = Repository.create(config_for_python3_hooks_repo, store)
+    ret = repo.run_hook('python3-hook', ['/dev/null'])
+    assert ret[0] == 0
+    assert ret[1] == "3.3\n['/dev/null']\nHello World\n"
+
+
+@pytest.mark.integration
+def test_run_versioned_node_hook(config_for_node_0_11_8_hooks_repo, store):
+    repo = Repository.create(config_for_node_0_11_8_hooks_repo, store)
+    ret = repo.run_hook('node-11-8-hook', ['/dev/null'])
+    assert ret[0] == 0
+    assert ret[1] == 'v0.11.8\nHello World\n'
+
+
+@pytest.mark.herpderp
+@skipif_slowtests_false
+@pytest.mark.integration
+def test_run_versioned_ruby_hook(config_for_ruby_1_9_3_p547_hooks_repo, store):
+    repo = Repository.create(config_for_ruby_1_9_3_p547_hooks_repo, store)
+    ret = repo.run_hook('ruby_hook', [])
+    assert ret[0] == 0
+    assert ret[1] == '1.9.3\n547\nHello world from a ruby hook\n'
+
+
 @pytest.mark.integration
 def test_lots_of_files(config_for_python_hooks_repo, store):
     repo = Repository.create(config_for_python_hooks_repo, store)
@@ -104,7 +130,7 @@ def test_sha(mock_repo_config):
 @pytest.mark.integration
 def test_languages(config_for_python_hooks_repo, store):
     repo = Repository.create(config_for_python_hooks_repo, store)
-    assert repo.languages == set(['python'])
+    assert repo.languages == set([('python', 'default')])
 
 
 def test_reinstall(config_for_python_hooks_repo, store):