Re: [PATCH] format-patch: Properly escape From_ lines when creating an mbox.

From: Carl Worth <cworth@cworth.org>
Date: Jun 10, 2010, 15:31 UTC
Message-ID: <87pqzyhpl2.fsf@yoom.home.cworth.org>
In-Reply-To: <7vaar3nds1.fsf@alter.siamese.dyndns.org>

On Thu, 10 Jun 2010 07:49:34 -0700, Junio C Hamano <gitster@pobox.com> wrote:

> Carl Worth <cworth@cworth.org> writes:
> Please disregard the above; I wasn't thinking straight.

No worries.

Show 7 quoted lines

> If your format-patch quotes ">*From " in the log message, and you unquote
> it somewhere in mailsplit to mailinfo pipeline, then the only time any
> funny interaction between the current git and your git would happen is
> when your git formats a commit with a line in its log that begins with
> "From " that cannot be a mistaken as a UNIX-From line and you use "am"
> from the current git on the output; the resulting commit would get an
> extra ">" left in the message, but that is a small price to pay.

Correct. That's the only downside I see. And it's clearly not a huge price. Checking the entire git.git history, I found only 12 messages that have a line beginning with "From " in the commit message, (ignoring the 5 I referred to earlier where the mbox line actually ended up in the commit message). Of these, three already have a ">From" in them.

So clearly this kind of thing is happening already. People are sometimes using systems that quote these lines and git isn't un-quoting. But it's really not that objectionable in the end.

> There is
> no other downside I can see (and the upside is that the output from your
> format-patch won't be split incorrectly, of course).

Good. And this series does fix actual bugs like the failure to rebase across a message with a "From ... <timestamp>" line as mentioned before.

> I find that the change to format-patch not to emit the UNIX-From line when
> generating one file per commit is somewhat iffy.  An upside is that the
> existing mailsplit-mailinfo pipeline already knows not to split such an
> input, so the change makes

This patch was written to reduce the likelihood of the downside mentioned above. But I understand the potential costs you outlined. I'll let you make the call on whether to include it. The rest of the patch series will work fine without this one. But leaving it out will lead to more occurrences of undesired ">From " in commit messages, (even with new "git am" as send-email or some other MUA will see the quoted ">From " and treat it as the intended payload, not an escaped "From ").

Show 5 quoted lines

> It is nice at the conceptual level, though.  By declaring individual file
> RFC2822 message (not mbox), it makes it very clear that it is MUA's
> responsibility to quote "From " in the payload when the output is used by
> MUA to compose and send a message.  IOW, we shouldn't be doing the quote
> in our output when operating in that mode.

I suppose we could maintain compatibility with any scripts, etc. by still emitting the initial "From " line, but declaring these files as messages (not mbox) and avoiding doing any quoting for them.

I think that gets us all the upsides with no downsides. I'll send one last patch for that.

-Carl

-- 
carl.d.worth@intel.com