[PATCH 2/2] commit: always populate GIT_AUTHOR_* variables

[Jeff King <peff@peff.net>]

To figure out the author ident for a commit, we call
determine_author_info(). This function collects information
from the environment, other commits (in the case of
"--amend" or "-c/-C"), and the "--author" option. It then
uses fmt_ident to generate the final ident string that goes
into the commit object. fmt_ident is therefore responsible
for any quality or validation checks on what is allowed to
go into a commit.

Before returning, though, we call split_ident_line on the
result, and feed the individual components to hooks via the
GIT_AUTHOR_* variables. Furthermore, we do extra validation
by feeding the split to sane_ident_split(), which is pickier
than fmt_ident (in particular, it will complain about an empty
email field).  If this parsing or validation fails, we skip
updating the environment variables.

This is bad, because it means that hooks may silently see a
different ident than what we are putting into the commit. We
should drop the extra sane_ident_split checks entirely, and
take whatever fmt_ident has fed us (and what will go into
the commit object).

If parsing fails, we should actually abort here rather than
continuing (and feeding the hooks bogus data). However,
split_ident_line should never fail here. The ident was just
generated by fmt_ident, so we know that it's sane. We can
use assert_split_ident to double-check this.

Note that we also teach that assertion to check that we
found a date (it always should, but until now, no caller
cared whether we found a date or not). Checking the return
value of sane_ident_split is enough to ensure we have the
name/email pointers set, and checking date_begin is enough
to know that all of the date/tz variables are set.

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/commit.c | 26 +++++---------------------
 1 file changed, 5 insertions(+), 21 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index 2be5506..f1a9e07 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -504,7 +504,7 @@ static int is_a_merge(const struct commit *current_head)
 
 static void assert_split_ident(struct ident_split *id, const struct strbuf *buf)
 {
-	if (split_ident_line(id, buf->buf, buf->len))
+	if (split_ident_line(id, buf->buf, buf->len) || !id->date_begin)
 		die("BUG: unable to parse our own ident: %s", buf->buf);
 }
 
@@ -518,20 +518,6 @@ static void export_one(const char *var, const char *s, const char *e, int hack)
 	strbuf_release(&buf);
 }
 
-static int sane_ident_split(struct ident_split *person)
-{
-	if (!person->name_begin || !person->name_end ||
-	    person->name_begin == person->name_end)
-		return 0; /* no human readable name */
-	if (!person->mail_begin || !person->mail_end ||
-	    person->mail_begin == person->mail_end)
-		return 0; /* no usable mail */
-	if (!person->date_begin || !person->date_end ||
-	    !person->tz_begin || !person->tz_end)
-		return 0;
-	return 1;
-}
-
 static int parse_force_date(const char *in, char *out, int len)
 {
 	if (len < 1)
@@ -606,12 +592,10 @@ static void determine_author_info(struct strbuf *author_ident)
 	}
 
 	strbuf_addstr(author_ident, fmt_ident(name, email, date, IDENT_STRICT));
-	if (!split_ident_line(&author, author_ident->buf, author_ident->len) &&
-	    sane_ident_split(&author)) {
-		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, '@');
-	}
+	assert_split_ident(&author, author_ident);
+	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 author_date_is_interesting(void)
-- 
2.2.0.454.g7eca6b7