git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v6 2/2] config: allow giving separate author and committer idents

From
Junio C Hamano <gitster@pobox.com>
Date
Feb 6, 2019, 05:03 UTC
Message-ID
<xmqqwomdqzik.fsf@gitster-ct.c.googlers.com>
In-Reply-To
<20190206000413.GA734@whubbs1.gaikai.biz>
William Hubbs <williamh@gentoo.org> writes:
> I attempted to save your patches to apply them, but didn't have any luck

I'll push a topic branch (not merged to any of the integration branches) ab/author-committer-ident-config later today at the https://github.com/gitster/git repository.

> Also, according to Junio's report, my patch is already merged to next,

In an earlier "What's cooking" I may have said that I plan to merge it to 'next', but I think the plan is now to leave it there for now until this discussion settles and the latest report should reflect that.

I just double-checked and wh/author-committer-ident-config is not in 'next'. Whew.

Here is a diff that turns wh/author-committer-ident-config into what ab/author-committer-ident-config has. There are some formatting changes, all of which I agree with, a bogus set_ident() refactoring that should not be used, in addition to some test changes.

 Documentation/git-commit-tree.txt |  3 +-
 builtin/am.c                      |  2 +-
 builtin/commit.c                  |  2 +-
 cache.h                           |  4 +-
 ident.c                           | 95 +++++++++++++--------------------------
 sequencer.c                       |  3 +-
 t/t7517-per-repo-email.sh         | 88 +++++++++++++++++++++++++-----------
 7 files changed, 101 insertions(+), 96 deletions(-)
diff --git a/Documentation/git-commit-tree.txt b/Documentation/git-commit-tree.txt
index 002dae625e..091e3a77ca 100644
--- a/Documentation/git-commit-tree.txt
+++ b/Documentation/git-commit-tree.txt
@@ -88,7 +88,8 @@ if set:
 (nb "<", ">" and "\n"s are stripped)
 
 In case (some of) these environment variables are not set, the information
-is taken from the configuration items user.name and user.email, or, if not
+is taken from the configuration items user.name and user.email, or the more
+specific author.{name,email} and committer.{name,email} variables, or, if not
 present, the environment variable EMAIL, or, if that is not set,
 system user name and the hostname used for outgoing mail (taken
 from `/etc/mailname` and falling back to the fully qualified hostname when
diff --git a/builtin/am.c b/builtin/am.c
index 3727d4d267..d4a1cbe828 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1594,7 +1594,7 @@ static void do_commit(const struct am_state *state)
 	}
 
 	author = fmt_ident(state->author_name, state->author_email,
-		WANT_AUTHOR_IDENT,
+			WANT_AUTHOR_IDENT,
 			state->ignore_date ? NULL : state->author_date,
 			IDENT_STRICT);
 
diff --git a/builtin/commit.c b/builtin/commit.c
index f96b90daeb..a7879d65d1 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -608,7 +608,7 @@ static void determine_author_info(struct strbuf *author_ident)
 	}
 
 	strbuf_addstr(author_ident, fmt_ident(name, email, WANT_AUTHOR_IDENT, date,
-				IDENT_STRICT));
+					      IDENT_STRICT));
 	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);
diff --git a/cache.h b/cache.h
index bb78eb9a3a..ca6ba1e423 100644
--- a/cache.h
+++ b/cache.h
@@ -1489,8 +1489,8 @@ enum want_ident {
 extern const char *git_author_info(int);
 extern const char *git_committer_info(int);
 extern const char *fmt_ident(const char *name, const char *email,
-		enum want_ident whose_ident,
-		const char *date_str, int);
+			     enum want_ident whose_ident,
+			     const char *date_str, int flag);
 extern const char *fmt_name(enum want_ident);
 extern const char *ident_default_name(void);
 extern const char *ident_default_email(void);
diff --git a/ident.c b/ident.c
index 9c2eb0a2d0..7c3be81ee1 100644
--- a/ident.c
+++ b/ident.c
@@ -359,7 +359,8 @@ N_("\n"
    "\n");
 
 const char *fmt_ident(const char *name, const char *email,
-		      enum want_ident whose_ident, const char *date_str, int flag)
+		      enum want_ident whose_ident, const char *date_str,
+		      int flag)
 {
 	static struct strbuf ident = STRBUF_INIT;
 	int strict = (flag & IDENT_STRICT);
@@ -508,70 +509,38 @@ int author_ident_sufficiently_given(void)
 	return ident_is_sufficient(author_ident_explicitly_given);
 }
 
-static int set_ident(const char *var, const char *value)
+static int set_ident_internal(const char *var, const char *value,
+			    struct strbuf *sb, const int flag)
 {
-	if (!strcmp(var, "author.name")) {
-		if (!value)
-			return config_error_nonbool(var);
-		strbuf_reset(&git_author_name);
-		strbuf_addstr(&git_author_name, value);
-		author_ident_explicitly_given |= IDENT_NAME_GIVEN;
-		ident_config_given |= IDENT_NAME_GIVEN;
-		return 0;
-	}
-
-	if (!strcmp(var, "author.email")) {
-		if (!value)
-			return config_error_nonbool(var);
-		strbuf_reset(&git_author_email);
-		strbuf_addstr(&git_author_email, value);
-		author_ident_explicitly_given |= IDENT_MAIL_GIVEN;
-		ident_config_given |= IDENT_MAIL_GIVEN;
-		return 0;
-	}
-
-	if (!strcmp(var, "committer.name")) {
-		if (!value)
-			return config_error_nonbool(var);
-		strbuf_reset(&git_committer_name);
-		strbuf_addstr(&git_committer_name, value);
-		committer_ident_explicitly_given |= IDENT_NAME_GIVEN;
-		ident_config_given |= IDENT_NAME_GIVEN;
-		return 0;
-	}
-
-	if (!strcmp(var, "committer.email")) {
-		if (!value)
-			return config_error_nonbool(var);
-		strbuf_reset(&git_committer_email);
-		strbuf_addstr(&git_committer_email, value);
-		committer_ident_explicitly_given |= IDENT_MAIL_GIVEN;
-		ident_config_given |= IDENT_MAIL_GIVEN;
-		return 0;
-	}
-
-	if (!strcmp(var, "user.name")) {
-		if (!value)
-			return config_error_nonbool(var);
-		strbuf_reset(&git_default_name);
-		strbuf_addstr(&git_default_name, value);
-		committer_ident_explicitly_given |= IDENT_NAME_GIVEN;
-		author_ident_explicitly_given |= IDENT_NAME_GIVEN;
-		ident_config_given |= IDENT_NAME_GIVEN;
-		return 0;
-	}
-
-	if (!strcmp(var, "user.email")) {
-		if (!value)
-			return config_error_nonbool(var);
-		strbuf_reset(&git_default_email);
-		strbuf_addstr(&git_default_email, value);
-		committer_ident_explicitly_given |= IDENT_MAIL_GIVEN;
-		author_ident_explicitly_given |= IDENT_MAIL_GIVEN;
-		ident_config_given |= IDENT_MAIL_GIVEN;
-		return 0;
-	}
+	if (!value)
+		return config_error_nonbool(var);
+	strbuf_reset(sb);
+	strbuf_addstr(sb, value);
+	author_ident_explicitly_given |= flag;
+	ident_config_given |= flag;
+	return 0;
+}
 
+static int set_ident(const char *var, const char *value)
+{
+	if (!strcmp(var, "author.name"))
+		return set_ident_internal(var, value, &git_author_name,
+					  IDENT_NAME_GIVEN);
+	else if (!strcmp(var, "author.email"))
+		return set_ident_internal(var, value, &git_author_email,
+					  IDENT_MAIL_GIVEN);
+	else if (!strcmp(var, "committer.name"))
+		return set_ident_internal(var, value, &git_committer_name,
+					  IDENT_NAME_GIVEN);
+	else if (!strcmp(var, "committer.email"))
+		return set_ident_internal(var, value, &git_committer_email,
+					  IDENT_MAIL_GIVEN);
+	else if (!strcmp(var, "user.name"))
+		return set_ident_internal(var, value, &git_default_name,
+					  IDENT_NAME_GIVEN);
+	else if (!strcmp(var, "user.email"))
+		return set_ident_internal(var, value, &git_default_email,
+					  IDENT_MAIL_GIVEN);
 	return 0;
 }
 
diff --git a/sequencer.c b/sequencer.c
index 3505d52bb9..98ba2106f6 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -836,7 +836,8 @@ static const char *read_author_ident(struct strbuf *buf)
 	}
 
 	strbuf_reset(&out);
-	strbuf_addstr(&out, fmt_ident(name, email, WANT_AUTHOR_IDENT, date, 0));
+	strbuf_addstr(&out, fmt_ident(name, email, WANT_AUTHOR_IDENT, date,
+				      0));
 	strbuf_swap(buf, &out);
 	strbuf_release(&out);
 	free(name);
diff --git a/t/t7517-per-repo-email.sh b/t/t7517-per-repo-email.sh
index b2401cec3e..e0182779ed 100755
--- a/t/t7517-per-repo-email.sh
+++ b/t/t7517-per-repo-email.sh
@@ -85,15 +85,49 @@ test_expect_success REBASE_P \
 	test_must_fail git rebase -p master
 '
 
+test_expect_success 'fallbacks for GIT_* and {user,author,committer}.{name,email}' '
+	# We must have committer in the object
+	test_must_fail test_env \
+		GIT_AUTHOR_NAME=author.name \
+		GIT_AUTHOR_EMAIL=author@email \
+		GIT_COMMITTER_NAME= \
+		GIT_COMMITTER_EMAIL= \
+		test_commit A 2>stderr &&
+	test_i18ngrep "empty ident name.*not allowed" stderr &&
+
+	# With no committer E-Mail we will have an empty field
+	test_env \
+		GIT_AUTHOR_NAME=author.name \
+		GIT_AUTHOR_EMAIL=author@email \
+		GIT_COMMITTER_NAME=committer.name \
+		GIT_COMMITTER_EMAIL= \
+		test_commit B 2>stderr &&
+	echo "author.name author@email committer.name " >expected &&
+	git log --format="%an %ae %cn %ce" -1 >actual &&
+	test_cmp expected actual &&
+
+	# Environment overrides config
+	test_config user.name author.config.name &&
+	test_env \
+		GIT_AUTHOR_NAME=author.name \
+		GIT_AUTHOR_EMAIL=author@email \
+		GIT_COMMITTER_NAME=committer.name \
+		GIT_COMMITTER_EMAIL= \
+		test_commit C 2>stderr &&
+	echo "author.name author@email committer.name " >expected &&
+	git log --format="%an %ae %cn %ce" -1 >actual &&
+	test_cmp expected actual
+'
+
 test_expect_success 'author.name overrides user.name' '
 	test_config user.name user &&
 	test_config user.email user@example.com &&
 	test_config author.name author &&
 	test_commit author-name-override-user &&
-	echo author user@example.com > expected-author &&
-	echo user user@example.com > expected-committer &&
-	git log --format="%an %ae" -1 > actual-author &&
-	git log --format="%cn %ce" -1 > actual-committer &&
+	echo author user@example.com >expected-author &&
+	echo user user@example.com >expected-committer &&
+	git log --format="%an %ae" -1 >actual-author &&
+	git log --format="%cn %ce" -1 >actual-committer &&
 	test_cmp expected-author actual-author &&
 	test_cmp expected-committer actual-committer
 '
@@ -103,10 +137,10 @@ test_expect_success 'author.email overrides user.email' '
 	test_config user.email user@example.com &&
 	test_config author.email author@example.com &&
 	test_commit author-email-override-user &&
-	echo user author@example.com > expected-author &&
-	echo user user@example.com > expected-committer &&
-	git log --format="%an %ae" -1 > actual-author &&
-	git log --format="%cn %ce" -1 > actual-committer &&
+	echo user author@example.com >expected-author &&
+	echo user user@example.com >expected-committer &&
+	git log --format="%an %ae" -1 >actual-author &&
+	git log --format="%cn %ce" -1 >actual-committer &&
 	test_cmp expected-author actual-author &&
 	test_cmp expected-committer actual-committer
 '
@@ -116,10 +150,10 @@ test_expect_success 'committer.name overrides user.name' '
 	test_config user.email user@example.com &&
 	test_config committer.name committer &&
 	test_commit committer-name-override-user &&
-	echo user user@example.com > expected-author &&
-	echo committer user@example.com > expected-committer &&
-	git log --format="%an %ae" -1 > actual-author &&
-	git log --format="%cn %ce" -1 > actual-committer &&
+	echo user user@example.com >expected-author &&
+	echo committer user@example.com >expected-committer &&
+	git log --format="%an %ae" -1 >actual-author &&
+	git log --format="%cn %ce" -1 >actual-committer &&
 	test_cmp expected-author actual-author &&
 	test_cmp expected-committer actual-committer
 '
@@ -129,10 +163,10 @@ test_expect_success 'committer.email overrides user.email' '
 	test_config user.email user@example.com &&
 	test_config committer.email committer@example.com &&
 	test_commit committer-email-override-user &&
-	echo user user@example.com > expected-author &&
-	echo user committer@example.com > expected-committer &&
-	git log --format="%an %ae" -1 > actual-author &&
-	git log --format="%cn %ce" -1 > actual-committer &&
+	echo user user@example.com >expected-author &&
+	echo user committer@example.com >expected-committer &&
+	git log --format="%an %ae" -1 >actual-author &&
+	git log --format="%cn %ce" -1 >actual-committer &&
 	test_cmp expected-author actual-author &&
 	test_cmp expected-committer actual-committer
 '
@@ -144,17 +178,17 @@ test_expect_success 'author and committer environment variables override config
 	test_config author.email author@example.com &&
 	test_config committer.name committer &&
 	test_config committer.email committer@example.com &&
-	GIT_AUTHOR_NAME=env_author && export GIT_AUTHOR_NAME &&
-	GIT_AUTHOR_EMAIL=env_author@example.com && export GIT_AUTHOR_EMAIL &&
-	GIT_COMMITTER_NAME=env_commit && export GIT_COMMITTER_NAME &&
-	GIT_COMMITTER_EMAIL=env_commit@example.com && export GIT_COMMITTER_EMAIL &&
-	test_commit env-override-conf &&
-	echo env_author env_author@example.com > expected-author &&
-	echo env_commit env_commit@example.com > expected-committer &&
-	git log --format="%an %ae" -1 > actual-author &&
-	git log --format="%cn %ce" -1 > actual-committer &&
-	sane_unset GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL &&
-	sane_unset GIT_COMMITTER_NAME GIT_COMMITTER_EMAIL &&
+
+	test_env \
+		GIT_AUTHOR_NAME=env_author \
+		GIT_AUTHOR_EMAIL=env_author@example.com \
+		GIT_COMMITTER_NAME=env_commit \
+		GIT_COMMITTER_EMAIL=env_commit@example.com \
+		test_commit env-override-conf &&
+	echo env_author env_author@example.com >expected-author &&
+	echo env_commit env_commit@example.com >expected-committer &&
+	git log --format="%an %ae" -1 >actual-author &&
+	git log --format="%cn %ce" -1 >actual-committer &&
 	test_cmp expected-author actual-author &&
 	test_cmp expected-committer actual-committer
 '
Previous: William HubbsNext: William Hubbs
Message 13 of 23 in “config: allow giving separate author and committer”
  1. 0/1 config: allow giving separate author and committerWilliam Hubbs, Feb 4, 2019
  2. 1/1 config: allow giving separate author and committer identsWilliam Hubbs, Feb 4, 2019
  3. Johannes SchindelinFeb 5, 2019
  4. Junio C HamanoFeb 5, 2019
  5. 0/2 New {author,committer}.{name,email} configÆvar Arnfjörð Bjarmason, Feb 5, 2019
  6. 1/2 ident: test how GIT_* and user.{name,email} interactÆvar Arnfjörð Bjarmason, Feb 5, 2019
  7. 2/2 config: allow giving separate author and committer identsÆvar Arnfjörð Bjarmason, Feb 5, 2019
  8. Junio C HamanoFeb 5, 2019
  9. Ævar Arnfjörð BjarmasonFeb 5, 2019
  10. William HubbsFeb 6, 2019
  11. William HubbsFeb 6, 2019
  12. William HubbsFeb 6, 2019
  13. Junio C HamanoFeb 6, 2019
  14. William HubbsFeb 13, 2019
  15. Junio C HamanoFeb 13, 2019
  16. William HubbsFeb 14, 2019
  17. Ævar Arnfjörð BjarmasonFeb 6, 2019
  18. Ævar Arnfjörð BjarmasonFeb 6, 2019
  19. Jeff KingFeb 6, 2019
  20. William HubbsFeb 6, 2019
  21. Junio C HamanoFeb 6, 2019
  22. William HubbsFeb 6, 2019
  23. Derrick StoleeApr 15, 2019

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.