diff --git a/pre_commit/error_handler.py b/pre_commit/error_handler.py index 60038f40..85d5602e 100644 --- a/pre_commit/error_handler.py +++ b/pre_commit/error_handler.py @@ -18,8 +18,19 @@ class PreCommitSystemExit(SystemExit): pass +def _to_bytes(exc): + try: + return bytes(exc) + except Exception: + return five.text(exc).encode('UTF-8') + + def _log_and_exit(msg, exc, formatted, write_fn=sys_stdout_write_wrapper): - error_msg = '{0}: {1}: {2}\n'.format(msg, type(exc).__name__, exc) + error_msg = b''.join(( + five.to_bytes(msg), b': ', + five.to_bytes(type(exc).__name__), b': ', + _to_bytes(exc), b'\n', + )) write_fn(error_msg) write_fn('Check the log at ~/.pre-commit/pre-commit.log\n') store = Store() diff --git a/pre_commit/languages/node.py b/pre_commit/languages/node.py index 1cf0d999..99b46df7 100644 --- a/pre_commit/languages/node.py +++ b/pre_commit/languages/node.py @@ -45,13 +45,14 @@ def install_environment( repo_cmd_runner.run(cmd) with in_env(repo_cmd_runner, version) as node_env: - node_env.run("cd '{prefix}' && npm install -g") + node_env.run("cd '{prefix}' && npm install -g", encoding=None) if additional_dependencies: node_env.run( "cd '{prefix}' && npm install -g " + ' '.join( shell_escape(dep) for dep in additional_dependencies - ) + ), + encoding=None, ) diff --git a/pre_commit/languages/python.py b/pre_commit/languages/python.py index 0ee0c04b..8e81f7b9 100644 --- a/pre_commit/languages/python.py +++ b/pre_commit/languages/python.py @@ -63,13 +63,14 @@ def install_environment( venv_cmd.extend(['-p', norm_version(version)]) repo_cmd_runner.run(venv_cmd) with in_env(repo_cmd_runner, version) as env: - env.run("cd '{prefix}' && pip install .") + env.run("cd '{prefix}' && pip install .", encoding=None) if additional_dependencies: env.run( "cd '{prefix}' && pip install " + ' '.join( shell_escape(dep) for dep in additional_dependencies - ) + ), + encoding=None, ) diff --git a/pre_commit/languages/ruby.py b/pre_commit/languages/ruby.py index aedf0bc6..a23df5d3 100644 --- a/pre_commit/languages/ruby.py +++ b/pre_commit/languages/ruby.py @@ -95,13 +95,15 @@ def install_environment( ruby_env.run( 'cd {prefix} && gem build *.gemspec && ' 'gem install --no-ri --no-rdoc *.gem', + encoding=None, ) if additional_dependencies: ruby_env.run( 'cd {prefix} && gem install --no-ri --no-rdoc ' + ' '.join( shell_escape(dep) for dep in additional_dependencies - ) + ), + encoding=None, ) diff --git a/pre_commit/util.py b/pre_commit/util.py index 30089463..3736d2e5 100644 --- a/pre_commit/util.py +++ b/pre_commit/util.py @@ -124,26 +124,38 @@ class CalledProcessError(RuntimeError): self.expected_returncode = expected_returncode self.output = output - def __str__(self): + def to_bytes(self): output = [] - for text in self.output: - if text: - output.append('\n ' + text.replace('\n', '\n ')) + for maybe_text in self.output: + if maybe_text: + output.append( + b'\n ' + + five.to_bytes(maybe_text).replace(b'\n', b'\n ') + ) else: - output.append('(none)') + output.append(b'(none)') - return ( - 'Command: {0!r}\n' - 'Return code: {1}\n' - 'Expected return code: {2}\n' - 'Output: {3}\n' - 'Errors: {4}\n'.format( - self.cmd, - self.returncode, - self.expected_returncode, - *output - ) - ) + return b''.join(( + five.to_bytes( + 'Command: {0!r}\n' + 'Return code: {1}\n' + 'Expected return code: {2}\n'.format( + self.cmd, self.returncode, self.expected_returncode + ) + ), + b'Output: ', output[0], b'\n', + b'Errors: ', output[1], b'\n', + )) + + def to_text(self): + return self.to_bytes().decode('UTF-8') + + if five.PY3: # pragma: no cover + __bytes__ = to_bytes + __str__ = to_text + else: + __str__ = to_bytes + __unicode__ = to_text def cmd_output(*cmd, **kwargs): diff --git a/testing/resources/not_installable_repo/hooks.yaml b/testing/resources/not_installable_repo/hooks.yaml new file mode 100644 index 00000000..48c1f9ef --- /dev/null +++ b/testing/resources/not_installable_repo/hooks.yaml @@ -0,0 +1,6 @@ +- id: foo + name: Foo + entry: foo + language: python + language_version: python2.7 + files: \.py$ diff --git a/testing/resources/not_installable_repo/setup.py b/testing/resources/not_installable_repo/setup.py new file mode 100644 index 00000000..ae5f6338 --- /dev/null +++ b/testing/resources/not_installable_repo/setup.py @@ -0,0 +1,17 @@ +# -*- coding: utf-8 -*- +from __future__ import print_function +from __future__ import unicode_literals + +import sys + + +def main(): + # Intentionally write mixed encoding to the output. This should not crash + # pre-commit and should write bytes to the output. + sys.stderr.write('☃'.encode('UTF-8') + '²'.encode('latin1') + b'\n') + # Return 1 to indicate failures + return 1 + + +if __name__ == '__main__': + exit(main()) diff --git a/tests/commands/run_test.py b/tests/commands/run_test.py index a0795f28..363743fb 100644 --- a/tests/commands/run_test.py +++ b/tests/commands/run_test.py @@ -371,6 +371,33 @@ def test_stdout_write_bug_py26( assert 'UnicodeDecodeError' not in stdout +def test_hook_install_failure(mock_out_store_directory, tempdir_factory): + git_path = make_consuming_repo(tempdir_factory, 'not_installable_repo') + with cwd(git_path): + install(Runner(git_path)) + + # Don't want to write to home directory + env = dict(os.environ, PRE_COMMIT_HOME=tempdir_factory.get()) + _, stdout, _ = cmd_output( + 'git', 'commit', '-m', 'Commit!', + # git commit puts pre-commit to stderr + stderr=subprocess.STDOUT, + env=env, + retcode=None, + encoding=None, + ) + assert b'UnicodeDecodeError' not in stdout + # Doesn't actually happen, but a reasonable assertion + assert b'UnicodeEncodeError' not in stdout + + # Sanity check our output + assert ( + b'An unexpected error has occurred: CalledProcessError: ' in + stdout + ) + assert '☃'.encode('UTF-8') + '²'.encode('latin1') in stdout + + def test_get_changed_files(): files = get_changed_files( '78c682a1d13ba20e7cb735313b9314a74365cd3a', diff --git a/tests/error_handler_test.py b/tests/error_handler_test.py index d8f966a8..d63511b7 100644 --- a/tests/error_handler_test.py +++ b/tests/error_handler_test.py @@ -11,6 +11,7 @@ import mock import pytest from pre_commit import error_handler +from pre_commit import five from pre_commit.errors import FatalError from pre_commit.util import cmd_output @@ -82,7 +83,9 @@ def test_log_and_exit(mock_out_store_directory): write_fn=mocked_write, ) - printed = ''.join(call[0][0] for call in mocked_write.call_args_list) + printed = ''.join( + five.to_text(call[0][0]) for call in mocked_write.call_args_list + ) assert printed == ( 'msg: FatalError: hai\n' 'Check the log at ~/.pre-commit/pre-commit.log\n'