git-mailinfo fixes for patch munging
PATCH 1/1 git-mailinfo fixes for patch munging
Don't translate the patch to UTF-8, instead preserve the data as is. Also allow overwriting the primary mail headers (addresses Linus's concern).
I also revert a test case that was included in the original patch. Now it makes sense why it was the way it was. :)
Cheers,
Don
builtin-mailinfo.c +12 -11
@@ -294,14 +294,14 @@ static char *header[MAX_HDR_PARSED] = { "From","Subject","Date", }; -static int check_header (char *line,char **hdr_data) +static int check_header (char *line,char **hdr_data,int overwrite) {int i;/* search for the interesting parts */ for (i =0 ; header[i]; i++) {int len =strlen (header[i]); -if (!hdr_data[i] && +if ((!hdr_data[i] || overwrite) && !strncasecmp (line, header[i], len) && line[len] ==':' &&isspace (line[len +1 ])) {/* Unwrap inline B and Q encoding, and optionally
@@ -614,6 +614,7 @@ static int find_boundary(void)static int handle_boundary (void ) { +char newline[]="\n" ; again:if (!memcmp (line+content_top->boundary_len,"--" ,2 )) {/* we hit an end boundary */
@@ -628,7 +629,7 @@ again: "can't recover\n");exit (1 ); } -handle_filter ("\n" ); +handle_filter (newline);/* skip to the next boundary */ if (!find_boundary ())
@@ -643,7 +644,7 @@ again:/* slurp in this section's info */ while (read_one_header_line (line,sizeof (line), fin)) -check_header (line, p_hdr_data); +check_header (line, p_hdr_data,1 );/* eat the blank line after section info */ return (fgets (line,sizeof (line), fin) !=NULL );
@@ -699,10 +700,14 @@ static int handle_commit_msg(char *line)if (!*cp)return 0 ; } -if ((still_looking =check_header (cp, s_hdr_data)) !=0 ) +if ((still_looking =check_header (cp, s_hdr_data,0 )) !=0 )return 0 ; } +/* normalize the log message to UTF-8. */ +if (metainfo_charset) +convert_to_utf8 (line, charset); +if (patchbreak (line)) {fclose (cmitmsg); cmitmsg =NULL ;
@@ -767,12 +772,8 @@ static void handle_body(void)return ; } -/* Unwrap transfer encoding and optionally -* normalize the log message to UTF-8. -*/ +/* Unwrap transfer encoding */ decode_transfer_encoding (line); -if (metainfo_charset) -convert_to_utf8 (line, charset);switch (transfer_encoding) {case TE_BASE64 :
@@ -875,7 +876,7 @@ int mailinfo(FILE *in, FILE *out, int ks, const char *encoding,/* process the email header */ while (read_one_header_line (line,sizeof (line), fin)) -check_header (line, p_hdr_data); +check_header (line, p_hdr_data,1 );handle_body ();handle_info ();
t/t5100/patch0005 +1 -1
@@ -61,7 +61,7 @@ diff --git a/git-cvsimport-script b/git-cvsimport-script push(@old,$fn); -- -David KÃ¥gedal +David Kågedal - To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to [email protected]
Quoted 8 lines · Don Zickus
Don't translate the patch to UTF-8, instead preserve the data as is. Also allow overwriting the primary mail headers (addresses Linus's concern).
I also revert a test case that was included in the original patch. Now it makes sense why it was the way it was. :)
Cheers,
Don
Thanks. Sign-off would have been nice.
Quoted 11 lines
builtin-mailinfo.c +1
-1@@ -294,14 +294,14 @@ static char *header[MAX_HDR_PARSED] = { "From","Subject","Date", }; -static int check_header (char *line,char **hdr_data) +static int check_header (char *line,char **hdr_data,int overwrite) {
builtin-mailinfo.c static char *header[MAX_HDR_PARSED] = { · line 307
Quoted 10 lines
builtin-mailinfo.c +1
-1int i;/* search for the interesting parts */ for (i =0 ; header[i]; i++) {int len =strlen (header[i]); -if (!hdr_data[i] && +if ((!hdr_data[i] || overwrite) && !strncasecmp (line, header[i], len) && line[len] ==':' &&isspace (line[len +1 ])) {/* Unwrap inline B and Q encoding, and optionally
This check_header is called from each multi-part boundary with overwrite=1, so if you have two parts and you have From: or
Subject: in the multi-part header (not in-body), wouldn't they overwrite what we already have? That is not desired, I would think.
For non multi-part case, what we traditionally have done is:
* Take Subject:, Date:, and From: from RFC2822 headers to prime the title and authorship information.
* The first lines of the body of the message (i.e. after the blank line that separates 2822 headers and the body) can look like the above header lines to override.
* A line that does not look like an overriding in-body header line is the first line of the commit log message. After that, nothing is taken as an overriding in-body header.
For a multi-part, I think we only processed the first part as the commit log message, potentially starting with the overriding in-body headers. In other words, in-body headers are what the user *types* to override what the MUA says in RFC2822 headers. As the stuff that follow the multi-part boundary (like content-type and transfer encoding) are of the MUA kind, I suspect we do not want it to override what the sender said in the earlier parts of the message.
Quoted 9 lines
builtin-mailinfo.c +1
-0@@ -614,6 +614,7 @@ static int find_boundary(void)static int handle_boundary (void ) { +char newline[]="\n" ; again:if (!memcmp (line+content_top->boundary_len,"--" ,2 )) {/* we hit an end boundary */
builtin-mailinfo.c again: · line 635
Quoted 8 lines
builtin-mailinfo.c +1
-1@@ -628,7 +629,7 @@ again: "can't recover\n");exit (1 ); } -handle_filter ("\n" ); +handle_filter (newline);/* skip to the next boundary */ if (!find_boundary ())
These two hunks certainly do not hurt, but why? Is this about the constness of the first parameter to handle_filter() and its call chain?
Having said that, the result of the patch is much better.
In fact, I couldn't "git am" this patch (the part that reverts the test vector) with the current tip of 'master' because of the breakage you are fixing with it ;-).
Now I can. So I'd probably take this patch for now.
Quoted 5 lines · Junio C Hamano
Quoted 3 lines · Don Zickus
Don't translate the patch to UTF-8, instead preserve the data as is. Also allow overwriting the primary mail headers (addresses Linus's concern).
Quoted 7 lines
Quoted 5 lines
I also revert a test case that was included in the original patch. Now it makes sense why it was the way it was. :)
Cheers,
DonThanks. Sign-off would have been nice.
Doh. Sorry. Should I repost with fix below?
Quoted 5 lines
This
check_headeris called from each multi-part boundary with overwrite=1, so if you have two parts and you have From: or
Subject: in the multi-part header (not in-body), wouldn't they overwrite what we already have? That is not desired, I would think.
Hmm. I guess I never thought about that case. You are right, that check can be changed to a zero (because the rfc2822 are checked elsewhere).
Quoted 15 lines
Quoted 15 lines
builtin-mailinfo.c +2
-1@@ -614,6 +614,7 @@ static int find_boundary(void)static int handle_boundary (void ) { +char newline[]="\n" ; again:if (!memcmp (line+content_top->boundary_len,"--" ,2 )) {/* we hit an end boundary */ @@ -628,7 +629,7 @@ again: "can't recover\n");exit (1 ); } -handle_filter ("\n" ); +handle_filter (newline);
Quoted 6 lines
builtin-mailinfo.c again: · line 635
Quoted 2 lines
builtin-mailinfo.c +0
-0/* skip to the next boundary */ if (!find_boundary ())These two hunks certainly do not hurt, but why? Is this about the constness of the first parameter to
handle_filter()and its call chain?
Yeah, I SEGFAULT'd when trying to convert_to_utf8() a fixed string. :-)
Cheers,
Don