Re: t4014 broken by 43ae9f47ab: format-patch: use default email for generating message ids

Jeff King [off-list ref] writes:

On Thu, May 24, 2012 at 04:15:53PM -0400, Jeff King wrote:

quoted

quoted

If you mean "why environment and not an API call?", then I would have to
agree.  ident_committer_email() call, that returns a sanitized version,
would have been a natural way to write this, if it were available.

I meant the latter. There is no such call, but I can make one. Let me
see how awkward it is.

Here it is.

  [1/7]: ident: refactor empty ident error message

This one is only tangentially related. I was going to touch the message
more in patch 3, but decided not to (details in that patch).

  [2/7]: ident: refactor NO_DATE flag in fmt_ident
  [3/7]: ident: let callers omit name with fmt_indent
  [4/7]: format-patch: use GIT_COMMITTER_EMAIL in message ids

These ones should fix Michael's failing test and restore the original
behavior.

  [5/7]: ident: rename IDENT_ERROR_ON_NO_NAME to IDENT_STRICT
  [6/7]: ident: reject bogus email addresses with IDENT_STRICT
  [7/7]: format-patch: do not use bogus email addresses in message ids

These ones prevent bogus message ids from being generated at all
(which is an improvement over the previous state).

All looked pretty straightforward and cleanly done.

We might want to further tighten 6/7 to verify user-supplied (i.e. non
default) e-mail for sanity, as I agree with the comment below --- lines of
that patch.

Also the check might want to be further tightened in the RFC 822/2822/5322
sense, but getting it correct will open a huge can of worms; I think the
check in 6/7 is a good place to stop, at least for now.

Thanks.

On Thu, May 24, 2012 at 05:08:15PM -0700, Junio C Hamano wrote:

quoted

  [5/7]: ident: rename IDENT_ERROR_ON_NO_NAME to IDENT_STRICT
  [6/7]: ident: reject bogus email addresses with IDENT_STRICT
  [7/7]: format-patch: do not use bogus email addresses in message ids

These ones prevent bogus message ids from being generated at all
(which is an improvement over the previous state).

All looked pretty straightforward and cleanly done.

We might want to further tighten 6/7 to verify user-supplied (i.e. non
default) e-mail for sanity, as I agree with the comment below --- lines of
that patch.

Yeah. I'd be fine with more tightening, but I wanted to just catch the
most common uncontroversial problem in the initial round. We can build
on top later if we want.

Also the check might want to be further tightened in the RFC 822/2822/5322
sense, but getting it correct will open a huge can of worms; I think the
check in 6/7 is a good place to stop, at least for now.

Yeah, I considered that, but got nervous thinking about the same can of
worms. It is not like people are complaining now, so I'd rather leave
it.

-Peff