Re: [PATCH 4/4] ident: do not ignore empty config name/email

[Junio C Hamano <gitster@pobox.com>]

Jeff King <peff@peff.net> writes:

> This one is perhaps questionable. Maybe somebody is relying on setting a
> per-repo user.name to override a ~/.gitconfig value and enforce
> auto-detection?

Thanks for splitting this step out.  1/4 and 2/4 are obvious
improvements, and 3/4 is a very sensible fix.  Compared to those
three, this one does smell questionable, because I do not quite see
any other reasonable fallback other than the auto-detection if the
user gives an empty ident on purpose.  

Erroring out to say "don't do that" is probably not too bad, but
perhaps we are being run by a script that is doing a best-effort
conversion from $ANOTHER_SCM using a list of known authors that is
incomplete, ending up feeding empty ident and allowing us to fall
back to attribute them to the user who runs the script.  I do not
see a point in breaking that user and having her or him update the
script to stuff in a truly bogus "Unknown <unknown>" name.

>
>  ident.c                       |  4 ++--
>  t/t7518-ident-corner-cases.sh | 11 +++++++++++
>  2 files changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/ident.c b/ident.c
> index ead09ff7f..c0364fe3a 100644
> --- a/ident.c
> +++ b/ident.c
> @@ -153,7 +153,7 @@ static void copy_email(const struct passwd *pw, struct strbuf *email,
>  
>  const char *ident_default_name(void)
>  {
> -	if (!git_default_name.len) {
> +	if (!(ident_config_given & IDENT_NAME_GIVEN) && !git_default_name.len) {
>  		copy_gecos(xgetpwuid_self(&default_name_is_bogus), &git_default_name);
>  		strbuf_trim(&git_default_name);
>  	}
> @@ -162,7 +162,7 @@ const char *ident_default_name(void)
>  
>  const char *ident_default_email(void)
>  {
> -	if (!git_default_email.len) {
> +	if (!(ident_config_given & IDENT_MAIL_GIVEN) && !git_default_email.len) {
>  		const char *email = getenv("EMAIL");
>  
>  		if (email && email[0]) {
> diff --git a/t/t7518-ident-corner-cases.sh b/t/t7518-ident-corner-cases.sh
> index 3d2560c3c..ef570ac62 100755
> --- a/t/t7518-ident-corner-cases.sh
> +++ b/t/t7518-ident-corner-cases.sh
> @@ -22,4 +22,15 @@ test_expect_success 'commit rejects all-crud name' '
>  		git commit --allow-empty -m foo
>  '
>  
> +# We must test the actual error message here, as an unwanted
> +# auto-detection could fail for other reasons.
> +test_expect_success 'empty configured name does not auto-detect' '
> +	(
> +		sane_unset GIT_AUTHOR_NAME &&
> +		test_must_fail \
> +			git -c user.name= commit --allow-empty -m foo 2>err &&
> +		test_i18ngrep "empty ident name" err
> +	)
> +'
> +
>  test_done