Re: git-format-patch should include a checksum

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 27, 2010, 03:01 UTC
Message-ID: <7vljfkw8ss.fsf@alter.siamese.dyndns.org>
In-Reply-To: <7ir5pccp9n.fsf@lanthane.pps.jussieu.fr>

Juliusz Chroboczek <Juliusz.Chroboczek@pps.jussieu.fr> writes:

Show 5 quoted lines

>> Do you have an example of such corrupted and incorrectly applied patches?
>> What kind of corruption are you talking about?
>
> The commit message getting rewrapped.  For some reason, the patch itself
> was not corrupted.

You hopefully check the message (and of course the patch) before applying, so "automatically reject and require force" wouldn't help you much in this case. Either you reject and tell the sender to resend, or you reflow it yourself to save the sender (and yourself) the hassle of round-trip.

And if you choose to do the latter, having to force it would actively inconvenience you.

> Another case is that of the commit message having its non-ASCII
> characters corrupted.

I've seen this one. It often is that the commit object records UTF-8, but somehow the MUA didn't mark it as such (or incorrectly marked it as ISO-8859-1), and mailinfo ended up doing unnecessary conversion. When this happens, not just the message but also the patch text is affected.

But to use "checksumming" as a solution for this, you need to think about what you checksum. Output from format-patch is "text/plain;charset=UTF-8" and requires 8-bit clean transport, but by cutting and pasting that into MUA you often end up with whatever MIME B/Q-quoting your MUA gives you, and mailinfo is actively unwraps them, so the right place to add an optional check _might_ be immediately after we run mailinfo. At that point, however, the author name and the subject line is in separate records from the body of the commit log message.

More realistically, the kind of MUA corruption I personally see most often is "text/plain; format-flowed"; I have a pre-applypatch hook to reject them altogether, but they tend to corrupt leading whitespaces so badly that the patch seldome applies. This is not something "checksumming" is the right tool to solve.

One thing that we _might_ consider doing is to reduce the default length of the function name we place on the hunk header line. For some reason, I see they get wrapped in messages I see, even when the proposed commit log message has overlong lines.