git gui replaces amend message when prepare-commit-msg hook is used

git gui replaces amend message when prepare-commit-msg hook is used

[Orgad Shaneh]

Hi,

If a prepare-commit-msg hook is used, git gui executes it for "New Commit".

If the "New Commit" is selected, and then immediately "Amend" (before
the hook returns), when the hook returns the message is replaced with
the one produced by the hook.

- Orgad

Re: git gui replaces amend message when prepare-commit-msg hook is used

[Fredrik Gustafsson]

On Thu, Jul 04, 2013 at 12:47:28PM +0300, Orgad Shaneh wrote:
> If a prepare-commit-msg hook is used, git gui executes it for "New Commit".
> 
> If the "New Commit" is selected, and then immediately "Amend" (before
> the hook returns), when the hook returns the message is replaced with
> the one produced by the hook.

I don't get it. The message from the hook is replaced with the message
from the hook?

What I don't get is how you can amend to a commit that doesn't yet
exists. How is that possible?

-- 
Med vänliga hälsningar
Fredrik Gustafsson

tel: 0733-608274
e-post: iveqy@iveqy.com

Re: git gui replaces amend message when prepare-commit-msg hook is used

[Orgad Shaneh]

On Thu, Jul 4, 2013 at 1:01 PM, Fredrik Gustafsson <iveqy@iveqy.com> wrote:
> On Thu, Jul 04, 2013 at 12:47:28PM +0300, Orgad Shaneh wrote:
>> If a prepare-commit-msg hook is used, git gui executes it for "New Commit".
>>
>> If the "New Commit" is selected, and then immediately "Amend" (before
>> the hook returns), when the hook returns the message is replaced with
>> the one produced by the hook.
>
> I don't get it. The message from the hook is replaced with the message
> from the hook?
>
> What I don't get is how you can amend to a commit that doesn't yet
> exists. How is that possible?
>
> --
> Med vänliga hälsningar
> Fredrik Gustafsson
>
> tel: 0733-608274
> e-post: iveqy@iveqy.com

Did I say anything about a commit that doesn't exist? I have a commit
which I want to amend. If I click the Amend button before the hook is
done, this commit's message is replaced (in the editor, not the actual
commit) with the hook's result.

The message that should appear in the editor should come from the
amended commit, not from the hook.

- Orgad

Re: git gui replaces amend message when prepare-commit-msg hook is used

[Fredrik Gustafsson]

On Thu, Jul 04, 2013 at 01:03:31PM +0300, Orgad Shaneh wrote:
> On Thu, Jul 4, 2013 at 1:01 PM, Fredrik Gustafsson <iveqy@iveqy.com> wrote:
> > On Thu, Jul 04, 2013 at 12:47:28PM +0300, Orgad Shaneh wrote:
> >> If a prepare-commit-msg hook is used, git gui executes it for "New Commit".
> >>
> >> If the "New Commit" is selected, and then immediately "Amend" (before
> >> the hook returns), when the hook returns the message is replaced with
> >> the one produced by the hook.
> >
> > I don't get it. The message from the hook is replaced with the message
> > from the hook?
> >
> > What I don't get is how you can amend to a commit that doesn't yet
> > exists. How is that possible?
> 
> Did I say anything about a commit that doesn't exist? I have a commit
> which I want to amend. If I click the Amend button before the hook is
> done, this commit's message is replaced (in the editor, not the actual
> commit) with the hook's result.

When you click on amend the prepare-commit-msg hook is runned. But you
say that you click amend before "the hook is done". Which hook are you
talking about in this case? Are you clicking twice on amend?

-- 
Med vänliga hälsningar
Fredrik Gustafsson

tel: 0733-608274
e-post: iveqy@iveqy.com

Re: git gui replaces amend message when prepare-commit-msg hook is used

[John Keeping]

On Thu, Jul 04, 2013 at 12:47:28PM +0300, Orgad Shaneh wrote:
> Hi,
> 
> If a prepare-commit-msg hook is used, git gui executes it for "New Commit".
> 
> If the "New Commit" is selected, and then immediately "Amend" (before
> the hook returns), when the hook returns the message is replaced with
> the one produced by the hook.

I think this is a problem with the hook you are running.  The hook is
given arguments specifying the message file and optionally the source of
whatever is already in the file (see githooks(5) for details).

It sounds like your hook is blindly overwriting the file, rather than
preserving its contents in the cases where you wish to do so.

Re: git gui replaces amend message when prepare-commit-msg hook is used

[Orgad Shaneh]

On Thu, Jul 4, 2013 at 1:34 PM, John Keeping <john@keeping.me.uk> wrote:
> On Thu, Jul 04, 2013 at 12:47:28PM +0300, Orgad Shaneh wrote:
>> Hi,
>>
>> If a prepare-commit-msg hook is used, git gui executes it for "New Commit".
>>
>> If the "New Commit" is selected, and then immediately "Amend" (before
>> the hook returns), when the hook returns the message is replaced with
>> the one produced by the hook.
>
> I think this is a problem with the hook you are running.  The hook is
> given arguments specifying the message file and optionally the source of
> whatever is already in the file (see githooks(5) for details).
>
> It sounds like your hook is blindly overwriting the file, rather than
> preserving its contents in the cases where you wish to do so.

Let me try to explain.

When git gui is executed, it calls the prepare-commit-msg script with
.git/PREPARE_COMMIT_MSG as an argument.

When amend is selected, the hook is *not* called at all (what would it
prepare? The message is already committed)

Use the following hook to reproduce:
--- snip ---
#!/bin/sh

sleep 5
echo "$@" >> /tmp/hook.log
echo 'Hello hook' > "$1"
--- snip ---

Now run git gui (or press F5 if it is already running), and before 5
seconds pass, click Amend last commit. You'll see the commit's
message, but when the 5 seconds pass it is replaced with "Hello hook".
That's the bug.

- Orgad

Re: git gui replaces amend message when prepare-commit-msg hook is used

[Fredrik Gustafsson]

On Thu, Jul 04, 2013 at 01:59:10PM +0300, Orgad Shaneh wrote:
> On Thu, Jul 4, 2013 at 1:34 PM, John Keeping <john@keeping.me.uk> wrote:
> > On Thu, Jul 04, 2013 at 12:47:28PM +0300, Orgad Shaneh wrote:
> >> Hi,
> >>
> >> If a prepare-commit-msg hook is used, git gui executes it for "New Commit".
> >>
> >> If the "New Commit" is selected, and then immediately "Amend" (before
> >> the hook returns), when the hook returns the message is replaced with
> >> the one produced by the hook.
> >
> > I think this is a problem with the hook you are running.  The hook is
> > given arguments specifying the message file and optionally the source of
> > whatever is already in the file (see githooks(5) for details).
> >
> > It sounds like your hook is blindly overwriting the file, rather than
> > preserving its contents in the cases where you wish to do so.
> 
> Let me try to explain.
> 
> When git gui is executed, it calls the prepare-commit-msg script with
> .git/PREPARE_COMMIT_MSG as an argument.
> 
> When amend is selected, the hook is *not* called at all (what would it
> prepare? The message is already committed)
> 
> Use the following hook to reproduce:
> --- snip ---
> #!/bin/sh
> 
> sleep 5
> echo "$@" >> /tmp/hook.log
> echo 'Hello hook' > "$1"
> --- snip ---
> 
> Now run git gui (or press F5 if it is already running), and before 5
> seconds pass, click Amend last commit. You'll see the commit's
> message, but when the 5 seconds pass it is replaced with "Hello hook".
> That's the bug.

Yeah I got what you mean, it looks like it's an update problem of the
test-box in git-gui. You can also get that textbox to be completely
empty if you click back and foward between "new commit" and "amend". The
hook is not always runned.

I can confirm this bug. I don't think it related to pure git but to
git-gui.

I imagine that "new commit" exectures the prepare-commit-msg hook and
replaces the textbox content with it. Something that should be correct,
but the you've already told the textbox to have an other value (the
value of the commit to amend).

There should be a check before writing to the textbox if "new commit"
still is set.

-- 
Med vänliga hälsningar
Fredrik Gustafsson

tel: 0733-608274
e-post: iveqy@iveqy.com

Re: git gui replaces amend message when prepare-commit-msg hook is used

[John Keeping]

On Thu, Jul 04, 2013 at 01:59:10PM +0300, Orgad Shaneh wrote:
> On Thu, Jul 4, 2013 at 1:34 PM, John Keeping <john@keeping.me.uk> wrote:
> > On Thu, Jul 04, 2013 at 12:47:28PM +0300, Orgad Shaneh wrote:
> >> Hi,
> >>
> >> If a prepare-commit-msg hook is used, git gui executes it for "New Commit".
> >>
> >> If the "New Commit" is selected, and then immediately "Amend" (before
> >> the hook returns), when the hook returns the message is replaced with
> >> the one produced by the hook.
> >
> > I think this is a problem with the hook you are running.  The hook is
> > given arguments specifying the message file and optionally the source of
> > whatever is already in the file (see githooks(5) for details).
> >
> > It sounds like your hook is blindly overwriting the file, rather than
> > preserving its contents in the cases where you wish to do so.
> 
> Let me try to explain.
> 
> When git gui is executed, it calls the prepare-commit-msg script with
> .git/PREPARE_COMMIT_MSG as an argument.
> 
> When amend is selected, the hook is *not* called at all (what would it
> prepare? The message is already committed)
> 
> Use the following hook to reproduce:
> --- snip ---
> #!/bin/sh
> 
> sleep 5
> echo "$@" >> /tmp/hook.log
> echo 'Hello hook' > "$1"
> --- snip ---
> 
> Now run git gui (or press F5 if it is already running), and before 5
> seconds pass, click Amend last commit. You'll see the commit's
> message, but when the 5 seconds pass it is replaced with "Hello hook".
> That's the bug.

Yes, and that's a bug in the hook.  The hook is called with a second
argument "commit" but it is ignoring this and blindly overwriting the
message.  githooks(5) says:

    prepare-commit-msg
        This hook is invoked by git commit right after preparing the default
        log message, and before the editor is started.

        It takes one to three parameters. The first is the name of the
        file that contains the commit log message. The second is the
        source of the commit message, and can be: message (if a -m or -F
        option was given); template (if a -t option was given or the
        configuration option commit.template is set); merge (if the
        commit is a merge or a .git/MERGE_MSG file exists); squash (if a
        .git/SQUASH_MSG file exists); or commit, followed by a commit
        SHA1 (if a -c, -C or --amend option was given).

        If the exit status is non-zero, git commit will abort.

        The purpose of the hook is to edit the message file in place,
        and it is not suppressed by the --no-verify option. A non-zero
        exit means a failure of the hook and aborts the commit. It
        should not be used as replacement for pre-commit hook.

Your problem is that your hook script is not checking $2 so it is
overwriting the message even when you do not want to do so.

Re: git gui replaces amend message when prepare-commit-msg hook is used

[Orgad Shaneh]

On Thu, Jul 4, 2013 at 2:19 PM, John Keeping <john@keeping.me.uk> wrote:
> On Thu, Jul 04, 2013 at 01:59:10PM +0300, Orgad Shaneh wrote:
>> On Thu, Jul 4, 2013 at 1:34 PM, John Keeping <john@keeping.me.uk> wrote:
>> > On Thu, Jul 04, 2013 at 12:47:28PM +0300, Orgad Shaneh wrote:
>> >> Hi,
>> >>
>> >> If a prepare-commit-msg hook is used, git gui executes it for "New Commit".
>> >>
>> >> If the "New Commit" is selected, and then immediately "Amend" (before
>> >> the hook returns), when the hook returns the message is replaced with
>> >> the one produced by the hook.
>> >
>> > I think this is a problem with the hook you are running.  The hook is
>> > given arguments specifying the message file and optionally the source of
>> > whatever is already in the file (see githooks(5) for details).
>> >
>> > It sounds like your hook is blindly overwriting the file, rather than
>> > preserving its contents in the cases where you wish to do so.
>>
>> Let me try to explain.
>>
>> When git gui is executed, it calls the prepare-commit-msg script with
>> .git/PREPARE_COMMIT_MSG as an argument.
>>
>> When amend is selected, the hook is *not* called at all (what would it
>> prepare? The message is already committed)
>>
>> Use the following hook to reproduce:
>> --- snip ---
>> #!/bin/sh
>>
>> sleep 5
>> echo "$@" >> /tmp/hook.log
>> echo 'Hello hook' > "$1"
>> --- snip ---
>>
>> Now run git gui (or press F5 if it is already running), and before 5
>> seconds pass, click Amend last commit. You'll see the commit's
>> message, but when the 5 seconds pass it is replaced with "Hello hook".
>> That's the bug.
>
> Yes, and that's a bug in the hook.  The hook is called with a second
> argument "commit" but it is ignoring this and blindly overwriting the
> message.  githooks(5) says:
>
>     prepare-commit-msg
>         This hook is invoked by git commit right after preparing the default
>         log message, and before the editor is started.
>
>         It takes one to three parameters. The first is the name of the
>         file that contains the commit log message. The second is the
>         source of the commit message, and can be: message (if a -m or -F
>         option was given); template (if a -t option was given or the
>         configuration option commit.template is set); merge (if the
>         commit is a merge or a .git/MERGE_MSG file exists); squash (if a
>         .git/SQUASH_MSG file exists); or commit, followed by a commit
>         SHA1 (if a -c, -C or --amend option was given).
>
>         If the exit status is non-zero, git commit will abort.
>
>         The purpose of the hook is to edit the message file in place,
>         and it is not suppressed by the --no-verify option. A non-zero
>         exit means a failure of the hook and aborts the commit. It
>         should not be used as replacement for pre-commit hook.
>
> Your problem is that your hook script is not checking $2 so it is
> overwriting the message even when you do not want to do so.

No, it isn't. Not by git-gui at least. Check /tmp/hook.log with the
hook I provided...

- Orgad

Re: git gui replaces amend message when prepare-commit-msg hook is used

[Antoine Pelisse]

>> Your problem is that your hook script is not checking $2 so it is
>> overwriting the message even when you do not want to do so.
>
> No, it isn't. Not by git-gui at least. Check /tmp/hook.log with the
> hook I provided...

So what you mean is that the hook is not executed with the correct parameters?
Could you please provide the output of the /tmp/hook.log file (I can't
reproduce right now) ?

Re: git gui replaces amend message when prepare-commit-msg hook is used

[Orgad Shaneh]

On Thu, Jul 4, 2013 at 3:42 PM, Antoine Pelisse <apelisse@gmail.com> wrote:
>>> Your problem is that your hook script is not checking $2 so it is
>>> overwriting the message even when you do not want to do so.
>>
>> No, it isn't. Not by git-gui at least. Check /tmp/hook.log with the
>> hook I provided...
>
> So what you mean is that the hook is not executed with the correct parameters?
> Could you please provide the output of the /tmp/hook.log file (I can't
> reproduce right now) ?

It only runs for "New commit" (possibly with merge or squash as
arguments). In my case the log is:
.git/PREPARE_COMMIT_MSG
.git/PREPARE_COMMIT_MSG
.git/PREPARE_COMMIT_MSG
...

Not running the hook for amend is another problem.

What I referred to was that handling the hook's result is done without
checking if the state has changed while it was running, like Fredrik
has already pointed out.

- Orgad

Re: git gui replaces amend message when prepare-commit-msg hook is used

[Antoine Pelisse]

On Thu, Jul 4, 2013 at 2:46 PM, Orgad Shaneh <orgads@gmail.com> wrote:
> On Thu, Jul 4, 2013 at 3:42 PM, Antoine Pelisse <apelisse@gmail.com> wrote:
>>>> Your problem is that your hook script is not checking $2 so it is
>>>> overwriting the message even when you do not want to do so.
>>>
>>> No, it isn't. Not by git-gui at least. Check /tmp/hook.log with the
>>> hook I provided...
>>
>> So what you mean is that the hook is not executed with the correct parameters?
>> Could you please provide the output of the /tmp/hook.log file (I can't
>> reproduce right now) ?
>
> It only runs for "New commit" (possibly with merge or squash as
> arguments). In my case the log is:
> .git/PREPARE_COMMIT_MSG
> .git/PREPARE_COMMIT_MSG
> .git/PREPARE_COMMIT_MSG
> ...
>
> Not running the hook for amend is another problem.
>
> What I referred to was that handling the hook's result is done without
> checking if the state has changed while it was running, like Fredrik
> has already pointed out.

Yep, I've had a quick look at that, and clearly the problem seems to
come from git-gui.
I have absolutely no knowledge of TCL so I won't be able to help.
Indeed, from what I've seen it looks like prepare-commit-message hook
is not called in amend case.

I cc'ed Pat Thoyts as he's the maintainer of git-gui.

Sorry for not being able to help any further on this one.

Antoine,