git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / Atom feed
* git gui replaces amend message when prepare-commit-msg hook is used
@ 2013-07-04  9:47 Orgad Shaneh
  2013-07-04 10:01 ` Fredrik Gustafsson
  2013-07-04 10:34 ` John Keeping
  0 siblings, 2 replies; 12+ messages in thread
From: Orgad Shaneh @ 2013-07-04  9:47 UTC (permalink / raw)
  To: git

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: git gui replaces amend message when prepare-commit-msg hook is used
  2013-07-04  9:47 git gui replaces amend message when prepare-commit-msg hook is used Orgad Shaneh
@ 2013-07-04 10:01 ` Fredrik Gustafsson
  2013-07-04 10:03   ` Orgad Shaneh
  2013-07-04 10:34 ` John Keeping
  1 sibling, 1 reply; 12+ messages in thread
From: Fredrik Gustafsson @ 2013-07-04 10:01 UTC (permalink / raw)
  To: Orgad Shaneh; +Cc: git

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: git gui replaces amend message when prepare-commit-msg hook is used
  2013-07-04 10:01 ` Fredrik Gustafsson
@ 2013-07-04 10:03   ` Orgad Shaneh
  2013-07-04 10:21     ` Fredrik Gustafsson
  0 siblings, 1 reply; 12+ messages in thread
From: Orgad Shaneh @ 2013-07-04 10:03 UTC (permalink / raw)
  To: Fredrik Gustafsson; +Cc: git

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: git gui replaces amend message when prepare-commit-msg hook is used
  2013-07-04 10:03   ` Orgad Shaneh
@ 2013-07-04 10:21     ` Fredrik Gustafsson
  0 siblings, 0 replies; 12+ messages in thread
From: Fredrik Gustafsson @ 2013-07-04 10:21 UTC (permalink / raw)
  To: Orgad Shaneh; +Cc: Fredrik Gustafsson, git

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: git gui replaces amend message when prepare-commit-msg hook is used
  2013-07-04  9:47 git gui replaces amend message when prepare-commit-msg hook is used Orgad Shaneh
  2013-07-04 10:01 ` Fredrik Gustafsson
@ 2013-07-04 10:34 ` John Keeping
  2013-07-04 10:59   ` Orgad Shaneh
  1 sibling, 1 reply; 12+ messages in thread
From: John Keeping @ 2013-07-04 10:34 UTC (permalink / raw)
  To: Orgad Shaneh; +Cc: git

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.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: git gui replaces amend message when prepare-commit-msg hook is used
  2013-07-04 10:34 ` John Keeping
@ 2013-07-04 10:59   ` Orgad Shaneh
  2013-07-04 11:17     ` Fredrik Gustafsson
  2013-07-04 11:19     ` John Keeping
  0 siblings, 2 replies; 12+ messages in thread
From: Orgad Shaneh @ 2013-07-04 10:59 UTC (permalink / raw)
  To: John Keeping; +Cc: git

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: git gui replaces amend message when prepare-commit-msg hook is used
  2013-07-04 10:59   ` Orgad Shaneh
@ 2013-07-04 11:17     ` Fredrik Gustafsson
  2013-07-04 11:19     ` John Keeping
  1 sibling, 0 replies; 12+ messages in thread
From: Fredrik Gustafsson @ 2013-07-04 11:17 UTC (permalink / raw)
  To: Orgad Shaneh; +Cc: John Keeping, git

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: git gui replaces amend message when prepare-commit-msg hook is used
  2013-07-04 10:59   ` Orgad Shaneh
  2013-07-04 11:17     ` Fredrik Gustafsson
@ 2013-07-04 11:19     ` John Keeping
  2013-07-04 11:37       ` Orgad Shaneh
  1 sibling, 1 reply; 12+ messages in thread
From: John Keeping @ 2013-07-04 11:19 UTC (permalink / raw)
  To: Orgad Shaneh; +Cc: git

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.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: git gui replaces amend message when prepare-commit-msg hook is used
  2013-07-04 11:19     ` John Keeping
@ 2013-07-04 11:37       ` Orgad Shaneh
  2013-07-04 12:42         ` Antoine Pelisse
  0 siblings, 1 reply; 12+ messages in thread
From: Orgad Shaneh @ 2013-07-04 11:37 UTC (permalink / raw)
  To: John Keeping; +Cc: git

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: git gui replaces amend message when prepare-commit-msg hook is used
  2013-07-04 11:37       ` Orgad Shaneh
@ 2013-07-04 12:42         ` Antoine Pelisse
  2013-07-04 12:46           ` Orgad Shaneh
  0 siblings, 1 reply; 12+ messages in thread
From: Antoine Pelisse @ 2013-07-04 12:42 UTC (permalink / raw)
  To: Orgad Shaneh; +Cc: John Keeping, git

>> 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) ?

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: git gui replaces amend message when prepare-commit-msg hook is used
  2013-07-04 12:42         ` Antoine Pelisse
@ 2013-07-04 12:46           ` Orgad Shaneh
  2013-07-05 19:13             ` Antoine Pelisse
  0 siblings, 1 reply; 12+ messages in thread
From: Orgad Shaneh @ 2013-07-04 12:46 UTC (permalink / raw)
  To: Antoine Pelisse; +Cc: John Keeping, git

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: git gui replaces amend message when prepare-commit-msg hook is used
  2013-07-04 12:46           ` Orgad Shaneh
@ 2013-07-05 19:13             ` Antoine Pelisse
  0 siblings, 0 replies; 12+ messages in thread
From: Antoine Pelisse @ 2013-07-05 19:13 UTC (permalink / raw)
  To: Orgad Shaneh, Pat Thoyts; +Cc: John Keeping, git

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,

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2013-07-05 19:13 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-07-04  9:47 git gui replaces amend message when prepare-commit-msg hook is used Orgad Shaneh
2013-07-04 10:01 ` Fredrik Gustafsson
2013-07-04 10:03   ` Orgad Shaneh
2013-07-04 10:21     ` Fredrik Gustafsson
2013-07-04 10:34 ` John Keeping
2013-07-04 10:59   ` Orgad Shaneh
2013-07-04 11:17     ` Fredrik Gustafsson
2013-07-04 11:19     ` John Keeping
2013-07-04 11:37       ` Orgad Shaneh
2013-07-04 12:42         ` Antoine Pelisse
2013-07-04 12:46           ` Orgad Shaneh
2013-07-05 19:13             ` Antoine Pelisse

git@vger.kernel.org list mirror (unofficial, one of many)

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V1 git git/ https://public-inbox.org/git \
		git@vger.kernel.org
	public-inbox-index git

Example config snippet for mirrors.
Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.io/gmane.comp.version-control.git
 note: .onion URLs require Tor: https://www.torproject.org/

code repositories for the project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

AGPL code for this site: git clone https://public-inbox.org/public-inbox.git