show color in hook outputs when attached to a tty

This commit is contained in:
Anthony Sottile
2019-10-12 15:57:40 -07:00
parent c8620f35e1
commit 7c3404ef1f
27 changed files with 200 additions and 76 deletions

View File

@@ -73,7 +73,7 @@ SKIPPED = 'Skipped'
NO_FILES = '(no files to check)'
def _run_single_hook(classifier, hook, args, skips, cols):
def _run_single_hook(classifier, hook, args, skips, cols, use_color):
filenames = classifier.filenames_for_hook(hook)
if hook.language == 'pcre':
@@ -118,7 +118,8 @@ def _run_single_hook(classifier, hook, args, skips, cols):
sys.stdout.flush()
diff_before = cmd_output_b('git', 'diff', '--no-ext-diff', retcode=None)
retcode, out = hook.run(tuple(filenames) if hook.pass_filenames else ())
filenames = tuple(filenames) if hook.pass_filenames else ()
retcode, out = hook.run(filenames, use_color)
diff_after = cmd_output_b('git', 'diff', '--no-ext-diff', retcode=None)
file_modifications = diff_before != diff_after
@@ -203,7 +204,9 @@ def _run_hooks(config, hooks, args, environ):
classifier = Classifier(filenames)
retval = 0
for hook in hooks:
retval |= _run_single_hook(classifier, hook, args, skips, cols)
retval |= _run_single_hook(
classifier, hook, args, skips, cols, args.color,
)
if retval and config['fail_fast']:
break
if retval and args.show_diff_on_failure and git.has_diff():

View File

@@ -38,16 +38,17 @@ from pre_commit.languages import system
# version - A version specified in the hook configuration or 'default'.
# """
#
# def run_hook(hook, file_args):
# def run_hook(hook, file_args, color):
# """Runs a hook and returns the returncode and output of running that
# hook.
#
# Args:
# hook - `Hook`
# file_args - The files to be run
# color - whether the hook should be given a pty (when supported)
#
# Returns:
# (returncode, stdout, stderr)
# (returncode, output)
# """
languages = {

View File

@@ -95,15 +95,15 @@ def docker_cmd(): # pragma: windows no cover
)
def run_hook(hook, file_args): # pragma: windows no cover
def run_hook(hook, file_args, color): # pragma: windows no cover
assert_docker_available()
# Rebuild the docker image in case it has gone missing, as many people do
# automated cleanup of docker images.
build_docker_image(hook.prefix, pull=False)
hook_cmd = helpers.to_cmd(hook)
entry_exe, cmd_rest = hook_cmd[0], hook_cmd[1:]
hook_cmd = hook.cmd
entry_exe, cmd_rest = hook.cmd[0], hook_cmd[1:]
entry_tag = ('--entrypoint', entry_exe, docker_tag(hook.prefix))
cmd = docker_cmd() + entry_tag + cmd_rest
return helpers.run_xargs(hook, cmd, file_args)
return helpers.run_xargs(hook, cmd, file_args, color=color)

View File

@@ -12,7 +12,7 @@ healthy = helpers.basic_healthy
install_environment = helpers.no_install
def run_hook(hook, file_args): # pragma: windows no cover
def run_hook(hook, file_args, color): # pragma: windows no cover
assert_docker_available()
cmd = docker_cmd() + helpers.to_cmd(hook)
return helpers.run_xargs(hook, cmd, file_args)
cmd = docker_cmd() + hook.cmd
return helpers.run_xargs(hook, cmd, file_args, color=color)

View File

@@ -9,7 +9,7 @@ healthy = helpers.basic_healthy
install_environment = helpers.no_install
def run_hook(hook, file_args):
def run_hook(hook, file_args, color):
out = hook.entry.encode('UTF-8') + b'\n\n'
out += b'\n'.join(f.encode('UTF-8') for f in file_args) + b'\n'
return 1, out

View File

@@ -81,6 +81,6 @@ def install_environment(prefix, version, additional_dependencies):
rmtree(pkgdir)
def run_hook(hook, file_args):
def run_hook(hook, file_args, color):
with in_env(hook.prefix):
return helpers.run_xargs(hook, helpers.to_cmd(hook), file_args)
return helpers.run_xargs(hook, hook.cmd, file_args, color=color)

View File

@@ -3,7 +3,6 @@ from __future__ import unicode_literals
import multiprocessing
import os
import random
import shlex
import six
@@ -25,10 +24,6 @@ def environment_dir(ENVIRONMENT_DIR, language_version):
return '{}-{}'.format(ENVIRONMENT_DIR, language_version)
def to_cmd(hook):
return tuple(shlex.split(hook.entry)) + tuple(hook.args)
def assert_version_default(binary, version):
if version != C.DEFAULT:
raise AssertionError(
@@ -83,8 +78,9 @@ def _shuffled(seq):
return seq
def run_xargs(hook, cmd, file_args):
def run_xargs(hook, cmd, file_args, **kwargs):
# Shuffle the files so that they more evenly fill out the xargs partitions,
# but do it deterministically in case a hook cares about ordering.
file_args = _shuffled(file_args)
return xargs(cmd, file_args, target_concurrency=target_concurrency(hook))
kwargs['target_concurrency'] = target_concurrency(hook)
return xargs(cmd, file_args, **kwargs)

View File

@@ -78,6 +78,6 @@ def install_environment(
)
def run_hook(hook, file_args): # pragma: windows no cover
def run_hook(hook, file_args, color): # pragma: windows no cover
with in_env(hook.prefix, hook.language_version):
return helpers.run_xargs(hook, helpers.to_cmd(hook), file_args)
return helpers.run_xargs(hook, hook.cmd, file_args, color=color)

View File

@@ -13,10 +13,10 @@ healthy = helpers.basic_healthy
install_environment = helpers.no_install
def run_hook(hook, file_args):
def run_hook(hook, file_args, color):
# For PCRE the entry is the regular expression to match
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.
return xargs(cmd, file_args, negate=True)
return xargs(cmd, file_args, negate=True, color=color)

View File

@@ -44,9 +44,9 @@ def _process_filename_at_once(pattern, filename):
return retv
def run_hook(hook, file_args):
def run_hook(hook, file_args, color):
exe = (sys.executable, '-m', __name__) + tuple(hook.args) + (hook.entry,)
return xargs(exe, file_args)
return xargs(exe, file_args, color=color)
def main(argv=None):

View File

@@ -151,9 +151,9 @@ def py_interface(_dir, _make_venv):
)
return retcode == 0
def run_hook(hook, file_args):
def run_hook(hook, file_args, color):
with in_env(hook.prefix, hook.language_version):
return helpers.run_xargs(hook, helpers.to_cmd(hook), file_args)
return helpers.run_xargs(hook, hook.cmd, file_args, color=color)
def install_environment(prefix, version, additional_dependencies):
additional_dependencies = tuple(additional_dependencies)

View File

@@ -124,6 +124,6 @@ def install_environment(
)
def run_hook(hook, file_args): # pragma: windows no cover
def run_hook(hook, file_args, color): # pragma: windows no cover
with in_env(hook.prefix, hook.language_version):
return helpers.run_xargs(hook, helpers.to_cmd(hook), file_args)
return helpers.run_xargs(hook, hook.cmd, file_args, color=color)

View File

@@ -89,6 +89,6 @@ def install_environment(prefix, version, additional_dependencies):
)
def run_hook(hook, file_args):
def run_hook(hook, file_args, color):
with in_env(hook.prefix):
return helpers.run_xargs(hook, helpers.to_cmd(hook), file_args)
return helpers.run_xargs(hook, hook.cmd, file_args, color=color)

View File

@@ -9,7 +9,7 @@ healthy = helpers.basic_healthy
install_environment = helpers.no_install
def run_hook(hook, file_args):
cmd = helpers.to_cmd(hook)
def run_hook(hook, file_args, color):
cmd = hook.cmd
cmd = (hook.prefix.path(cmd[0]),) + cmd[1:]
return helpers.run_xargs(hook, cmd, file_args)
return helpers.run_xargs(hook, cmd, file_args, color=color)

View File

@@ -51,6 +51,6 @@ def install_environment(
)
def run_hook(hook, file_args): # pragma: windows no cover
def run_hook(hook, file_args, color): # pragma: windows no cover
with in_env(hook.prefix):
return helpers.run_xargs(hook, helpers.to_cmd(hook), file_args)
return helpers.run_xargs(hook, hook.cmd, file_args, color=color)

View File

@@ -9,5 +9,5 @@ healthy = helpers.basic_healthy
install_environment = helpers.no_install
def run_hook(hook, file_args):
return helpers.run_xargs(hook, helpers.to_cmd(hook), file_args)
def run_hook(hook, file_args, color):
return helpers.run_xargs(hook, hook.cmd, file_args, color=color)

View File

@@ -5,6 +5,7 @@ import io
import json
import logging
import os
import shlex
import pre_commit.constants as C
from pre_commit import five
@@ -54,6 +55,10 @@ _KEYS = tuple(item.key for item in MANIFEST_HOOK_DICT.items)
class Hook(collections.namedtuple('Hook', ('src', 'prefix') + _KEYS)):
__slots__ = ()
@property
def cmd(self):
return tuple(shlex.split(self.entry)) + tuple(self.args)
@property
def install_key(self):
return (
@@ -95,9 +100,9 @@ class Hook(collections.namedtuple('Hook', ('src', 'prefix') + _KEYS)):
# Write our state to indicate we're installed
_write_state(self.prefix, venv, _state(self.additional_dependencies))
def run(self, file_args):
def run(self, file_args, color):
lang = languages[self.language]
return lang.run_hook(self, file_args)
return lang.run_hook(self, file_args, color)
@classmethod
def create(cls, src, prefix, dct):

View File

@@ -117,29 +117,28 @@ class CalledProcessError(RuntimeError):
__str__ = to_text
def cmd_output_b(*cmd, **kwargs):
retcode = kwargs.pop('retcode', 0)
popen_kwargs = {
'stdin': subprocess.PIPE,
'stdout': subprocess.PIPE,
'stderr': subprocess.PIPE,
}
def _cmd_kwargs(*cmd, **kwargs):
# py2/py3 on windows are more strict about the types here
cmd = tuple(five.n(arg) for arg in cmd)
kwargs['env'] = {
five.n(key): five.n(value)
for key, value in kwargs.pop('env', {}).items()
} or None
popen_kwargs.update(kwargs)
for arg in ('stdin', 'stdout', 'stderr'):
kwargs.setdefault(arg, subprocess.PIPE)
return cmd, kwargs
def cmd_output_b(*cmd, **kwargs):
retcode = kwargs.pop('retcode', 0)
cmd, kwargs = _cmd_kwargs(*cmd, **kwargs)
try:
cmd = parse_shebang.normalize_cmd(cmd)
except parse_shebang.ExecutableNotFoundError as e:
returncode, stdout_b, stderr_b = e.to_output()
else:
proc = subprocess.Popen(cmd, **popen_kwargs)
proc = subprocess.Popen(cmd, **kwargs)
stdout_b, stderr_b = proc.communicate()
returncode = proc.returncode
@@ -158,6 +157,72 @@ def cmd_output(*cmd, **kwargs):
return returncode, stdout, stderr
if os.name != 'nt': # pragma: windows no cover
from os import openpty
import termios
class Pty(object):
def __init__(self):
self.r = self.w = None
def __enter__(self):
self.r, self.w = openpty()
# tty flags normally change \n to \r\n
attrs = termios.tcgetattr(self.r)
attrs[1] &= ~(termios.ONLCR | termios.OPOST)
termios.tcsetattr(self.r, termios.TCSANOW, attrs)
return self
def close_w(self):
if self.w is not None:
os.close(self.w)
self.w = None
def close_r(self):
assert self.r is not None
os.close(self.r)
self.r = None
def __exit__(self, exc_type, exc_value, traceback):
self.close_w()
self.close_r()
def cmd_output_p(*cmd, **kwargs):
assert kwargs.pop('retcode') is None
assert kwargs['stderr'] == subprocess.STDOUT, kwargs['stderr']
cmd, kwargs = _cmd_kwargs(*cmd, **kwargs)
try:
cmd = parse_shebang.normalize_cmd(cmd)
except parse_shebang.ExecutableNotFoundError as e:
return e.to_output()
with open(os.devnull) as devnull, Pty() as pty:
kwargs.update({'stdin': devnull, 'stdout': pty.w, 'stderr': pty.w})
proc = subprocess.Popen(cmd, **kwargs)
pty.close_w()
buf = b''
while True:
try:
bts = os.read(pty.r, 4096)
except OSError as e:
if e.errno == errno.EIO:
bts = b''
else:
raise
else:
buf += bts
if not bts:
break
return proc.wait(), buf, None
else: # pragma: no cover
cmd_output_p = cmd_output_b
def rmtree(path):
"""On windows, rmtree fails for readonly dirs."""
def handle_remove_readonly(func, path, exc):

View File

@@ -13,6 +13,7 @@ import six
from pre_commit import parse_shebang
from pre_commit.util import cmd_output_b
from pre_commit.util import cmd_output_p
def _environ_size(_env=None):
@@ -108,9 +109,11 @@ def xargs(cmd, varargs, **kwargs):
negate: Make nonzero successful and zero a failure
target_concurrency: Target number of partitions to run concurrently
"""
color = kwargs.pop('color', False)
negate = kwargs.pop('negate', False)
target_concurrency = kwargs.pop('target_concurrency', 1)
max_length = kwargs.pop('_max_length', _get_platform_max_length())
cmd_fn = cmd_output_p if color else cmd_output_b
retcode = 0
stdout = b''
@@ -122,7 +125,7 @@ def xargs(cmd, varargs, **kwargs):
partitions = partition(cmd, varargs, target_concurrency, max_length)
def run_cmd_partition(run_cmd):
return cmd_output_b(
return cmd_fn(
*run_cmd, retcode=None, stderr=subprocess.STDOUT, **kwargs
)

View File

@@ -1,4 +1,8 @@
- id: stdout-stderr
name: stdout-stderr
language: script
entry: ./entry
entry: ./stdout-stderr-entry
- id: tty-check
name: tty-check
language: script
entry: ./tty-check-entry

View File

@@ -0,0 +1,12 @@
#!/usr/bin/env python
import sys
def main():
print('stdin: {}'.format(sys.stdin.isatty()))
print('stdout: {}'.format(sys.stdout.isatty()))
print('stderr: {}'.format(sys.stderr.isatty()))
if __name__ == '__main__':
exit(main())

View File

@@ -39,7 +39,7 @@ def test_ENVIRONMENT_DIR(language):
@pytest.mark.parametrize('language', all_languages)
def test_run_hook_argpsec(language):
expected_argspec = ArgSpec(args=['hook', 'file_args'])
expected_argspec = ArgSpec(args=['hook', 'file_args', 'color'])
argspec = getargspec(languages[language].run_hook)
assert argspec == expected_argspec

View File

@@ -91,7 +91,7 @@ def test_normexe_does_not_exist_sep():
assert excinfo.value.args == ('Executable `./i-dont-exist-lol` not found',)
@pytest.mark.xfail(os.name == 'nt', reason='posix only',)
@pytest.mark.xfail(os.name == 'nt', reason='posix only')
def test_normexe_not_executable(tmpdir): # pragma: windows no cover
tmpdir.join('exe').ensure()
with tmpdir.as_cwd(), pytest.raises(OSError) as excinfo:

View File

@@ -70,10 +70,11 @@ def _test_hook_repo(
expected,
expected_return_code=0,
config_kwargs=None,
color=False,
):
path = make_repo(tempdir_factory, repo_path)
config = make_config_from_repo(path, **(config_kwargs or {}))
ret, out = _get_hook(config, store, hook_id).run(args)
ret, out = _get_hook(config, store, hook_id).run(args, color=color)
assert ret == expected_return_code
assert _norm_out(out) == expected
@@ -137,7 +138,8 @@ def test_switch_language_versions_doesnt_clobber(tempdir_factory, store):
def run_on_version(version, expected_output):
config = make_config_from_repo(path)
config['hooks'][0]['language_version'] = version
ret, out = _get_hook(config, store, 'python3-hook').run([])
hook = _get_hook(config, store, 'python3-hook')
ret, out = hook.run([], color=False)
assert ret == 0
assert _norm_out(out) == expected_output
@@ -373,6 +375,17 @@ def test_intermixed_stdout_stderr(tempdir_factory, store):
)
@pytest.mark.xfail(os.name == 'nt', reason='ptys are posix-only')
def test_output_isatty(tempdir_factory, store):
_test_hook_repo(
tempdir_factory, store, 'stdout_stderr_repo',
'tty-check',
[],
b'stdin: False\nstdout: True\nstderr: True\n',
color=True,
)
def _make_grep_repo(language, entry, store, args=()):
config = {
'repo': 'local',
@@ -403,20 +416,20 @@ class TestPygrep(object):
def test_grep_hook_matching(self, greppable_files, store):
hook = _make_grep_repo(self.language, 'ello', store)
ret, out = hook.run(('f1', 'f2', 'f3'))
ret, out = hook.run(('f1', 'f2', 'f3'), color=False)
assert ret == 1
assert _norm_out(out) == b"f1:1:hello'hi\n"
def test_grep_hook_case_insensitive(self, greppable_files, store):
hook = _make_grep_repo(self.language, 'ELLO', store, args=['-i'])
ret, out = hook.run(('f1', 'f2', 'f3'))
ret, out = hook.run(('f1', 'f2', 'f3'), color=False)
assert ret == 1
assert _norm_out(out) == b"f1:1:hello'hi\n"
@pytest.mark.parametrize('regex', ('nope', "foo'bar", r'^\[INFO\]'))
def test_grep_hook_not_matching(self, regex, greppable_files, store):
hook = _make_grep_repo(self.language, regex, store)
ret, out = hook.run(('f1', 'f2', 'f3'))
ret, out = hook.run(('f1', 'f2', 'f3'), color=False)
assert (ret, out) == (0, b'')
@@ -430,7 +443,7 @@ class TestPCRE(TestPygrep):
# file to make sure it still fails. This is not the case when naively
# using a system hook with `grep -H -n '...'`
hook = _make_grep_repo('pcre', 'ello', store)
ret, out = hook.run((os.devnull,) * 15000 + ('f1',))
ret, out = hook.run((os.devnull,) * 15000 + ('f1',), color=False)
assert ret == 1
assert _norm_out(out) == b"f1:1:hello'hi\n"
@@ -441,7 +454,7 @@ class TestPCRE(TestPygrep):
with mock.patch.object(parse_shebang, 'find_executable', no_grep):
hook = _make_grep_repo('pcre', 'ello', store)
ret, out = hook.run(('f1', 'f2', 'f3'))
ret, out = hook.run(('f1', 'f2', 'f3'), color=False)
assert ret == 1
expected = 'Executable `{}` not found'.format(pcre.GREP).encode()
assert out == expected
@@ -543,7 +556,7 @@ def test_local_golang_additional_dependencies(store):
'additional_dependencies': ['github.com/golang/example/hello'],
}],
}
ret, out = _get_hook(config, store, 'hello').run(())
ret, out = _get_hook(config, store, 'hello').run((), color=False)
assert ret == 0
assert _norm_out(out) == b'Hello, Go examples!\n'
@@ -559,7 +572,7 @@ def test_local_rust_additional_dependencies(store):
'additional_dependencies': ['cli:hello-cli:0.2.2'],
}],
}
ret, out = _get_hook(config, store, 'hello').run(())
ret, out = _get_hook(config, store, 'hello').run((), color=False)
assert ret == 0
assert _norm_out(out) == b'Hello World!\n'
@@ -576,12 +589,12 @@ def test_fail_hooks(store):
}],
}
hook = _get_hook(config, store, 'fail')
ret, out = hook.run(('changelog/1234.bugfix', 'changelog/wat'))
ret, out = hook.run(('changelog/123.bugfix', 'changelog/wat'), color=False)
assert ret == 1
assert out == (
b'make sure to name changelogs as .rst!\n'
b'\n'
b'changelog/1234.bugfix\n'
b'changelog/123.bugfix\n'
b'changelog/wat\n'
)
@@ -645,7 +658,7 @@ def test_control_c_control_c_on_install(tempdir_factory, store):
# However, it should be perfectly runnable (reinstall after botched
# install)
install_hook_envs(hooks, store)
ret, out = hook.run(())
ret, out = hook.run((), color=False)
assert ret == 0
@@ -667,7 +680,7 @@ def test_invalidated_virtualenv(tempdir_factory, store):
cmd_output_b('rm', '-rf', *paths)
# pre-commit should rebuild the virtualenv and it should be runnable
ret, out = _get_hook(config, store, 'foo').run(())
ret, out = _get_hook(config, store, 'foo').run((), color=False)
assert ret == 0
@@ -707,12 +720,14 @@ def test_tags_on_repositories(in_tmpdir, tempdir_factory, store):
git2 = _create_repo_with_tags(tempdir_factory, 'script_hooks_repo', tag)
config1 = make_config_from_repo(git1, rev=tag)
ret1, out1 = _get_hook(config1, store, 'prints_cwd').run(('-L',))
hook1 = _get_hook(config1, store, 'prints_cwd')
ret1, out1 = hook1.run(('-L',), color=False)
assert ret1 == 0
assert out1.strip() == _norm_pwd(in_tmpdir)
config2 = make_config_from_repo(git2, rev=tag)
ret2, out2 = _get_hook(config2, store, 'bash_hook').run(('bar',))
hook2 = _get_hook(config2, store, 'bash_hook')
ret2, out2 = hook2.run(('bar',), color=False)
assert ret2 == 0
assert out2 == b'bar\nHello World\n'
@@ -736,7 +751,7 @@ def test_local_python_repo(store, local_python_config):
hook = _get_hook(local_python_config, store, 'foo')
# language_version should have been adjusted to the interpreter version
assert hook.language_version != C.DEFAULT
ret, out = hook.run(('filename',))
ret, out = hook.run(('filename',), color=False)
assert ret == 0
assert _norm_out(out) == b"['filename']\nHello World\n"

View File

@@ -2,12 +2,14 @@ from __future__ import unicode_literals
import os.path
import stat
import subprocess
import pytest
from pre_commit.util import CalledProcessError
from pre_commit.util import clean_path_on_failure
from pre_commit.util import cmd_output
from pre_commit.util import cmd_output_p
from pre_commit.util import parse_version
from pre_commit.util import rmtree
from pre_commit.util import tmpdir
@@ -83,9 +85,15 @@ def test_tmpdir():
def test_cmd_output_exe_not_found():
ret, out, _ = cmd_output('i-dont-exist', retcode=None)
ret, out, _ = cmd_output('dne', retcode=None)
assert ret == 1
assert out == 'Executable `i-dont-exist` not found'
assert out == 'Executable `dne` not found'
def test_cmd_output_p_exe_not_found():
ret, out, _ = cmd_output_p('dne', retcode=None, stderr=subprocess.STDOUT)
assert ret == 1
assert out == b'Executable `dne` not found'
def test_parse_version():

View File

@@ -3,6 +3,7 @@ from __future__ import absolute_import
from __future__ import unicode_literals
import concurrent.futures
import os
import sys
import time
@@ -217,3 +218,14 @@ def test_xargs_propagate_kwargs_to_cmd():
ret, stdout = xargs.xargs(cmd, ('1',), env=env)
assert ret == 0
assert b'Pre commit is awesome' in stdout
@pytest.mark.xfail(os.name == 'nt', reason='posix only')
def test_xargs_color_true_makes_tty():
retcode, out = xargs.xargs(
(sys.executable, '-c', 'import sys; print(sys.stdout.isatty())'),
('1',),
color=True,
)
assert retcode == 0
assert out == b'True\n'