# Re: [PATCH] include/asm-arm/: Spelling fixes 9 messages from 2007-12-17 to 2008-01-08. Participants: J. Bruce Fields, Joe Perches, Jeff King, Junio C Hamano, Josh Triplett. Thread: https://gitlist.dev/t/11328 ## J. Bruce Fields, 2007-12-17 20:12 Subject: Re: [PATCH] include/asm-arm/: Spelling fixes Message-ID: <20071217201219.GC13515@fieldses.org> URL: https://gitlist.dev/e/20071217201219.GC13515%40fieldses.org In-Reply-To: <1197921847.27386.16.camel@localhost> ``` Hope you don't mind my cc'ing the git list: On Mon, Dec 17, 2007 at 12:04:07PM -0800, Joe Perches wrote: > On Mon, 2007-12-17 at 14:56 -0500, J. Bruce Fields wrote: > > My mail client seems to be flagging all your messages as duplicates of each > > other. > > > > Hm. It may be that your headers have two Message-Id's... > > Message-Id: <1197919875-5288-3-git-send-email-joe@perches.com> > > X-Mailer: git-send-email 1.5.3.7.949.g2221a6 > > Message-Id: > > <5703e57f925f31fc0eb38873bd7f10fc44f99cb4.1197918889.git.joe@perches.com > > > > > > > ... the second of which is pretty weird (and is the same for every message. Is > > that a git-send-email bug or something else? > > Dunno. I did a git-format-patch, then git-send-email. Weird. Anyone know how that could happen? --b. ``` ## Joe Perches, 2007-12-17 20:22 Subject: Re: [PATCH] include/asm-arm/: Spelling fixes Message-ID: <1197922971.27386.32.camel@localhost> URL: https://gitlist.dev/e/1197922971.27386.32.camel%40localhost In-Reply-To: <20071217201219.GC13515@fieldses.org> ``` On Mon, 2007-12-17 at 15:12 -0500, J. Bruce Fields wrote: > Hope you don't mind my cc'ing the git list: Nope. Not a bit. I had patches that were committed in word order that I wanted to aggregate by subsystem. I took all the patches, created a branch, committed all the patches at once, then used these commands: (patch_order is a directory list) $ let count=0 ; for line in $(cat patch_order) ; do ((count++)); \ printf -v tmp "%04u-Spelling-%s" $count $line ; \ tmp=${tmp//\//-} ; tmp=${tmp// /}; \ git-format-patch -N --thread --start-number $count -s \ -o patch3 \ --subject-prefix="[PATCH] Spelling: $line" master $line ; done [converted the subjects appropriately] $ git-send-email --smtp-server --smtp-user \ --from "Joe Perches " \ --to "linux-kernel@vger.kernel.org" \ --cc "Andrew Morton " \ --cc-cmd "../get_maintainer.pl --nogit" \ --no-thread --suppress-from patch3 I probably just reversed the "--thread" on git-format-patch and send-email. cheers, Joe ``` ## Jeff King, 2007-12-17 20:51 Subject: Re: [PATCH] include/asm-arm/: Spelling fixes Message-ID: <20071217205134.GB2105@coredump.intra.peff.net> URL: https://gitlist.dev/e/20071217205134.GB2105%40coredump.intra.peff.net In-Reply-To: <1197922971.27386.32.camel@localhost> ``` On Mon, Dec 17, 2007 at 12:22:51PM -0800, Joe Perches wrote: > $ let count=0 ; for line in $(cat patch_order) ; do ((count++)); \ > printf -v tmp "%04u-Spelling-%s" $count $line ; \ > tmp=${tmp//\//-} ; tmp=${tmp// /}; \ > git-format-patch -N --thread --start-number $count -s \ > -o patch3 \ > --subject-prefix="[PATCH] Spelling: $line" master $line ; done > [converted the subjects appropriately] > $ git-send-email --smtp-server --smtp-user \ > --from "Joe Perches " \ > --to "linux-kernel@vger.kernel.org" \ > --cc "Andrew Morton " \ > --cc-cmd "../get_maintainer.pl --nogit" \ > --no-thread --suppress-from patch3 Ah. The problem is that git-send-email unconditionally adds a message-id. Usually git-format-patch doesn't add one, but for obvious reasons, it must when doing --thread. Here is a fix. -- >8 -- git-send-email: avoid duplicate message-ids We used to unconditionally add a message-id to the outgoing email without bothering to check if it already had one. Instead, let's use the existing one. Signed-off-by: Jeff King --- It will also happily add duplicate --in-reply-to and --references headers, but those can be suppressed with --no-thread. Arguably, it should detect this case and turn on --no-thread, but looking at git-send-email makes me want to claw my eyes out. git-send-email.perl | 5 ++++- 1 files changed, 4 insertions(+), 1 deletions(-) diff --git a/git-send-email.perl b/git-send-email.perl index 1d6f466..083466a 100755 --- a/git-send-email.perl +++ b/git-send-email.perl @@ -580,7 +580,7 @@ sub send_message $ccline = "\nCc: $cc"; } my $sanitized_sender = sanitize_address($sender); - make_message_id(); + make_message_id() unless defined($message_id); my $header = "From: $sanitized_sender To: $to${ccline} @@ -718,6 +718,9 @@ foreach my $t (@files) { } push @xh, $_; } + elsif (/^Message-Id: (.*)/i) { + $message_id = $1; + } elsif (!/^Date:\s/ && /^[-A-Za-z]+:\s+\S/) { push @xh, $_; } -- 1.5.4.rc0.1146.gc97f-dirty ``` ## Junio C Hamano, 2007-12-17 21:27 Subject: Re: [PATCH] include/asm-arm/: Spelling fixes Message-ID: <7vy7bt57wn.fsf@gitster.siamese.dyndns.org> URL: https://gitlist.dev/e/7vy7bt57wn.fsf%40gitster.siamese.dyndns.org In-Reply-To: <20071217205134.GB2105@coredump.intra.peff.net> ``` Jeff King writes: > Ah. The problem is that git-send-email unconditionally adds a > message-id. Usually git-format-patch doesn't add one, but for obvious > reasons, it must when doing --thread. Here is a fix. > diff --git a/git-send-email.perl b/git-send-email.perl > index 1d6f466..083466a 100755 > --- a/git-send-email.perl > +++ b/git-send-email.perl > @@ -580,7 +580,7 @@ sub send_message > $ccline = "\nCc: $cc"; > } > my $sanitized_sender = sanitize_address($sender); > - make_message_id(); > + make_message_id() unless defined($message_id); Isn't this called inside a loop? If the outgoing message does not originally have "Message-Id:", does the loop correctly reinitialize $message_id to undef, or does this change make everybody reuse the same $message_id over and over again? I have a feeling that --thread to format-patch is a misfeature. Why is it needed if you are feeding the output to send-email? I wonder if stripping existing "Message-Id:" away just like we strip away "Date:" from @xh would be a much saner fix. ``` ## Jeff King, 2007-12-17 23:05 Subject: Re: [PATCH] include/asm-arm/: Spelling fixes Message-ID: <20071217230558.GD2105@coredump.intra.peff.net> URL: https://gitlist.dev/e/20071217230558.GD2105%40coredump.intra.peff.net In-Reply-To: <7vy7bt57wn.fsf@gitster.siamese.dyndns.org> ``` On Mon, Dec 17, 2007 at 01:27:20PM -0800, Junio C Hamano wrote: > > my $sanitized_sender = sanitize_address($sender); > > - make_message_id(); > > + make_message_id() unless defined($message_id); > > Isn't this called inside a loop? If the outgoing message does not > originally have "Message-Id:", does the loop correctly reinitialize > $message_id to undef, or does this change make everybody reuse the same > $message_id over and over again? Yes, sorry. I realized it right after I sent the other out, but then a repairman showed up to fix my non-working furnace. :) The following needs to be squashed in (alternatively, the message_id doesn't need to be a loop variable, so it could be cleaned up. But part of me says that git-send-email is beyond hope for being clean). diff --git a/git-send-email.perl b/git-send-email.perl index 083466a..248d035 100755 --- a/git-send-email.perl +++ b/git-send-email.perl @@ -808,6 +808,7 @@ foreach my $t (@files) { $references = "$message_id"; } } + $message_id = undef; } if ($compose) { > I have a feeling that --thread to format-patch is a misfeature. Why is > it needed if you are feeding the output to send-email? I think it is a case of --thread being added for people not using send-email, and then getting it misused. I am just trying to add a sanity check to send-email in case the user does something silly (though one could certainly argue that it is already hopelessly tied to git-format-patch, and fixing git-format-patch is the right way to go). > I wonder if stripping existing "Message-Id:" away just like we strip > away "Date:" from @xh would be a much saner fix. That is definitely wrong if we expect to re-use the in-reply-to and references headers that already exist (though obviously we could strip out all three of those headers and re-add our own). I don't have a strong opinion. I never use git-send-email myself, but was just trying to fix a reported bug. -Peff ``` ## Jeff King, 2007-12-17 23:12 Subject: Re: [PATCH] include/asm-arm/: Spelling fixes Message-ID: <20071217231209.GA5558@coredump.intra.peff.net> URL: https://gitlist.dev/e/20071217231209.GA5558%40coredump.intra.peff.net In-Reply-To: <20071217230558.GD2105@coredump.intra.peff.net> ``` On Mon, Dec 17, 2007 at 06:05:58PM -0500, Jeff King wrote: > > I wonder if stripping existing "Message-Id:" away just like we strip > > away "Date:" from @xh would be a much saner fix. > > That is definitely wrong if we expect to re-use the in-reply-to and > references headers that already exist (though obviously we could strip > out all three of those headers and re-add our own). > > I don't have a strong opinion. I never use git-send-email myself, but > was just trying to fix a reported bug. Actually, I don't think stripping the message-id is ever the right thing. The user put it in there for some purpose, and "ours not to reason why" (ours but to do and die). IOW, it is not possible for us to know what we are breaking by changing the message-id. It could simply be the reply-to header in the following messages, or it could be the reply-to in some message we have not and will not ever see. Even if we assume git-send-email only ever gets the unmunged output of git-format-patch, we do not necessarily know it has been fed all of the patches during a single run. -Peff ``` ## Junio C Hamano, 2007-12-17 23:28 Subject: Re: [PATCH] include/asm-arm/: Spelling fixes Message-ID: <7v7ijc6gvf.fsf@gitster.siamese.dyndns.org> URL: https://gitlist.dev/e/7v7ijc6gvf.fsf%40gitster.siamese.dyndns.org In-Reply-To: <20071217230558.GD2105@coredump.intra.peff.net> ``` Jeff King writes: >> I wonder if stripping existing "Message-Id:" away just like we strip >> away "Date:" from @xh would be a much saner fix. > > That is definitely wrong if we expect to re-use the in-reply-to and > references headers that already exist (though obviously we could strip > out all three of those headers and re-add our own). Ah, you are right. And undef $message_id you squashed in definitely belongs there -- "prepare for the next round" section. ``` ## Josh Triplett, 2008-01-08 11:15 Subject: Re: [PATCH] include/asm-arm/: Spelling fixes Message-ID: <47835B5F.3030204@freedesktop.org> URL: https://gitlist.dev/e/47835B5F.3030204%40freedesktop.org In-Reply-To: <7vy7bt57wn.fsf@gitster.siamese.dyndns.org> ``` Junio C Hamano wrote: > Jeff King writes: >> Ah. The problem is that git-send-email unconditionally adds a >> message-id. Usually git-format-patch doesn't add one, but for obvious >> reasons, it must when doing --thread. Here is a fix. > >> diff --git a/git-send-email.perl b/git-send-email.perl >> index 1d6f466..083466a 100755 >> --- a/git-send-email.perl >> +++ b/git-send-email.perl >> @@ -580,7 +580,7 @@ sub send_message >> $ccline = "\nCc: $cc"; >> } >> my $sanitized_sender = sanitize_address($sender); >> - make_message_id(); >> + make_message_id() unless defined($message_id); > > Isn't this called inside a loop? If the outgoing message does not > originally have "Message-Id:", does the loop correctly reinitialize > $message_id to undef, or does this change make everybody reuse the same > $message_id over and over again? > > I have a feeling that --thread to format-patch is a misfeature. Why is > it needed if you are feeding the output to send-email? I added that option; see (d1566f7883f727f38bf442af3fdb69d36e6fcea2, cc35de8470541e389b7d2bdda4c901574720fa81, and da56645dd7c1175fc2ed1628ac35fdd35e705641). I use git-imap-send, not git-send-email, and I wanted to thread my patches. - Josh Triplett ``` ## Josh Triplett, 2008-01-08 11:16 Subject: Re: [PATCH] include/asm-arm/: Spelling fixes Message-ID: <47835BAA.3090100@freedesktop.org> URL: https://gitlist.dev/e/47835BAA.3090100%40freedesktop.org In-Reply-To: <7vy7bt57wn.fsf@gitster.siamese.dyndns.org> ``` Junio C Hamano wrote: > Jeff King writes: >> Ah. The problem is that git-send-email unconditionally adds a >> message-id. Usually git-format-patch doesn't add one, but for obvious >> reasons, it must when doing --thread. Here is a fix. > >> diff --git a/git-send-email.perl b/git-send-email.perl >> index 1d6f466..083466a 100755 >> --- a/git-send-email.perl >> +++ b/git-send-email.perl >> @@ -580,7 +580,7 @@ sub send_message >> $ccline = "\nCc: $cc"; >> } >> my $sanitized_sender = sanitize_address($sender); >> - make_message_id(); >> + make_message_id() unless defined($message_id); > > Isn't this called inside a loop? If the outgoing message does not > originally have "Message-Id:", does the loop correctly reinitialize > $message_id to undef, or does this change make everybody reuse the same > $message_id over and over again? > > I have a feeling that --thread to format-patch is a misfeature. Why is > it needed if you are feeding the output to send-email? I added that option; see (d1566f7883f727f38bf442af3fdb69d36e6fcea2, cc35de8470541e389b7d2bdda4c901574720fa81, and da56645dd7c1175fc2ed1628ac35fdd35e705641). I use git-imap-send, not git-send-email, and I wanted to thread my patches. - Josh Triplett ```