Merge pull request #1668 from pre-commit/avoid_shims

don't use system for ruby/node if it is a shim exe

pre_commit/languages/helpers.py

@@ -1,6 +1,7 @@
 import multiprocessing
 import os
 import random
+import re
 from typing import Any
 from typing import List
 from typing import Optional
@@ -10,6 +11,7 @@ from typing import Tuple
 from typing import TYPE_CHECKING
 
 import pre_commit.constants as C
+from pre_commit import parse_shebang
 from pre_commit.hook import Hook
 from pre_commit.prefix import Prefix
 from pre_commit.util import cmd_output_b
@@ -20,6 +22,25 @@ if TYPE_CHECKING:
 
 FIXED_RANDOM_SEED = 1542676187
 
+SHIMS_RE = re.compile(r'[/\\]shims[/\\]')
+
+
+def exe_exists(exe: str) -> bool:
+    found = parse_shebang.find_executable(exe)
+    if found is None:  # exe exists
+        return False
+
+    homedir = os.path.expanduser('~')
+    try:
+        common: Optional[str] = os.path.commonpath((found, homedir))
+    except ValueError:  # on windows, different drives raises ValueError
+        common = None
+
+    return (
+        not SHIMS_RE.search(found) and  # it is not in a /shims/ directory
+        common != homedir  # it is not in the home directory
+    )
+
 
 def run_setup_cmd(prefix: Prefix, cmd: Tuple[str, ...]) -> None:
     cmd_output_b(*cmd, cwd=prefix.prefix_dir)

pre_commit/languages/node.py

@@ -7,7 +7,6 @@ from typing import Sequence
 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
@@ -31,7 +30,7 @@ def get_default_version() -> str:
         return C.DEFAULT
     # if node is already installed, we can save a bunch of setup time by
     # using the installed version
-    elif all(parse_shebang.find_executable(exe) for exe in ('node', 'npm')):
+    elif all(helpers.exe_exists(exe) for exe in ('node', 'npm')):
         return 'system'
     else:
         return C.DEFAULT

pre_commit/languages/ruby.py

@@ -8,7 +8,6 @@ from typing import Sequence
 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
@@ -26,7 +25,7 @@ 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')):
+    if all(helpers.exe_exists(exe) for exe in ('ruby', 'gem')):
         return 'system'
     else:
         return C.DEFAULT

tests/languages/helpers_test.py

@@ -1,17 +1,60 @@
 import multiprocessing
-import os
+import os.path
 import sys
 from unittest import mock
 
 import pytest
 
 import pre_commit.constants as C
+from pre_commit import parse_shebang
 from pre_commit.languages import helpers
 from pre_commit.prefix import Prefix
 from pre_commit.util import CalledProcessError
 from testing.auto_namedtuple import auto_namedtuple
 
 
+@pytest.fixture
+def find_exe_mck():
+    with mock.patch.object(parse_shebang, 'find_executable') as mck:
+        yield mck
+
+
+@pytest.fixture
+def homedir_mck():
+    def fake_expanduser(pth):
+        assert pth == '~'
+        return os.path.normpath('/home/me')
+
+    with mock.patch.object(os.path, 'expanduser', fake_expanduser):
+        yield
+
+
+def test_exe_exists_does_not_exist(find_exe_mck, homedir_mck):
+    find_exe_mck.return_value = None
+    assert helpers.exe_exists('ruby') is False
+
+
+def test_exe_exists_exists(find_exe_mck, homedir_mck):
+    find_exe_mck.return_value = os.path.normpath('/usr/bin/ruby')
+    assert helpers.exe_exists('ruby') is True
+
+
+def test_exe_exists_false_if_shim(find_exe_mck, homedir_mck):
+    find_exe_mck.return_value = os.path.normpath('/foo/shims/ruby')
+    assert helpers.exe_exists('ruby') is False
+
+
+def test_exe_exists_false_if_homedir(find_exe_mck, homedir_mck):
+    find_exe_mck.return_value = os.path.normpath('/home/me/somedir/ruby')
+    assert helpers.exe_exists('ruby') is False
+
+
+def test_exe_exists_commonpath_raises_ValueError(find_exe_mck, homedir_mck):
+    find_exe_mck.return_value = os.path.normpath('/usr/bin/ruby')
+    with mock.patch.object(os.path, 'commonpath', side_effect=ValueError):
+        assert helpers.exe_exists('ruby') is True
+
+
 def test_basic_get_default_version():
     assert helpers.basic_get_default_version() == C.DEFAULT