[PATCH-v2 1/4] git-commit: support variable number of hook arguments

[PATCH-v2 1/4] git-commit: support variable number of hook arguments

[Paolo Bonzini]

This is a preparatory patch to allow using run_hook for the
prepare-commit-msg hook.
---
 builtin-commit.c |   61 +++++++++++++++++++++++++++++++-----------------------
 1 files changed, 35 insertions(+), 26 deletions(-)

	Compared to the previous submission, this one includes the
	range check.

diff --git a/builtin-commit.c b/builtin-commit.c
index 2ffca40..405c8b5 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -343,6 +343,40 @@ static int run_status(FILE *fp, const char *index_file, const char *prefix, int
 	return s.commitable;
 }
 
+static int run_hook(const char *index_file, const char *name, ...)
+{
+	struct child_process hook;
+	const char *argv[10], *env[2];
+	char index[PATH_MAX];
+	va_list args;
+	int i;
+
+	va_start(args, name);
+	argv[0] = git_path("hooks/%s", name);
+	i = 0;
+	do {
+		if (++i >= ARRAY_SIZE(argv))
+			die ("run_hook(): too many arguments");
+		argv[i] = va_arg(args, const char *);
+	} while (argv[i]);
+	va_end(args);
+
+	snprintf(index, sizeof(index), "GIT_INDEX_FILE=%s", index_file);
+	env[0] = index;
+	env[1] = NULL;
+
+	if (access(argv[0], X_OK) < 0)
+		return 0;
+
+	memset(&hook, 0, sizeof(hook));
+	hook.argv = argv;
+	hook.no_stdin = 1;
+	hook.stdout_to_stderr = 1;
+	hook.env = env;
+
+	return run_command(&hook);
+}
+
 static const char sign_off_header[] = "Signed-off-by: ";
 
 static int prepare_log_message(const char *index_file, const char *prefix)
@@ -677,31 +711,6 @@ int cmd_status(int argc, const char **argv, const char *prefix)
 	return commitable ? 0 : 1;
 }
 
-static int run_hook(const char *index_file, const char *name, const char *arg)
-{
-	struct child_process hook;
-	const char *argv[3], *env[2];
-	char index[PATH_MAX];
-
-	argv[0] = git_path("hooks/%s", name);
-	argv[1] = arg;
-	argv[2] = NULL;
-	snprintf(index, sizeof(index), "GIT_INDEX_FILE=%s", index_file);
-	env[0] = index;
-	env[1] = NULL;
-
-	if (access(argv[0], X_OK) < 0)
-		return 0;
-
-	memset(&hook, 0, sizeof(hook));
-	hook.argv = argv;
-	hook.no_stdin = 1;
-	hook.stdout_to_stderr = 1;
-	hook.env = env;
-
-	return run_command(&hook);
-}
-
 static void print_summary(const char *prefix, const unsigned char *sha1)
 {
 	struct rev_info rev;
@@ -876,7 +885,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 		launch_editor(git_path(commit_editmsg), NULL, env);
 	}
 	if (!no_verify &&
-	    run_hook(index_file, "commit-msg", git_path(commit_editmsg))) {
+	    run_hook(index_file, "commit-msg", git_path(commit_editmsg), NULL)) {
 		rollback_index_files();
 		exit(1);
 	}
-- 
1.5.3.4.910.gc5122-dirty

[PATCH-v2 2/4] git-commit: set GIT_EDITOR=: if editor will not be launched

[Paolo Bonzini]

This is a preparatory patch that provides a simple way for the future
prepare-commit-msg hook to discover if the editor will be launched.
---
 Documentation/hooks.txt |    4 ++++
 builtin-commit.c        |    2 ++
 2 files changed, 6 insertions(+), 0 deletions(-)

	This has not changed.

diff --git a/Documentation/hooks.txt b/Documentation/hooks.txt
index f110162..e8d80cf 100644
--- a/Documentation/hooks.txt
+++ b/Documentation/hooks.txt
@@ -61,6 +61,10 @@ The default 'pre-commit' hook, when enabled, catches introduction
 of lines with trailing whitespaces and aborts the commit when
 such a line is found.
 
+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.
+
 commit-msg
 ----------
 
diff --git a/builtin-commit.c b/builtin-commit.c
index 405c8b5..7d1059c 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -599,6 +599,8 @@ static int parse_and_validate_options(int argc, const char *argv[],
 		use_editor = 0;
 	if (edit_flag)
 		use_editor = 1;
+	if (!use_editor)
+		setenv("GIT_EDITOR", ":", 1);
 
 	if (get_sha1("HEAD", head_sha1))
 		initial_commit = 1;
-- 
1.5.3.4.910.gc5122-dirty

[PATCH-v2 3/4] git-commit: Refactor creation of log message.

[Paolo Bonzini]

This patch moves the code of run_commit, up to writing the trees, editing
the message and running the commit-msg hook to prepare_log_message.  It also
renames the latter to prepare_to_commit.

This simplifies a little the code for the next patch.
---
 builtin-commit.c  |  171 +++++++++++++++++++++++++++--------------------------
 t/t7502-commit.sh |   28 +++++++++
 2 files changed, 116 insertions(+), 83 deletions(-)

	While not strictly necessary, I moved code that invokes pre-commit
	too, because it seemed more logical.  Both for pre-commit and
	for the old prepare_log_message you have to call rollback_index_files
	upon failure.

	I added a testcase that the editor is invoked *after* checking
	for incomplete merges.

diff --git a/builtin-commit.c b/builtin-commit.c
index 7d1059c..d8945ac 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -377,9 +377,17 @@ static int run_hook(const char *index_file, const char *name, ...)
 	return run_command(&hook);
 }
 
+static int is_a_merge(const unsigned char *sha1)
+{
+	struct commit *commit = lookup_commit(sha1);
+	if (!commit || parse_commit(commit))
+		die("could not parse HEAD commit");
+	return !!(commit->parents && commit->parents->next);
+}
+
 static const char sign_off_header[] = "Signed-off-by: ";
 
-static int prepare_log_message(const char *index_file, const char *prefix)
+static int prepare_to_commit(const char *index_file, const char *prefix)
 {
 	struct stat statbuf;
 	int commitable, saved_color_setting;
@@ -387,6 +395,9 @@ static int prepare_log_message(const char *index_file, const char *prefix)
 	char *buffer;
 	FILE *fp;
 
+	if (!no_verify && run_hook(index_file, "pre-commit", NULL))
+		return 0;
+
 	strbuf_init(&sb, 0);
 	if (message.len) {
 		strbuf_addbuf(&sb, &message);
@@ -447,13 +458,38 @@ static int prepare_log_message(const char *index_file, const char *prefix)
 
 	strbuf_release(&sb);
 
-	if (!use_editor) {
+	if (use_editor) {
+		if (in_merge)
+			fprintf(fp,
+				"#\n"
+				"# It looks like you may be committing a MERGE.\n"
+				"# If this is not correct, please remove the file\n"
+				"#	%s\n"
+				"# and try again.\n"
+				"#\n",
+				git_path("MERGE_HEAD"));
+
+		fprintf(fp,
+			"\n"
+			"# Please enter the commit message for your changes.\n"
+			"# (Comment lines starting with '#' will ");
+		if (cleanup_mode == CLEANUP_ALL)
+			fprintf(fp, "not be included)\n");
+		else /* CLEANUP_SPACE, that is. */
+			fprintf(fp, "be kept.\n"
+				"# You can remove them yourself if you want to)\n");
+		if (only_include_assumed)
+			fprintf(fp, "# %s\n", only_include_assumed);
+
+		saved_color_setting = wt_status_use_color;
+		wt_status_use_color = 0;
+		commitable = run_status(fp, index_file, prefix, 1);
+		wt_status_use_color = saved_color_setting;
+	} else {
 		struct rev_info rev;
 		unsigned char sha1[20];
 		const char *parent = "HEAD";
 
-		fclose(fp);
-
 		if (!active_nr && read_cache() < 0)
 			die("Cannot read index");
 
@@ -461,48 +497,56 @@ static int prepare_log_message(const char *index_file, const char *prefix)
 			parent = "HEAD^1";
 
 		if (get_sha1(parent, sha1))
-			return !!active_nr;
+			commitable = !!active_nr;
+		else {
+			init_revisions(&rev, "");
+			rev.abbrev = 0;
+			setup_revisions(0, NULL, &rev, parent);
+			DIFF_OPT_SET(&rev.diffopt, QUIET);
+			DIFF_OPT_SET(&rev.diffopt, EXIT_WITH_STATUS);
+			run_diff_index(&rev, 1 /* cached */);
+
+			commitable = !!DIFF_OPT_TST(&rev.diffopt, HAS_CHANGES);
+		}
+	}
 
-		init_revisions(&rev, "");
-		rev.abbrev = 0;
-		setup_revisions(0, NULL, &rev, parent);
-		DIFF_OPT_SET(&rev.diffopt, QUIET);
-		DIFF_OPT_SET(&rev.diffopt, EXIT_WITH_STATUS);
-		run_diff_index(&rev, 1 /* cached */);
+	fclose(fp);
+
+	if (!commitable && !in_merge && !allow_empty &&
+	    !(amend && is_a_merge(head_sha1))) {
+		run_status(stdout, index_file, prefix, 0);
+		unlink(commit_editmsg);
+		return 0;
+	}
 
-		return !!DIFF_OPT_TST(&rev.diffopt, HAS_CHANGES);
+	/*
+	 * Re-read the index as pre-commit hook could have updated it,
+	 * and write it out as a tree.  We must do this before we invoke
+	 * the editor and after we invoke run_status above.
+	 */
+	discard_cache();
+	read_cache_from(index_file);
+	if (!active_cache_tree)
+		active_cache_tree = cache_tree();
+	if (cache_tree_update(active_cache_tree,
+			      active_cache, active_nr, 0, 0) < 0) {
+		error("Error building trees");
+		return 0;
 	}
 
-	if (in_merge)
-		fprintf(fp,
-			"#\n"
-			"# It looks like you may be committing a MERGE.\n"
-			"# If this is not correct, please remove the file\n"
-			"#	%s\n"
-			"# and try again.\n"
-			"#\n",
-			git_path("MERGE_HEAD"));
-
-	fprintf(fp,
-		"\n"
-		"# Please enter the commit message for your changes.\n"
-		"# (Comment lines starting with '#' will ");
-	if (cleanup_mode == CLEANUP_ALL)
-		fprintf(fp, "not be included)\n");
-	else /* CLEANUP_SPACE, that is. */
-		fprintf(fp, "be kept.\n"
-			"# You can remove them yourself if you want to)\n");
-	if (only_include_assumed)
-		fprintf(fp, "# %s\n", only_include_assumed);
-
-	saved_color_setting = wt_status_use_color;
-	wt_status_use_color = 0;
-	commitable = run_status(fp, index_file, prefix, 1);
-	wt_status_use_color = saved_color_setting;
+	if (use_editor) {
+		char index[PATH_MAX];
+		const char *env[2] = { index, NULL };
+		snprintf(index, sizeof(index), "GIT_INDEX_FILE=%s", index_file);
+		launch_editor(git_path(commit_editmsg), NULL, env);
+	}
 
-	fclose(fp);
+	if (!no_verify &&
+	    run_hook(index_file, "commit-msg", git_path(commit_editmsg), NULL)) {
+		return 0;
+	}
 
-	return commitable;
+	return 1;
 }
 
 /*
@@ -761,14 +805,6 @@ int git_commit_config(const char *k, const char *v)
 	return git_status_config(k, v);
 }
 
-static int is_a_merge(const unsigned char *sha1)
-{
-	struct commit *commit = lookup_commit(sha1);
-	if (!commit || parse_commit(commit))
-		die("could not parse HEAD commit");
-	return !!(commit->parents && commit->parents->next);
-}
-
 static const char commit_utf8_warn[] =
 "Warning: commit message does not conform to UTF-8.\n"
 "You may want to amend it after fixing the message, or set the config\n"
@@ -800,34 +836,14 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 
 	index_file = prepare_index(argc, argv, prefix);
 
-	if (!no_verify && run_hook(index_file, "pre-commit", NULL)) {
-		rollback_index_files();
-		return 1;
-	}
-
-	if (!prepare_log_message(index_file, prefix) && !in_merge &&
-	    !allow_empty && !(amend && is_a_merge(head_sha1))) {
-		run_status(stdout, index_file, prefix, 0);
+	/* Set up everything for writing the commit object.  This includes
+	   running hooks, writing the trees, and interacting with the user.  */
+	if (!prepare_to_commit(index_file, prefix)) {
 		rollback_index_files();
-		unlink(commit_editmsg);
 		return 1;
 	}
 
 	/*
-	 * Re-read the index as pre-commit hook could have updated it,
-	 * and write it out as a tree.
-	 */
-	discard_cache();
-	read_cache_from(index_file);
-	if (!active_cache_tree)
-		active_cache_tree = cache_tree();
-	if (cache_tree_update(active_cache_tree,
-			      active_cache, active_nr, 0, 0) < 0) {
-		rollback_index_files();
-		die("Error building trees");
-	}
-
-	/*
 	 * The commit object
 	 */
 	strbuf_init(&sb, 0);
@@ -878,19 +894,8 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 		strbuf_addf(&sb, "encoding %s\n", git_commit_encoding);
 	strbuf_addch(&sb, '\n');
 
-	/* Get the commit message and validate it */
+	/* Finally, get the commit message */
 	header_len = sb.len;
-	if (use_editor) {
-		char index[PATH_MAX];
-		const char *env[2] = { index, NULL };
-		snprintf(index, sizeof(index), "GIT_INDEX_FILE=%s", index_file);
-		launch_editor(git_path(commit_editmsg), NULL, env);
-	}
-	if (!no_verify &&
-	    run_hook(index_file, "commit-msg", git_path(commit_editmsg), NULL)) {
-		rollback_index_files();
-		exit(1);
-	}
 	if (strbuf_read_file(&sb, git_path(commit_editmsg), 0) < 0) {
 		rollback_index_files();
 		die("could not read commit message");
diff --git a/t/t7502-commit.sh b/t/t7502-commit.sh
index aaf497e..5ab8e23 100755
--- a/t/t7502-commit.sh
+++ b/t/t7502-commit.sh
@@ -154,4 +154,33 @@ test_expect_success 'cleanup commit messages (strip,-F,-e)' '
 
 '
 
+pwd=`pwd`
+cat >> .git/FAKE_EDITOR << EOF
+#! /bin/sh
+echo editor started > "$pwd/.git/result"
+exit 0
+EOF
+chmod +x .git/FAKE_EDITOR
+
+test_expect_success 'do not fire editor in the presence of conflicts' '
+
+	git clean
+	echo f>g
+	git add g
+	git commit -myes
+	git branch second
+	echo master>g
+	echo g>h
+	git add g h
+	git commit -mmaster
+	git checkout second
+	echo second>g
+	git add g
+	git commit -msecond
+	git cherry-pick -n master
+	echo "editor not started" > .git/result
+	GIT_EDITOR=`pwd`/.git/FAKE_EDITOR git commit && exit 1  # should fail
+	test "`cat .git/result`" = "editor not started"
+'
+
 test_done
-- 
1.5.3.4.910.gc5122-dirty

[PATCH-v2 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.

The hook is not suppressed by the --no-verify option, and the
exit status is ignored by git.

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.
---
 Documentation/git-commit.txt        |    4 +-
 Documentation/hooks.txt             |   32 ++++++++-
 builtin-commit.c                    |   16 ++++
 t/t7505-prepare-commit-msg-hook.sh  |  130 +++++++++++++++++++++++++++++++++++
 templates/hooks--commit-msg         |    3 +
 templates/hooks--prepare-commit-msg |   14 ++++
 6 files changed, 196 insertions(+), 3 deletions(-)
 create mode 100755 t/t7505-prepare-commit-msg-hook.sh
 create mode 100644 templates/hooks--prepare-commit-msg

	I removed tests that involved a failing hook, since the
	exit status is documented to be irrelevant.

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..02c8f9d 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,35 @@ 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 hook is not suppressed by the `\--no-verify` option, and
+the exit status is ignored by `git-commit`.
+
+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..3f2d3a4 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,9 @@ static int prepare_to_commit(const char *index_file, const char *prefix)
 		return 0;
 	}
 
+	run_hook(index_file, "prepare-commit-msg",
+		 git_path(commit_editmsg), hook_arg, NULL);
+
 	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..440e197
--- /dev/null
+++ b/t/t7505-prepare-commit-msg-hook.sh
@@ -0,0 +1,130 @@
+#!/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"
+
+'
+
+
+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-v2 4/4] git-commit: add a prepare-commit-msg hook

[Johannes Schindelin]

Hi,

On Tue, 5 Feb 2008, Paolo Bonzini wrote:

> The hook is not suppressed by the --no-verify option, and the
> exit status is ignored by git.

Umm.  Should the exit status really be ignored?  What _use_ would it be 
then?

Ciao,
Dscho

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

[Paolo Bonzini]

Johannes Schindelin wrote:
> Hi,
> 
> On Tue, 5 Feb 2008, Paolo Bonzini wrote:
> 
>> The hook is not suppressed by the --no-verify option, and the
>> exit status is ignored by git.
> 
> Umm.  Should the exit status really be ignored?  What _use_ would it be 
> then?

None; in the previous version I used an exit status of 1 to kill the 
commit.  But since this hook, unlike others, is meant to prepare things 
for the user, the hook itself was not suppressed by --no-verify, only 
checking the exit status.  Junio thought it was a mess, so I now 
completely ignore the exit status.

I just wrote the above sentence because prepare-commit-msg is the only 
git-commit hook whose exit status is ignored.

Paolo

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

[Johannes Schindelin]

Hi,

On Tue, 5 Feb 2008, Paolo Bonzini wrote:

> Johannes Schindelin wrote:
> 
> > On Tue, 5 Feb 2008, Paolo Bonzini wrote:
> > 
> > > The hook is not suppressed by the --no-verify option, and the exit 
> > > status is ignored by git.
> > 
> > Umm.  Should the exit status really be ignored?  What _use_ would it 
> > be then?
> 
> None; in the previous version I used an exit status of 1 to kill the 
> commit. But since this hook, unlike others, is meant to prepare things 
> for the user, the hook itself was not suppressed by --no-verify, only 
> checking the exit status.  Junio thought it was a mess, so I now 
> completely ignore the exit status.
> 
> I just wrote the above sentence because prepare-commit-msg is the only 
> git-commit hook whose exit status is ignored.

I might be missing something, but should this not be solved by having the 
prepare-commit-msg exit 0?  I mean, it is conceivable that such a bad 
error occurs in the hook that the commit should be stopped then and there.

Ciao,
Dscho

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

[Paolo Bonzini]

> I might be missing something

No, you are not.

> but should this not be solved by having the 
> prepare-commit-msg exit 0?  I mean, it is conceivable that such a bad 
> error occurs in the hook that the commit should be stopped then and there.

So you say, don't suppress the hook with --no-verify *and* exit if the 
hook returns 1, even if --no-verify is given.  That's fine by me.

Paolo

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

[Junio C Hamano]

Paolo Bonzini <bonzini@gnu.org> writes:

>> I might be missing something
>
> No, you are not.
>
>> but should this not be solved by having the prepare-commit-msg exit
>> 0?  I mean, it is conceivable that such a bad error occurs in the
>> hook that the commit should be stopped then and there.
>
> So you say, don't suppress the hook with --no-verify *and* exit if the
> hook returns 1, even if --no-verify is given.  That's fine by me.

Fine by me as well, and I would prefer it that way.