Repository: pre-commit/pre-commit
Pull request #1117 (2019-08-15 06:17:35)

Stabilize python default version lookup
For example, for sys.executable:
>   /usr/bin/python3 -> python3.7

...the default lookup may return either `python3` or `python3.7`. Make the order deterministic by iterating over tuple, not set, of candidates.

scop: An effect of the instability is that pre-commit will end up installing dependencies several times in separate redundant virtualenvs over multiple runs, depending on which version happened to be returned.

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 697e3cd..ba491cf 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -261,7 +261,7 @@
 
 ### Features
 - Run hooks in parallel
-    - individual hooks may opt out of parallel execution with `require_serial: true`
+    - individual hooks may opt out of parallel exection with `require_serial: true`
     - #510 issue by @chriskuehl.
     - #851 PR by @chriskuehl.
 
@@ -440,7 +440,7 @@
 ### Fixes
 - Fix integration with go 1.10 and `pkg` directory
     - #725 PR by @asottile
-- Restore support for `git<1.8.5` (inadvertently removed in 1.7.0)
+- Restore support for `git<1.8.5` (inadvertantly removed in 1.7.0)
     - #723 issue by @JohnLyman.
     - #724 PR by @asottile.
 
diff --git a/pre_commit/languages/python.py b/pre_commit/languages/python.py
index 5d48fb8..6d125a4 100644
--- a/pre_commit/languages/python.py
+++ b/pre_commit/languages/python.py
@@ -43,14 +43,13 @@ def _find_by_py_launcher(version):  # pragma: no cover (windows only)
             pass
 
 
-def _get_default_version():  # pragma: no cover (platform dependent)
+def _find_by_sys_executable():
     def _norm(path):
         _, exe = os.path.split(path.lower())
         exe, _, _ = exe.partition('.exe')
         if find_executable(exe) and exe not in {'python', 'pythonw'}:
             return exe
 
-    # First attempt from `sys.executable` (or the realpath)
     # On linux, I see these common sys.executables:
     #
     # system `python`: /usr/bin/python -> python2.7
@@ -59,10 +58,18 @@ def _get_default_version():  # pragma: no cover (platform dependent)
     # virtualenv v -ppython2: v/bin/python -> python2
     # virtualenv v -ppython2.7: v/bin/python -> python2.7
     # virtualenv v -ppypy: v/bin/python -> v/bin/pypy
-    for path in {sys.executable, os.path.realpath(sys.executable)}:
+    for path in (sys.executable, os.path.realpath(sys.executable)):
         exe = _norm(path)
         if exe:
             return exe
+    return None
+
+
+def _get_default_version():  # pragma: no cover (platform dependent)
+    # First attempt from `sys.executable` (or the realpath)
+    exe = _find_by_sys_executable()
+    if exe:
+        return exe
 
     # Next try the `pythonX.X` executable
     exe = 'python{}.{}'.format(*sys.version_info)


Test patch:
diff --git a/tests/languages/python_test.py b/tests/languages/python_test.py
index 426d3ec..d9d8ecd 100644
--- a/tests/languages/python_test.py
+++ b/tests/languages/python_test.py
@@ -32,3 +32,19 @@ def test_sys_executable_matches(v):
 def test_sys_executable_matches_does_not_match(v):
     with mock.patch.object(sys, 'version_info', (3, 6, 7)):
         assert not python._sys_executable_matches(v)
+
+
+@pytest.mark.parametrize(
+    ('exe', 'realpath', 'expected'), (
+        ('/usr/bin/python3', '/usr/bin/python3.7', 'python3'),
+        ('/usr/bin/python', '/usr/bin/python3.7', 'python3.7'),
+        ('/usr/bin/python', '/usr/bin/python', None),
+        ('/usr/bin/python3.6m', '/usr/bin/python3.6m', 'python3.6m'),
+        ('v/bin/python', 'v/bin/pypy', 'pypy'),
+    ),
+)
+def test_find_by_sys_executable(exe, realpath, expected):
+    with mock.patch.object(sys, 'executable', exe):
+        with mock.patch.object(os.path, 'realpath', return_value=realpath):
+            with mock.patch.object(python, 'find_executable', lambda x: x):
+                assert python._find_by_sys_executable() == expected
diff --git a/tests/staged_files_only_test.py b/tests/staged_files_only_test.py
index 107c149..2410bff 100644
--- a/tests/staged_files_only_test.py
+++ b/tests/staged_files_only_test.py
@@ -328,7 +328,7 @@ def test_whitespace_errors(in_git_dir, patch_dir):
     test_crlf(in_git_dir, patch_dir, True, True, 'true')
 
 
-def test_autocrlf_committed_crlf(in_git_dir, patch_dir):
+def test_autocrlf_commited_crlf(in_git_dir, patch_dir):
     """Regression test for #570"""
     cmd_output('git', 'config', '--local', 'core.autocrlf', 'false')
     _write(b'1\r\n2\r\n')