Merge branch 'jc/commit-hook-authorship' into maint

"git commit --author=$name" did not tell the name that was being
recorded in the resulting commit to hooks, even though it does do so
when the end user overrode the authorship via the "GIT_AUTHOR_NAME"
environment variable.

* jc/commit-hook-authorship:
  commit: pass author/committer info to hooks
  t7503: does pre-commit-hook learn authorship?
  ident.c: add split_ident_line() to parse formatted ident line

5 files changed, 141 insertions, 47 deletions

diff --git a/builtin/commit.c b/builtin/commit.c
index 3714582e19..7808be30e1 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/cache.h b/cache.h
index e5e1aa4e15..a8aceb5aee 100644
--- a/cache.h
+++ b/cache.h
@@ -928,6 +928,22 @@ extern const char *fmt_name(const char *name, const char *email);
 extern const char *git_editor(void);
 extern const char *git_pager(int stdout_is_tty);
 
+struct ident_split {
+	const char *name_begin;
+	const char *name_end;
+	const char *mail_begin;
+	const char *mail_end;
+	const char *date_begin;
+	const char *date_end;
+	const char *tz_begin;
+	const char *tz_end;
+};
+/*
+ * Signals an success with 0, but time part of the result may be NULL
+ * if the input lacks timestamp and zone
+ */
+extern int split_ident_line(struct ident_split *, const char *, int);
+
 struct checkout {
 	const char *base_dir;
 	int base_dir_len;
diff --git a/ident.c b/ident.c
index f619619b82..87c697c2b0 100644
--- a/ident.c
+++ b/ident.c
@@ -220,6 +220,74 @@ static int copy(char *buf, size_t size, int offset, const char *src)
 	return offset;
 }
 
+/*
+ * Reverse of fmt_ident(); given an ident line, split the fields
+ * to allow the caller to parse it.
+ * Signal a success by returning 0, but date/tz fields of the result
+ * can still be NULL if the input line only has the name/email part
+ * (e.g. reading from a reflog entry).
+ */
+int split_ident_line(struct ident_split *split, const char *line, int len)
+{
+	const char *cp;
+	size_t span;
+	int status = -1;
+
+	memset(split, 0, sizeof(*split));
+
+	split->name_begin = line;
+	for (cp = line; *cp && cp < line + len; cp++)
+		if (*cp == '<') {
+			split->mail_begin = cp + 1;
+			break;
+		}
+	if (!split->mail_begin)
+		return status;
+
+	for (cp = split->mail_begin - 2; line < cp; cp--)
+		if (!isspace(*cp)) {
+			split->name_end = cp + 1;
+			break;
+		}
+	if (!split->name_end)
+		return status;
+
+	for (cp = split->mail_begin; cp < line + len; cp++)
+		if (*cp == '>') {
+			split->mail_end = cp;
+			break;
+		}
+	if (!split->mail_end)
+		return status;
+
+	for (cp = split->mail_end + 1; cp < line + len && isspace(*cp); cp++)
+		;
+	if (line + len <= cp)
+		goto person_only;
+	split->date_begin = cp;
+	span = strspn(cp, "0123456789");
+	if (!span)
+		goto person_only;
+	split->date_end = split->date_begin + span;
+	for (cp = split->date_end; cp < line + len && isspace(*cp); cp++)
+		;
+	if (line + len <= cp || (*cp != '+' && *cp != '-'))
+		goto person_only;
+	split->tz_begin = cp;
+	span = strspn(cp + 1, "0123456789");
+	if (!span)
+		goto person_only;
+	split->tz_end = split->tz_begin + 1 + span;
+	return 0;
+
+person_only:
+	split->date_begin = NULL;
+	split->date_end = NULL;
+	split->tz_begin = NULL;
+	split->tz_end = NULL;
+	return 0;
+}
+
 static const char *env_hint =
 "\n"
 "*** Please tell me who you are.\n"
diff --git a/pretty.c b/pretty.c
index 8688b8f2d4..f2dee308b8 100644
--- a/pretty.c
+++ b/pretty.c
@@ -531,41 +531,24 @@ static size_t format_person_part(struct strbuf *sb, char part,
 {
 	/* currently all placeholders have same length */
 	const int placeholder_len = 2;
-	int start, end, tz = 0;
+	int tz;
 	unsigned long date = 0;
-	char *ep;
-	const char *name_start, *name_end, *mail_start, *mail_end, *msg_end = msg+len;
 	char person_name[1024];
 	char person_mail[1024];
+	struct ident_split s;
+	const char *name_start, *name_end, *mail_start, *mail_end;
 
-	/* advance 'end' to point to email start delimiter */
-	for (end = 0; end < len && msg[end] != '<'; end++)
-		; /* do nothing */
-
-	/*
-	 * When end points at the '<' that we found, it should have
-	 * matching '>' later, which means 'end' must be strictly
-	 * below len - 1.
-	 */
-	if (end >= len - 2)
+	if (split_ident_line(&s, msg, len) < 0)
 		goto skip;
 
-	/* Seek for both name and email part */
-	name_start = msg;
-	name_end = msg+end;
-	while (name_end > name_start && isspace(*(name_end-1)))
-		name_end--;
-	mail_start = msg+end+1;
-	mail_end = mail_start;
-	while (mail_end < msg_end && *mail_end != '>')
-		mail_end++;
-	if (mail_end == msg_end)
-		goto skip;
-	end = mail_end-msg;
+	name_start = s.name_begin;
+	name_end = s.name_end;
+	mail_start = s.mail_begin;
+	mail_end = s.mail_end;
 
 	if (part == 'N' || part == 'E') { /* mailmap lookup */
-		strlcpy(person_name, name_start, name_end-name_start+1);
-		strlcpy(person_mail, mail_start, mail_end-mail_start+1);
+		strlcpy(person_name, name_start, name_end - name_start + 1);
+		strlcpy(person_mail, mail_start, mail_end - mail_start + 1);
 		mailmap_name(person_mail, sizeof(person_mail), person_name, sizeof(person_name));
 		name_start = person_name;
 		name_end = name_start + strlen(person_name);
@@ -581,28 +564,20 @@ static size_t format_person_part(struct strbuf *sb, char part,
 		return placeholder_len;
 	}
 
-	/* advance 'start' to point to date start delimiter */
-	for (start = end + 1; start < len && isspace(msg[start]); start++)
-		; /* do nothing */
-	if (start >= len)
-		goto skip;
-	date = strtoul(msg + start, &ep, 10);
-	if (msg + start == ep)
+	if (!s.date_begin)
 		goto skip;
 
+	date = strtoul(s.date_begin, NULL, 10);
+
 	if (part == 't') {	/* date, UNIX timestamp */
-		strbuf_add(sb, msg + start, ep - (msg + start));
+		strbuf_add(sb, s.date_begin, s.date_end - s.date_begin);
 		return placeholder_len;
 	}
 
 	/* parse tz */
-	for (start = ep - msg + 1; start < len && isspace(msg[start]); start++)
-		; /* do nothing */
-	if (start + 1 < len) {
-		tz = strtoul(msg + start + 1, NULL, 10);
-		if (msg[start] == '-')
-			tz = -tz;
-	}
+	tz = strtoul(s.tz_begin + 1, NULL, 10);
+	if (*s.tz_begin == '-')
+		tz = -tz;
 
 	switch (part) {
 	case 'd':	/* date */
@@ -621,8 +596,9 @@ static size_t format_person_part(struct strbuf *sb, char part,
 
 skip:
 	/*
-	 * bogus commit, 'sb' cannot be updated, but we still need to
-	 * compute a valid return value.
+	 * reading from either a bogus commit, or a reflog entry with
+	 * %gn, %ge, etc.; 'sb' cannot be updated, but we still need
+	 * to compute a valid return value.
 	 */
 	if (part == 'n' || part == 'e' || part == 't' || part == 'd'
 	    || part == 'D' || part == 'r' || part == 'i')
diff --git a/t/t7503-pre-commit-hook.sh b/t/t7503-pre-commit-hook.sh
index ee7f0cd459..984889b39d 100755
--- a/t/t7503-pre-commit-hook.sh
+++ b/t/t7503-pre-commit-hook.sh
@@ -118,4 +118,22 @@ test_expect_success 'with failing hook requiring GIT_PREFIX' '
 	git checkout -- file
 '
 
+test_expect_success 'check the author in hook' '
+	write_script "$HOOK" <<-\EOF &&
+	test "$GIT_AUTHOR_NAME" = "New Author" &&
+	test "$GIT_AUTHOR_EMAIL" = "newauthor@example.com"
+	EOF
+	test_must_fail git commit --allow-empty -m "by a.u.thor" &&
+	(
+		GIT_AUTHOR_NAME="New Author" &&
+		GIT_AUTHOR_EMAIL="newauthor@example.com" &&
+		export GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL &&
+		git commit --allow-empty -m "by new.author via env" &&
+		git show -s
+	) &&
+	git commit --author="New Author <newauthor@example.com>" \
+		--allow-empty -m "by new.author via command line" &&
+	git show -s
+'
+
 test_done