threads / patch / 30163

patchsend-email: add 'specify-author' option

Subject: [PATCH RESEND] send-email: add 'specify-author' option

## tl;dr

14 messages between Apr 6, 2012 and Apr 12, 2012. Diffs are folded; open one to read it.

replies: 13people: 4as markdown or json

Felipe Contreras· Apr 6, 2012, 12:21 UTC · lore

Some mail servers (Microsoft Exchange) mangle the 'From' header, so while applying the patches, the author has to be fixed manually.

This option allows to always specify the author of the commit in the body of the message, even if the committer is the author.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Documentation/git-send-email.txt |    4 ++++
 git-send-email.perl              |    9 +++++++--
 t/t9001-send-email.sh            |   12 ++++++++++++
 3 files changed, 23 insertions(+), 2 deletions(-)
Show changes to 3 files +23 −2

Documentation/git-send-email.txt, git-send-email.perl, t/t9001-send-email.sh

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 3241170..9bd1379 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -126,6 +126,10 @@ The --to option must be repeated for each user you want on the to list.
 +
 Note that no attempts whatsoever are made to validate the encoding.
 
+--specify-author::
+	Always specify the author of the commit in the body of the message,
+	even if the committer is the author. This is useful if the 'From'
+	header is mangled by some mail server.
 
 Sending
 ~~~~~~~
diff --git a/git-send-email.perl b/git-send-email.perl
index ef30c55..f18ad16 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -57,6 +57,8 @@ git send-email [options] <file | directory | rev-list options >
     --annotate                     * Review each patch that will be sent in an editor.
     --compose                      * Open an editor for introduction.
     --8bit-encoding         <str>  * Encoding to assume 8bit mails if undeclared
+    --specify-author               * Always specify the author of the commit in
+			             the body of the message.
 
   Sending:
     --envelope-sender       <str>  * Email envelope sender.
@@ -198,6 +200,7 @@ my ($identity, $aliasfiletype, @alias_files, $smtp_domain);
 my ($validate, $confirm);
 my (@suppress_cc);
 my ($auto_8bit_encoding);
+my ($specify_author);
 
 my ($debug_net_smtp) = 0;		# Net::SMTP, see send_message()
 
@@ -210,7 +213,8 @@ my %config_bool_settings = (
     "signedoffbycc" => [\$signed_off_by_cc, undef],
     "signedoffcc" => [\$signed_off_by_cc, undef],      # Deprecated
     "validate" => [\$validate, 1],
-    "multiedit" => [\$multiedit, undef]
+    "multiedit" => [\$multiedit, undef],
+    "specifyauthor" => [\$specify_author, undef],
 );
 
 my %config_settings = (
@@ -316,6 +320,7 @@ my $rc = GetOptions("h" => \$help,
 		    "format-patch!" => \$format_patch,
 		    "8bit-encoding=s" => \$auto_8bit_encoding,
 		    "force" => \$force,
+		    "specify-author!" => \$specify_author,
 	 );
 
 usage() if $help;
@@ -1310,7 +1315,7 @@ foreach my $t (@files) {
 		$subject = quote_rfc2047($subject, $auto_8bit_encoding);
 	}
 
-	if (defined $author and $author ne $sender) {
+	if (defined $author and ($author ne $sender or $specify_author)) {
 		$message = "From: $author\n\n$message";
 		if (defined $author_encoding) {
 			if ($has_content_type) {
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 8c12c65..1dab17e 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -281,6 +281,18 @@ test_expect_success $PREREQ 'Author From: not in message body' '
 	! grep "From: A <author@example.com>" msgbody1
 '
 
+test_expect_success $PREREQ 'Author From: in message body (forced)' '
+	clean_fake_sendmail &&
+	git send-email \
+		--from="A <author@example.com>" \
+		--to=nobody@example.com \
+		--smtp-server="$(pwd)/fake.sendmail" \
+		--specify-author \
+		$patches &&
+	sed "1,/^\$/d" < msgtxt1 > msgbody1 &&
+	grep "From: A <author@example.com>" msgbody1
+'
+
 test_expect_success $PREREQ 'allow long lines with --no-validate' '
 	git send-email \
 		--from="Example <nobody@example.com>" \
-- 
1.7.9.6
Felipe Contreras· Apr 6, 2012, 12:32 UTC · re: Felipe Contreras · lore

Re: [PATCH RESEND] send-email: add 'specify-author' option

On Fri, Apr 6, 2012 at 3:21 PM, Felipe Contreras <felipe.contreras@gmail.com> wrote:

Show 7 quoted lines
> Some mail servers (Microsoft Exchange) mangle the 'From' header, so
> while applying the patches, the author has to be fixed manually.
>
> This option allows to always specify the author of the commit in the
> body of the message, even if the committer is the author.
>
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>

Some recipients don't exist any more; you might want to reply to this message instead.

Cheers.
-- 
Felipe Contreras
Junio C Hamano· Apr 6, 2012, 17:14 UTC · re: Felipe Contreras · lore

Re: [PATCH RESEND] send-email: add 'specify-author' option

Felipe Contreras <felipe.contreras@gmail.com> writes:
Show 8 quoted lines
> Some mail servers (Microsoft Exchange) mangle the 'From' header, so
> while applying the patches, the author has to be fixed manually.
>
> This option allows to always specify the author of the commit in the
> body of the message, even if the committer is the author.
>
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---

I wouldn't say that the existing option names to send-email are great, but I have to say that the one added by this is simply horrible ;-)

The first paragraph of the proposed commit log message states the problem it tries to address very clearly, which is good, but is "From: " the only thing that needs this? I am wondering if this should be named and behave more like "--duplicate-header" or "--in-body-header".

Felipe Contreras· Apr 6, 2012, 19:42 UTC · re: Junio C Hamano · lore

Re: [PATCH RESEND] send-email: add 'specify-author' option

On Fri, Apr 6, 2012 at 8:14 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 18 quoted lines
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> Some mail servers (Microsoft Exchange) mangle the 'From' header, so
>> while applying the patches, the author has to be fixed manually.
>>
>> This option allows to always specify the author of the commit in the
>> body of the message, even if the committer is the author.
>>
>> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
>> ---
>
> I wouldn't say that the existing option names to send-email are great, but
> I have to say that the one added by this is simply horrible ;-)
>
> The first paragraph of the proposed commit log message states the problem
> it tries to address very clearly, which is good, but is "From: " the only
> thing that needs this?  I am wondering if this should be named and behave
> more like "--duplicate-header" or "--in-body-header".

I have never seen any other 'in-body-header' other than From, and I don't see how that would be useful. Anybody else?

-- 
Felipe Contreras
Junio C Hamano· Apr 6, 2012, 20:22 UTC · re: Felipe Contreras · lore

Re: [PATCH RESEND] send-email: add 'specify-author' option

Felipe Contreras <felipe.contreras@gmail.com> writes:
Show 16 quoted lines
> On Fri, Apr 6, 2012 at 8:14 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Felipe Contreras <felipe.contreras@gmail.com> writes:
>>
>>> Some mail servers (Microsoft Exchange) mangle the 'From' header, so
>>> while applying the patches, the author has to be fixed manually.
>> ...
>> I wouldn't say that the existing option names to send-email are great, but
>> I have to say that the one added by this is simply horrible ;-)
>>
>> The first paragraph of the proposed commit log message states the problem
>> it tries to address very clearly, which is good, but is "From: " the only
>> thing that needs this?  I am wondering if this should be named and behave
>> more like "--duplicate-header" or "--in-body-header".
>
> I have never seen any other 'in-body-header' other than From, and I
> don't see how that would be useful. Anybody else?

The "Subject:" is very often used in the wild, when responding to an existing discussion thread with a patch, without changing the topic of the thread (I would say it is used more than "From: " override).

When using send-email to start a thread anew, this use case is much less of a problem, but I wouldn't be surprised if a broken MSA/MTA mangled the subject (especially imagine a non-ASCII ones) incorrectly which would be helped with exactly the same in-body-header mechanism.

You probably meant "I do not want to hear from Junio" by your last half-sentence, but I replied anyway ;-).

Felipe Contreras· Apr 6, 2012, 21:48 UTC · re: Junio C Hamano · lore

Re: [PATCH RESEND] send-email: add 'specify-author' option

On Fri, Apr 6, 2012 at 11:22 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 22 quoted lines
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> On Fri, Apr 6, 2012 at 8:14 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>> Felipe Contreras <felipe.contreras@gmail.com> writes:
>>>
>>>> Some mail servers (Microsoft Exchange) mangle the 'From' header, so
>>>> while applying the patches, the author has to be fixed manually.
>>> ...
>>> I wouldn't say that the existing option names to send-email are great, but
>>> I have to say that the one added by this is simply horrible ;-)
>>>
>>> The first paragraph of the proposed commit log message states the problem
>>> it tries to address very clearly, which is good, but is "From: " the only
>>> thing that needs this?  I am wondering if this should be named and behave
>>> more like "--duplicate-header" or "--in-body-header".
>>
>> I have never seen any other 'in-body-header' other than From, and I
>> don't see how that would be useful. Anybody else?
>
> The "Subject:" is very often used in the wild, when responding to an
> existing discussion thread with a patch, without changing the topic of the
> thread (I would say it is used more than "From: " override).
Hmm, but that is different, isn't it?
AFAIK people use this format:

--- $headers

Message

Patch (format-patch output: headers, commit message, diff) ---

In this case 'git am' would ignore the patch headers. The only way 'git am' would override $headers, is if the first part of the body has new headers:

--- $headers

$patch_headers
$patch_commit_message

$patch_diff ---

IOW; if there's no message at the beginning of the body.
> When using send-email to start a thread anew, this use case is much less
> of a problem, but I wouldn't be surprised if a broken MSA/MTA mangled the
> subject (especially imagine a non-ASCII ones) incorrectly which would be
> helped with exactly the same in-body-header mechanism.

I can't foresee that, but I guess we can do it anyway. So which would be the fields to repeat? From, Date, and Subject?

> You probably meant "I do not want to hear from Junio" by your last
> half-sentence, but I replied anyway ;-).
I meant I wanted to know which fields would be useful to put in the body.
Cheers.
-- 
Felipe Contreras
Junio C Hamano· Apr 6, 2012, 22:30 UTC · re: Felipe Contreras · lore

Re: [PATCH RESEND] send-email: add 'specify-author' option

Felipe Contreras <felipe.contreras@gmail.com> writes:
Show 31 quoted lines
>> The "Subject:" is very often used in the wild, when responding to an
>> existing discussion thread with a patch, without changing the topic of the
>> thread (I would say it is used more than "From: " override).
>
> Hmm, but that is different, isn't it?
>
> AFAIK people use this format:
>
> ---
> $headers
>
> Message
>
> Patch (format-patch output: headers, commit message, diff)
> ---
>
> In this case 'git am' would ignore the patch headers. The only way
> 'git am' would override $headers, is if the first part of the body has
> new headers:
>
> ---
> $headers
>
> $patch_headers
>
> $patch_commit_message
>
> $patch_diff
> ---
>
> IOW; if there's no message at the beginning of the body.

Your notation is a bit unclear to me, but I take that $headers mean the e-mail headers, and $patch_headers mean what we often call "in-body" headers; in other words, your patch is "duplicate my authorship in $patch_headers because my MSA/MTA mangles my name in $headers." Am I following you well so far?

What I meant to say was that perhaps the approach can help the same class of issues where other fields in $headers can be corrupted and the user wants duplicate in "in-body", assuming that it is less likely to be eaten, and non-ASCII subject was one example that immediately came to my mind.

So in that sense, it is not much a different issue.
> I can't foresee that, but I guess we can do it anyway. So which would
> be the fields to repeat? From, Date, and Subject?
I would say From and Subject are equally worth considering.
Felipe Contreras· Apr 7, 2012, 11:46 UTC · re: Junio C Hamano · lore

Re: [PATCH RESEND] send-email: add 'specify-author' option

On Sat, Apr 7, 2012 at 1:30 AM, Junio C Hamano <gitster@pobox.com> wrote:
Show 39 quoted lines
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>>> The "Subject:" is very often used in the wild, when responding to an
>>> existing discussion thread with a patch, without changing the topic of the
>>> thread (I would say it is used more than "From: " override).
>>
>> Hmm, but that is different, isn't it?
>>
>> AFAIK people use this format:
>>
>> ---
>> $headers
>>
>> Message
>>
>> Patch (format-patch output: headers, commit message, diff)
>> ---
>>
>> In this case 'git am' would ignore the patch headers. The only way
>> 'git am' would override $headers, is if the first part of the body has
>> new headers:
>>
>> ---
>> $headers
>>
>> $patch_headers
>>
>> $patch_commit_message
>>
>> $patch_diff
>> ---
>>
>> IOW; if there's no message at the beginning of the body.
>
> Your notation is a bit unclear to me, but I take that $headers mean the
> e-mail headers, and $patch_headers mean what we often call "in-body"
> headers; in other words, your patch is "duplicate my authorship in
> $patch_headers because my MSA/MTA mangles my name in $headers."  Am I
> following you well so far?

Yes, but 'git send-email' already does that; when the author and sender are not the same.

Show 6 quoted lines
> What I meant to say was that perhaps the approach can help the same class
> of issues where other fields in $headers can be corrupted and the user
> wants duplicate in "in-body", assuming that it is less likely to be eaten,
> and non-ASCII subject was one example that immediately came to my mind.
>
> So in that sense, it is not much a different issue.
No, but it requires new code, whereas my patch only exercises existing code.
>> I can't foresee that, but I guess we can do it anyway. So which would
>> be the fields to repeat? From, Date, and Subject?
>
> I would say From and Subject are equally worth considering.
So only From and Subject? Not Date?
Cheers.
-- 
Felipe Contreras
Junio C Hamano· Apr 8, 2012, 04:45 UTC · re: Felipe Contreras · lore

Re: [PATCH RESEND] send-email: add 'specify-author' option

Felipe Contreras <felipe.contreras@gmail.com> writes:
> So only From and Subject? Not Date?

I do not foresee MSA/MTA breaking a Date field; RFC2822 dates are ASCII-only no?

Michael Witten· Apr 12, 2012, 15:58 UTC · re: Junio C Hamano · lore

Re: [PATCH RESEND] send-email: add 'specify-author' option

On Fri, 06 Apr 2012 15:30:32 -0700, Junio C Hamano wrote:
Show 6 quoted lines
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> I can't foresee that, but I guess we can do it anyway. So which would
>> be the fields to repeat? From, Date, and Subject?
> 
> I would say From and Subject are equally worth considering.
Famous last words: "I can't foresee that."
My impression from Junio's initial comment:
>>> The first paragraph of the proposed commit log message states the problem
>>> it tries to address very clearly, which is good, but is "From: " the only
>>> thing that needs this?  I am wondering if this should be named and behave
>>> more like "--duplicate-header" or "--in-body-header".

is that a more general solution should be sought, particularly because we indeed can't foresee people's needs. I envisioned `--in-body-header' taking as an argument a colon-separated list of headers that should be inlined; let the user mix and match as he pleases, and save us trouble in the future when new headers pop up (I can't forsee that happening, but, well, who knows?).

Naturally, a configuration variable could be useful.

Sincerely, Michael Witten

Michael Witten· Apr 12, 2012, 15:36 UTC · re: Felipe Contreras · lore

Re: [PATCH RESEND] send-email: add 'specify-author' option

On Fri, 6 Apr 2012 22:42:16 +0300, Felipe Contreras wrote:
Show 22 quoted lines
> On Fri, Apr 6, 2012 at 8:14 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Felipe Contreras <felipe.contreras@gmail.com> writes:
>>
>>> Some mail servers (Microsoft Exchange) mangle the 'From' header, so
>>> while applying the patches, the author has to be fixed manually.
>>>
>>> This option allows to always specify the author of the commit in the
>>> body of the message, even if the committer is the author.
>>>
>>> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
>>> ---
>>
>> I wouldn't say that the existing option names to send-email are great, but
>> I have to say that the one added by this is simply horrible ;-)
>>
>> The first paragraph of the proposed commit log message states the problem
>> it tries to address very clearly, which is good, but is "From: " the only
>> thing that needs this? I am wondering if this should be named and behave
>> more like "--duplicate-header" or "--in-body-header".
>
> I have never seen any other 'in-body-header' other than From, and I
> don't see how that would be useful. Anybody else?

I sometimes like to set an explicit in-body `Date' header. However, Junio has disagreed vocally about this practice:

  Subject: Dates in Commits and other issues of style
           (Re: [RFC 2/5] Pretty Print: show tz when using DATE_LOCAL)
  Message-ID: <811b01a9-f10e-4444-9e5e-581adaf059c2-mfwitten@gmail.com>
  http://article.gmane.org/gmane.comp.version-control.git/171936

Sincerely, Michael Witten

Junio C Hamano· Apr 12, 2012, 17:40 UTC · re: Michael Witten · lore

Re: [PATCH RESEND] send-email: add 'specify-author' option

Michael Witten <mfwitten@gmail.com> writes:
Show 7 quoted lines
> I sometimes like to set an explicit in-body `Date' header. However, Junio
> has disagreed vocally about this practice:
>
>   Subject: Dates in Commits and other issues of style
>            (Re: [RFC 2/5] Pretty Print: show tz when using DATE_LOCAL)
>   Message-ID: <811b01a9-f10e-4444-9e5e-581adaf059c2-mfwitten@gmail.com>
>   http://article.gmane.org/gmane.comp.version-control.git/171936

I do not want to see that used for patches meant for me to pick up from mbox; the reason is explained in the message the cited one is in response to.

It is an entirely different matter if such a "feature" should or should not exist in the tool. I do not think I would use it myself, and as I said, I do not want anybody to use it when sending patches to this list, but that does not mean Git should not support it.

Thomas Rast· Apr 6, 2012, 19:47 UTC · re: Felipe Contreras · lore

Re: [PATCH RESEND] send-email: add 'specify-author' option

Felipe Contreras <felipe.contreras@gmail.com> writes:
Show 5 quoted lines
> Some mail servers (Microsoft Exchange) mangle the 'From' header, so
> while applying the patches, the author has to be fixed manually.
>
> This option allows to always specify the author of the commit in the
> body of the message, even if the committer is the author.

I imagine a user forced to send her email over such a broken server would have this problem all the time. Wouldn't a config option be in order?

-- 
Thomas Rast
trast@{inf,student}.ethz.ch
Felipe Contreras· Apr 6, 2012, 21:30 UTC · re: Thomas Rast · lore

Re: [PATCH RESEND] send-email: add 'specify-author' option

On Fri, Apr 6, 2012 at 10:47 PM, Thomas Rast <trast@inf.ethz.ch> wrote:
Show 11 quoted lines
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> Some mail servers (Microsoft Exchange) mangle the 'From' header, so
>> while applying the patches, the author has to be fixed manually.
>>
>> This option allows to always specify the author of the commit in the
>> body of the message, even if the committer is the author.
>
> I imagine a user forced to send her email over such a broken server
> would have this problem all the time.  Wouldn't a config option be in
> order?
There is a config option, but I guess I forgot to add the documentation.
Cheers.
-- 
Felipe Contreras

← back to recent threads