Re: [PATCH] merge: Honor prepare-commit-msg return code

[Antoine Pelisse <apelisse@gmail.com>]

> Oh, that wasn't meant as a complaint.  I am tempted to rewrite the
> log message like so, though:
>
>     65969d4 (merge: honor prepare-commit-msg hook, 2011-02-14) tried to
>     make "git commit" and "git merge" consistent, because a merge that
>     required user assistance has to be concluded with "git commit", but
>     only "git commit" triggered prepare-commit-msg hook.  When it added
>     a call to run the prepare-commit-msg hook, however, it forgot to
>     check the exit code from the hook like "git commit" does, and ended
>     up replacing one inconsistency with another.

That's fine with me

>>> diff --git a/t/t7505-prepare-commit-msg-hook.sh b/t/t7505-prepare-commit-msg-hook.sh
>>> index bc497bc..3573751 100755
>>> --- a/t/t7505-prepare-commit-msg-hook.sh
>>> +++ b/t/t7505-prepare-commit-msg-hook.sh
>>> @@ -172,11 +172,12 @@ test_expect_success 'with failing hook (merge)' '
>>>         git checkout -B other HEAD@{1} &&
>>>         echo "more" >> file &&
>>>         git add file &&
>>> -       chmod -x $HOOK &&
>>> +       rm -f "$HOOK" &&
>>>         git commit -m other &&
>>> -       chmod +x $HOOK &&
>>> +       write_script "$HOOK" <<-EOF
>>> +       exit 1
>>> +       EOF
>>>         git checkout - &&
>>> -       head=`git rev-parse HEAD` &&
>>>         test_must_fail git merge other
>>>
>>>  '
>>
>> What about moving the hook file then ? Not very important to me, just
>> a suggestion as it would keep the shebang.
>
> Strictly speaking, the way $HOOK is prepared in the original is
> wrong.  The script is always run under "#!/bin/sh" instead of the
> shell the user told us to use with $SHELL_PATH.  For a simple one
> liner that only exits with 1, it does not matter, though.
>
> Many test scripts got this wrong and that was the reason we later
> added write_script helper function to the test suite.

So let's keep your suggestion and squash the commit.

Thanks,