From 91a547ed6180224045953ba0c992ac0aa558ac26 Mon Sep 17 00:00:00 2001 From: Joe Bateson Date: Wed, 25 Nov 2015 11:14:01 -0800 Subject: [PATCH] Output a message when a hook fails due to file modification --- pre_commit/commands/run.py | 16 ++++++++++++++-- .../modified_file_returns_zero_repo/bin/hook3.sh | 6 ++++++ .../modified_file_returns_zero_repo/hooks.yaml | 7 ++++++- tests/commands/run_test.py | 14 ++++++++++---- 4 files changed, 36 insertions(+), 7 deletions(-) create mode 100755 testing/resources/modified_file_returns_zero_repo/bin/hook3.sh diff --git a/pre_commit/commands/run.py b/pre_commit/commands/run.py index d4929aca..e01c7f9f 100644 --- a/pre_commit/commands/run.py +++ b/pre_commit/commands/run.py @@ -89,8 +89,10 @@ def _run_single_hook(hook, repo, args, write, skips=frozenset()): retcode, stdout, stderr = repo.run_hook(hook, filenames) diff_after = cmd_output('git', 'diff', retcode=None, encoding=None) + file_modifications = diff_before != diff_after + # If the hook makes changes, fail the commit - if diff_before != diff_after: + if file_modifications: retcode = 1 if retcode: @@ -104,9 +106,19 @@ def _run_single_hook(hook, repo, args, write, skips=frozenset()): write(color.format_color(pass_fail, print_color, args.color) + '\n') - if (stdout or stderr) and (retcode or args.verbose): + if (stdout or stderr or file_modifications) and (retcode or args.verbose): write('hookid: {0}\n'.format(hook['id'])) write('\n') + + # Print a message if failing due to file modifications + if file_modifications: + write('Files were modified by this hook.') + + if stdout or stderr: + write(' Additional output:\n') + + write('\n') + for output in (stdout, stderr): assert type(output) is bytes, type(output) if output.strip(): diff --git a/testing/resources/modified_file_returns_zero_repo/bin/hook3.sh b/testing/resources/modified_file_returns_zero_repo/bin/hook3.sh new file mode 100755 index 00000000..3180eb3c --- /dev/null +++ b/testing/resources/modified_file_returns_zero_repo/bin/hook3.sh @@ -0,0 +1,6 @@ +#!/usr/bin/env bash + +for f in $@; do + # Non UTF-8 bytes + echo -e '\x01\x97' > "$f" +done diff --git a/testing/resources/modified_file_returns_zero_repo/hooks.yaml b/testing/resources/modified_file_returns_zero_repo/hooks.yaml index 62219a7d..8d79ef39 100644 --- a/testing/resources/modified_file_returns_zero_repo/hooks.yaml +++ b/testing/resources/modified_file_returns_zero_repo/hooks.yaml @@ -2,9 +2,14 @@ name: Bash hook entry: bin/hook.sh language: script - files: '' + files: 'foo.py' - id: bash_hook2 name: Bash hook entry: bin/hook2.sh language: script files: '' +- id: bash_hook3 + name: Bash hook + entry: bin/hook3.sh + language: script + files: 'bar.py' diff --git a/tests/commands/run_test.py b/tests/commands/run_test.py index 4e1c950d..77ae41a3 100644 --- a/tests/commands/run_test.py +++ b/tests/commands/run_test.py @@ -40,9 +40,9 @@ def repo_with_failing_hook(tempdir_factory): yield git_path -def stage_a_file(): - cmd_output('touch', 'foo.py') - cmd_output('git', 'add', 'foo.py') +def stage_a_file(filename='foo.py'): + cmd_output('touch', filename) + cmd_output('git', 'add', filename) def get_write_mock_output(write_mock): @@ -127,16 +127,22 @@ def test_hook_that_modifies_but_returns_zero( tempdir_factory, 'modified_file_returns_zero_repo', ) with cwd(git_path): + stage_a_file('bar.py') _test_run( git_path, {}, ( # The first should fail b'Failed', - # With a modified file (the hook's output) + # With a modified file (default message + the hook's output) + b'Files were modified by this hook. Additional output:\n\n' b'Modified: foo.py', # The next hook should pass despite the first modifying b'Passed', + # The next hook should fail + b'Failed', + # bar.py was modified, but provides no additional output + b'Files were modified by this hook.\n', ), 1, True,