All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: bs.x.ttp@recursor.net, git@vger.kernel.org
Subject: Re: [PATCH 4/4] ident: do not ignore empty config name/email
Date: Thu, 23 Feb 2017 20:08:23 -0500	[thread overview]
Message-ID: <20170224010823.my4wmdyezjuqajfx@sigill.intra.peff.net> (raw)
In-Reply-To: <xmqqfuj47hfk.fsf@gitster.mtv.corp.google.com>

On Thu, Feb 23, 2017 at 12:58:39PM -0800, Junio C Hamano wrote:

> 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.

The outcomes are basically:

  1. In strict mode (making a commit, etc), we'll die with "empty name
     not allowed". My thinking was that this is less confusing for the
     user.

  2. In non-strict mode, we'd use a blank name instead of trying your
     username (or dying if you don't have an /etc/passwd entry).

> 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.

Keep in mind this _only_ affects Git's config variables. So a script
feeding git via GIT_AUTHOR_NAME, etc, shouldn't change at all with this
code. If your script is doing "git -c user.name=whatever commit", I
think you should reconsider your script. :)

So I dunno. I could really go either way on it. Feel free to drop it, or
even move it into a separate topic to be cooked longer.

-Peff

  reply	other threads:[~2017-02-24  1:08 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-03  4:13 possible bug: inconsistent CLI behaviour for empty user.name bs.x.ttp
2017-02-23  8:11 ` Jeff King
2017-02-23  8:12   ` [PATCH 1/4] ident: mark error messages for translation Jeff King
2017-02-23  8:13   ` [PATCH 2/4] ident: handle NULL email when complaining of empty name Jeff King
2017-02-23  8:15   ` [PATCH 3/4] ident: reject all-crud ident name Jeff King
2017-02-23  8:17   ` [PATCH 4/4] ident: do not ignore empty config name/email Jeff King
2017-02-23 20:58     ` Junio C Hamano
2017-02-24  1:08       ` Jeff King [this message]
2017-02-24  4:11         ` Junio C Hamano
2017-02-24  4:18           ` Jeff King
2017-02-27 15:08             ` Dennis Kaarsemaker
2017-02-27 20:42               ` Junio C Hamano
2017-02-28  5:28                 ` Christian Couder

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170224010823.my4wmdyezjuqajfx@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=bs.x.ttp@recursor.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.