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

On Tue, 08 Jun 2010 20:50:01 -0700, Junio C Hamano [off-list ref] wrote:

Carl Worth [off-list ref] 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