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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Antoine Pelisse <apelisse@xxxxxxxxx> writes:

>>> prepare-commit-msg hook is run when committing to prepare the log
>>> message. If the exit-status is non-zero, the commit should be aborted.
>>
>> I was scratching my head why you CC'ed Jay, until I dug up 65969d4
>> (merge: honor prepare-commit-msg hook, 2011-02-14).
>
> I did as suggested in "SubmittingPatches" :)

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.

>> 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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]