threads / patch / 24039

v2, 2 partsgit-gui - provide commit base to the hooks/pre-commit script

Subject: [PATCH 2/2 v2] git-gui - provide commit base to the hooks/pre-commit script

## tl;dr

6 messages between Jun 8, 2010 and Jun 22, 2010. Diffs are folded; open one to read it.

replies: 5people: 3as markdown or json

Mark Levedahl· Jun 8, 2010, 02:56 UTC · lore

[PATCH 1/2 v2] commit.c - provide base commit to the hooks/pre-commit script

If hooks/pre-commit acts based upon the changes to be checked in rather than just the resulting content, the script needs to know which commit to use. For a normal commit, this is HEAD, but when amending this is HEAD~1. So, this modifies commit.c to pass $1 as HEAD|HEAD~1 depending upon the commit type. Existing scripts are unaffected as they did not expect any argument so will silently ignore this extra bit of info.

Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
---
 Documentation/githooks.txt         |   10 ++++++----
 builtin/commit.c                   |    2 +-
 templates/hooks--pre-commit.sample |    9 +++++----
 3 files changed, 12 insertions(+), 9 deletions(-)
Show changes to 3 files +12 −9

Documentation/githooks.txt, builtin/commit.c, templates/hooks--pre-commit.sample

diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
index 7183aa9..d785b6c 100644
--- a/Documentation/githooks.txt
+++ b/Documentation/githooks.txt
@@ -73,10 +73,12 @@ pre-commit
 ~~~~~~~~~~
 
 This hook is invoked by 'git commit', and can be bypassed
-with `\--no-verify` option.  It takes no parameter, and is
-invoked before obtaining the proposed commit log message and
-making a commit.  Exiting with non-zero status from this script
-causes the 'git commit' to abort.
+with `\--no-verify` option.  It takes a single parameter naming
+the current commit's parent (HEAD | HEAD~1), and is invoked before
+obtaining the proposed commit log message and making a commit.
+Note that $1=HEAD~1 indicates an amended commit is in process.
+Exiting with non-zero status from this script causes the 'git
+commit' to abort.
 
 The default 'pre-commit' hook, when enabled, catches introduction
 of lines with trailing whitespaces and aborts the commit when
diff --git a/builtin/commit.c b/builtin/commit.c
index ddf77e4..49f799e 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -544,7 +544,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 	const char *hook_arg2 = NULL;
 	int ident_shown = 0;
 
-	if (!no_verify && run_hook(index_file, "pre-commit", NULL))
+	if (!no_verify && run_hook(index_file, "pre-commit", amend ? "HEAD~1" : "HEAD", NULL))
 		return 0;
 
 	if (message.len) {
diff --git a/templates/hooks--pre-commit.sample b/templates/hooks--pre-commit.sample
index b187c4b..e72cf9c 100755
--- a/templates/hooks--pre-commit.sample
+++ b/templates/hooks--pre-commit.sample
@@ -1,15 +1,16 @@
 #!/bin/sh
 #
 # An example hook script to verify what is about to be committed.
-# Called by "git commit" with no arguments.  The hook should
-# exit with non-zero status after issuing an appropriate message if
-# it wants to stop the commit.
+# Called by "git commit" with a single argument indicating the
+# commit-base (HEAD|HEAD~1).  The hook should exit with non-zero
+# status after issuing an appropriate message if it wants to stop
+# the commit.
 #
 # To enable this hook, rename this file to "pre-commit".
 
 if git rev-parse --verify HEAD >/dev/null 2>&1
 then
-	against=HEAD
+	against=$1
 else
 	# Initial commit: diff against an empty tree object
 	against=4b825dc642cb6eb9a060e54bf8d69288fbee4904
-- 
1.7.1.269.g9fb6
Mark Levedahl· Jun 8, 2010, 02:56 UTC · re: Mark Levedahl · lore

If hooks/pre-commit acts based upon the changes to be checked in rather than just the resulting content, the script needs to know which commit to use. For a normal commit, this is HEAD, but when amending this is HEAD~1. So, this modifies commit.tcl to pass $1 as HEAD|HEAD~1 depending upon the commit type. Existing scripts are unaffected as they did not expect any argument so will silently ignore this extra bit of info.

Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
---
 git-gui/lib/commit.tcl |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)
Show changes to git-gui/lib/commit.tcl +7 −1
diff --git a/git-gui/lib/commit.tcl b/git-gui/lib/commit.tcl
index 7f459cd..a62edc9 100644
--- a/git-gui/lib/commit.tcl
+++ b/git-gui/lib/commit.tcl
@@ -225,7 +225,13 @@ A good commit message has the following format:
 
 	# -- Run the pre-commit hook.
 	#
-	set fd_ph [githook_read pre-commit]
+	if {[string compare $commit_type "normal"] == 0} {
+		set against "HEAD"
+	} else {
+		set against "HEAD~1"
+	}
+
+	set fd_ph [githook_read pre-commit $against]
 	if {$fd_ph eq {}} {
 		commit_commitmsg $curHEAD $msg_p
 		return
-- 
1.7.1.269.g9fb6
Mark Levedahl· Jun 19, 2010, 02:59 UTC · re: Mark Levedahl · lore

Re: [PATCH 2/2 v2] git-gui - provide commit base to the hooks/pre-commit script

On 06/07/2010 10:56 PM, Mark Levedahl wrote:
Show 9 quoted lines
> If hooks/pre-commit acts based upon the changes to be checked in
> rather than just the resulting content, the script needs to know which
> commit to use. For a normal commit, this is HEAD, but when amending this
> is HEAD~1. So, this modifies commit.tcl to pass $1 as HEAD|HEAD~1 depending
> upon the commit type. Existing scripts are unaffected as they did not
> expect any argument so will silently ignore this extra bit of info.
>
> Signed-off-by: Mark Levedahl<mlevedahl@gmail.com>
>    

Given the discussion on the first series (starting with)<1275759590-16342-1-git-send-email-mlevedahl@gmail.com>, and the absence of objection to v2, I presume this sequence is ok?

Mark
Junio C Hamano· Jun 21, 2010, 04:54 UTC · re: Mark Levedahl · lore

Re: [PATCH 2/2 v2] git-gui - provide commit base to the hooks/pre-commit script

Mark Levedahl <mlevedahl@gmail.com> writes:
> Given the discussion on the first series (starting with)
> <1275759590-16342-1-git-send-email-mlevedahl@gmail.com>, and the absence
> of objection to v2, I presume this sequence is ok?

I generally take absense of responses as absense of interest and/or support, not absense of objection.

For this particular patch, I am mostly indifferent (i.e. no objection, but no strong support neither).

Jeff King· Jun 21, 2010, 05:00 UTC · re: Junio C Hamano · lore

Re: [PATCH 2/2 v2] git-gui - provide commit base to the hooks/pre-commit script

On Sun, Jun 20, 2010 at 09:54:39PM -0700, Junio C Hamano wrote:
Show 11 quoted lines
> Mark Levedahl <mlevedahl@gmail.com> writes:
> 
> > Given the discussion on the first series (starting with)
> > <1275759590-16342-1-git-send-email-mlevedahl@gmail.com>, and the absence
> > of objection to v2, I presume this sequence is ok?
> 
> I generally take absense of responses as absense of interest and/or
> support, not absense of objection.
> 
> For this particular patch, I am mostly indifferent (i.e. no objection, but
> no strong support neither).

Ditto. I don't personally have a use, but all of my concerns were answered.

-Peff
Mark Levedahl· Jun 22, 2010, 01:07 UTC · re: Jeff King · lore

Re: [PATCH 2/2 v2] git-gui - provide commit base to the hooks/pre-commit script

On 06/21/2010 01:00 AM, Jeff King wrote:
Show 14 quoted lines
> On Sun, Jun 20, 2010 at 09:54:39PM -0700, Junio C Hamano wrote:
>
>    
>> I generally take absense of responses as absense of interest and/or
>> support, not absense of objection.
>>
>> For this particular patch, I am mostly indifferent (i.e. no objection, but
>> no strong support neither).
>>      
> Ditto. I don't personally have a use, but all of my concerns were
> answered.
>
> -Peff
>    
Well, I have a use else I wouldn't have written this :^).

Absent this patch, the default commit hook is always computing a diff against HEAD and that is wrong when amending a commit. Folks may not care, but that is an error so this patch fixes an existing bug. I'll leave it at that.

Mark

← back to recent threads