threads / patch / 24018

patch, 2 partscommit.c - provide commit-type to the hooks/pre-commit script

Subject: [PATCH 1/2] commit.c - provide commit-type to the hooks/pre-commit script

## tl;dr

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

replies: 6people: 3as markdown or json

Mark Levedahl· Jun 5, 2010, 17:39 UTC · 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.c to pass $1 as normal|amend 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         |    8 +++++---
 builtin/commit.c                   |    2 +-
 templates/hooks--pre-commit.sample |   10 ++++++++--
 3 files changed, 14 insertions(+), 6 deletions(-)
Show changes to 3 files +14 −6

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

diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
index 7183aa9..dbe1a1a 100644
--- a/Documentation/githooks.txt
+++ b/Documentation/githooks.txt
@@ -73,9 +73,11 @@ 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
+with `\--no-verify` option.  It takes a single parameter indicating
+the commit type (normal|amend), and is invoked before obtaining the
+proposed commit log message and making a commit.  For $1=normal,
+the commit is against HEAD, while for $1=amend, the commit is
+against HEAD~1.  Exiting with non-zero status from this script
 causes the 'git commit' to abort.
 
 The default 'pre-commit' hook, when enabled, catches introduction
diff --git a/builtin/commit.c b/builtin/commit.c
index ddf77e4..8543f09 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 ? "amend" : "normal", NULL))
 		return 0;
 
 	if (message.len) {
diff --git a/templates/hooks--pre-commit.sample b/templates/hooks--pre-commit.sample
index b187c4b..0c0c79f 100755
--- a/templates/hooks--pre-commit.sample
+++ b/templates/hooks--pre-commit.sample
@@ -1,7 +1,8 @@
 #!/bin/sh
 #
 # An example hook script to verify what is about to be committed.
-# Called by "git commit" with no arguments.  The hook should
+# Called by "git commit" with a single argument indicating the
+# commit-type (normal|amend). The hook should
 # exit with non-zero status after issuing an appropriate message if
 # it wants to stop the commit.
 #
@@ -9,7 +10,12 @@
 
 if git rev-parse --verify HEAD >/dev/null 2>&1
 then
-	against=HEAD
+	if test "$1" = "amend"
+	then
+		against=HEAD~1
+	else
+		against=HEAD
+	fi
 else
 	# Initial commit: diff against an empty tree object
 	against=4b825dc642cb6eb9a060e54bf8d69288fbee4904
-- 
1.7.1.270.g2a29a
Mark Levedahl· Jun 5, 2010, 17:43 UTC · re: Mark Levedahl · lore

[PATCH 2/2] git-gui - provide commit-type 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 normal|amend 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 |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
Show changes to git-gui/lib/commit.tcl +1 −1
diff --git a/git-gui/lib/commit.tcl b/git-gui/lib/commit.tcl
index 7f459cd..8150fa6 100644
--- a/git-gui/lib/commit.tcl
+++ b/git-gui/lib/commit.tcl
@@ -225,7 +225,7 @@ A good commit message has the following format:
 
 	# -- Run the pre-commit hook.
 	#
-	set fd_ph [githook_read pre-commit]
+	set fd_ph [githook_read pre-commit $commit_type]
 	if {$fd_ph eq {}} {
 		commit_commitmsg $curHEAD $msg_p
 		return
-- 
1.7.1.270.g2a29a
Jeff King· Jun 6, 2010, 22:10 UTC · re: Mark Levedahl · lore

Re: [PATCH 1/2] commit.c - provide commit-type to the hooks/pre-commit script

On Sat, Jun 05, 2010 at 01:39:50PM -0400, Mark Levedahl wrote:
Show 10 quoted lines
> -	if (!no_verify && run_hook(index_file, "pre-commit", NULL))
> +	if (!no_verify && run_hook(index_file, "pre-commit", amend ? "amend" : "normal", NULL))
> [...]
> -	against=HEAD
> +	if test "$1" = "amend"
> +	then
> +		against=HEAD~1
> +	else
> +		against=HEAD
> +	fi

Is there a reason to use the magic "amend" and "normal" words, if scripts are just going to end up changing them back into HEAD~1 and HEAD anyway?

-Peff
Mark Levedahl· Jun 7, 2010, 00:21 UTC · re: Jeff King · lore

Re: [PATCH 1/2] commit.c - provide commit-type to the hooks/pre-commit script

On 06/06/2010 06:10 PM, Jeff King wrote:
Show 21 quoted lines
> On Sat, Jun 05, 2010 at 01:39:50PM -0400, Mark Levedahl wrote:
>
>    
>> -	if (!no_verify&&  run_hook(index_file, "pre-commit", NULL))
>> +	if (!no_verify&&  run_hook(index_file, "pre-commit", amend ? "amend" : "normal", NULL))
>> [...]
>> -	against=HEAD
>> +	if test "$1" = "amend"
>> +	then
>> +		against=HEAD~1
>> +	else
>> +		against=HEAD
>> +	fi
>>      
> Is there a reason to use the magic "amend" and "normal" words, if
> scripts are just going to end up changing them back into HEAD~1 and HEAD
> anyway?
>
> -Peff
>
>    

Well, git-gui's internal $commit_type variable has the values normal|amend, so I went with those. So , yes there is a reason, but maybe not a great one :^). Given some consensus, this is trivial to change. I'll let things cook a while longer before sending a new patch.

Mark
Johannes Sixt· Jun 7, 2010, 06:26 UTC · re: Jeff King · lore

Re: [PATCH 1/2] commit.c - provide commit-type to the hooks/pre-commit script

Am 6/7/2010 0:10, schrieb Jeff King:
Show 16 quoted lines
> On Sat, Jun 05, 2010 at 01:39:50PM -0400, Mark Levedahl wrote:
> 
>> -	if (!no_verify && run_hook(index_file, "pre-commit", NULL))
>> +	if (!no_verify && run_hook(index_file, "pre-commit", amend ? "amend" : "normal", NULL))
>> [...]
>> -	against=HEAD
>> +	if test "$1" = "amend"
>> +	then
>> +		against=HEAD~1
>> +	else
>> +		against=HEAD
>> +	fi
> 
> Is there a reason to use the magic "amend" and "normal" words, if
> scripts are just going to end up changing them back into HEAD~1 and HEAD
> anyway?

pre-commit might act differently when a commit is amended, the most likely reason I can think of is to always allow to amend. When you have only a SHA1, you can get that information only with an additional process.

-- Hannes
Jeff King· Jun 7, 2010, 06:38 UTC · re: Johannes Sixt · lore

Re: [PATCH 1/2] commit.c - provide commit-type to the hooks/pre-commit script

On Mon, Jun 07, 2010 at 08:26:53AM +0200, Johannes Sixt wrote:
Show 7 quoted lines
> > Is there a reason to use the magic "amend" and "normal" words, if
> > scripts are just going to end up changing them back into HEAD~1 and HEAD
> > anyway?
> 
> pre-commit might act differently when a commit is amended, the most likely
> reason I can think of is to always allow to amend. When you have only a
> SHA1, you can get that information only with an additional process.

Actually, I meant to provide the hook with the literal words "HEAD~1" and "HEAD", not the sha1. So they are effectively magic words, but they also happen to be useful for directly feeding to git commands.

It also extends naturally to indicating a merge commit ("HEAD" or "HEAD~1" followed by some other ref). I don't know if that is useful or not, but it seems like the same realm of information as whether or not we are amending.

-Peff
Mark Levedahl· Jun 8, 2010, 02:55 UTC · re: Jeff King · lore

Re: [PATCH 1/2] commit.c - provide commit-type to the hooks/pre-commit script

On 06/07/2010 02:38 AM, Jeff King wrote:
Show 12 quoted lines
> Actually, I meant to provide the hook with the literal words "HEAD~1"
> and "HEAD", not the sha1. So they are effectively magic words, but they
> also happen to be useful for directly feeding to git commands.
>
> It also extends naturally to indicating a merge commit ("HEAD" or
> "HEAD~1" followed by some other ref). I don't know if that is useful or
> not, but it seems like the same realm of information as whether or not
> we are amending.
>
> -Peff
>
>    

I agree with this, though I have no itch regarding all of the commit parents for a merge commit so I'll leave that for another to scratch. So, I'm sending a new series to use HEAD~1 and HEAD rather than amend and normal.

Mark

← back to recent threads