Merge pull request #1509 from pre-commit/system_language_version_ruby

skip rbenv if ruby and gem are installed with default language_version

azure-pipelines.yml

@@ -19,6 +19,7 @@ jobs:
     toxenvs: [py37]
     os: windows
     pre_test:
+    - task: UseRubyVersion@0
     - powershell: Write-Host "##vso[task.prependpath]$env:CONDA\Scripts"
       displayName: Add conda to PATH
     - powershell: |

pre_commit/languages/ruby.py

@@ -1,4 +1,5 @@
 import contextlib
+import functools
 import os.path
 import shutil
 import tarfile
@@ -7,6 +8,7 @@
 from typing import Tuple
 
 import pre_commit.constants as C
+from pre_commit import parse_shebang
 from pre_commit.envcontext import envcontext
 from pre_commit.envcontext import PatchesT
 from pre_commit.envcontext import UNSET
@@ -19,33 +21,51 @@
 from pre_commit.util import resource_bytesio
 
 ENVIRONMENT_DIR = 'rbenv'
-get_default_version = helpers.basic_get_default_version
 healthy = helpers.basic_healthy
 
 
+@functools.lru_cache(maxsize=1)
+def get_default_version() -> str:
+    if all(parse_shebang.find_executable(exe) for exe in ('ruby', 'gem')):
+        return 'system'
+    else:
+        return C.DEFAULT
+
+
 def get_env_patch(
         venv: str,
         language_version: str,
-) -> PatchesT:  # pragma: win32 no cover
+) -> PatchesT:
     patches: PatchesT = (
         ('GEM_HOME', os.path.join(venv, 'gems')),
         ('GEM_PATH', UNSET),
-        ('RBENV_ROOT', venv),
         ('BUNDLE_IGNORE_CONFIG', '1'),
-        (
-            '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'),
-            ),
-        ),
     )
-    if language_version != C.DEFAULT:
-        patches += (('RBENV_VERSION', language_version),)
+    if language_version == 'system':
+        patches += (
+            (
+                'PATH', (
+                    os.path.join(venv, 'gems', 'bin'), os.pathsep,
+                    Var('PATH'),
+                ),
+            ),
+        )
+    else:  # pragma: win32 no cover
+        patches += (
+            ('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'),
+                ),
+            ),
+        )
     return patches
 
 
-@contextlib.contextmanager  # pragma: win32 no cover
+@contextlib.contextmanager
 def in_env(
         prefix: Prefix,
         language_version: str,
@@ -65,7 +85,7 @@ def _extract_resource(filename: str, dest: str) -> None:
 
 def _install_rbenv(
         prefix: Prefix,
-        version: str = C.DEFAULT,
+        version: str,
 ) -> None:  # pragma: win32 no cover
     directory = helpers.environment_dir(ENVIRONMENT_DIR, version)
 
@@ -92,21 +112,22 @@ def _install_ruby(
 
 def install_environment(
         prefix: Prefix, version: str, additional_dependencies: Sequence[str],
-) -> None:  # pragma: win32 no cover
+) -> None:
     additional_dependencies = tuple(additional_dependencies)
     directory = helpers.environment_dir(ENVIRONMENT_DIR, version)
     with clean_path_on_failure(prefix.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(prefix, version=version)
-        with in_env(prefix, version):
-            # Need to call this before installing so rbenv's directories are
-            # set up
-            helpers.run_setup_cmd(prefix, ('rbenv', 'init', '-'))
-            if version != C.DEFAULT:
+        if version != 'system':  # pragma: win32 no cover
+            _install_rbenv(prefix, version)
+            with in_env(prefix, version):
+                # Need to call this before installing so rbenv's directories
+                # are set up
+                helpers.run_setup_cmd(prefix, ('rbenv', 'init', '-'))
+                # XXX: this will *always* fail if `version == C.DEFAULT`
                 _install_ruby(prefix, version)
-            # Need to call this after installing to set up the shims
-            helpers.run_setup_cmd(prefix, ('rbenv', 'rehash'))
+                # Need to call this after installing to set up the shims
+                helpers.run_setup_cmd(prefix, ('rbenv', 'rehash'))
+
+        with in_env(prefix, version):
             helpers.run_setup_cmd(
                 prefix, ('gem', 'build', *prefix.star('.gemspec')),
             )
@@ -123,6 +144,6 @@ def run_hook(
         hook: Hook,
         file_args: Sequence[str],
         color: bool,
-) -> Tuple[int, bytes]:  # pragma: win32 no cover
+) -> Tuple[int, bytes]:
     with in_env(hook.prefix, hook.language_version):
         return helpers.run_xargs(hook, hook.cmd, file_args, color=color)

testing/util.py

@@ -38,10 +38,6 @@ def cmd_output_mocked_pre_commit_home(
     parse_shebang.find_executable('swift') is None,
     reason="swift isn't installed or can't be found",
 )
-xfailif_windows_no_ruby = pytest.mark.xfail(
-    os.name == 'nt',
-    reason='Ruby support not yet implemented on windows.',
-)
 xfailif_windows = pytest.mark.xfail(os.name == 'nt', reason='windows')
 
 

tests/languages/ruby_test.py

@@ -1,15 +1,39 @@
 import os.path
+from unittest import mock
 
+import pytest
+
+import pre_commit.constants as C
+from pre_commit import parse_shebang
 from pre_commit.languages import ruby
 from pre_commit.prefix import Prefix
 from pre_commit.util import cmd_output
-from testing.util import xfailif_windows_no_ruby
+from testing.util import xfailif_windows
+
+
+ACTUAL_GET_DEFAULT_VERSION = ruby.get_default_version.__wrapped__
+
+
+@pytest.fixture
+def find_exe_mck():
+    with mock.patch.object(parse_shebang, 'find_executable') as mck:
+        yield mck
+
+
+def test_uses_default_version_when_not_available(find_exe_mck):
+    find_exe_mck.return_value = None
+    assert ACTUAL_GET_DEFAULT_VERSION() == C.DEFAULT
+
+
+def test_uses_system_if_both_gem_and_ruby_are_available(find_exe_mck):
+    find_exe_mck.return_value = '/path/to/exe'
+    assert ACTUAL_GET_DEFAULT_VERSION() == 'system'
 
 
-@xfailif_windows_no_ruby
+@xfailif_windows  # pragma: win32 no cover
 def test_install_rbenv(tempdir_factory):
     prefix = Prefix(tempdir_factory.get())
-    ruby._install_rbenv(prefix)
+    ruby._install_rbenv(prefix, C.DEFAULT)
     # Should have created rbenv directory
     assert os.path.exists(prefix.path('rbenv-default'))
 
@@ -18,7 +42,7 @@ def test_install_rbenv(tempdir_factory):
         cmd_output('rbenv', '--help')
 
 
-@xfailif_windows_no_ruby
+@xfailif_windows  # pragma: win32 no cover
 def test_install_rbenv_with_version(tempdir_factory):
     prefix = Prefix(tempdir_factory.get())
     ruby._install_rbenv(prefix, version='1.9.3p547')

tests/repository_test.py

@@ -34,7 +34,6 @@
 from testing.util import skipif_cant_run_docker
 from testing.util import skipif_cant_run_swift
 from testing.util import xfailif_windows
-from testing.util import xfailif_windows_no_ruby
 
 
 def _norm_out(b):
@@ -260,15 +259,14 @@ def test_run_versioned_node_hook(tempdir_factory, store):
     )
 
 
-@xfailif_windows_no_ruby
 def test_run_a_ruby_hook(tempdir_factory, store):
     _test_hook_repo(
         tempdir_factory, store, 'ruby_hooks_repo',
         'ruby_hook', [os.devnull], b'Hello world from a ruby hook\n',
     )
 
 
-@xfailif_windows_no_ruby
+@xfailif_windows  # pragma: win32 no cover
 def test_run_versioned_ruby_hook(tempdir_factory, store):
     _test_hook_repo(
         tempdir_factory, store, 'ruby_versioned_hooks_repo',
@@ -278,7 +276,7 @@ def test_run_versioned_ruby_hook(tempdir_factory, store):
     )
 
 
-@xfailif_windows_no_ruby
+@xfailif_windows  # pragma: win32 no cover
 def test_run_ruby_hook_with_disable_shared_gems(
         tempdir_factory,
         store,
@@ -524,7 +522,6 @@ def test_additional_dependencies_roll_forward(tempdir_factory, store):
         assert 'mccabe' not in cmd_output('pip', 'freeze', '-l')[1]
 
 
-@xfailif_windows_no_ruby  # pragma: win32 no cover
 def test_additional_ruby_dependencies_installed(tempdir_factory, store):
     path = make_repo(tempdir_factory, 'ruby_hooks_repo')
     config = make_config_from_repo(path)