Jeff King <p...@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
