All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: Angus Hammond <angusgh@gmail.com>, git@vger.kernel.org
Subject: Re: [PATCH 1/2] drop length limitations on gecos-derived names and emails
Date: Mon, 14 May 2012 22:32:20 -0400	[thread overview]
Message-ID: <20120515023220.GA22947@sigill.intra.peff.net> (raw)
In-Reply-To: <20120515015437.GA13833@sigill.intra.peff.net>

On Mon, May 14, 2012 at 09:54:37PM -0400, Jeff King wrote:

> Of course we've still polluted this crappy fake name into
> git_default_name, so that later calls with error_on_no_name will see it
> and not error. I think so far it hasn't mattered because the only user
> of this "warn" code is format-patch, which otherwise does not care about
> ident (and doesn't even end up using the name at all!). And I doubt this
> code path gets triggered much anyway; do people really run
> "GIT_COMMITTER_NAME= git format-patch"?
> 
> I can just leave it as it's not really hurting anybody, I think. But I
> was refactoring in the area and it just seemed flaky and questionable. I
> wonder if we can simply get rid of the IDENT_WARN_ON_NO_NAME code path
> entirely. The use here is grabbing the email address to use as part of a
> message id. Could we just call setup_ident and then read from
> git_default_email directly? There's no need to respect
> GIT_COMMITTER_EMAIL here at all.

Hmm, I was mistaken. This code path also gets followed whenever
IDENT_ERROR_ON_NO_NAME is not set (regardless of IDENT_WARN_ON_NO_NAME).
So other programs may accidentally get this pollution of
git_default_name and show a username when we _could_ have shown the name
from config. I can see the pollution in a debugger in "git commit", but
I don't think you can actually trigger a commit with it, because later
calls to fmt_ident use ERROR_ON_NO_NAME.

I really wonder if we can just get rid of all of the calls which do not
use ERROR_ON_NO_NAME. As far as I can tell, they are all part of
programs which later end up using ERROR_ON_NO_NAME anyway.

-Peff

  reply	other threads:[~2012-05-15  2:32 UTC|newest]

Thread overview: 68+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-05-10 19:06 [PATCH 1/2] Change error messages in ident.c Make error messages caused by failed reads of the /etc/passwd file easier to understand. Signed-off-by: Angus Hammond <angusgh@gmail.com> Angus Hammond
2012-05-10 19:06 ` [PATCH 2/2] Remove diagnostics section from commit-tree and var man pages New error messages shouldn't need explaining like the old ones did so just delete the diagnostics section of the man pages. " Angus Hammond
2012-05-10 19:21   ` Angus Hammond
2012-05-10 19:23 ` [PATCH 1/2] Change error messages in ident.c Jeff King
2012-05-10 19:56   ` Jeff King
2012-05-11 22:53     ` Junio C Hamano
2012-05-11 23:13       ` Jeff King
2012-05-14 16:28         ` [PATCH 1/2] drop length limitations on gecos-derived names and emails Jeff King
2012-05-14 17:05           ` Jeff King
2012-05-14 21:02           ` Jeff King
2012-05-14 21:13             ` Jeff King
2012-05-15  1:54               ` Jeff King
2012-05-15  2:32                 ` Jeff King [this message]
2012-05-15 15:03                 ` Junio C Hamano
2012-05-15 17:47                   ` Jeff King
2012-05-15 18:10                     ` Junio C Hamano
2012-05-18 23:05                       ` [PATCH 0/13] ident cleanups and bugfixes Jeff King
2012-05-18 23:07                         ` [PATCH 01/13] ident: split setup_ident into separate functions Jeff King
2012-05-18 23:09                         ` [PATCH 02/13] http-push: do not access git_default_email directly Jeff King
2012-05-18 23:10                         ` [PATCH 03/13] fmt-merge-msg: don't use static buffer in record_person Jeff King
2012-05-18 23:11                         ` [PATCH 04/13] move identity config parsing to ident.c Jeff King
2012-05-18 23:11                         ` [PATCH 05/13] move git_default_* variables " Jeff King
2012-05-21  4:07                           ` Junio C Hamano
2012-05-21  5:41                             ` Jeff King
2012-05-21  6:41                               ` Jeff King
2012-05-18 23:13                         ` [PATCH 06/13] format-patch: use default email for generating message ids Jeff King
2012-05-21  2:58                           ` Junio C Hamano
2012-05-21  6:36                             ` Jeff King
2012-05-18 23:14                         ` [PATCH 07/13] fmt_ident: drop IDENT_WARN_ON_NO_NAME code Jeff King
2012-05-18 23:19                         ` [PATCH 08/13] ident: don't write fallback username into git_default_name Jeff King
2012-05-21  2:54                           ` Junio C Hamano
2012-05-21  6:31                             ` Jeff King
2012-05-21  9:11                               ` Junio C Hamano
2012-05-21 23:09                                 ` [PATCHv2 0/15] ident cleanups git_default_name Jeff King
2012-05-21 23:09                                   ` [PATCHv2 01/15] ident: split setup_ident into separate functions Jeff King
2012-05-21 23:09                                   ` [PATCHv2 02/15] http-push: do not access git_default_email directly Jeff King
2012-05-21 23:09                                   ` [PATCHv2 03/15] fmt-merge-msg: don't use static buffer in record_person Jeff King
2012-05-21 23:09                                   ` [PATCHv2 04/15] move identity config parsing to ident.c Jeff King
2012-05-21 23:09                                   ` [PATCHv2 05/15] move git_default_* variables " Jeff King
2012-05-21 23:10                                   ` [PATCHv2 06/15] ident: trim trailing newline from /etc/mailname Jeff King
2012-05-21 23:10                                   ` [PATCHv2 07/15] format-patch: use default email for generating message ids Jeff King
2012-05-21 23:10                                   ` [PATCHv2 08/15] fmt_ident: drop IDENT_WARN_ON_NO_NAME code Jeff King
2012-05-21 23:10                                   ` [PATCHv2 09/15] ident: don't write fallback username into git_default_name Jeff King
2012-05-21 23:10                                   ` [PATCHv2 10/15] drop length limitations on gecos-derived names and emails Jeff King
2013-01-24 23:21                                     ` [regression] " Jonathan Nieder
2013-01-25  1:05                                       ` Jeff King
2013-01-25 18:46                                         ` Junio C Hamano
2013-01-25 22:10                                           ` Jeff King
2012-05-21 23:10                                   ` [PATCHv2 11/15] ident: report passwd errors with a more friendly message Jeff King
2012-05-21 23:10                                   ` [PATCHv2 12/15] ident: use full dns names to generate email addresses Jeff King
2012-05-21 23:10                                   ` [PATCHv2 13/15] ident: use a dynamic strbuf in fmt_ident Jeff King
2012-05-21 23:10                                   ` [PATCHv2 14/15] ident: trim whitespace from default name/email Jeff King
2012-05-22 16:55                                     ` Junio C Hamano
2012-05-22 17:12                                       ` Jeff King
2012-05-22 17:21                                         ` Junio C Hamano
2012-05-21 23:10                                   ` [PATCHv2 15/15] format-patch: refactor get_patch_filename Jeff King
2012-05-18 23:20                         ` [PATCH 09/13] drop length limitations on gecos-derived names and emails Jeff King
2012-05-18 23:21                         ` [PATCH 10/13] ident: report passwd errors with a more friendly message Jeff King
2012-05-18 23:22                         ` [PATCH 11/13] ident: use full dns names to generate email addresses Jeff King
2012-05-18 23:23                         ` [PATCH 12/13] ident: use a dynamic strbuf in fmt_ident Jeff King
2012-05-18 23:24                         ` [PATCH 13/13] format-patch: refactor get_patch_filename Jeff King
2012-05-14 16:36         ` [PATCH 2/2] ident: report passwd errors with a more friendly message Jeff King
2012-05-10 20:04   ` [PATCH 1/2] Change error messages in ident.c Junio C Hamano
2012-05-10 20:22     ` Jeff King
2012-05-10 20:28       ` Junio C Hamano
2012-05-10 19:43 ` [PATCH 1/2] Change error messages in ident.c Make error messages caused by failed reads of the /etc/passwd file easier to understand. Signed-off-by: Angus Hammond <angusgh@gmail.com> Junio C Hamano
2012-05-10 19:57   ` Angus Hammond
2012-05-11 11:35 ` Nguyen Thai Ngoc Duy

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=20120515023220.GA22947@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=angusgh@gmail.com \
    --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.