[PATCH 3/3] commit: pass author/committer info to hooks

When lying the author name via GIT_AUTHOR_NAME environment variable
to "git commit", the hooks run by the command saw it and could act
on the name that will be recorded in the final commit. When the user
uses the "--author" option from the command line, the command should
give the same information to the hook, and back when "git command"
was a scripted Porcelain, it did set the environment variable and
hooks can learn the author name from it.

However, when the command was reimplemented in C, the rewritten code
was not very faithful to the original, and hooks stopped getting the
authorship information given with "--author".  Fix this by exporting
the necessary environment variables.

Signed-off-by: Junio C Hamano <redacted>
---

 * This is the last patch of the simpler of the two approaches, that
   builds on top of the two common preparatory patches.  It uses setenv()
   to directly affect the execution environment of "git commit" process,
   which is closer to the original scripted Porcelain implementation.

 builtin/commit.c           |   22 +++++++++++++++++++---
 t/t7503-pre-commit-hook.sh |    2 +-
 2 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index eae5a29..57a60f9 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c

@@ -533,9 +533,20 @@ static int is_a_merge(const struct commit *current_head)
 
 static const char sign_off_header[] = "Signed-off-by: ";
 
+static void export_one(const char *var, const char *s, const char *e, int hack)
+{
+	struct strbuf buf = STRBUF_INIT;
+	if (hack)
+		strbuf_addch(&buf, hack);
+	strbuf_addf(&buf, "%.*s", (int)(e - s), s);
+	setenv(var, buf.buf, 1);
+	strbuf_release(&buf);
+}
+
 static void determine_author_info(struct strbuf *author_ident)
 {
 	char *name, *email, *date;
+	struct ident_split author;
 
 	name = getenv("GIT_AUTHOR_NAME");
 	email = getenv("GIT_AUTHOR_EMAIL");

@@ -585,6 +596,11 @@ static void determine_author_info(struct strbuf *author_ident)
 		date = force_date;
 	strbuf_addstr(author_ident, fmt_ident(name, email, date,
 					      IDENT_ERROR_ON_NO_NAME));
+	if (!split_ident_line(&author, author_ident->buf, author_ident->len)) {
+		export_one("GIT_AUTHOR_NAME", author.name_begin, author.name_end, 0);
+		export_one("GIT_AUTHOR_EMAIL", author.mail_begin, author.mail_end, 0);
+		export_one("GIT_AUTHOR_DATE", author.date_begin, author.tz_end, '@');
+	}
 }
 
 static int ends_rfc2822_footer(struct strbuf *sb)

@@ -652,6 +668,9 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 	int ident_shown = 0;
 	int clean_message_contents = (cleanup_mode != CLEANUP_NONE);
 
+	/* This checks and barfs if author is badly specified */
+	determine_author_info(author_ident);
+
 	if (!no_verify && run_hook(index_file, "pre-commit", NULL))
 		return 0;
 

@@ -771,9 +790,6 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 
 	strbuf_release(&sb);
 
-	/* This checks and barfs if author is badly specified */
-	determine_author_info(author_ident);
-
 	/* This checks if committer ident is explicitly given */
 	strbuf_addstr(&committer_ident, git_committer_info(0));
 	if (use_editor && include_status) {

diff --git a/t/t7503-pre-commit-hook.sh b/t/t7503-pre-commit-hook.sh
index fc6de5b..9301a0c 100755
--- a/t/t7503-pre-commit-hook.sh
+++ b/t/t7503-pre-commit-hook.sh

@@ -118,7 +118,7 @@ test_expect_success 'with failing hook requiring GIT_PREFIX' '
 	git checkout -- file
 '
 
-test_expect_failure 'check the author in hook' '
+test_expect_success 'check the author in hook' '
 	cat >"$HOOK" <<-\EOF &&
 	test "$GIT_AUTHOR_NAME" = "New Author" &&
 	test "$GIT_AUTHOR_EMAIL" = "newauthor@example.com"

-- 
1.7.10.rc0.33.g8866af

On Sun, Mar 11, 2012 at 04:11:38AM -0700, Junio C Hamano wrote:

However, when the command was reimplemented in C, the rewritten code
was not very faithful to the original, and hooks stopped getting the
authorship information given with "--author".  Fix this by exporting
the necessary environment variables.
[...]
 * This is the last patch of the simpler of the two approaches, that
   builds on top of the two common preparatory patches.  It uses setenv()
   to directly affect the execution environment of "git commit" process,
   which is closer to the original scripted Porcelain implementation.

FWIW, I like this approach better. It's simpler, and I think it is
perfectly sane to define "--author" as "act as if the contents were in
GIT_AUTHOR_*"[1]. We do the same thing already with "git --git-dir",
"git --work-tree", and other variables, and it keeps the code simple by
making the environment variable the definitive source.

-Peff

[1] Your approach is slightly more complex, in that it handles "-c"
    (which is a good thing). But I think the principle is the same.