Skip to content

Commit 66b1d39

Browse files
asottileasottile
committed
Allow arbitrary bytes in output. Resolves pre-commit#245 
1 parent 826aa4c commit 66b1d39

File tree

11 files changed

+104
-50
lines changed

11 files changed

+104
-50
lines changed

‎pre_commit/commands/run.py‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,8 +102,9 @@ def _run_single_hook(hook, repo, args, write, skips=frozenset()):
102102
write('hookid: {0}\n'.format(hook['id']))
103103
write('\n')
104104
for output in (stdout, stderr):
105+
assert type(output) is bytes, type(output)
105106
if output.strip():
106-
write(output.strip() + '\n')
107+
write(output.strip() + b'\n')
107108
write('\n')
108109

109110
return retcode

‎pre_commit/languages/helpers.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ def run_hook(env, hook, file_args):
2222
' '.join(['xargs', '-0', '-s4000', hook['entry']] + quoted_args),
2323
stdin=file_args_to_stdin(file_args),
2424
retcode=None,
25+
encoding=None,
2526
)
2627

2728

‎pre_commit/languages/pcre.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,4 +24,5 @@ def run_hook(repo_cmd_runner, hook, file_args):
2424
],
2525
stdin=file_args_to_stdin(file_args),
2626
retcode=None,
27+
encoding=None,
2728
)

‎pre_commit/languages/script.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,4 +17,5 @@ def run_hook(repo_cmd_runner, hook, file_args):
1717
# TODO: this is duplicated in pre_commit/languages/helpers.py
1818
stdin=file_args_to_stdin(file_args),
1919
retcode=None,
20+
encoding=None,
2021
)

‎pre_commit/languages/system.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,4 +18,5 @@ def run_hook(repo_cmd_runner, hook, file_args):
1818
['xargs', '-0'] + shlex.split(hook['entry']) + hook['args'],
1919
stdin=file_args_to_stdin(file_args),
2020
retcode=None,
21+
encoding=None,
2122
)

‎testing/resources/arbitrary_bytes_repo/hooks.yaml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
- id: python3-hook
2+
name: Python 3 Hook
3+
entry: python3-hook
4+
language: python
5+
language_version: python3.3
6+
files: \.py$

‎testing/resources/arbitrary_bytes_repo/python3_hook/__init__.py‎

Whitespace-only changes.

‎testing/resources/arbitrary_bytes_repo/python3_hook/main.py‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
# -*- coding: utf-8 -*-
2+
from __future__ import print_function
3+
from __future__ import unicode_literals
4+
5+
import sys
6+
7+
8+
def func():
9+
# Intentionally write mixed encoding to the output. This should not crash
10+
# pre-commit and should write bytes to the output.
11+
sys.stdout.buffer.write('☃'.encode('UTF-8') + '²'.encode('latin1') + b'\n')
12+
# Return 1 to trigger printing
13+
return 1

‎testing/resources/arbitrary_bytes_repo/setup.py‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
from setuptools import find_packages
2+
from setuptools import setup
3+
4+
setup(
5+
name='python3_hook',
6+
version='0.0.0',
7+
packages=find_packages('.'),
8+
entry_points={
9+
'console_scripts': ['python3-hook = python3_hook.main:func'],
10+
},
11+
)

‎tests/commands/run_test.py‎

Lines changed: 43 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
# -*- coding: UTF-8 -*-
22
from __future__ import unicode_literals
33

4+
import functools
45
import io
56
import os
67
import os.path
@@ -16,6 +17,7 @@
1617
from pre_commit.commands.run import get_changed_files
1718
from pre_commit.commands.run import run
1819
from pre_commit.ordereddict import OrderedDict
20+
from pre_commit.output import sys_stdout_write_wrapper
1921
from pre_commit.runner import Runner
2022
from pre_commit.util import cmd_output
2123
from pre_commit.util import cwd
@@ -44,7 +46,7 @@ def stage_a_file():
4446

4547

4648
def get_write_mock_output(write_mock):
47-
return ''.join(call[0][0] for call in write_mock.call_args_list)
49+
return b''.join(call[0][0] for call in write_mock.write.call_args_list)
4850

4951

5052
def _get_opts(
@@ -76,7 +78,8 @@ def _get_opts(
7678
def _do_run(repo, args, environ={}):
7779
runner = Runner(repo)
7880
write_mock = mock.Mock()
79-
ret = run(runner, args, write=write_mock, environ=environ)
81+
write_fn = functools.partial(sys_stdout_write_wrapper, stream=write_mock)
82+
ret = run(runner, args, write=write_fn, environ=environ)
8083
printed = get_write_mock_output(write_mock)
8184
return ret, printed
8285

@@ -97,32 +100,43 @@ def test_run_all_hooks_failing(
97100
_test_run(
98101
repo_with_failing_hook,
99102
{},
100-
('Failing hook', 'Failed', 'hookid: failing_hook', 'Fail\nfoo.py\n'),
103+
(
104+
b'Failing hook',
105+
b'Failed',
106+
b'hookid: failing_hook',
107+
b'Fail\nfoo.py\n',
108+
),
101109
1,
102110
True,
103111
)
104112

105113

114+
def test_arbitrary_bytes_hook(tmpdir_factory, mock_out_store_directory):
115+
git_path = make_consuming_repo(tmpdir_factory, 'arbitrary_bytes_repo')
116+
with cwd(git_path):
117+
_test_run(git_path, {}, (b'\xe2\x98\x83\xb2\n',), 1, True)
118+
119+
106120
@pytest.mark.parametrize(
107121
('options', 'outputs', 'expected_ret', 'stage'),
108122
(
109-
({}, ('Bash hook', 'Passed'), 0, True),
110-
({'verbose': True}, ('foo.py\nHello World',), 0, True),
111-
({'hook': 'bash_hook'}, ('Bash hook', 'Passed'), 0, True),
112-
({'hook': 'nope'}, ('No hook with id `nope`',), 1, True),
123+
({}, (b'Bash hook', b'Passed'), 0, True),
124+
({'verbose': True}, (b'foo.py\nHello World',), 0, True),
125+
({'hook': 'bash_hook'}, (b'Bash hook', b'Passed'), 0, True),
126+
({'hook': 'nope'}, (b'No hook with id `nope`',), 1, True),
113127
(
114128
{'all_files': True, 'verbose': True},
115-
('foo.py'),
129+
(b'foo.py',),
116130
0,
117131
True,
118132
),
119133
(
120134
{'files': ('foo.py',), 'verbose': True},
121-
('foo.py'),
135+
(b'foo.py',),
122136
0,
123137
True,
124138
),
125-
({}, ('Bash hook', '(no files to check)', 'Skipped'), 0, False),
139+
({}, (b'Bash hook', b'(no files to check)', b'Skipped'), 0, False),
126140
)
127141
)
128142
def test_run(
@@ -150,7 +164,7 @@ def test_origin_source_error_msg(
150164
):
151165
args = _get_opts(origin=origin, source=source)
152166
ret, printed = _do_run(repo_with_passing_hook, args)
153-
warning_msg = 'Specify both --origin and --source.'
167+
warning_msg = b'Specify both --origin and --source.'
154168
if expect_failure:
155169
assert ret == 1
156170
assert warning_msg in printed
@@ -183,7 +197,7 @@ def test_no_stash(
183197
args = _get_opts(no_stash=no_stash, all_files=all_files)
184198
ret, printed = _do_run(repo_with_passing_hook, args)
185199
assert ret == 0
186-
warning_msg = '[WARNING] Unstaged files detected.'
200+
warning_msg = b'[WARNING] Unstaged files detected.'
187201
if expect_stash:
188202
assert warning_msg in printed
189203
else:
@@ -200,7 +214,7 @@ def test_has_unmerged_paths(output, expected):
200214
def test_merge_conflict(in_merge_conflict, mock_out_store_directory):
201215
ret, printed = _do_run(in_merge_conflict, _get_opts())
202216
assert ret == 1
203-
assert 'Unmerged files. Resolve before committing.' in printed
217+
assert b'Unmerged files. Resolve before committing.' in printed
204218

205219

206220
def test_merge_conflict_modified(in_merge_conflict, mock_out_store_directory):
@@ -211,13 +225,15 @@ def test_merge_conflict_modified(in_merge_conflict, mock_out_store_directory):
211225

212226
ret, printed = _do_run(in_merge_conflict, _get_opts())
213227
assert ret == 1
214-
assert 'Unmerged files. Resolve before committing.' in printed
228+
assert b'Unmerged files. Resolve before committing.' in printed
215229

216230

217231
def test_merge_conflict_resolved(in_merge_conflict, mock_out_store_directory):
218232
cmd_output('git', 'add', '.')
219233
ret, printed = _do_run(in_merge_conflict, _get_opts())
220-
for msg in ('Checking merge-conflict files only.', 'Bash hook', 'Passed'):
234+
for msg in (
235+
b'Checking merge-conflict files only.', b'Bash hook', b'Passed',
236+
):
221237
assert msg in printed
222238

223239

@@ -242,22 +258,22 @@ def test_skip_hook(repo_with_passing_hook, mock_out_store_directory):
242258
ret, printed = _do_run(
243259
repo_with_passing_hook, _get_opts(), {'SKIP': 'bash_hook'},
244260
)
245-
for msg in ('Bash hook', 'Skipped'):
261+
for msg in (b'Bash hook', b'Skipped'):
246262
assert msg in printed
247263

248264

249265
def test_hook_id_not_in_non_verbose_output(
250266
repo_with_passing_hook, mock_out_store_directory
251267
):
252268
ret, printed = _do_run(repo_with_passing_hook, _get_opts(verbose=False))
253-
assert '[bash_hook]' not in printed
269+
assert b'[bash_hook]' not in printed
254270

255271

256272
def test_hook_id_in_verbose_output(
257273
repo_with_passing_hook, mock_out_store_directory,
258274
):
259275
ret, printed = _do_run(repo_with_passing_hook, _get_opts(verbose=True))
260-
assert '[bash_hook] Bash hook' in printed
276+
assert b'[bash_hook] Bash hook' in printed
261277

262278

263279
def test_multiple_hooks_same_id(
@@ -272,7 +288,7 @@ def test_multiple_hooks_same_id(
272288

273289
ret, output = _do_run(repo_with_passing_hook, _get_opts())
274290
assert ret == 0
275-
assert output.count('Bash hook') == 2
291+
assert output.count(b'Bash hook') == 2
276292

277293

278294
def test_non_ascii_hook_id(
@@ -383,7 +399,7 @@ def test_local_hook_passes(
383399
_test_run(
384400
repo_with_passing_hook,
385401
options={},
386-
expected_outputs=[''],
402+
expected_outputs=[b''],
387403
expected_ret=0,
388404
stage=False
389405
)
@@ -412,7 +428,7 @@ def test_local_hook_fails(
412428
_test_run(
413429
repo_with_passing_hook,
414430
options={},
415-
expected_outputs=[''],
431+
expected_outputs=[b''],
416432
expected_ret=1,
417433
stage=False
418434
)
@@ -428,8 +444,8 @@ def test_allow_unstaged_config_option(
428444

429445
args = _get_opts(allow_unstaged_config=True)
430446
ret, printed = _do_run(repo_with_passing_hook, args)
431-
assert 'You have an unstaged config file' in printed
432-
assert 'have specified the --allow-unstaged-config option.' in printed
447+
assert b'You have an unstaged config file' in printed
448+
assert b'have specified the --allow-unstaged-config option.' in printed
433449
assert ret == 0
434450

435451

@@ -446,7 +462,7 @@ def test_no_allow_unstaged_config_option(
446462
modify_config(repo_with_passing_hook)
447463
args = _get_opts(allow_unstaged_config=False)
448464
ret, printed = _do_run(repo_with_passing_hook, args)
449-
assert 'Your .pre-commit-config.yaml is unstaged.' in printed
465+
assert b'Your .pre-commit-config.yaml is unstaged.' in printed
450466
assert ret == 1
451467

452468

@@ -456,7 +472,7 @@ def test_no_stash_suppresses_allow_unstaged_config_option(
456472
modify_config(repo_with_passing_hook)
457473
args = _get_opts(allow_unstaged_config=False, no_stash=True)
458474
ret, printed = _do_run(repo_with_passing_hook, args)
459-
assert 'Your .pre-commit-config.yaml is unstaged.' not in printed
475+
assert b'Your .pre-commit-config.yaml is unstaged.' not in printed
460476

461477

462478
def test_all_files_suppresses_allow_unstaged_config_option(
@@ -465,7 +481,7 @@ def test_all_files_suppresses_allow_unstaged_config_option(
465481
modify_config(repo_with_passing_hook)
466482
args = _get_opts(all_files=True)
467483
ret, printed = _do_run(repo_with_passing_hook, args)
468-
assert 'Your .pre-commit-config.yaml is unstaged.' not in printed
484+
assert b'Your .pre-commit-config.yaml is unstaged.' not in printed
469485

470486

471487
def test_files_suppresses_allow_unstaged_config_option(
@@ -474,4 +490,4 @@ def test_files_suppresses_allow_unstaged_config_option(
474490
modify_config(repo_with_passing_hook)
475491
args = _get_opts(files=['.pre-commit-config.yaml'])
476492
ret, printed = _do_run(repo_with_passing_hook, args)
477-
assert 'Your .pre-commit-config.yaml is unstaged.' not in printed
493+
assert b'Your .pre-commit-config.yaml is unstaged.' not in printed

0 commit comments

Comments
 (0)