Formatting problem send_mail in version 2.10.0

Formatting problem send_mail in version 2.10.0

[Larry Finger]

I have recently switched to openSUSE Leap 42.2 and found that some of the 
features of send_mail no longer work. The problem occurs when trying to add 
information to a Cc to Stable.

The initial pass through the patch produces the output
(body) Adding cc: Stable <stable@vger.kernel.org> [4.8+] from line 'Cc: Stable 
<stable@vger.kernel.org> [4.8+]'

That is correct, but the actual Cc list contains
         Stable <stable@vger.kernel.org[4.8+]>,

The mangled address is not legal and the mail attempt fails.

Thanks,

Larry

Re: Formatting problem send_mail in version 2.10.0

[Jeff King]

On Mon, Oct 10, 2016 at 04:00:56PM -0500, Larry Finger wrote:

> I have recently switched to openSUSE Leap 42.2 and found that some of the
> features of send_mail no longer work. The problem occurs when trying to add
> information to a Cc to Stable.
> 
> The initial pass through the patch produces the output
> (body) Adding cc: Stable <stable@vger.kernel.org> [4.8+] from line 'Cc:
> Stable <stable@vger.kernel.org> [4.8+]'
> 
> That is correct, but the actual Cc list contains
>         Stable <stable@vger.kernel.org[4.8+]>,
> 
> The mangled address is not legal and the mail attempt fails.

I can't reproduce the problem with this simple setup:

	git init
	echo content >file && git add file
	git commit -F- <<-\EOF
	the subject

	the body

	Cc: Stable <stable@vger.kernel.org> [4.8+]
	EOF

If I then run:

	git send-email -1 --to=peff@peff.net --dry-run

I get:

	/tmp/MH8SfHOjCv/0001-the-subject.patch
	(mbox) Adding cc: Jeff King <peff@peff.net> from line 'From: Jeff King <peff@peff.net>'
	(body) Adding cc: Stable <stable@vger.kernel.org> [4.8+] from line 'Cc: Stable <stable@vger.kernel.org> [4.8+]'
	Dry-OK. Log says:
	Sendmail: /usr/sbin/sendmail -i peff@peff.net stable@vger.kernel.org
	From: Jeff King <peff@peff.net>
	To: peff@peff.net
	Cc: "Stable [4.8+]" <stable@vger.kernel.org>
	Subject: [PATCH] the subject
	Date: Mon, 10 Oct 2016 17:44:25 -0400
	Message-Id: <20161010214425.9761-1-peff@peff.net>
	X-Mailer: git-send-email 2.10.1.527.g93d4615
	
	Result: OK

So it looks like it parsed the address, and shifted the "4.8+" bit into
the name, which seems reasonable. Does my example behave differently on
your system? If not, can you see what's different between your
real-world case and the example?

It might also be related to which perl modules are available. We'll use
Mail::Address if you have it, but some fallback routines if you don't.
They may behave differently.

Alternatively, if this used to work, you might try bisecting it.

-Peff

Re: Formatting problem send_mail in version 2.10.0

[Jeff King]

[+cc authors of b1c8a11, which regressed this case; I'll quote liberally
     to give context]

On Mon, Oct 10, 2016 at 05:48:56PM -0400, Jeff King wrote:

> I can't reproduce the problem with this simple setup:
> 
> 	git init
> 	echo content >file && git add file
> 	git commit -F- <<-\EOF
> 	the subject
> 
> 	the body
> 
> 	Cc: Stable <stable@vger.kernel.org> [4.8+]
> 	EOF
> 
> If I then run:
> 
> 	git send-email -1 --to=peff@peff.net --dry-run
> 
> I get:
> 
> 	/tmp/MH8SfHOjCv/0001-the-subject.patch
> 	(mbox) Adding cc: Jeff King <peff@peff.net> from line 'From: Jeff King <peff@peff.net>'
> 	(body) Adding cc: Stable <stable@vger.kernel.org> [4.8+] from line 'Cc: Stable <stable@vger.kernel.org> [4.8+]'
> 	Dry-OK. Log says:
> 	Sendmail: /usr/sbin/sendmail -i peff@peff.net stable@vger.kernel.org
> 	From: Jeff King <peff@peff.net>
> 	To: peff@peff.net
> 	Cc: "Stable [4.8+]" <stable@vger.kernel.org>
> 	Subject: [PATCH] the subject
> 	Date: Mon, 10 Oct 2016 17:44:25 -0400
> 	Message-Id: <20161010214425.9761-1-peff@peff.net>
> 	X-Mailer: git-send-email 2.10.1.527.g93d4615
> 	
> 	Result: OK
> 
> So it looks like it parsed the address, and shifted the "4.8+" bit into
> the name, which seems reasonable. Does my example behave differently on
> your system? If not, can you see what's different between your
> real-world case and the example?
> 
> It might also be related to which perl modules are available. We'll use
> Mail::Address if you have it, but some fallback routines if you don't.
> They may behave differently.
> 
> Alternatively, if this used to work, you might try bisecting it.

Ah, it is Mail::Address. It gets this case right, but if I uninstall it,
then the cc becomes:

  Cc: Stable <stable@vger.kernel.org[4.8+]>

that you saw, which is broken. Older versions of git, even without
Mail::Address, got this right. The breakage bisects to b1c8a11
(send-email: allow multiple emails using --cc, --to and --bcc,
2015-06-30) from v2.6.0, but I didn't dig deeper into the cause.

-Peff

Re: Formatting problem send_mail in version 2.10.0

[Larry Finger]

On 10/10/2016 04:57 PM, Jeff King wrote:
> [+cc authors of b1c8a11, which regressed this case; I'll quote liberally
>      to give context]
>
> On Mon, Oct 10, 2016 at 05:48:56PM -0400, Jeff King wrote:
>
>> I can't reproduce the problem with this simple setup:
>>
>> 	git init
>> 	echo content >file && git add file
>> 	git commit -F- <<-\EOF
>> 	the subject
>>
>> 	the body
>>
>> 	Cc: Stable <stable@vger.kernel.org> [4.8+]
>> 	EOF
>>
>> If I then run:
>>
>> 	git send-email -1 --to=peff@peff.net --dry-run
>>
>> I get:
>>
>> 	/tmp/MH8SfHOjCv/0001-the-subject.patch
>> 	(mbox) Adding cc: Jeff King <peff@peff.net> from line 'From: Jeff King <peff@peff.net>'
>> 	(body) Adding cc: Stable <stable@vger.kernel.org> [4.8+] from line 'Cc: Stable <stable@vger.kernel.org> [4.8+]'
>> 	Dry-OK. Log says:
>> 	Sendmail: /usr/sbin/sendmail -i peff@peff.net stable@vger.kernel.org
>> 	From: Jeff King <peff@peff.net>
>> 	To: peff@peff.net
>> 	Cc: "Stable [4.8+]" <stable@vger.kernel.org>
>> 	Subject: [PATCH] the subject
>> 	Date: Mon, 10 Oct 2016 17:44:25 -0400
>> 	Message-Id: <20161010214425.9761-1-peff@peff.net>
>> 	X-Mailer: git-send-email 2.10.1.527.g93d4615
>> 	
>> 	Result: OK
>>
>> So it looks like it parsed the address, and shifted the "4.8+" bit into
>> the name, which seems reasonable. Does my example behave differently on
>> your system? If not, can you see what's different between your
>> real-world case and the example?
>>
>> It might also be related to which perl modules are available. We'll use
>> Mail::Address if you have it, but some fallback routines if you don't.
>> They may behave differently.
>>
>> Alternatively, if this used to work, you might try bisecting it.
>
> Ah, it is Mail::Address. It gets this case right, but if I uninstall it,
> then the cc becomes:
>
>   Cc: Stable <stable@vger.kernel.org[4.8+]>
>
> that you saw, which is broken. Older versions of git, even without
> Mail::Address, got this right. The breakage bisects to b1c8a11
> (send-email: allow multiple emails using --cc, --to and --bcc,
> 2015-06-30) from v2.6.0, but I didn't dig deeper into the cause.

I did not have Mail::Address installed, but adding it did not help.

I solved my immediate problem by moving the [4.8+] between Stable and the 
starting <. The result is spaced funny, but at least the info is there.

Larry

Re: Formatting problem send_mail in version 2.10.0

[Jeff King]

On Mon, Oct 10, 2016 at 06:35:01PM -0500, Larry Finger wrote:

> > Ah, it is Mail::Address. It gets this case right, but if I uninstall it,
> > then the cc becomes:
> > 
> >   Cc: Stable <stable@vger.kernel.org[4.8+]>
> > 
> > that you saw, which is broken. Older versions of git, even without
> > Mail::Address, got this right. The breakage bisects to b1c8a11
> > (send-email: allow multiple emails using --cc, --to and --bcc,
> > 2015-06-30) from v2.6.0, but I didn't dig deeper into the cause.
> 
> I did not have Mail::Address installed, but adding it did not help.

Weird. On my system:

  $ git send-email -1 --to=peff@peff.net --dry-run | grep ^Cc
  Cc: Stable <stable@vger.kernel.org[4.8+]>

  $ sudo apt-get install libmailtools-perl
  ... apt-get cruft ...

  $ git send-email -1 --to=peff@peff.net --dry-run | grep ^Cc
  Cc: "Stable [4.8+]" <stable@vger.kernel.org>

I wonder if the version matters (mine is 2.13-1, packaged by Debian).

> I solved my immediate problem by moving the [4.8+] between Stable and the
> starting <. The result is spaced funny, but at least the info is there.

If you can change the format, I suspect:

  Cc: "Stable: [v4.8+]" <stable@vger.kernel.org>

will probably yield the most consistent results.

-Peff

Re: Formatting problem send_mail in version 2.10.0

[Matthieu Moy]

Jeff King <peff@peff.net> writes:

> [+cc authors of b1c8a11, which regressed this case; I'll quote liberally
>      to give context]
>
> On Mon, Oct 10, 2016 at 05:48:56PM -0400, Jeff King wrote:
>
>> I can't reproduce the problem with this simple setup:
>> 
>> 	git init
>> 	echo content >file && git add file
>> 	git commit -F- <<-\EOF
>> 	the subject
>> 
>> 	the body
>> 
>> 	Cc: Stable <stable@vger.kernel.org> [4.8+]

Is this RFC2822 compliant (https://tools.ietf.org/html/rfc2822)? Not an
expert of the norm, but my understanding is that you're allowed to use
either "Name <addr@domain.com>" (name-addr) or addr@domain.com
(addr-spec), and that comments are allowed within parenthesis like
"Stable <stable@vger.kernel.org> (4.8+)".

What is this [4.8+] supposed to mean?

The guilty function is parse_mailboxes in perl/Git.pm. It should be
rather easy to modify it but I need to understand the spec before I can
try to implement anything.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: Formatting problem send_mail in version 2.10.0

[Larry Finger]

On 10/11/2016 02:39 AM, Matthieu Moy wrote:
> Jeff King <peff@peff.net> writes:
>
>> [+cc authors of b1c8a11, which regressed this case; I'll quote liberally
>>      to give context]
>>
>> On Mon, Oct 10, 2016 at 05:48:56PM -0400, Jeff King wrote:
>>
>>> I can't reproduce the problem with this simple setup:
>>>
>>> 	git init
>>> 	echo content >file && git add file
>>> 	git commit -F- <<-\EOF
>>> 	the subject
>>>
>>> 	the body
>>>
>>> 	Cc: Stable <stable@vger.kernel.org> [4.8+]
>
> Is this RFC2822 compliant (https://tools.ietf.org/html/rfc2822)? Not an
> expert of the norm, but my understanding is that you're allowed to use
> either "Name <addr@domain.com>" (name-addr) or addr@domain.com
> (addr-spec), and that comments are allowed within parenthesis like
> "Stable <stable@vger.kernel.org> (4.8+)".
>
> What is this [4.8+] supposed to mean?
>
> The guilty function is parse_mailboxes in perl/Git.pm. It should be
> rather easy to modify it but I need to understand the spec before I can
> try to implement anything.

That added information at the end is intended to be passed on to the stable 
group. In this case, the patch needs to be applied to kernel versions 4.8 and later.

Placing the info inside parentheses causes it to be dropped from the outgoing 
mail as follows:

(body) Adding cc: Stable <stable@vger.kernel.org> (4.8+) from line 'Cc: Stable 
<stable@vger.kernel.org> (4.8+)'
--snip--
Cc:     Stable <stable@vger.kernel.org>,

Larry

Re: Formatting problem send_mail in version 2.10.0

[Matthieu Moy]

Larry Finger <Larry.Finger@lwfinger.net> writes:

> That added information at the end is intended to be passed on to the
> stable group. In this case, the patch needs to be applied to kernel
> versions 4.8 and later.

OK, but where do people fetch this information from?

When you use git send-email, the content of the Cc: trailers ends up
both in the body of the message and in the Cc: field of the same
message.

If you need the mention to appear in the body of the message, then using
parenthesis is fine: git send-email won't remove it (more precisely,
"send-email" will call "format-patch" which won't remove it).

Not an objection to patching send-email anyway, but if there's a simple
and RFC-compliant way to do what you're looking for, we can as well use
it (possibly in addition to patching).

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: Formatting problem send_mail in version 2.10.0

[Larry Finger]

On 10/11/2016 11:18 AM, Matthieu Moy wrote:
> Larry Finger <Larry.Finger@lwfinger.net> writes:
>
>> That added information at the end is intended to be passed on to the
>> stable group. In this case, the patch needs to be applied to kernel
>> versions 4.8 and later.
>
> OK, but where do people fetch this information from?

This format is used in a patch for the kernel. When the patch is merged into 
mainline, stable@vger.kernel.org gets sent an E-mail with a copy of the original 
patch. Maintainers of the indicated systems then merge the patch with their 
stable version.
>
> When you use git send-email, the content of the Cc: trailers ends up
> both in the body of the message and in the Cc: field of the same
> message.
>
> If you need the mention to appear in the body of the message, then using
> parenthesis is fine: git send-email won't remove it (more precisely,
> "send-email" will call "format-patch" which won't remove it).
>
> Not an objection to patching send-email anyway, but if there's a simple
> and RFC-compliant way to do what you're looking for, we can as well use
> it (possibly in addition to patching).

I do not want it in the body of the message. I just want to pass a hint to the 
stable maintainer(s).

As noted earlier, this has worked for a very long time, and I think the previous 
behavior should be restored.

Larry

>

Re: Formatting problem send_mail in version 2.10.0

[Matthieu Moy]

Larry Finger <Larry.Finger@lwfinger.net> writes:

> On 10/11/2016 11:18 AM, Matthieu Moy wrote:
>> Larry Finger <Larry.Finger@lwfinger.net> writes:
>>
>>> That added information at the end is intended to be passed on to the
>>> stable group. In this case, the patch needs to be applied to kernel
>>> versions 4.8 and later.
>>
>> OK, but where do people fetch this information from?
>
> This format is used in a patch for the kernel. When the patch is
> merged into mainline, stable@vger.kernel.org gets sent an E-mail with
> a copy of the original patch. Maintainers of the indicated systems
> then merge the patch with their stable version.

Sorry, but this does not answer my question. I'll rephrase: when
people behind stable@vger.kernel.org get the message, how do they know
which version of the kernel they should apply it to?

> I do not want it in the body of the message. I just want to pass a
> hint to the stable maintainer(s).

If it's not in the body of the message, then where is it?

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: Formatting problem send_mail in version 2.10.0

[Larry Finger]

On 10/12/2016 02:36 AM, Matthieu Moy wrote:
> Larry Finger <Larry.Finger@lwfinger.net> writes:
>
>> On 10/11/2016 11:18 AM, Matthieu Moy wrote:
>>> Larry Finger <Larry.Finger@lwfinger.net> writes:
>>>
>>>> That added information at the end is intended to be passed on to the
>>>> stable group. In this case, the patch needs to be applied to kernel
>>>> versions 4.8 and later.
>>>
>>> OK, but where do people fetch this information from?
>>
>> This format is used in a patch for the kernel. When the patch is
>> merged into mainline, stable@vger.kernel.org gets sent an E-mail with
>> a copy of the original patch. Maintainers of the indicated systems
>> then merge the patch with their stable version.
>
> Sorry, but this does not answer my question. I'll rephrase: when
> people behind stable@vger.kernel.org get the message, how do they know
> which version of the kernel they should apply it to?
>
>> I do not want it in the body of the message. I just want to pass a
>> hint to the stable maintainer(s).
>
> If it's not in the body of the message, then where is it?

That information is supplied in the "Cc: Stable" line at the end. That is the 
point for the "[4.8+]" notation. That means apply to kernel 4.8 and later. 
Without this information, the patch author has to monitor the mainline kernel 
for the application of the main patch, and then send a separate E-mail to Stable 
indicating the version info. When the info is included, then the process is 
automatic. I hope you understand now.

Larry

Re: Formatting problem send_mail in version 2.10.0

[Matthieu Moy]

Larry Finger <Larry.Finger@lwfinger.net> writes:

> On 10/12/2016 02:36 AM, Matthieu Moy wrote:
>> Larry Finger <Larry.Finger@lwfinger.net> writes:
>>
>>> On 10/11/2016 11:18 AM, Matthieu Moy wrote:
>>>> Larry Finger <Larry.Finger@lwfinger.net> writes:
>>>>
>>>>> That added information at the end is intended to be passed on to the
>>>>> stable group. In this case, the patch needs to be applied to kernel
>>>>> versions 4.8 and later.
>>>>
>>>> OK, but where do people fetch this information from?
>>>
>>> This format is used in a patch for the kernel. When the patch is
>>> merged into mainline, stable@vger.kernel.org gets sent an E-mail with
>>> a copy of the original patch. Maintainers of the indicated systems
>>> then merge the patch with their stable version.
>>
>> Sorry, but this does not answer my question. I'll rephrase: when
>> people behind stable@vger.kernel.org get the message, how do they know
>> which version of the kernel they should apply it to?
>>
>>> I do not want it in the body of the message. I just want to pass a
>>> hint to the stable maintainer(s).
>>
>> If it's not in the body of the message, then where is it?
>
> That information is supplied in the "Cc: Stable" line at the end.

OK, so it *is* in the body of the email message.

If I understand correctly, the important is that:

* "git send-email" should accept sending a message for a commit
  containing a Cc: <address@host> [number] line.

* The message should be sent to <address@host>

But I still don't get why using the (4.8+) notation wouldn't work. It
would still end up in the Cc: line in the body of the message, and the
message would still be sent to the same address. It is *not* dropped
from the message, just from the email headers of the message being sent.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: Formatting problem send_mail in version 2.10.0

[Larry Finger]

On 10/12/2016 02:36 AM, Matthieu Moy wrote:
> Larry Finger <Larry.Finger@lwfinger.net> writes:
>
>> On 10/11/2016 11:18 AM, Matthieu Moy wrote:
>>> Larry Finger <Larry.Finger@lwfinger.net> writes:
>>>
>>>> That added information at the end is intended to be passed on to the
>>>> stable group. In this case, the patch needs to be applied to kernel
>>>> versions 4.8 and later.
>>>
>>> OK, but where do people fetch this information from?
>>
>> This format is used in a patch for the kernel. When the patch is
>> merged into mainline, stable@vger.kernel.org gets sent an E-mail with
>> a copy of the original patch. Maintainers of the indicated systems
>> then merge the patch with their stable version.
>
> Sorry, but this does not answer my question. I'll rephrase: when
> people behind stable@vger.kernel.org get the message, how do they know
> which version of the kernel they should apply it to?
>
>> I do not want it in the body of the message. I just want to pass a
>> hint to the stable maintainer(s).
>
> If it's not in the body of the message, then where is it?

This point is clarified in the thread 
http://marc.info/?l=linux-wireless&m=147625930203434&w=2, which is with my 
upstream maintainer.

Larry

Re: Formatting problem send_mail in version 2.10.0

[Matthieu Moy]

Larry Finger <Larry.Finger@lwfinger.net> writes:

> On 10/12/2016 02:36 AM, Matthieu Moy wrote:
>> Larry Finger <Larry.Finger@lwfinger.net> writes:
>>
>>> On 10/11/2016 11:18 AM, Matthieu Moy wrote:
>>>> Larry Finger <Larry.Finger@lwfinger.net> writes:
>>>>
>>>>> That added information at the end is intended to be passed on to the
>>>>> stable group. In this case, the patch needs to be applied to kernel
>>>>> versions 4.8 and later.
>>>>
>>>> OK, but where do people fetch this information from?
>>>
>>> This format is used in a patch for the kernel. When the patch is
>>> merged into mainline, stable@vger.kernel.org gets sent an E-mail with
>>> a copy of the original patch. Maintainers of the indicated systems
>>> then merge the patch with their stable version.
>>
>> Sorry, but this does not answer my question. I'll rephrase: when
>> people behind stable@vger.kernel.org get the message, how do they know
>> which version of the kernel they should apply it to?
>>
>>> I do not want it in the body of the message. I just want to pass a
>>> hint to the stable maintainer(s).
>>
>> If it's not in the body of the message, then where is it?
>
> This point is clarified in the thread
> http://marc.info/?l=linux-wireless&m=147625930203434&w=2, which is
> with my upstream maintainer.

Which explicitly states that the syntax is not [$number], but # $number,
right?

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: Formatting problem send_mail in version 2.10.0

[Larry Finger]

On 10/12/2016 10:45 AM, Matthieu Moy wrote:
> Larry Finger <Larry.Finger@lwfinger.net> writes:
>
>> On 10/12/2016 02:36 AM, Matthieu Moy wrote:
>>> Larry Finger <Larry.Finger@lwfinger.net> writes:
>>>
>>>> On 10/11/2016 11:18 AM, Matthieu Moy wrote:
>>>>> Larry Finger <Larry.Finger@lwfinger.net> writes:
>>>>>
>>>>>> That added information at the end is intended to be passed on to the
>>>>>> stable group. In this case, the patch needs to be applied to kernel
>>>>>> versions 4.8 and later.
>>>>>
>>>>> OK, but where do people fetch this information from?
>>>>
>>>> This format is used in a patch for the kernel. When the patch is
>>>> merged into mainline, stable@vger.kernel.org gets sent an E-mail with
>>>> a copy of the original patch. Maintainers of the indicated systems
>>>> then merge the patch with their stable version.
>>>
>>> Sorry, but this does not answer my question. I'll rephrase: when
>>> people behind stable@vger.kernel.org get the message, how do they know
>>> which version of the kernel they should apply it to?
>>>
>>>> I do not want it in the body of the message. I just want to pass a
>>>> hint to the stable maintainer(s).
>>>
>>> If it's not in the body of the message, then where is it?
>>
>> This point is clarified in the thread
>> http://marc.info/?l=linux-wireless&m=147625930203434&w=2, which is
>> with my upstream maintainer.
>
> Which explicitly states that the syntax is not [$number], but # $number,
> right?

Yes it does; however v2.10.0 mangles this into

Stable <stable@vger.kernel.org#4.8+>

and my mailer refuses to send that.

Larry

Re: Formatting problem send_mail in version 2.10.0

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

>>> If it's not in the body of the message, then where is it?
>>
>> This point is clarified in the thread
>> http://marc.info/?l=linux-wireless&m=147625930203434&w=2, which is
>> with my upstream maintainer.
>
> Which explicitly states that the syntax is not [$number], but # $number,
> right?

But I do not think that works, either.  Let's step back.

People write things like these

    Cc: Stable <stable@vger.kernel.org> # 4.8
    Cc: Stable <stable@vger.kernel.org> [4.8+]

in the trailer part in the body of the message.  Are these lines
meant to be usable if they appear as Cc: headers of an outgoing
piece of e-mail as-is?

That question has two subissues.

 * Are these RFC compliant?
 * Do real-world servers accept them?

I did a quick test with my e-mail provider, by sending a test
message with the above (with <stable@vger.kernel.org> replaced with
my address, of course) on Cc: and the answer to the second question
appears to be no.

When I send the former, I get this:

    550 5.1.1 <4.8>: Recipient address rejected: User unknown...'
    errormsg='recipient address 4.8 not accepted by the server'

The latter gives me this:

    550 5.1.1 <[4.8+]>: Recipient address rejected: User unknown...'
    errormsg='recipient address 4.8 not accepted by the server'

So even if these were accepted by some servers, it is not sane
to assume that these lines are meant to be copied to Cc: headers
when sending out.  "send-email --cc" needs to salvage these
malformed lines.

For me, it seems that I get correct envelope rcpt-to addresses for
both of them (i.e. "stable@vger.kernel.org" and nothing else appears
on the command line of sendmail invocation), even though I seem to
get a strange Cc: that are visible to humans:

    Cc: "Stable # 4 . 8" <stable@vger.kernel.org>,
        "Stable [4.8+]" <stable@vger.kernel.org>

I seem to have libmailtools-perl 2.12 on my system.

Just like Peff who tried to disable Mail::Address, it seems that
I get complaint on both of them.

"error: unable to extract a valid address from:" is followed by

    Stable <stable@vger.kernel.org#4.8>
    Stable <stable@vger.kernel.org[4.8+]>

which is not ideal.

If I were to issue a decree, I would say that people should stop
suggesting to put RFC-bogus things on the Cc: line.  As you
mentioned, things like:

    Cc: Stable (4.8+) <stable@vger.kernel.org>
    Cc: "Stable 4.8+" <stable@vger.kernel.org>

are perfectly readable alternative that are still RFC-kosher.

Things may have appeared to work by accident, and things may still
work by accident, depending on the vintage and availability of
Mail::Address package (which seems to be the case), but it is not
worth risking random breakages that depends on what other people
use in the first place, no?

That is, even though people seem to expect "send-email --cc" to
somehow be able to judge that " # 4.8" and " [4.8+]" are cruft not
meant as part of a valid address, I do not think it is a realistic
expectation.  How would it know "Cc: Stable <add@re.ss> 4.8, 4.9"
has garbage " 4.8, 4.9" that needs to be stripped, while "Cc: Stable
<add@re.ss> 4.8, torvalds@linux-foundation.org" has two valid
addresses that need to be CC'ed and " 4.8" is the only thing that is
unwanted?

I'd welcome to be proven wrong with a working patch that makes
"send-email --cc" to be hyper intelligent ;-), though.

Re: Formatting problem send_mail in version 2.10.0

[Jeff King]

On Wed, Oct 12, 2016 at 01:53:52PM -0700, Junio C Hamano wrote:

> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
> 
> >>> If it's not in the body of the message, then where is it?
> >>
> >> This point is clarified in the thread
> >> http://marc.info/?l=linux-wireless&m=147625930203434&w=2, which is
> >> with my upstream maintainer.
> >
> > Which explicitly states that the syntax is not [$number], but # $number,
> > right?
> 
> But I do not think that works, either.  Let's step back.
> 
> People write things like these
> 
>     Cc: Stable <stable@vger.kernel.org> # 4.8
>     Cc: Stable <stable@vger.kernel.org> [4.8+]
> 
> in the trailer part in the body of the message.  Are these lines
> meant to be usable if they appear as Cc: headers of an outgoing
> piece of e-mail as-is?

I think the answer is pretty clearly no. It's just that historically we
have auto-munged it into something useful. I think the viable options
are basically:

  1. Tell people not to do that, and to do something RFC compliant like
     "Stable [4.8+]" <stable@vger.kernel.org>. This is a little funny
     for git because we otherwise do not require things like
     rfc-compliant quoting for our name/email pairs. But it Just Works
     without anybody having to write extra code, or worry about corner
     cases in parsing.

  2. Drop everything after the trailing ">". This gives a valid rfc2822
     cc, and people can pick the "# 4.8" from the cc line in the body.

  3. Rewrite

       A <B@C> D

     into

       A D <B@C>

     regardless of what is in "D". This retains the information in the
     rfc2822 cc.

Starting from scratch, I'd say that (2) seems like a good combination of
simplicity and friendliness.  But (3) matches what we have done
historically (and still do at least for some values of "D", and
depending on the presence of Mail::Address).

Once we decide on a behavior, it seems like we should be able to apply
it consistently with or without Mail::Address by grabbing the bits after
the final ">".

Larry seems to be against (2), but I'm not sure I understand why pulling
the value from the in-body cc (which gets copied into the commit message
by git-am, too) would be a problem.

-Peff

Re: Formatting problem send_mail in version 2.10.0

[Matthieu Moy]

Jeff King <peff@peff.net> writes:

>   2. Drop everything after the trailing ">". This gives a valid rfc2822
>      cc, and people can pick the "# 4.8" from the cc line in the body.

That would work for me, but it's inconsistent with Mail::Address and I'd
really like to avoid having a behavior depending on libraries installed.

Plus, consistency with Mail::Address gives us consistency with any other
program using Mail::Address.

>   3. Rewrite
>
>        A <B@C> D
>
>      into
>
>        A D <B@C>
>
>      regardless of what is in "D". This retains the information in the
>      rfc2822 cc.

There's another one I considered:

   4. Consider '#.*' as a comment when parsing trailers (but not in
      other places where we parse addresses)

This is much harder to define properly because of

Cc: "Foo # Bar" <email@example.com>

=> we'd need to handle the ".*" syntax when stripping comments. And
again, that wouldn't be consistent with Mail::Address.

So, I ended up implementing 3., which actually isn't hard, and gives
code IMHO cleaner than it used to be.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

[PATCH] parse_mailboxes: accept extra text after <...> address

[Matthieu Moy]

The test introduced in this commit succeeds without the patch to Git.pm
if Mail::Address is installed, but fails otherwise because our in-house
parser does not accept any text after the email address. They succeed
both with and without Mail::Address after this commit.

Mail::Address accepts extra text and considers it as part of the name,
iff the address is surrounded with <...>. The implementation mimics
this behavior as closely as possible.

This mostly restores the behavior we had before b1c8a11 (send-email:
allow multiple emails using --cc, --to and --bcc, 2015-06-30), but we
keep the possibility to handle comma-separated lists.

Reported-by: Larry Finger <Larry.Finger@lwfinger.net>
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
 perl/Git.pm           | 13 +++++++------
 t/t9001-send-email.sh | 29 +++++++++++++++++++++++++++++
 2 files changed, 36 insertions(+), 6 deletions(-)

diff --git a/perl/Git.pm b/perl/Git.pm
index ce7e4e8da394..ca769246216c 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -879,6 +879,7 @@ sub parse_mailboxes {
 	# divide the string in tokens of the above form
 	my $re_token = qr/(?:$re_quote|$re_word|$re_comment|\S)/;
 	my @tokens = map { $_ =~ /\s*($re_token)\s*/g } @_;
+	my $end_of_addr_seen = 0;
 
 	# add a delimiter to simplify treatment for the last mailbox
 	push @tokens, ",";
@@ -888,10 +889,10 @@ sub parse_mailboxes {
 		if ($token =~ /^[,;]$/) {
 			# if buffer still contains undeterminated strings
 			# append it at the end of @address or @phrase
-			if (@address) {
-				push @address, @buffer;
-			} else {
+			if ($end_of_addr_seen) {
 				push @phrase, @buffer;
+			} else {
+				push @address, @buffer;
 			}
 
 			my $str_phrase = join ' ', @phrase;
@@ -915,16 +916,16 @@ sub parse_mailboxes {
 			push @addr_list, $str_mailbox if ($str_mailbox);
 
 			@phrase = @address = @comment = @buffer = ();
+			$end_of_addr_seen = 0;
 		} elsif ($token =~ /^\(/) {
 			push @comment, $token;
 		} elsif ($token eq "<") {
 			push @phrase, (splice @address), (splice @buffer);
 		} elsif ($token eq ">") {
+			$end_of_addr_seen = 1;
 			push @address, (splice @buffer);
-		} elsif ($token eq "@") {
+		} elsif ($token eq "@" && !$end_of_addr_seen) {
 			push @address, (splice @buffer), "@";
-		} elsif ($token eq ".") {
-			push @address, (splice @buffer), ".";
 		} else {
 			push @buffer, $token;
 		}
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index b3355d2c7016..3dc4a3454d22 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -140,6 +140,35 @@ test_expect_success $PREREQ 'Verify commandline' '
 	test_cmp expected commandline1
 '
 
+test_expect_success $PREREQ 'setup expect for cc trailer' "
+cat >expected-cc <<\EOF
+!recipient@example.com!
+!author@example.com!
+!one@example.com!
+!two@example.com!
+!three@example.com!
+!four@example.com!
+!five@example.com!
+EOF
+"
+
+test_expect_success $PREREQ 'cc trailer with various syntax' '
+	test_commit cc-trailer &&
+	test_when_finished "git reset --hard HEAD^" &&
+	git commit --amend -F - <<-EOF &&
+	Test Cc: trailers.
+
+	Cc: one@example.com
+	Cc: <two@example.com> # this is part of the name
+	Cc: <three@example.com>, <four@example.com> # not.five@example.com
+	Cc: "Some # Body" <five@example.com> [part.of.name.too]
+	EOF
+	clean_fake_sendmail &&
+	git send-email -1 --to=recipient@example.com \
+		--smtp-server="$(pwd)/fake.sendmail" &&
+	test_cmp expected-cc commandline1
+'
+
 test_expect_success $PREREQ 'setup expect' "
 cat >expected-show-all-headers <<\EOF
 0001-Second.patch
-- 
2.10.0.rc0.1.g07c9292

Re: Formatting problem send_mail in version 2.10.0

[Kevin Daudt]

On Wed, Oct 12, 2016 at 07:13:22PM -0400, Jeff King wrote:
> On Wed, Oct 12, 2016 at 01:53:52PM -0700, Junio C Hamano wrote:
> 
> > Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
> > 
> > >>> If it's not in the body of the message, then where is it?
> > >>
> > >> This point is clarified in the thread
> > >> http://marc.info/?l=linux-wireless&m=147625930203434&w=2, which is
> > >> with my upstream maintainer.
> > >
> > > Which explicitly states that the syntax is not [$number], but # $number,
> > > right?
> > 
> > But I do not think that works, either.  Let's step back.
> > 
> > People write things like these
> > 
> >     Cc: Stable <stable@vger.kernel.org> # 4.8
> >     Cc: Stable <stable@vger.kernel.org> [4.8+]
> > 
> > in the trailer part in the body of the message.  Are these lines
> > meant to be usable if they appear as Cc: headers of an outgoing
> > piece of e-mail as-is?
> 
> I think the answer is pretty clearly no. It's just that historically we
> have auto-munged it into something useful. I think the viable options
> are basically:
> 
>   1. Tell people not to do that, and to do something RFC compliant like
>      "Stable [4.8+]" <stable@vger.kernel.org>. This is a little funny
>      for git because we otherwise do not require things like
>      rfc-compliant quoting for our name/email pairs. But it Just Works
>      without anybody having to write extra code, or worry about corner
>      cases in parsing.
> 
>   2. Drop everything after the trailing ">". This gives a valid rfc2822
>      cc, and people can pick the "# 4.8" from the cc line in the body.

Comments, surrounded by parenthesis are allowed after the ">" according
to the RFC, just plain dropping everything comming after that would
break that support.

Re: Formatting problem send_mail in version 2.10.0

[Matthieu Moy]

Kevin Daudt <me@ikke.info> writes:

> On Wed, Oct 12, 2016 at 07:13:22PM -0400, Jeff King wrote:
>
>> I think the answer is pretty clearly no. It's just that historically we
>> have auto-munged it into something useful. I think the viable options
>> are basically:
>> 
>>   1. Tell people not to do that, and to do something RFC compliant like
>>      "Stable [4.8+]" <stable@vger.kernel.org>. This is a little funny
>>      for git because we otherwise do not require things like
>>      rfc-compliant quoting for our name/email pairs. But it Just Works
>>      without anybody having to write extra code, or worry about corner
>>      cases in parsing.
>> 
>>   2. Drop everything after the trailing ">". This gives a valid rfc2822
>>      cc, and people can pick the "# 4.8" from the cc line in the body.
>
> Comments, surrounded by parenthesis are allowed after the ">" according
> to the RFC, just plain dropping everything comming after that would
> break that support.

Our in-house parser does consider (...) comments, and my patch does not
change how they are handled: they are still kept after the address part.

However, another piece of code does strip everything behind ">":

sub sanitize_address {
        ...
	# remove garbage after email address
	$recipient =~ s/(.*>).*$/$1/;

introduced in 831a488b76e0 (git-send-email: remove garbage after email
address, 2012-11-22).

IMHO, it's OK to continue doing this: removing comments from To: and Cc:
is not really a problem (and I think we've seen nobody complain about it
since 2012). But after my patch, these two lines can probably safely be
removed, as there can no longer be "garbage" after the email, only
comments.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: Formatting problem send_mail in version 2.10.0

[Matthieu Moy]

Junio C Hamano <gitster@pobox.com> writes:

> People write things like these
>
>     Cc: Stable <stable@vger.kernel.org> # 4.8
>     Cc: Stable <stable@vger.kernel.org> [4.8+]
>
> in the trailer part in the body of the message.  Are these lines
> meant to be usable if they appear as Cc: headers of an outgoing
> piece of e-mail as-is?

I think this is not the right question. The relevant one is: should
these lines be accepted by git and turned into something usable if they
appear as Cc: headers of an outgoing piece of e-mail?

I.e. "Be liberal in what you accept, and conservative in what you send".

If you have Mail::Address installed, this is already possible. With
pre-2.6, I did not re-test, but I think it was using the addresses
as-is, which probably worked but AFAICT created non-RCF-compliant
emails.

> "error: unable to extract a valid address from:" is followed by
>
>     Stable <stable@vger.kernel.org#4.8>
>     Stable <stable@vger.kernel.org[4.8+]>
>
> which is not ideal.

I'd actually even say "broken" ;-). If we decide to reject these, we
should at least give a sensible error message.

> If I were to issue a decree, I would say that people should stop
> suggesting to put RFC-bogus things on the Cc: line.  As you
> mentioned, things like:
>
>     Cc: Stable (4.8+) <stable@vger.kernel.org>
>     Cc: "Stable 4.8+" <stable@vger.kernel.org>
>
> are perfectly readable alternative that are still RFC-kosher.

I do support those, but if there's an established tradition of using
# ... trailer, then I don't think we should be the ones forcing it to
stop.

> Things may have appeared to work by accident, and things may still
> work by accident, depending on the vintage and availability of
> Mail::Address package (which seems to be the case), but it is not
> worth risking random breakages that depends on what other people
> use in the first place, no?

The "depending on the availability of Mail::Address" is what bothers me
most. Suppose we make a strong statement here that this # 4.8 should
stop. Then some users will listen to that statements, but others won't
read the thread, test with their own git that it works, and recommend it
to users for whom it doesn't.

> That is, even though people seem to expect "send-email --cc" to
> somehow be able to judge that " # 4.8" and " [4.8+]" are cruft not
> meant as part of a valid address, I do not think it is a realistic
> expectation.  How would it know "Cc: Stable <add@re.ss> 4.8, 4.9"
> has garbage " 4.8, 4.9" that needs to be stripped, while "Cc: Stable
> <add@re.ss> 4.8, torvalds@linux-foundation.org" has two valid
> addresses that need to be CC'ed and " 4.8" is the only thing that is
> unwanted?

We clearly can't guess, but we can be consistent with Mail::Address, so
that git's behavior depends less on its availability.

Patch follows doing that.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: Formatting problem send_mail in version 2.10.0

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

> We clearly can't guess, but we can be consistent with Mail::Address, so
> that git's behavior depends less on its availability.
>
> Patch follows doing that.

Thanks.  I love that somebody counters with a much better way to
solve it whenever I suggest an outrageous non-solution to a problem
around here ;-)

Re: Formatting problem send_mail in version 2.10.0

[Jeff King]

On Tue, Oct 11, 2016 at 09:39:58AM +0200, Matthieu Moy wrote:

> >> I can't reproduce the problem with this simple setup:
> >> 
> >> 	git init
> >> 	echo content >file && git add file
> >> 	git commit -F- <<-\EOF
> >> 	the subject
> >> 
> >> 	the body
> >> 
> >> 	Cc: Stable <stable@vger.kernel.org> [4.8+]
> 
> Is this RFC2822 compliant (https://tools.ietf.org/html/rfc2822)? Not an
> expert of the norm, but my understanding is that you're allowed to use
> either "Name <addr@domain.com>" (name-addr) or addr@domain.com
> (addr-spec), and that comments are allowed within parenthesis like
> "Stable <stable@vger.kernel.org> (4.8+)".

I'm not sure. I don't recall seeing anything like it in the wild before,
but I find it interesting that we behave differently than Mail::Address
(which I generally assume to adhere to a mix of spec and common
practice). I couldn't find anything relevant in rfc2822.

> What is this [4.8+] supposed to mean?
> 
> The guilty function is parse_mailboxes in perl/Git.pm. It should be
> rather easy to modify it but I need to understand the spec before I can
> try to implement anything.

It seems to be syntactically invalid as an rfc2822 address. If it's in
common use on trailer lines[1], I think the only sane things are to drop
it, or to stick it in the name. Between the two, I'd argue for the
latter, as that matches what Git historically has done.

I also found it interesting that:

  Cc: Stable [4.8+] <stable@vger.kernel.org>

ends up as:

  Cc: "Stable [ v4 . 8+ ]" <stable@vger.kernel.org>

I think this is also syntactically invalid as an rfc2822 address, but we
have a habit in Git of treating single-line "name <email>" in a pretty
lax manner, and just letting any character except "<" exist in the name
field. I wonder if we should do something similar here.

-Peff

[1] Running `git log | grep -i '^ *cc:' | grep '\['` on linux.git
    shows that it is a common pattern there, though there are other uses
    of brackets that probably would not want to include their contents
    in the name.

    It also looks like:

      Cc: stable@vger.kernel.org # 4.8+

    is a common pattern.

    So I suspect we really are in the realm of micro-formats here, and
    it is less about what rfc2822 says, and what people would find it
    useful for send-email to do with these bits after the address (and
    possibly what other people's scripts do with them).