xargs returns nonzero for negate + not found exe (fixes pcre + not found pre-commit#447)

asottile · asottile · commit a157e1a63f2b · 2016-12-04T15:07:35.000-08:00
diff --git a/pre_commit/languages/pcre.py b/pre_commit/languages/pcre.py
@@ -1,11 +1,12 @@
 from __future__ import unicode_literals
 
-from sys import platform
+import sys
 
 from pre_commit.xargs import xargs
 
 
 ENVIRONMENT_DIR = None
+GREP = 'ggrep' if sys.platform == 'darwin' else 'grep'
 
 
 def install_environment(
@@ -19,10 +20,7 @@ def install_environment(
 
 def run_hook(repo_cmd_runner, hook, file_args):
     # For PCRE the entry is the regular expression to match
-    cmd = (
-        'ggrep' if platform == 'darwin' else 'grep',
-        '-H', '-n', '-P',
-    ) + tuple(hook['args']) + (hook['entry'],)
+    cmd = (GREP, '-H', '-n', '-P') + tuple(hook['args']) + (hook['entry'],)
 
     # Grep usually returns 0 for matches, and nonzero for non-matches so we
     # negate it here.
diff --git a/pre_commit/parse_shebang.py b/pre_commit/parse_shebang.py
@@ -11,7 +11,8 @@
 
 
 class ExecutableNotFoundError(OSError):
-    pass
+    def to_output(self):
+        return (1, self.args[0].encode('UTF-8'), b'')
 
 
 def parse_bytesio(bytesio):
diff --git a/pre_commit/util.py b/pre_commit/util.py
@@ -172,16 +172,16 @@ def cmd_output(*cmd, **kwargs):
     try:
         cmd = parse_shebang.normalize_cmd(cmd)
     except parse_shebang.ExecutableNotFoundError as e:
-        returncode, stdout, stderr = (-1, e.args[0].encode('UTF-8'), b'')
+        returncode, stdout, stderr = e.to_output()
     else:
         popen_kwargs.update(kwargs)
         proc = __popen(cmd, **popen_kwargs)
         stdout, stderr = proc.communicate()
-        if encoding is not None and stdout is not None:
-            stdout = stdout.decode(encoding)
-        if encoding is not None and stderr is not None:
-            stderr = stderr.decode(encoding)
         returncode = proc.returncode
+    if encoding is not None and stdout is not None:
+        stdout = stdout.decode(encoding)
+    if encoding is not None and stderr is not None:
+        stderr = stderr.decode(encoding)
 
     if retcode is not None and retcode != returncode:
         raise CalledProcessError(
diff --git a/pre_commit/xargs.py b/pre_commit/xargs.py
@@ -1,6 +1,7 @@
 from __future__ import absolute_import
 from __future__ import unicode_literals
 
+from pre_commit import parse_shebang
 from pre_commit.util import cmd_output
 
 
@@ -52,6 +53,11 @@ def xargs(cmd, varargs, **kwargs):
     stdout = b''
     stderr = b''
 
+    try:
+        parse_shebang.normexe(cmd[0])
+    except parse_shebang.ExecutableNotFoundError as e:
+        return e.to_output()
+
     for run_cmd in partition(cmd, varargs, **kwargs):
         proc_retcode, proc_out, proc_err = cmd_output(
             *run_cmd, encoding=None, retcode=None
diff --git a/tests/repository_test.py b/tests/repository_test.py
@@ -12,11 +12,13 @@
 import pytest
 
 from pre_commit import five
+from pre_commit import parse_shebang
 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 pcre
 from pre_commit.languages import python
 from pre_commit.languages import ruby
 from pre_commit.repository import Repository
@@ -187,6 +189,25 @@ def test_missing_executable(tempdir_factory, store):
     )
 
 
+@pytest.mark.integration
+def test_missing_pcre_support(tempdir_factory, store):
+    orig_find_executable = parse_shebang.find_executable
+
+    def no_grep(exe, **kwargs):
+        if exe == pcre.GREP:
+            return None
+        else:
+            return orig_find_executable(exe, **kwargs)
+
+    with mock.patch.object(parse_shebang, 'find_executable', no_grep):
+        _test_hook_repo(
+            tempdir_factory, store, 'pcre_hooks_repo',
+            'regex-with-quotes', ['/dev/null'],
+            'Executable `{}` not found'.format(pcre.GREP).encode('UTF-8'),
+            expected_return_code=1,
+        )
+
+
 @pytest.mark.integration
 def test_run_a_script_hook(tempdir_factory, store):
     _test_hook_repo(
diff --git a/tests/util_test.py b/tests/util_test.py
@@ -6,6 +6,7 @@
 import pytest
 
 from pre_commit.util import clean_path_on_failure
+from pre_commit.util import cmd_output
 from pre_commit.util import cwd
 from pre_commit.util import memoize_by_cwd
 from pre_commit.util import tmpdir
@@ -81,3 +82,9 @@ def test_tmpdir():
     with tmpdir() as tempdir:
         assert os.path.exists(tempdir)
     assert not os.path.exists(tempdir)
+
+
+def test_cmd_output_exe_not_found():
+    ret, out, _ = cmd_output('i-dont-exist', retcode=None)
+    assert ret == 1
+    assert out == 'Executable `i-dont-exist` not found'
diff --git a/tests/xargs_test.py b/tests/xargs_test.py
@@ -64,6 +64,11 @@ def test_xargs_negate():
     assert ret == 1
 
 
+def test_xargs_negate_command_not_found():
+    ret, _, _ = xargs.xargs(('cmd-not-found',), ('1',), negate=True)
+    assert ret != 0
+
+
 def test_xargs_retcode_normal():
     ret, _, _ = xargs.xargs(exit_cmd, ('0',), _max_length=max_length)
     assert ret == 0
