[PATCH] Pass amend to pre-commit hook

[PATCH] Pass amend to pre-commit hook

[Alan Clucas]

Pass a single parameter 'amend' to the pre-commit hook when performing a
commit amend.

This allows 'incremental improvement' pre-commit hooks to prevent new
code from violating a rule, but also allow the pre-commit hook to pass an
amended commit where the amend has reverted back to the original
code (which may not pass that same rule).

Example:
I have a new whitespace rule. Old code violates this rule and will not
be fixed up for blame reasons.
My pre-commit hook detects _new_ lines which violate the rule and
rejects them, however, my original commit passes.
I amend the code to revert back to the original code (which violates the
rule). Without this change I cannot detect this is an amend and reject the
change (unless --no-verify).
With this I can detect this is an amend and verify the patch as a whole
is not in violation of the rule.

Signed-off-by: Alan Clucas <alan.clucas@teamwpc.co.uk>
---
Hello,
This is my first submission to git, so hopefully I've managed to get the 
formatting right. This patch should be explained above, and would also
help out the folks at overcommit who have this (pretty horrid) solution to
the same issue:
https://github.com/brigade/overcommit/issues/146
https://github.com/brigade/overcommit/pull/167
Thanks,
Alan Clucas

 Documentation/githooks.txt | 10 ++++++----
 builtin/commit.c           |  2 +-
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
index 7ba0ac9..49d7adb 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 zero or one parameters.
+If a parameter is given it will be 'amend' indicating this is
+a commit amend (if an `--amend` option was given). It 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.
 
 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 63772d0..936a614 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -671,7 +671,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 	/* This checks and barfs if author is badly specified */
 	determine_author_info(author_ident);
 
-	if (!no_verify && run_commit_hook(use_editor, index_file, "pre-commit", NULL))
+	if (!no_verify && run_commit_hook(use_editor, index_file, "pre-commit", amend?"amend":NULL, NULL))
 		return 0;
 
 	if (squash_message) {
-- 
2.4.1.168.g1ea28e1

Re: [PATCH] Pass amend to pre-commit hook

[Jeff King]

On Mon, Sep 14, 2015 at 01:14:20PM +0100, Alan Clucas wrote:

> Pass a single parameter 'amend' to the pre-commit hook when performing a
> commit amend.

I think this is a sensible thing to want, and it has come up a few
times. I'm not sure why the last round didn't get merged, though. Looks
like it just slipped through the cracks.

Here are the relevant threads:

  http://thread.gmane.org/gmane.comp.version-control.git/260122

  http://thread.gmane.org/gmane.comp.version-control.git/260245

Looks like there was some question of what to pass in the normal,
non-amend case. I've added interested parties from the original thread
to the cc here.

-Peff

Re: [PATCH] Pass amend to pre-commit hook

[Alan Clucas]

On 14/09/15 15:47, Jeff King wrote:
> On Mon, Sep 14, 2015 at 01:14:20PM +0100, Alan Clucas wrote:
>
>> Pass a single parameter 'amend' to the pre-commit hook when performing a
>> commit amend.
>
> I think this is a sensible thing to want, and it has come up a few
> times. I'm not sure why the last round didn't get merged, though. Looks
> like it just slipped through the cracks.
>
> Here are the relevant threads:
>
>    http://thread.gmane.org/gmane.comp.version-control.git/260122
>
>    http://thread.gmane.org/gmane.comp.version-control.git/260245
>
> Looks like there was some question of what to pass in the normal,
> non-amend case. I've added interested parties from the original thread
> to the cc here.
>
> -Peff
>

Ah thanks. My google-fu didn't find any of those threads, I should have 
tried specifically searching the archives I guess.

Maybe with a little cajoling we could get some version of this passed. 
The issue that came up before was my worry, I didn't like defining the 
interface that way in case it needed extending... (but other hooks also 
have bad interfaces, so wasn't sure what precedent to follow).

Alan

Re: [PATCH] Pass amend to pre-commit hook

[Øystein Walle]

Jeff King <peff <at> peff.net> writes:

> 
> On Mon, Sep 14, 2015 at 01:14:20PM +0100, Alan Clucas wrote:
> 
> > Pass a single parameter 'amend' to the pre-commit hook when performing a
> > commit amend.
> 
> I think this is a sensible thing to want, and it has come up a few
> times. I'm not sure why the last round didn't get merged, though. Looks
> like it just slipped through the cracks.
> 
> Here are the relevant threads:
> 
>   http://thread.gmane.org/gmane.comp.version-control.git/260122
> 
>   http://thread.gmane.org/gmane.comp.version-control.git/260245
> 
> Looks like there was some question of what to pass in the normal,
> non-amend case. I've added interested parties from the original thread
> to the cc here.
> 
> -Peff
> 

There were so many different ways of solving them that we weren't able
to decide between them:

Assuming we give the string "amend" as the hook's argv[1], what to do
when --amend is not used? We can pass nothing, or the empty string, or
the string "noamend". Then there was the suggestion of exporting
GIT_AMEND=1 or something like that. In any case, what to do when we want
to pass more arguments? Should we let the hook check argv[1] to decide
whether --amend was used, argv[2] to check whether {some scenario here}
is the case? Or make the hook author effectively implement options
parsing?

And then it died out...

In my totally unprofessional opinion anything more complex than maybe
passing "amend" in argv[1] is unwarranted. Since I posted my first patch
almost a year ago there has been no discussion regarding passing other
information along to pre-commit. Furthermore --amend is the only thing I
know of that a pre-commit hook cannot discover on its own. On the other
hand the desire for this has popped up at least twice on #git in the
same time span.

Alan Clucas' solution looks fine to me. It is essentially the same as
mine. But mine had tests and whatnot ;-)

Øsse