git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: "Stefan-W. Hahn" <stefan.hahn@s-hahn.de>
Cc: Junio C Hamano <gitster@pobox.com>, git@vger.kernel.org
Subject: [PATCH 2/3] mailinfo: always clean up rfc822 header folding
Date: Thu, 26 May 2011 16:53:38 -0400	[thread overview]
Message-ID: <20110526205338.GB31340@sigill.intra.peff.net> (raw)
In-Reply-To: <20110526203625.GA31018@sigill.intra.peff.net>

Without the "-k" option, mailinfo will convert a folded
subject header like:

  Subject: this is a
    subject that doesn't
    fit on one line

into a single line. With "-k", however, we assumed that
these newlines were significant and represented something
that the sending side would want us to preserve.

For messages created by format-patch, this assumption was
broken by a1f6baa (format-patch: wrap long header lines,
2011-02-23).  For messages sent by arbitrary MUAs, this was
probably never a good assumption to make, as they may have
been folding subjects in accordance with rfc822's line
length recommendations all along.

This patch now joins folded lines with a single whitespace
character. This treats header folding purely as a syntactic
feature of the transport mechanism, not as something that
format-patch is trying to tell us about the original
subject.

Signed-off-by: Jeff King <peff@peff.net>
---
The astute reader will notice that even with this patch, there is still
a regression when using new versions of format-patch (with a1f6baa) with
an older version of "git am". But you only see it when using "am -k", so
interoperability is probably not a huge deal:

  1. Before this patch, "am -k" was arguably broken anyway for applying
     random patches via email, since MUAs may have been doing arbitrary
     header folding. So we can probably discount people running "am -k"
     on random input as insane.

  2. People doing "git format-patch -k | git am -k" will presumably use
     the same version for both, and are OK.

  3. People doing "git format-patch -k >file", followed by upgrading
     git, and t hen "git am file" are still OK, since the newer version
     of "am" handles the output of both old and new format-patch.

  4. The problematic case is "git format-patch -k >file" with v1.7.5 or
     newer, then _downgrading_ git, then using "git am -k" to apply. Or
     more likely, using a newer version to create an mbox, shipping the
     mbox to another machine, and then using an older "git am" to apply.

So I don't see it as all that likely a problem in practice. If we do
care, we can't fix it with a simple patch. We would have to revert the
header-folding from format-patch, fix am, wait N time units until all of
the old "am" no longer exists, and then re-apply.

 builtin/mailinfo.c     |    2 +-
 t/t4152-am-subjects.sh |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index 71e6262..bfb32b7 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -400,7 +400,7 @@ static int read_one_header_line(struct strbuf *line, FILE *in)
 			break;
 		if (strbuf_getline(&continuation, in, '\n'))
 			break;
-		continuation.buf[0] = '\n';
+		continuation.buf[0] = ' ';
 		strbuf_rtrim(&continuation);
 		strbuf_addbuf(line, &continuation);
 	}
diff --git a/t/t4152-am-subjects.sh b/t/t4152-am-subjects.sh
index 7222c06..37e5c03 100755
--- a/t/t4152-am-subjects.sh
+++ b/t/t4152-am-subjects.sh
@@ -58,7 +58,7 @@ test_expect_success 'long subject preserved (format-patch | am)' '
 test_expect_success 'long subject preserved (format-patch -k | am)' '
 	check_subject long-k
 '
-test_expect_failure 'long subject preserved (format-patch -k | am -k)' '
+test_expect_success 'long subject preserved (format-patch -k | am -k)' '
 	check_subject long-k -k
 '
 
-- 
1.7.4.5.26.g0c6a2

  parent reply	other threads:[~2011-05-26 20:53 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-05-24 16:02 commit a1f6baa5 (wrap long header lines) breaks my habit Stefan-W. Hahn
2011-05-24 16:27 ` Junio C Hamano
2011-05-24 16:46   ` Stefan-W. Hahn
2011-05-24 20:07     ` Jeff King
2011-05-25 15:40       ` Stefan-W. Hahn
2011-05-26 20:36         ` Jeff King
2011-05-26 20:41           ` [PATCH 1/3] t: test subject handling in format-patch / am pipeline Jeff King
2011-05-26 20:53           ` Jeff King [this message]
2011-05-26 20:55           ` [PATCH 3/3] format-patch: preserve subject newlines with -k Jeff King
2011-05-26 21:18             ` Junio C Hamano
2011-05-26 21:19               ` Jeff King
2011-05-26 22:24                 ` Jeff King
2011-05-26 22:27                   ` [PATCH 3/5] pretty: add pp_commit_easy function for simple callers Jeff King
2011-05-26 22:47                     ` Junio C Hamano
2011-05-26 22:27                   ` [PATCH 4/5] clean up calling conventions for pretty.c functions Jeff King
2011-05-26 22:28                   ` [PATCH 5/5] format-patch: preserve subject newlines with -k Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20110526205338.GB31340@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=stefan.hahn@s-hahn.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).