[PATCH-v4 4/4] git-commit: add a prepare-commit-msg hook

[PATCH-v4 4/4] git-commit: add a prepare-commit-msg hook

[Paolo Bonzini]

The prepare-commit-msg hook is run whenever a "fresh" commit message
is prepared, just before it is shown in the editor (if it is).
It can modify the commit message in-place and/or abort the commit.

It takes one to three parameters.  The first is the name of the file that
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 as the third parameter (if a -c, -C or --amend option
was given).

If the exit status is non-zero, `git-commit` will abort.  The hook is
not suppressed by the `\--no-verify` option.

The hook is allowed to edit the message file in place.  It can also be
used for the same purpose as the pre-commit message, if the verification
should not be skipped for automatic commits (e.g. during interactive
rebasing).

The default `prepare-commit-msg` comments out the `Conflicts:` part of
a merge's commit message; other examples are commented out, including
adding a Signed-off-by line at the bottom of the commit messsage,
that the user can then edit or discard altogether.

Signed-off-by: Paolo Bonzini <bonzini@gnu.org>
---
 Documentation/git-commit.txt        |    4 +-
 Documentation/hooks.txt             |   26 ++++++
 builtin-commit.c                    |   19 ++++
 t/t7505-prepare-commit-msg-hook.sh  |  155 +++++++++++++++++++++++++++++++++++
 templates/hooks--commit-msg         |    3 +
 templates/hooks--prepare-commit-msg |   36 ++++++++
 6 files changed, 241 insertions(+), 2 deletions(-)
 create mode 100755 t/t7505-prepare-commit-msg-hook.sh
 create mode 100644 templates/hooks--prepare-commit-msg

	This includes all the comments made to v3 by Junio.
	In particular, the second parameter to the hook is
	always a "kind of message source", and the commit ID is
	passed as the third parameter.

	The way it can be used instead of the pre-commit hook
	is now documented differently: it is for verification
	that must *not* be skipped even for automatic commits
	(if there is such a beast).

	The add-S-o-b hook is commented out.  I added another
	example that adds "#" in front of the Conflicts section.
	I like it because it shows how to use the "kind of
	message source".

	Thanks for the thorough review!

diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index c3725b2..b4ae61f 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -280,8 +280,8 @@ order).
 
 HOOKS
 -----
-This command can run `commit-msg`, `pre-commit`, and
-`post-commit` hooks.  See link:hooks.html[hooks] for more
+This command can run `commit-msg`, `prepare-commit-msg`, `pre-commit`,
+and `post-commit` hooks.  See link:hooks.html[hooks] for more
 information.
 
 
diff --git a/Documentation/hooks.txt b/Documentation/hooks.txt
index e8d80cf..b91d85e 100644
--- a/Documentation/hooks.txt
+++ b/Documentation/hooks.txt
@@ -65,6 +65,32 @@ All the `git-commit` hooks are invoked with the environment
 variable `GIT_EDITOR=:` if the command will not bring up an editor
 to modify the commit message.
 
+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 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 hook is not suppressed by the `\--no-verify` option.
+
+The hook is allowed to edit the message file in place.  It can also be
+used for the same purpose as the pre-commit message, if the verification
+should not be skipped for automatic commits (e.g. during interactive
+rebasing).
+
+The default `prepare-commit-msg` comments out the `Conflicts:` part of
+a merge's commit message.
+
 commit-msg
 ----------
 
diff --git a/builtin-commit.c b/builtin-commit.c
index d8945ac..9468d27 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -394,6 +394,8 @@ static int prepare_to_commit(const char *index_file, const char *prefix)
 	struct strbuf sb;
 	char *buffer;
 	FILE *fp;
+	const char *hook_arg1 = NULL;
+	const char *hook_arg2 = NULL;
 
 	if (!no_verify && run_hook(index_file, "pre-commit", NULL))
 		return 0;
@@ -401,32 +403,45 @@ static int prepare_to_commit(const char *index_file, const char *prefix)
 	strbuf_init(&sb, 0);
 	if (message.len) {
 		strbuf_addbuf(&sb, &message);
+		hook_arg1 = "message";
 	} else if (logfile && !strcmp(logfile, "-")) {
 		if (isatty(0))
 			fprintf(stderr, "(reading log message from standard input)\n");
 		if (strbuf_read(&sb, 0, 0) < 0)
 			die("could not read log from standard input");
+		hook_arg1 = "message";
 	} else if (logfile) {
 		if (strbuf_read_file(&sb, logfile, 0) < 0)
 			die("could not read log file '%s': %s",
 			    logfile, strerror(errno));
+		hook_arg1 = "message";
 	} else if (use_message) {
 		buffer = strstr(use_message_buffer, "\n\n");
 		if (!buffer || buffer[2] == '\0')
 			die("commit has empty message");
 		strbuf_add(&sb, buffer + 2, strlen(buffer + 2));
+		hook_arg1 = "commit";
+		hook_arg2 = use_message;
 	} else if (!stat(git_path("MERGE_MSG"), &statbuf)) {
 		if (strbuf_read_file(&sb, git_path("MERGE_MSG"), 0) < 0)
 			die("could not read MERGE_MSG: %s", strerror(errno));
+		hook_arg1 = "merge";
 	} else if (!stat(git_path("SQUASH_MSG"), &statbuf)) {
 		if (strbuf_read_file(&sb, git_path("SQUASH_MSG"), 0) < 0)
 			die("could not read SQUASH_MSG: %s", strerror(errno));
+		hook_arg1 = "squash";
 	} else if (template_file && !stat(template_file, &statbuf)) {
 		if (strbuf_read_file(&sb, template_file, 0) < 0)
 			die("could not read %s: %s",
 			    template_file, strerror(errno));
+		hook_arg1 = "template";
 	}
 
+	/* This final case does not modify the template message, it just sets
+	   the argument to the prepare-commit-msg hook.  */
+	else if (in_merge)
+		hook_arg1 = "merge";
+
 	fp = fopen(git_path(commit_editmsg), "w");
 	if (fp == NULL)
 		die("could not open %s", git_path(commit_editmsg));
@@ -534,6 +549,10 @@ static int prepare_to_commit(const char *index_file, const char *prefix)
 		return 0;
 	}
 
+	if (run_hook(index_file, "prepare-commit-msg",
+		     git_path(commit_editmsg), hook_arg1, hook_arg2, NULL))
+		return 0;
+
 	if (use_editor) {
 		char index[PATH_MAX];
 		const char *env[2] = { index, NULL };
diff --git a/t/t7505-prepare-commit-msg-hook.sh b/t/t7505-prepare-commit-msg-hook.sh
new file mode 100755
index 0000000..7ddec99
--- /dev/null
+++ b/t/t7505-prepare-commit-msg-hook.sh
@@ -0,0 +1,155 @@
+#!/bin/sh
+
+test_description='prepare-commit-msg hook'
+
+. ./test-lib.sh
+
+test_expect_success 'with no hook' '
+
+	echo "foo" > file &&
+	git add file &&
+	git commit -m "first"
+
+'
+
+# set up fake editor for interactive editing
+cat > fake-editor <<'EOF'
+#!/bin/sh
+exit 0
+EOF
+chmod +x fake-editor
+FAKE_EDITOR="$(pwd)/fake-editor"
+export FAKE_EDITOR
+
+# now install hook that always succeeds and adds a message
+HOOKDIR="$(git rev-parse --git-dir)/hooks"
+HOOK="$HOOKDIR/prepare-commit-msg"
+mkdir -p "$HOOKDIR"
+cat > "$HOOK" <<'EOF'
+#!/bin/sh
+if test "$2" = commit; then
+  source=$(git-rev-parse "$3")
+else
+  source=${2-default}
+fi
+if test "$GIT_EDITOR" = :; then
+  sed -e "1s/.*/$source (no editor)/" "$1" > msg.tmp
+else
+  sed -e "1s/.*/$source/" "$1" > msg.tmp
+fi
+mv msg.tmp "$1"
+exit 0
+EOF
+chmod +x "$HOOK"
+
+echo dummy template > "$(git rev-parse --git-dir)/template"
+
+test_expect_success 'with hook (-m)' '
+
+	echo "more" >> file &&
+	git add file &&
+	git commit -m "more" &&
+	test "`git log -1 --pretty=format:%s`" = "message (no editor)"
+
+'
+
+test_expect_success 'with hook (-m editor)' '
+
+	echo "more" >> file &&
+	git add file &&
+	GIT_EDITOR="$FAKE_EDITOR" git commit -e -m "more more" &&
+	test "`git log -1 --pretty=format:%s`" = message
+
+'
+
+test_expect_success 'with hook (-t)' '
+
+	echo "more" >> file &&
+	git add file &&
+	git commit -t "$(git rev-parse --git-dir)/template" &&
+	test "`git log -1 --pretty=format:%s`" = template
+
+'
+
+test_expect_success 'with hook (-F)' '
+
+	echo "more" >> file &&
+	git add file &&
+	(echo more | git commit -F -) &&
+	test "`git log -1 --pretty=format:%s`" = "message (no editor)"
+
+'
+
+test_expect_success 'with hook (-F editor)' '
+
+	echo "more" >> file &&
+	git add file &&
+	(echo more more | GIT_EDITOR="$FAKE_EDITOR" git commit -e -F -) &&
+	test "`git log -1 --pretty=format:%s`" = message
+
+'
+
+test_expect_success 'with hook (-C)' '
+
+	head=`git rev-parse HEAD` &&
+	echo "more" >> file &&
+	git add file &&
+	git commit -C $head &&
+	test "`git log -1 --pretty=format:%s`" = "$head (no editor)"
+
+'
+
+test_expect_success 'with hook (editor)' '
+
+	echo "more more" >> file &&
+	git add file &&
+	GIT_EDITOR="$FAKE_EDITOR" git commit &&
+	test "`git log -1 --pretty=format:%s`" = default
+
+'
+
+test_expect_success 'with hook (--amend)' '
+
+	head=`git rev-parse HEAD` &&
+	echo "more" >> file &&
+	git add file &&
+	GIT_EDITOR="$FAKE_EDITOR" git commit --amend &&
+	test "`git log -1 --pretty=format:%s`" = "$head"
+
+'
+
+test_expect_success 'with hook (-c)' '
+
+	head=`git rev-parse HEAD` &&
+	echo "more" >> file &&
+	git add file &&
+	GIT_EDITOR="$FAKE_EDITOR" git commit -c $head &&
+	test "`git log -1 --pretty=format:%s`" = "$head"
+
+'
+
+cat > "$HOOK" <<'EOF'
+#!/bin/sh
+exit 1
+EOF
+
+test_expect_success 'with failing hook' '
+
+	head=`git rev-parse HEAD` &&
+	echo "more" >> file &&
+	git add file &&
+	! GIT_EDITOR="$FAKE_EDITOR" git commit -c $head
+
+'
+
+test_expect_success 'with failing hook (--no-verify)' '
+
+	head=`git rev-parse HEAD` &&
+	echo "more" >> file &&
+	git add file &&
+	! GIT_EDITOR="$FAKE_EDITOR" git commit --no-verify -c $head
+
+'
+
+
+test_done
diff --git a/templates/hooks--commit-msg b/templates/hooks--commit-msg
index c5cdb9d..4ef86eb 100644
--- a/templates/hooks--commit-msg
+++ b/templates/hooks--commit-msg
@@ -9,6 +9,9 @@
 # To enable this hook, make this file executable.
 
 # Uncomment the below to add a Signed-off-by line to the message.
+# Doing this in a hook is a bad idea in general, but the prepare-commit-msg
+# hook is more suited to it.
+#
 # SOB=$(git var GIT_AUTHOR_IDENT | sed -n 's/^\(.*>\).*$/Signed-off-by: \1/p')
 # grep -qs "^$SOB" "$1" || echo "$SOB" >> "$1"
 
diff --git a/templates/hooks--prepare-commit-msg b/templates/hooks--prepare-commit-msg
new file mode 100644
index 0000000..8509ba0
--- /dev/null
+++ b/templates/hooks--prepare-commit-msg
@@ -0,0 +1,36 @@
+#!/bin/sh
+#
+# An example hook script to prepare the commit log message.
+# Called by git-commit with the name of the file that has the
+# commit message, followed by the description of the commit
+# message's source.  The hook should exit with non-zero
+# status after issuing an appropriate message if it wants to stop the
+# commit.  The hook is allowed to edit the commit message file.
+#
+# To enable this hook, make this file executable.
+
+# This hook includes three examples.  The first comments out the
+# "Conflicts:" part of a merge commit.
+#
+# The second includes the output of "git diff --name-status -r"
+# into the message, just before the "git status" output.  It is
+# commented because it doesn't cope with --amend or with squashed
+# commits.
+#
+# The third example adds a Signed-off-by line to the message, that can
+# still be edited.  This is rarely a good idea.
+
+case "$2 $3" in
+  merge)
+    sed -i '/^Conflicts:/,/#/!b;s/^/# &/;s/^# #/#/' "$1" ;;
+
+# ""|template)
+#   perl -i -pe '
+#      print "\n" . `git diff --cached --name-status -r`
+#	 if /^#/ && $first++ == 0' "$1" ;;
+
+  *) ;;
+esac
+
+# SOB=$(git var GIT_AUTHOR_IDENT | sed -n 's/^\(.*>\).*$/Signed-off-by: \1/p')
+# grep -qs "^$SOB" "$1" || echo "$SOB" >> "$1"
-- 
1.5.3.4.910.gc5122-dirty

[PATCH-v5 4/4] git-commit: add a prepare-commit-msg hook

[Paolo Bonzini]

The prepare-commit-msg hook is run whenever a "fresh" commit message
is prepared, just before it is shown in the editor (if it is).
It can modify the commit message in-place and/or abort the commit.

It takes one to three parameters.  The first is the name of the file that
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 as the third parameter (if a -c, -C or --amend option
was given).

If the exit status is non-zero, git-commit will abort.  The hook is
not suppressed by the --no-verify option, so it should not be used
as a replacement for the pre-commit hook.

The hook is allowed to edit the message file in place.  It can also be
used for the same purpose as the pre-commit message, if the verification
should not be skipped for automatic commits (e.g. during interactive
rebasing).

The default prepare-commit-msg comments out the `Conflicts:` part of
a merge's commit message; other examples are commented out, including
adding a Signed-off-by line at the bottom of the commit messsage,
that the user can then edit or discard altogether.

Signed-off-by: Paolo Bonzini <bonzini@gnu.org>
---
 Documentation/git-commit.txt        |    4 +-
 Documentation/hooks.txt             |   26 ++++++
 builtin-commit.c                    |   19 ++++
 t/t7505-prepare-commit-msg-hook.sh  |  155 +++++++++++++++++++++++++++++++++++
 templates/hooks--commit-msg         |    3 +
 templates/hooks--prepare-commit-msg |   36 ++++++++
 6 files changed, 241 insertions(+), 2 deletions(-)
 create mode 100755 t/t7505-prepare-commit-msg-hook.sh
 create mode 100644 templates/hooks--prepare-commit-msg

diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index c3725b2..b4ae61f 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -280,8 +280,8 @@ order).
 
 HOOKS
 -----
-This command can run `commit-msg`, `pre-commit`, and
-`post-commit` hooks.  See link:hooks.html[hooks] for more
+This command can run `commit-msg`, `prepare-commit-msg`, `pre-commit`,
+and `post-commit` hooks.  See link:hooks.html[hooks] for more
 information.
 
 
diff --git a/Documentation/hooks.txt b/Documentation/hooks.txt
index e8d80cf..b91d85e 100644
--- a/Documentation/hooks.txt
+++ b/Documentation/hooks.txt
@@ -65,6 +65,32 @@ All the `git-commit` hooks are invoked with the environment
 variable `GIT_EDITOR=:` if the command will not bring up an editor
 to modify the commit message.
 
+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 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 hook is not suppressed by the `\--no-verify` option.
+
+The hook is allowed to edit the message file in place.  It can also be
+used for the same purpose as the pre-commit message, if the verification
+should not be skipped for automatic commits (e.g. during interactive
+rebasing).
+
+The default `prepare-commit-msg` comments out the `Conflicts:` part of
+a merge's commit message.
+
 commit-msg
 ----------
 
diff --git a/builtin-commit.c b/builtin-commit.c
index d8945ac..9468d27 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -394,6 +394,8 @@ static int prepare_to_commit(const char *index_file, const char *prefix)
 	struct strbuf sb;
 	char *buffer;
 	FILE *fp;
+	const char *hook_arg1 = NULL;
+	const char *hook_arg2 = NULL;
 
 	if (!no_verify && run_hook(index_file, "pre-commit", NULL))
 		return 0;
@@ -401,32 +403,45 @@ static int prepare_to_commit(const char *index_file, const char *prefix)
 	strbuf_init(&sb, 0);
 	if (message.len) {
 		strbuf_addbuf(&sb, &message);
+		hook_arg1 = "message";
 	} else if (logfile && !strcmp(logfile, "-")) {
 		if (isatty(0))
 			fprintf(stderr, "(reading log message from standard input)\n");
 		if (strbuf_read(&sb, 0, 0) < 0)
 			die("could not read log from standard input");
+		hook_arg1 = "message";
 	} else if (logfile) {
 		if (strbuf_read_file(&sb, logfile, 0) < 0)
 			die("could not read log file '%s': %s",
 			    logfile, strerror(errno));
+		hook_arg1 = "message";
 	} else if (use_message) {
 		buffer = strstr(use_message_buffer, "\n\n");
 		if (!buffer || buffer[2] == '\0')
 			die("commit has empty message");
 		strbuf_add(&sb, buffer + 2, strlen(buffer + 2));
+		hook_arg1 = "commit";
+		hook_arg2 = use_message;
 	} else if (!stat(git_path("MERGE_MSG"), &statbuf)) {
 		if (strbuf_read_file(&sb, git_path("MERGE_MSG"), 0) < 0)
 			die("could not read MERGE_MSG: %s", strerror(errno));
+		hook_arg1 = "merge";
 	} else if (!stat(git_path("SQUASH_MSG"), &statbuf)) {
 		if (strbuf_read_file(&sb, git_path("SQUASH_MSG"), 0) < 0)
 			die("could not read SQUASH_MSG: %s", strerror(errno));
+		hook_arg1 = "squash";
 	} else if (template_file && !stat(template_file, &statbuf)) {
 		if (strbuf_read_file(&sb, template_file, 0) < 0)
 			die("could not read %s: %s",
 			    template_file, strerror(errno));
+		hook_arg1 = "template";
 	}
 
+	/* This final case does not modify the template message, it just sets
+	   the argument to the prepare-commit-msg hook.  */
+	else if (in_merge)
+		hook_arg1 = "merge";
+
 	fp = fopen(git_path(commit_editmsg), "w");
 	if (fp == NULL)
 		die("could not open %s", git_path(commit_editmsg));
@@ -534,6 +549,10 @@ static int prepare_to_commit(const char *index_file, const char *prefix)
 		return 0;
 	}
 
+	if (run_hook(index_file, "prepare-commit-msg",
+		     git_path(commit_editmsg), hook_arg1, hook_arg2, NULL))
+		return 0;
+
 	if (use_editor) {
 		char index[PATH_MAX];
 		const char *env[2] = { index, NULL };
diff --git a/t/t7505-prepare-commit-msg-hook.sh b/t/t7505-prepare-commit-msg-hook.sh
new file mode 100755
index 0000000..7ddec99
--- /dev/null
+++ b/t/t7505-prepare-commit-msg-hook.sh
@@ -0,0 +1,155 @@
+#!/bin/sh
+
+test_description='prepare-commit-msg hook'
+
+. ./test-lib.sh
+
+test_expect_success 'with no hook' '
+
+	echo "foo" > file &&
+	git add file &&
+	git commit -m "first"
+
+'
+
+# set up fake editor for interactive editing
+cat > fake-editor <<'EOF'
+#!/bin/sh
+exit 0
+EOF
+chmod +x fake-editor
+FAKE_EDITOR="$(pwd)/fake-editor"
+export FAKE_EDITOR
+
+# now install hook that always succeeds and adds a message
+HOOKDIR="$(git rev-parse --git-dir)/hooks"
+HOOK="$HOOKDIR/prepare-commit-msg"
+mkdir -p "$HOOKDIR"
+cat > "$HOOK" <<'EOF'
+#!/bin/sh
+if test "$2" = commit; then
+  source=$(git-rev-parse "$3")
+else
+  source=${2-default}
+fi
+if test "$GIT_EDITOR" = :; then
+  sed -e "1s/.*/$source (no editor)/" "$1" > msg.tmp
+else
+  sed -e "1s/.*/$source/" "$1" > msg.tmp
+fi
+mv msg.tmp "$1"
+exit 0
+EOF
+chmod +x "$HOOK"
+
+echo dummy template > "$(git rev-parse --git-dir)/template"
+
+test_expect_success 'with hook (-m)' '
+
+	echo "more" >> file &&
+	git add file &&
+	git commit -m "more" &&
+	test "`git log -1 --pretty=format:%s`" = "message (no editor)"
+
+'
+
+test_expect_success 'with hook (-m editor)' '
+
+	echo "more" >> file &&
+	git add file &&
+	GIT_EDITOR="$FAKE_EDITOR" git commit -e -m "more more" &&
+	test "`git log -1 --pretty=format:%s`" = message
+
+'
+
+test_expect_success 'with hook (-t)' '
+
+	echo "more" >> file &&
+	git add file &&
+	git commit -t "$(git rev-parse --git-dir)/template" &&
+	test "`git log -1 --pretty=format:%s`" = template
+
+'
+
+test_expect_success 'with hook (-F)' '
+
+	echo "more" >> file &&
+	git add file &&
+	(echo more | git commit -F -) &&
+	test "`git log -1 --pretty=format:%s`" = "message (no editor)"
+
+'
+
+test_expect_success 'with hook (-F editor)' '
+
+	echo "more" >> file &&
+	git add file &&
+	(echo more more | GIT_EDITOR="$FAKE_EDITOR" git commit -e -F -) &&
+	test "`git log -1 --pretty=format:%s`" = message
+
+'
+
+test_expect_success 'with hook (-C)' '
+
+	head=`git rev-parse HEAD` &&
+	echo "more" >> file &&
+	git add file &&
+	git commit -C $head &&
+	test "`git log -1 --pretty=format:%s`" = "$head (no editor)"
+
+'
+
+test_expect_success 'with hook (editor)' '
+
+	echo "more more" >> file &&
+	git add file &&
+	GIT_EDITOR="$FAKE_EDITOR" git commit &&
+	test "`git log -1 --pretty=format:%s`" = default
+
+'
+
+test_expect_success 'with hook (--amend)' '
+
+	head=`git rev-parse HEAD` &&
+	echo "more" >> file &&
+	git add file &&
+	GIT_EDITOR="$FAKE_EDITOR" git commit --amend &&
+	test "`git log -1 --pretty=format:%s`" = "$head"
+
+'
+
+test_expect_success 'with hook (-c)' '
+
+	head=`git rev-parse HEAD` &&
+	echo "more" >> file &&
+	git add file &&
+	GIT_EDITOR="$FAKE_EDITOR" git commit -c $head &&
+	test "`git log -1 --pretty=format:%s`" = "$head"
+
+'
+
+cat > "$HOOK" <<'EOF'
+#!/bin/sh
+exit 1
+EOF
+
+test_expect_success 'with failing hook' '
+
+	head=`git rev-parse HEAD` &&
+	echo "more" >> file &&
+	git add file &&
+	! GIT_EDITOR="$FAKE_EDITOR" git commit -c $head
+
+'
+
+test_expect_success 'with failing hook (--no-verify)' '
+
+	head=`git rev-parse HEAD` &&
+	echo "more" >> file &&
+	git add file &&
+	! GIT_EDITOR="$FAKE_EDITOR" git commit --no-verify -c $head
+
+'
+
+
+test_done
diff --git a/templates/hooks--commit-msg b/templates/hooks--commit-msg
index c5cdb9d..4ef86eb 100644
--- a/templates/hooks--commit-msg
+++ b/templates/hooks--commit-msg
@@ -9,6 +9,9 @@
 # To enable this hook, make this file executable.
 
 # Uncomment the below to add a Signed-off-by line to the message.
+# Doing this in a hook is a bad idea in general, but the prepare-commit-msg
+# hook is more suited to it.
+#
 # SOB=$(git var GIT_AUTHOR_IDENT | sed -n 's/^\(.*>\).*$/Signed-off-by: \1/p')
 # grep -qs "^$SOB" "$1" || echo "$SOB" >> "$1"
 
diff --git a/templates/hooks--prepare-commit-msg b/templates/hooks--prepare-commit-msg
new file mode 100644
index 0000000..8509ba0
--- /dev/null
+++ b/templates/hooks--prepare-commit-msg
@@ -0,0 +1,36 @@
+#!/bin/sh
+#
+# An example hook script to prepare the commit log message.
+# Called by git-commit with the name of the file that has the
+# commit message, followed by the description of the commit
+# message's source.  The hook should exit with non-zero
+# status after issuing an appropriate message if it wants to stop the
+# commit.  The hook is allowed to edit the commit message file.
+#
+# To enable this hook, make this file executable.
+
+# This hook includes three examples.  The first comments out the
+# "Conflicts:" part of a merge commit.
+#
+# The second includes the output of "git diff --name-status -r"
+# into the message, just before the "git status" output.  It is
+# commented because it doesn't cope with --amend or with squashed
+# commits.
+#
+# The third example adds a Signed-off-by line to the message, that can
+# still be edited.  This is rarely a good idea.
+
+case "$2 $3" in
+  merge)
+    sed -i '/^Conflicts:/,/#/!b;s/^/# &/;s/^# #/#/' "$1" ;;
+
+# ""|template)
+#   perl -i -pe '
+#      print "\n" . `git diff --cached --name-status -r`
+#	 if /^#/ && $first++ == 0' "$1" ;;
+
+  *) ;;
+esac
+
+# SOB=$(git var GIT_AUTHOR_IDENT | sed -n 's/^\(.*>\).*$/Signed-off-by: \1/p')
+# grep -qs "^$SOB" "$1" || echo "$SOB" >> "$1"
-- 
1.5.3.4.910.gc5122-dirty

[PATCH-v3 4/4] git-commit: add a prepare-commit-msg hook

[Paolo Bonzini]

The prepare-commit-msg hook is run whenever a "fresh" commit message
is prepared, just before it is shown in the editor (if it is).
It can modify the commit message in-place and/or abort the commit.

It takes two parameters.  The first 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 a commit SHA1 (if a
-c, -C or --amend option was given).  The second
parameter if the name of the file that the commit log message.

If the exit status is non-zero, `git-commit` will abort.
The hook is not suppressed by the --no-verify option, and
this is the only hook that can abort a commit if --no-verify is
given.

While the default hook just adds a Signed-off-by line at the bottom
of the commit messsage, the hook is more intended to build a template
for the commit message following project standards, that the user
can then edit or discard altogether.

Signed-off-by: Paolo Bonzini <bonzini@gnu.org>
---
 Documentation/git-commit.txt        |    4 +-
 Documentation/hooks.txt             |   34 ++++++++-
 builtin-commit.c                    |   17 ++++
 t/t7505-prepare-commit-msg-hook.sh  |  153 +++++++++++++++++++++++++++++++++++
 templates/hooks--commit-msg         |    3 +
 templates/hooks--prepare-commit-msg |   14 +++
 6 files changed, 222 insertions(+), 3 deletions(-)
 create mode 100755 t/t7505-prepare-commit-msg-hook.sh
 create mode 100644 templates/hooks--prepare-commit-msg

	Other parts are unchanged from v2.

diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index c3725b2..b4ae61f 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -280,8 +280,8 @@ order).
 
 HOOKS
 -----
-This command can run `commit-msg`, `pre-commit`, and
-`post-commit` hooks.  See link:hooks.html[hooks] for more
+This command can run `commit-msg`, `prepare-commit-msg`, `pre-commit`,
+and `post-commit` hooks.  See link:hooks.html[hooks] for more
 information.
 
 
diff --git a/Documentation/hooks.txt b/Documentation/hooks.txt
index e8d80cf..6df5c42 100644
--- a/Documentation/hooks.txt
+++ b/Documentation/hooks.txt
@@ -55,7 +55,8 @@ 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.
+causes the `git-commit` to abort.  This hook can also modify
+the index.
 
 The default 'pre-commit' hook, when enabled, catches introduction
 of lines with trailing whitespaces and aborts the commit when
@@ -65,6 +66,37 @@ All the `git-commit` hooks are invoked with the environment
 variable `GIT_EDITOR=:` if the command will not bring up an editor
 to modify the commit message.
 
+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 two parameters.  The first 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 a commit SHA1 (if a
+`\-c`, `\-C` or `\--amend` option was given).  The second
+parameter if the name of the file that the commit log message.
+
+If the exit status is non-zero, `git-commit` will abort.
+The hook is not suppressed by the `\--no-verify` option, and
+this is the only hook that can abort a commit if `\--no-verify`
+is given.
+
+The hook is allowed to edit the message file in place, and
+can be used to augment the default commit message with some
+project standard information.  It can also be used for the same
+purpose as the pre-commit message, if the verification has
+to be skipped for automatic commits (e.g. during rebasing).
+
+The default 'prepare-commit-msg' hook adds a Signed-off-by line
+(doing it with a hook is not necessarily a good idea, but doing
+it in 'commit-msg' is worse because you are not reminded in
+the editor).
+
 commit-msg
 ----------
 
diff --git a/builtin-commit.c b/builtin-commit.c
index d8945ac..84ead34 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -394,6 +394,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix)
 	struct strbuf sb;
 	char *buffer;
 	FILE *fp;
+	const char *hook_arg = NULL;
 
 	if (!no_verify && run_hook(index_file, "pre-commit", NULL))
 		return 0;
@@ -401,32 +402,44 @@ static int prepare_to_commit(const char *index_file, const char *prefix)
 	strbuf_init(&sb, 0);
 	if (message.len) {
 		strbuf_addbuf(&sb, &message);
+		hook_arg = "message";
 	} else if (logfile && !strcmp(logfile, "-")) {
 		if (isatty(0))
 			fprintf(stderr, "(reading log message from standard input)\n");
 		if (strbuf_read(&sb, 0, 0) < 0)
 			die("could not read log from standard input");
+		hook_arg = "message";
 	} else if (logfile) {
 		if (strbuf_read_file(&sb, logfile, 0) < 0)
 			die("could not read log file '%s': %s",
 			    logfile, strerror(errno));
+		hook_arg = "message";
 	} else if (use_message) {
 		buffer = strstr(use_message_buffer, "\n\n");
 		if (!buffer || buffer[2] == '\0')
 			die("commit has empty message");
 		strbuf_add(&sb, buffer + 2, strlen(buffer + 2));
+		hook_arg = use_message;
 	} else if (!stat(git_path("MERGE_MSG"), &statbuf)) {
 		if (strbuf_read_file(&sb, git_path("MERGE_MSG"), 0) < 0)
 			die("could not read MERGE_MSG: %s", strerror(errno));
+		hook_arg = "merge";
 	} else if (!stat(git_path("SQUASH_MSG"), &statbuf)) {
 		if (strbuf_read_file(&sb, git_path("SQUASH_MSG"), 0) < 0)
 			die("could not read SQUASH_MSG: %s", strerror(errno));
+		hook_arg = "squash";
 	} else if (template_file && !stat(template_file, &statbuf)) {
 		if (strbuf_read_file(&sb, template_file, 0) < 0)
 			die("could not read %s: %s",
 			    template_file, strerror(errno));
+		hook_arg = "template";
 	}
 
+	/* This final case does not modify the template message, it just sets
+	   the argument to the prepare-commit-msg hook.  */
+	else if (in_merge)
+		hook_arg = "merge";
+
 	fp = fopen(git_path(commit_editmsg), "w");
 	if (fp == NULL)
 		die("could not open %s", git_path(commit_editmsg));
@@ -534,6 +547,10 @@ static int prepare_to_commit(const char *index_file, const char *prefix)
 		return 0;
 	}
 
+	if (run_hook(index_file, "prepare-commit-msg",
+		     git_path(commit_editmsg), hook_arg, NULL))
+		return 0;
+
 	if (use_editor) {
 		char index[PATH_MAX];
 		const char *env[2] = { index, NULL };
diff --git a/t/t7505-prepare-commit-msg-hook.sh b/t/t7505-prepare-commit-msg-hook.sh
new file mode 100755
index 0000000..9632f97
--- /dev/null
+++ b/t/t7505-prepare-commit-msg-hook.sh
@@ -0,0 +1,153 @@
+#!/bin/sh
+
+test_description='prepare-commit-msg hook'
+
+. ./test-lib.sh
+
+test_expect_success 'with no hook' '
+
+	echo "foo" > file &&
+	git add file &&
+	git commit -m "first"
+
+'
+
+# set up fake editor for interactive editing
+cat > fake-editor <<'EOF'
+#!/bin/sh
+exit 0
+EOF
+chmod +x fake-editor
+FAKE_EDITOR="$(pwd)/fake-editor"
+export FAKE_EDITOR
+
+# now install hook that always succeeds and adds a message
+HOOKDIR="$(git rev-parse --git-dir)/hooks"
+HOOK="$HOOKDIR/prepare-commit-msg"
+mkdir -p "$HOOKDIR"
+cat > "$HOOK" <<'EOF'
+#!/bin/sh
+if test "$2" = HEAD; then
+  set "$1" $(git-rev-parse HEAD)
+fi
+if test "$GIT_EDITOR" = :; then
+  sed -e "1s/.*/${2-default} (no editor)/" "$1" > msg.tmp
+else
+  sed -e "1s/.*/${2-default}/" "$1" > msg.tmp
+fi
+mv msg.tmp "$1"
+exit 0
+EOF
+chmod +x "$HOOK"
+
+echo dummy template > "$(git rev-parse --git-dir)/template"
+
+test_expect_success 'with hook (-m)' '
+
+	echo "more" >> file &&
+	git add file &&
+	git commit -m "more" &&
+	test "`git log -1 --pretty=format:%s`" = "message (no editor)"
+
+'
+
+test_expect_success 'with hook (-m editor)' '
+
+	echo "more" >> file &&
+	git add file &&
+	GIT_EDITOR="$FAKE_EDITOR" git commit -e -m "more more" &&
+	test "`git log -1 --pretty=format:%s`" = message
+
+'
+
+test_expect_success 'with hook (-t)' '
+
+	echo "more" >> file &&
+	git add file &&
+	git commit -t "$(git rev-parse --git-dir)/template" &&
+	test "`git log -1 --pretty=format:%s`" = template
+
+'
+
+test_expect_success 'with hook (-F)' '
+
+	echo "more" >> file &&
+	git add file &&
+	(echo more | git commit -F -) &&
+	test "`git log -1 --pretty=format:%s`" = "message (no editor)"
+
+'
+
+test_expect_success 'with hook (-F editor)' '
+
+	echo "more" >> file &&
+	git add file &&
+	(echo more more | GIT_EDITOR="$FAKE_EDITOR" git commit -e -F -) &&
+	test "`git log -1 --pretty=format:%s`" = message
+
+'
+
+test_expect_success 'with hook (-C)' '
+
+	head=`git rev-parse HEAD` &&
+	echo "more" >> file &&
+	git add file &&
+	git commit -C $head &&
+	test "`git log -1 --pretty=format:%s`" = "$head (no editor)"
+
+'
+
+test_expect_success 'with hook (editor)' '
+
+	echo "more more" >> file &&
+	git add file &&
+	GIT_EDITOR="$FAKE_EDITOR" git commit &&
+	test "`git log -1 --pretty=format:%s`" = default
+
+'
+
+test_expect_success 'with hook (--amend)' '
+
+	head=`git rev-parse HEAD` &&
+	echo "more" >> file &&
+	git add file &&
+	GIT_EDITOR="$FAKE_EDITOR" git commit --amend &&
+	test "`git log -1 --pretty=format:%s`" = "$head"
+
+'
+
+test_expect_success 'with hook (-c)' '
+
+	head=`git rev-parse HEAD` &&
+	echo "more" >> file &&
+	git add file &&
+	GIT_EDITOR="$FAKE_EDITOR" git commit -c $head &&
+	test "`git log -1 --pretty=format:%s`" = "$head"
+
+'
+
+cat > "$HOOK" <<'EOF'
+#!/bin/sh
+exit 1
+EOF
+
+test_expect_success 'with failing hook' '
+
+	head=`git rev-parse HEAD` &&
+	echo "more" >> file &&
+	git add file &&
+	! GIT_EDITOR="$FAKE_EDITOR" git commit -c $head
+
+'
+
+test_expect_success 'with failing hook (--no-verify)' '
+
+	head=`git rev-parse HEAD` &&
+	echo "more" >> file &&
+	git add file &&
+	! GIT_EDITOR="$FAKE_EDITOR" git commit --no-verify -c $head
+
+'
+
+
+test_done
diff --git a/templates/hooks--commit-msg b/templates/hooks--commit-msg
index c5cdb9d..4ef86eb 100644
--- a/templates/hooks--commit-msg
+++ b/templates/hooks--commit-msg
@@ -9,6 +9,9 @@
 # To enable this hook, make this file executable.
 
 # Uncomment the below to add a Signed-off-by line to the message.
+# Doing this in a hook is a bad idea in general, but the prepare-commit-msg
+# hook is more suited to it.
+#
 # SOB=$(git var GIT_AUTHOR_IDENT | sed -n 's/^\(.*>\).*$/Signed-off-by: \1/p')
 # grep -qs "^$SOB" "$1" || echo "$SOB" >> "$1"
 
diff --git a/templates/hooks--prepare-commit-msg b/templates/hooks--prepare-commit-msg
new file mode 100644
index 0000000..176283b
--- /dev/null
+++ b/templates/hooks--prepare-commit-msg
@@ -0,0 +1,14 @@
+#!/bin/sh
+#
+# An example hook script to prepare the commit log message.
+# Called by git-commit with one argument, the name of the file
+# that has the commit message.  The hook should exit with non-zero
+# status after issuing an appropriate message if it wants to stop the
+# commit.  The hook is allowed to edit the commit message file.
+#
+# To enable this hook, make this file executable.
+
+# This example adds a Signed-off-by line to the message, that can
+# still be edited.
+SOB=$(git var GIT_AUTHOR_IDENT | sed -n 's/^\(.*>\).*$/Signed-off-by: \1/p')
+grep -qs "^$SOB" "$1" || echo "$SOB" >> "$1"
-- 
1.5.3.4.910.gc5122-dirty

Re: [PATCH-v3 4/4] git-commit: add a prepare-commit-msg hook

[Junio C Hamano]

Paolo Bonzini <bonzini@gnu.org> writes:

> The prepare-commit-msg hook is run whenever a "fresh" commit message
> is prepared, just before it is shown in the editor (if it is).
> It can modify the commit message in-place and/or abort the commit.
>
> It takes two parameters.  The first 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 a commit SHA1 (if a
> -c, -C or --amend option was given).  The second
> parameter if the name of the file that the commit log message.
>
> If the exit status is non-zero, `git-commit` will abort.
> The hook is not suppressed by the --no-verify option, and
> this is the only hook that can abort a commit if --no-verify is
> given.

I doubt the last two lines should be said.  What it says is
correct, but that is because we happen to have only two hooks
that are not at all about validation (after this patch is
applied), and the other one is post-commit that is _defined_ to
ignore the error.  It should be the norm for hook failure to
abort the execution of the main command.

> Signed-off-by: Paolo Bonzini <bonzini@gnu.org>

Good.  The earlier three patches lacked S-o-b.

> diff --git a/Documentation/hooks.txt b/Documentation/hooks.txt
> index e8d80cf..6df5c42 100644
> --- a/Documentation/hooks.txt
> +++ b/Documentation/hooks.txt
> @@ -55,7 +55,8 @@ 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.
> +causes the `git-commit` to abort.  This hook can also modify
> +the index.
>  
>  The default 'pre-commit' hook, when enabled, catches introduction
>  of lines with trailing whitespaces and aborts the commit when

This hunk is a good documentation fix but does not belong to the
series.

> @@ -65,6 +66,37 @@ All the `git-commit` hooks are invoked with the environment
>  variable `GIT_EDITOR=:` if the command will not bring up an editor
>  to modify the commit message.
>  
> +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 two parameters.  The first 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 a commit SHA1 (if a
> +`\-c`, `\-C` or `\--amend` option was given).  The second
> +parameter if the name of the file that the commit log message.

The second parameter "is"???

I suspect "or a commit SHA1" is a wrong interface.  Wouldn't it
be much easier for the hook to parse its arguments if you give a
token "commit" and the commit object name as separate arguments?

> +If the exit status is non-zero, `git-commit` will abort.
> +The hook is not suppressed by the `\--no-verify` option, and
> +this is the only hook that can abort a commit if `\--no-verify`
> +is given.

I'd suggest removing ", and this is.....given".

> +The hook is allowed to edit the message file in place, and
> +can be used to augment the default commit message with some
> +project standard information.

"with some project standard information."  Is that something you
need to say here?  It is allowed to augment the default commit
message in _any way_, isn't it?

> +It can also be used for the same
> +purpose as the pre-commit message, if the verification has
> +to be skipped for automatic commits (e.g. during rebasing).

As we seem to have agreed to make its exit status matter, it
cannot be used for that.  A non-zero exit from the script is an
error and not verification failure.

> +The default 'prepare-commit-msg' hook adds a Signed-off-by line
> +(doing it with a hook is not necessarily a good idea, but doing
> +it in 'commit-msg' is worse because you are not reminded in
> +the editor).

I am moderately opposed to call something that blindly adds
S-o-b "the default".  S-o-b should be a conscious choice.  I am
Ok with an "example" for people whose work contract says "what I
write is always Open Source", but that is far from the norm, so
it can hardly be the default nor necessarily is a good example.

Perhaps we can add diffstat with '#' prefix as an example, like
this, instead?

	#!/bin/sh
	# Note that this needs to be tightened to deal with the root
	# commit sensibly.
	git diff --name-status HEAD -- >>"$1"

> diff --git a/templates/hooks--commit-msg b/templates/hooks--commit-msg
> index c5cdb9d..4ef86eb 100644
> --- a/templates/hooks--commit-msg
> +++ b/templates/hooks--commit-msg
> @@ -9,6 +9,9 @@
>  # To enable this hook, make this file executable.
>  
>  # Uncomment the below to add a Signed-off-by line to the message.
> +# Doing this in a hook is a bad idea in general, but the prepare-commit-msg
> +# hook is more suited to it.
> +#
>  # SOB=$(git var GIT_AUTHOR_IDENT | sed -n 's/^\(.*>\).*$/Signed-off-by: \1/p')
>  # grep -qs "^$SOB" "$1" || echo "$SOB" >> "$1"

And this is a good change.

Re: [PATCH-v4 4/4] git-commit: add a prepare-commit-msg hook

[Junio C Hamano]

Paolo Bonzini <bonzini@gnu.org> writes:

> 	The way it can be used instead of the pre-commit hook
> 	is now documented differently: it is for verification
> 	that must *not* be skipped even for automatic commits
> 	(if there is such a beast).

I really do not think we should encourage this as a verification
method.  If you encourage it as such, I am sure people would
complain "Why can't I bypass this hook with --no-verify!!!".
I do not want to spend cycles defending the behaviour to error
out on non-zero exit from the hook.

Instead, I think you should _stress_ that people must _NOT_ use
this as a replacement for pre-commit hook.

The same comment applies to the comment in the example hook.

Re: [PATCH-v5 4/4] git-commit: add a prepare-commit-msg hook

[Junio C Hamano]

Paolo Bonzini <bonzini@gnu.org> writes:

> If the exit status is non-zero, git-commit will abort.  The hook is
> not suppressed by the --no-verify option, so it should not be used
> as a replacement for the pre-commit hook.

I see this matches _exactly_ what I wanted to say.  I however am
suspecting that it may not match what you think the hook should
do, though.

For example,

> The hook is allowed to edit the message file in place.  It can also be
> used for the same purpose as the pre-commit message, if the verification
> should not be skipped for automatic commits (e.g. during interactive
> rebasing).

I, who is claiming that the purpose of this hook is about
preparing the message and not about validating and interfering
with the commit, would never say that.  It's not just "allowed";
editing is the _sole_ reason for its existence.

I'd make it even stronger and clearer:

    The purpose of the hook is to edit the message file in place
    before it gets presented to the user for further editing.
    It is not suppressed by the --no-verify option, because it
    is not about validating what is to be committed.  A non-zero
    exit from the hook means the hook failed, and aborts the
    commit (in other words, non-zero exit from the hook is an
    abnormal condition, perhaps a bug in the hook itself, and
    should never be about the hook not liking the commit being
    created).

But that is apparently quite different from what you wrote.  So
I am sensing some miscommunication here.  I suspect that you are
not convinced by my claim that the hook's sole purpose should be
preparing the commit message to be edited, and that it should
never be used for validation.  Yet an early part of your commit
log message pretends that you are agreeing with me.  The rest of
the patch however still seems to think differently.

And if you think the hook should actively interfere with the
commit, please argue for that case first, without sending an
incoherent patch.  As I often say, unlike Linus, I am not always
right.

> +If the exit status is non-zero, `git-commit` will abort.
> +The hook is not suppressed by the `\--no-verify` option.
> +
> +The hook is allowed to edit the message file in place.  It can also be
> +used for the same purpose as the pre-commit message, if the verification
> +should not be skipped for automatic commits (e.g. during interactive
> +rebasing).

The same comment applies here.

> +++ b/templates/hooks--prepare-commit-msg
> @@ -0,0 +1,36 @@
> +#!/bin/sh
> +#
> +# An example hook script to prepare the commit log message.
> +# Called by git-commit with the name of the file that has the
> +# commit message, followed by the description of the commit
> +# message's source.  The hook should exit with non-zero
> +# status after issuing an appropriate message if it wants to stop the
> +# commit.  The hook is allowed to edit the commit message file.

The same comment applies here as well.

There may be cases where an unmaskable validation hook is
desired.  But even if that is the case, I see little reason to
tie it to the act of commit message preparation.

[PATCH-v6 4/4] git-commit: add a prepare-commit-msg hook

[Paolo Bonzini]

The prepare-commit-msg hook is run whenever a "fresh" commit message
is prepared, just before it is shown in the editor (if it is).
It can modify the commit message in-place and/or abort the commit.

It takes one to three parameters.  The first is the name of the file that
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 as the third parameter (if a -c, -C or --amend option
was given).

The purpose of the hook is to edit the message file in place before it
gets presented to the user for further editing.  It is not suppressed
by the --no-verify option, because it is not about validating what is
to be committed.  A non-zero exit from the hook means the hook failed,
and aborts the commit (in other words, non-zero exit from the hook is an
abnormal condition, perhaps a bug in the hook itself, and should never
be about the hook not liking the commit being created).

The default prepare-commit-msg comments out the `Conflicts:` part of
a merge's commit message; other examples are commented out, including
adding a Signed-off-by line at the bottom of the commit messsage,
that the user can then edit or discard altogether.

Signed-off-by: Paolo Bonzini <bonzini@gnu.org>
---
 Documentation/git-commit.txt        |    4 +-
 Documentation/hooks.txt             |   26 ++++++
 builtin-commit.c                    |   19 ++++
 t/t7505-prepare-commit-msg-hook.sh  |  155 +++++++++++++++++++++++++++++++++++
 templates/hooks--commit-msg         |    3 +
 templates/hooks--prepare-commit-msg |   39 +++++++++
 6 files changed, 244 insertions(+), 2 deletions(-)
 create mode 100755 t/t7505-prepare-commit-msg-hook.sh
 create mode 100644 templates/hooks--prepare-commit-msg

	No miscommunication.  Just typing `git commit --amend` instead
	of `git commit --amend -a`.  I should use citool more.

	> There may be cases where an unmaskable validation hook is
	> desired.  But even if that is the case, I see little reason to
	> tie it to the act of commit message preparation.

	Agreed, it should be more like a config option.

diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index c3725b2..b4ae61f 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -280,8 +280,8 @@ order).
 
 HOOKS
 -----
-This command can run `commit-msg`, `pre-commit`, and
-`post-commit` hooks.  See link:hooks.html[hooks] for more
+This command can run `commit-msg`, `prepare-commit-msg`, `pre-commit`,
+and `post-commit` hooks.  See link:hooks.html[hooks] for more
 information.
 
 
diff --git a/Documentation/hooks.txt b/Documentation/hooks.txt
index e8d80cf..83f2fcb 100644
--- a/Documentation/hooks.txt
+++ b/Documentation/hooks.txt
@@ -65,6 +65,32 @@ All the `git-commit` hooks are invoked with the environment
 variable `GIT_EDITOR=:` if the command will not bring up an editor
 to modify the commit message.
 
+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 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).
+
+The purpose of the hook is to edit the message file in place before it
+gets presented to the user for further editing.  It is not suppressed
+by the \`--no-verify` option, because it is not about validating what is
+to be committed.  A non-zero exit from the hook means the hook failed,
+and aborts the commit (in other words, non-zero exit from the hook is an
+abnormal condition, perhaps a bug in the hook itself, and should never
+be about the hook not liking the commit being created).
+
+The default `prepare-commit-msg` comments out the `Conflicts:` part of
+a merge's commit message.
+
 commit-msg
 ----------
 
diff --git a/builtin-commit.c b/builtin-commit.c
index d8945ac..9468d27 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -394,6 +394,8 @@ static int prepare_to_commit(const char *index_file, const char *prefix)
 	struct strbuf sb;
 	char *buffer;
 	FILE *fp;
+	const char *hook_arg1 = NULL;
+	const char *hook_arg2 = NULL;
 
 	if (!no_verify && run_hook(index_file, "pre-commit", NULL))
 		return 0;
@@ -401,32 +403,45 @@ static int prepare_to_commit(const char *index_file, const char *prefix)
 	strbuf_init(&sb, 0);
 	if (message.len) {
 		strbuf_addbuf(&sb, &message);
+		hook_arg1 = "message";
 	} else if (logfile && !strcmp(logfile, "-")) {
 		if (isatty(0))
 			fprintf(stderr, "(reading log message from standard input)\n");
 		if (strbuf_read(&sb, 0, 0) < 0)
 			die("could not read log from standard input");
+		hook_arg1 = "message";
 	} else if (logfile) {
 		if (strbuf_read_file(&sb, logfile, 0) < 0)
 			die("could not read log file '%s': %s",
 			    logfile, strerror(errno));
+		hook_arg1 = "message";
 	} else if (use_message) {
 		buffer = strstr(use_message_buffer, "\n\n");
 		if (!buffer || buffer[2] == '\0')
 			die("commit has empty message");
 		strbuf_add(&sb, buffer + 2, strlen(buffer + 2));
+		hook_arg1 = "commit";
+		hook_arg2 = use_message;
 	} else if (!stat(git_path("MERGE_MSG"), &statbuf)) {
 		if (strbuf_read_file(&sb, git_path("MERGE_MSG"), 0) < 0)
 			die("could not read MERGE_MSG: %s", strerror(errno));
+		hook_arg1 = "merge";
 	} else if (!stat(git_path("SQUASH_MSG"), &statbuf)) {
 		if (strbuf_read_file(&sb, git_path("SQUASH_MSG"), 0) < 0)
 			die("could not read SQUASH_MSG: %s", strerror(errno));
+		hook_arg1 = "squash";
 	} else if (template_file && !stat(template_file, &statbuf)) {
 		if (strbuf_read_file(&sb, template_file, 0) < 0)
 			die("could not read %s: %s",
 			    template_file, strerror(errno));
+		hook_arg1 = "template";
 	}
 
+	/* This final case does not modify the template message, it just sets
+	   the argument to the prepare-commit-msg hook.  */
+	else if (in_merge)
+		hook_arg1 = "merge";
+
 	fp = fopen(git_path(commit_editmsg), "w");
 	if (fp == NULL)
 		die("could not open %s", git_path(commit_editmsg));
@@ -534,6 +549,10 @@ static int prepare_to_commit(const char *index_file, const char *prefix)
 		return 0;
 	}
 
+	if (run_hook(index_file, "prepare-commit-msg",
+		     git_path(commit_editmsg), hook_arg1, hook_arg2, NULL))
+		return 0;
+
 	if (use_editor) {
 		char index[PATH_MAX];
 		const char *env[2] = { index, NULL };
diff --git a/t/t7505-prepare-commit-msg-hook.sh b/t/t7505-prepare-commit-msg-hook.sh
new file mode 100755
index 0000000..7ddec99
--- /dev/null
+++ b/t/t7505-prepare-commit-msg-hook.sh
@@ -0,0 +1,155 @@
+#!/bin/sh
+
+test_description='prepare-commit-msg hook'
+
+. ./test-lib.sh
+
+test_expect_success 'with no hook' '
+
+	echo "foo" > file &&
+	git add file &&
+	git commit -m "first"
+
+'
+
+# set up fake editor for interactive editing
+cat > fake-editor <<'EOF'
+#!/bin/sh
+exit 0
+EOF
+chmod +x fake-editor
+FAKE_EDITOR="$(pwd)/fake-editor"
+export FAKE_EDITOR
+
+# now install hook that always succeeds and adds a message
+HOOKDIR="$(git rev-parse --git-dir)/hooks"
+HOOK="$HOOKDIR/prepare-commit-msg"
+mkdir -p "$HOOKDIR"
+cat > "$HOOK" <<'EOF'
+#!/bin/sh
+if test "$2" = commit; then
+  source=$(git-rev-parse "$3")
+else
+  source=${2-default}
+fi
+if test "$GIT_EDITOR" = :; then
+  sed -e "1s/.*/$source (no editor)/" "$1" > msg.tmp
+else
+  sed -e "1s/.*/$source/" "$1" > msg.tmp
+fi
+mv msg.tmp "$1"
+exit 0
+EOF
+chmod +x "$HOOK"
+
+echo dummy template > "$(git rev-parse --git-dir)/template"
+
+test_expect_success 'with hook (-m)' '
+
+	echo "more" >> file &&
+	git add file &&
+	git commit -m "more" &&
+	test "`git log -1 --pretty=format:%s`" = "message (no editor)"
+
+'
+
+test_expect_success 'with hook (-m editor)' '
+
+	echo "more" >> file &&
+	git add file &&
+	GIT_EDITOR="$FAKE_EDITOR" git commit -e -m "more more" &&
+	test "`git log -1 --pretty=format:%s`" = message
+
+'
+
+test_expect_success 'with hook (-t)' '
+
+	echo "more" >> file &&
+	git add file &&
+	git commit -t "$(git rev-parse --git-dir)/template" &&
+	test "`git log -1 --pretty=format:%s`" = template
+
+'
+
+test_expect_success 'with hook (-F)' '
+
+	echo "more" >> file &&
+	git add file &&
+	(echo more | git commit -F -) &&
+	test "`git log -1 --pretty=format:%s`" = "message (no editor)"
+
+'
+
+test_expect_success 'with hook (-F editor)' '
+
+	echo "more" >> file &&
+	git add file &&
+	(echo more more | GIT_EDITOR="$FAKE_EDITOR" git commit -e -F -) &&
+	test "`git log -1 --pretty=format:%s`" = message
+
+'
+
+test_expect_success 'with hook (-C)' '
+
+	head=`git rev-parse HEAD` &&
+	echo "more" >> file &&
+	git add file &&
+	git commit -C $head &&
+	test "`git log -1 --pretty=format:%s`" = "$head (no editor)"
+
+'
+
+test_expect_success 'with hook (editor)' '
+
+	echo "more more" >> file &&
+	git add file &&
+	GIT_EDITOR="$FAKE_EDITOR" git commit &&
+	test "`git log -1 --pretty=format:%s`" = default
+
+'
+
+test_expect_success 'with hook (--amend)' '
+
+	head=`git rev-parse HEAD` &&
+	echo "more" >> file &&
+	git add file &&
+	GIT_EDITOR="$FAKE_EDITOR" git commit --amend &&
+	test "`git log -1 --pretty=format:%s`" = "$head"
+
+'
+
+test_expect_success 'with hook (-c)' '
+
+	head=`git rev-parse HEAD` &&
+	echo "more" >> file &&
+	git add file &&
+	GIT_EDITOR="$FAKE_EDITOR" git commit -c $head &&
+	test "`git log -1 --pretty=format:%s`" = "$head"
+
+'
+
+cat > "$HOOK" <<'EOF'
+#!/bin/sh
+exit 1
+EOF
+
+test_expect_success 'with failing hook' '
+
+	head=`git rev-parse HEAD` &&
+	echo "more" >> file &&
+	git add file &&
+	! GIT_EDITOR="$FAKE_EDITOR" git commit -c $head
+
+'
+
+test_expect_success 'with failing hook (--no-verify)' '
+
+	head=`git rev-parse HEAD` &&
+	echo "more" >> file &&
+	git add file &&
+	! GIT_EDITOR="$FAKE_EDITOR" git commit --no-verify -c $head
+
+'
+
+
+test_done
diff --git a/templates/hooks--commit-msg b/templates/hooks--commit-msg
index c5cdb9d..4ef86eb 100644
--- a/templates/hooks--commit-msg
+++ b/templates/hooks--commit-msg
@@ -9,6 +9,9 @@
 # To enable this hook, make this file executable.
 
 # Uncomment the below to add a Signed-off-by line to the message.
+# Doing this in a hook is a bad idea in general, but the prepare-commit-msg
+# hook is more suited to it.
+#
 # SOB=$(git var GIT_AUTHOR_IDENT | sed -n 's/^\(.*>\).*$/Signed-off-by: \1/p')
 # grep -qs "^$SOB" "$1" || echo "$SOB" >> "$1"
 
diff --git a/templates/hooks--prepare-commit-msg b/templates/hooks--prepare-commit-msg
new file mode 100644
index 0000000..8e5bc45
--- /dev/null
+++ b/templates/hooks--prepare-commit-msg
@@ -0,0 +1,39 @@
+#!/bin/sh
+#
+# An example hook script to prepare the commit log message.
+# Called by git-commit with the name of the file that has the
+# commit message, followed by the description of the commit
+# message's source.
+#
+# The hook is designed to edit the message file in place.
+# If the exit status is non-zero, git-commit will abort.
+# The hook is not suppressed by the --no-verify option, so it
+# should *not* be used as a replacement for the `pre-commit` hook.
+#
+# To enable this hook, make this file executable.
+
+# This hook includes three examples.  The first comments out the
+# "Conflicts:" part of a merge commit.
+#
+# The second includes the output of "git diff --name-status -r"
+# into the message, just before the "git status" output.  It is
+# commented because it doesn't cope with --amend or with squashed
+# commits.
+#
+# The third example adds a Signed-off-by line to the message, that can
+# still be edited.  This is rarely a good idea.
+
+case "$2 $3" in
+  merge)
+    sed -i '/^Conflicts:/,/#/!b;s/^/# &/;s/^# #/#/' "$1" ;;
+
+# ""|template)
+#   perl -i -pe '
+#      print "\n" . `git diff --cached --name-status -r`
+#	 if /^#/ && $first++ == 0' "$1" ;;
+
+  *) ;;
+esac
+
+# SOB=$(git var GIT_AUTHOR_IDENT | sed -n 's/^\(.*>\).*$/Signed-off-by: \1/p')
+# grep -qs "^$SOB" "$1" || echo "$SOB" >> "$1"
-- 
1.5.3.4.910.gc5122-dirty