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

[Carl Worth <cworth@cworth.org>]

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

On Tue, 08 Jun 2010 20:50:01 -0700, Junio C Hamano <gitster@pobox.com> wrote:
> Carl Worth <cworth@cworth.org> writes:
> Especially because your implementation quotes lines that begin with "From "
> unconditionally (even when the tail end of the line would never be a
> valid-looking timestamp).  Such an output will confuse existing mailsplit,
> but the worst part of the story is that somebody who is applying a series
> of patches will _not_ notice the breakage.  The payload of the second and
> subsequent messages will likely be concatenated as if it were part of the
> first message, ignoring cruft between patches, but the resulting tree
> would likely to be the same as what the sending end intended.

I agree that anything that results in multiple patches being (silently!)
concatenated would be catastrophic and I do not recommend accepting any
patches that could result in failures like that.

Could you describe in more detail how the implementation could lead to a
case like that? I'm not seeing it myself. But if you can show me, I'll
be happy to attempt a fix.

In particular, I don't see how any of the new quoting will confuse
existing mailsplit. The splitting itself shouldn't be changed. And at
worst, using new "git format-patch" with old mailsplit could result in a
">From " getting into a commit message where a "From " should be.

We could reduce the occurrence of that problem by being less aggressive
with "From " quoting, (for example, examining whether the tail of the
line looks like a timestamp before quoting). The cost there would be
fairly minor. It would increase the occurrence of a failure to pass a
">From " correctly from a new "git am" to a new "git mailsplit". [*]

I don't see a way to eliminate both problems other than specifying that
git's mbox format is a non-standard mbox format that looks specifically
for From_ lines ending in timestamps and is not capable of containing an
arbitrary message, (namely messages with lines that begin with "From "
and end with timestamps).

That would be a particularly unsatisfying solution for me, since I'm
trying to implement an mbox-export option in a mail client as a general
feature (that happens to work with git) rather than implementing a
git-specific export option.

-Carl

[*] It would seem a strange strategy to make new git compatible with old
git while not being perfectly compatible with itself going forward, but
that is a possibility.

-- 
carl.d.worth@intel.com

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]