Re: [PATCH 4/4] ident: do not ignore empty config name/email
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Feb 23, 2017, 20:58 UTC
- Message-ID
- <xmqqfuj47hfk.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <20170223081708.ge34zjkmpsolocqx@sigill.intra.peff.net>
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.
Show 47 quoted lines
>
> 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