git-mailinfo munges the patch?

git-mailinfo munges the patch?

[Junio C Hamano]

I noticed that the new mailinfo when splitting a message into
cover letter and the patch text seems to munge the patch text,
applying the same "if content-type is not there then assume
latin-1 and recode to utf-8" logic that is applied to the commit
log message.  That munging should not be done to the patch text,
and it appears the current code botches it.

I am a bit too busy with day job today and haven't had a chance
to look into this problem fully, but 1.5.0.3 does not seem to
have this problem but post 87ab7992 mailinfo is problematic.

Re: git-mailinfo munges the patch?

[Don Zickus]

On Thu, Mar 29, 2007 at 01:18:51PM -0700, Junio C Hamano wrote:
> I noticed that the new mailinfo when splitting a message into
> cover letter and the patch text seems to munge the patch text,
> applying the same "if content-type is not there then assume
> latin-1 and recode to utf-8" logic that is applied to the commit
> log message.  That munging should not be done to the patch text,
> and it appears the current code botches it.

Ok.  I see what you are saying with the old code.  Sorry about that.  Do
you have a sample file that I can play with to test my fix?

Cheers,
Don

Re: git-mailinfo munges the patch?

[Linus Torvalds]

On Thu, 29 Mar 2007, Don Zickus wrote:
> 
> Ok.  I see what you are saying with the old code.  Sorry about that.  Do
> you have a sample file that I can play with to test my fix?

On that note - here's an unrelated simple case that the old mailinfo got 
right, but the new one seems to screw up: multiple Subject: lines.

The old one would make later Subject: lines override the earlier ones, and 
I depended on that when I fix up peoples emails to me manually (you 
wouldn't believe how bad explanations or subject lines people use for 
perfectly good patches ;)

The current mailinfo seems to just take the first one. 

		Linus

Re: git-mailinfo munges the patch?

[Junio C Hamano]

[-- Attachment #1: Type: text/plain, Size: 1534 bytes --]

Don Zickus <dzickus@redhat.com> writes:

> On Thu, Mar 29, 2007 at 01:18:51PM -0700, Junio C Hamano wrote:
>> I noticed that the new mailinfo when splitting a message into
>> cover letter and the patch text seems to munge the patch text,
>> applying the same "if content-type is not there then assume
>> latin-1 and recode to utf-8" logic that is applied to the commit
>> log message.  That munging should not be done to the patch text,
>> and it appears the current code botches it.
>
> Ok.  I see what you are saying with the old code.  Sorry about that.  Do
> you have a sample file that I can play with to test my fix?

Attached are two files.

The first one is to be split with git-mailsplit and then
processed with git-mailinfo; I just tried to make sure what I
expect to see as the output is given by 1.5.0 but not with the
current 'master'.

The second one is a bundle of the repository the test data comes
from.  You could use it like this:

	$ mkdir foo && cd foo && git init
        $ git pull ../sample.bdl master

The repository has one branch, 'master', with two commits on
it.  The first one creates an empty file 'a', and the second one
adds a line to the file, which has a Spanish "tilde on top of N"
in it, and the contents of the file is encoded in ISO 8859-1.

The commit message of the second one is plain ascii but it could
be in UTF-8.

The point is that patch can have payload from different files
that are possibly in different encodings, so shoud really be
treated as binary data without mangling.

Thanks.


[-- Attachment #2: sample input --]
[-- Type: application/octet-stream, Size: 548 bytes --]

From 119dca7e9d905b73ca94a1d625d75209d241ec59 Mon Sep 17 00:00:00 2001
From: Junio C Hamano <junkio@cox.net>
Date: Thu, 29 Mar 2007 14:16:01 -0700
Subject: Add a line with ISO 8859-1 contents

This adds a line with tilde-on-top-of-N character in ISO 8859-1.
This commit log message itself is in plain ascii but it could be in
UTF-8.'
---
 a |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/a b/a
index e69de29..6ccbec4 100644
--- a/a
+++ b/a
@@ -0,0 +1 @@
+ISO 8859 1 Ñ (Spanish en with tilde on top)
-- 
1.5.1.rc3.5.ge91e


[-- Attachment #3: sample repository bundle --]
[-- Type: application/octet-stream, Size: 646 bytes --]

Re: git-mailinfo munges the patch?

[Don Zickus]

On Thu, Mar 29, 2007 at 02:19:26PM -0700, Linus Torvalds wrote:
> 
> 
> On Thu, 29 Mar 2007, Don Zickus wrote:
> > 
> > Ok.  I see what you are saying with the old code.  Sorry about that.  Do
> > you have a sample file that I can play with to test my fix?
> 
> On that note - here's an unrelated simple case that the old mailinfo got 
> right, but the new one seems to screw up: multiple Subject: lines.
> 
> The old one would make later Subject: lines override the earlier ones, and 
> I depended on that when I fix up peoples emails to me manually (you 
> wouldn't believe how bad explanations or subject lines people use for 
> perfectly good patches ;)

I see what happened.  The old code allowed rewriting of the mail headers
but blocked rewriting of the inbody headers.  For some reason I thought it
was by accident that the mail headers were allow to be rewritten.  Oops.
:(

I wrote the code to have both header types block rewriting.  I'll change
that.  

Cheers,
Don

Re: git-mailinfo munges the patch?

[Don Zickus]

On Thu, Mar 29, 2007 at 02:24:39PM -0700, Junio C Hamano wrote:
> Don Zickus <dzickus@redhat.com> writes:
> 
> > On Thu, Mar 29, 2007 at 01:18:51PM -0700, Junio C Hamano wrote:
> >> I noticed that the new mailinfo when splitting a message into
> >> cover letter and the patch text seems to munge the patch text,
> >> applying the same "if content-type is not there then assume
> >> latin-1 and recode to utf-8" logic that is applied to the commit
> >> log message.  That munging should not be done to the patch text,
> >> and it appears the current code botches it.
> >
> > Ok.  I see what you are saying with the old code.  Sorry about that.  Do
> > you have a sample file that I can play with to test my fix?
> 
> Attached are two files.

Thanks.  I'll play with them later tonight.

Cheers,
Don