Re: [PATCH 06/13] format-patch: use default email for generating message ids

From: Jeff King <peff@peff.net>
Date: May 21, 2012, 06:36 UTC
Message-ID: <20120521063643.GC2077@sigill.intra.peff.net>
In-Reply-To: <7vzk928clf.fsf@alter.siamese.dyndns.org>

On Sun, May 20, 2012 at 07:58:04PM -0700, Junio C Hamano wrote:

Show 10 quoted lines

> > Technically this is a regression if you really wanted:
> >
> >   GIT_COMMITTER_EMAIL=some.addr@example.com \
> >   git format-patch --thread=deep
> >
> > to make your environment variable part of the message-ids. I don't think
> > it matters, but I can adjust it if we care.
> 
> Is it because you no longer explicitly ask for "committer" and get generic
> "who am I" bit from the ident infrastructure?

Right. Calling git_committer_info will also check getenv("GIT_COMMITTER_EMAIL"), but we don't bother to do that here. We could change the code to:

  const char *email = getenv("GIT_COMMITTER_EMAIL");
  if (!email)
          email = ident_default_email();

if it matters. I don't think it's worth building an alternate version of git_committer_info that doesn't do the "name" half of the info.

Show 8 quoted lines

> I wouldn't be surprised if some automated "commit email notification
> reacting to push" bot in post-receive hook is using the environment
> variable to affect the message ID.  I would doubt that would break the
> message as long as the message ID generated from this codepath stays
> valid, though, so I wouldn't worry about complaints along the lines of
> "you started using names different from what you used to use".  As long as
> we don't die due to "Hey bot, you do not seem to have a valid e-mail
> address!", I don't think we need to worry about it.

No, we'll never die as a result. But if you have a poorly configured machine, you might get "user@host.(none)". Or I suppose you could leak information about the username of the post-receive process. In both cases, we do this already, so the only change is if you are trying to override that with GIT_COMMITTER_EMAIL.

So it's a little far-fetched, which is why I didn't bother. But the code above is quite simple, so maybe it is better to be conservative.

-Peff