William Hubbs <williamh@xxxxxxxxxx> 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@xxxxxxxxxxx && test_config author.name author && test_commit author-name-override-user && - echo author user@xxxxxxxxxxx > expected-author && - echo user user@xxxxxxxxxxx > expected-committer && - git log --format="%an %ae" -1 > actual-author && - git log --format="%cn %ce" -1 > actual-committer && + echo author user@xxxxxxxxxxx >expected-author && + echo user user@xxxxxxxxxxx >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@xxxxxxxxxxx && test_config author.email author@xxxxxxxxxxx && test_commit author-email-override-user && - echo user author@xxxxxxxxxxx > expected-author && - echo user user@xxxxxxxxxxx > expected-committer && - git log --format="%an %ae" -1 > actual-author && - git log --format="%cn %ce" -1 > actual-committer && + echo user author@xxxxxxxxxxx >expected-author && + echo user user@xxxxxxxxxxx >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@xxxxxxxxxxx && test_config committer.name committer && test_commit committer-name-override-user && - echo user user@xxxxxxxxxxx > expected-author && - echo committer user@xxxxxxxxxxx > expected-committer && - git log --format="%an %ae" -1 > actual-author && - git log --format="%cn %ce" -1 > actual-committer && + echo user user@xxxxxxxxxxx >expected-author && + echo committer user@xxxxxxxxxxx >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@xxxxxxxxxxx && test_config committer.email committer@xxxxxxxxxxx && test_commit committer-email-override-user && - echo user user@xxxxxxxxxxx > expected-author && - echo user committer@xxxxxxxxxxx > expected-committer && - git log --format="%an %ae" -1 > actual-author && - git log --format="%cn %ce" -1 > actual-committer && + echo user user@xxxxxxxxxxx >expected-author && + echo user committer@xxxxxxxxxxx >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@xxxxxxxxxxx && test_config committer.name committer && test_config committer.email committer@xxxxxxxxxxx && - GIT_AUTHOR_NAME=env_author && export GIT_AUTHOR_NAME && - GIT_AUTHOR_EMAIL=env_author@xxxxxxxxxxx && export GIT_AUTHOR_EMAIL && - GIT_COMMITTER_NAME=env_commit && export GIT_COMMITTER_NAME && - GIT_COMMITTER_EMAIL=env_commit@xxxxxxxxxxx && export GIT_COMMITTER_EMAIL && - test_commit env-override-conf && - echo env_author env_author@xxxxxxxxxxx > expected-author && - echo env_commit env_commit@xxxxxxxxxxx > 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@xxxxxxxxxxx \ + GIT_COMMITTER_NAME=env_commit \ + GIT_COMMITTER_EMAIL=env_commit@xxxxxxxxxxx \ + test_commit env-override-conf && + echo env_author env_author@xxxxxxxxxxx >expected-author && + echo env_commit env_commit@xxxxxxxxxxx >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 '