Re: Maintainers, please add Message-Id: when merging patches

[Laszlo Ersek]

On 01/23/20 18:18, Kevin Wolf wrote:
> Am 22.01.2020 um 13:28 hat Kevin Wolf geschrieben:
>> Am 22.01.2020 um 13:02 hat Stefan Hajnoczi geschrieben:
>>> Around 66% of qemu.git commits since v4.1.0 include a Message-Id: tag.  
>>> Hooray!
>>>
>>> Message-Id: references the patch email that a commit was merged from.
>>> This information is helpful to anyone wishing to refer back to email
>>> discussions and patch series.
>>>
>>> Please use git-am(1) -m/--message-id or set am.messageid in your 
>>> git-config(1).
>>
>> I've had -m in my scripts for a while (last time someone asked me to
>> make the change, I guess), but it wasn't effective, because my .muttrc
>> has 'set pipe_decode' enabled, which doesn't only decode the output, but
>> also throws away most headers.
>>
>> I seem to remember that this was necessary at some point because
>> otherwise some mails just wouldn't apply. Maybe 'git am' works better
>> these days and can actually parse the mails that used to give me
>> problems. I'll give it a try and disable pipe_decode.
> 
> Here is the first patch for which it failed for me:
> 
> Message-ID: <address@hidden>
> 
> The problem seems to be related to line endings because the patch that
> git-apply sees eventually has "\r\n" whereas the file to be patched has
> only "\n".
> 
> If I understand correctly (this is a bit of guesswork after reading man
> pages and trying out a few options), git-mailsplit would normally get
> rid of the "\r". However, this specific patch email is base64 encoded,
> so the encoded "\r" characters survive this stage.
> 
> git-mailinfo later decodes the email, but doesn't seem to do anything
> about "\r" again, so it survives this one as well. This means feeding a
> patch with the wrong line endings to git-apply, which just fails.
> 
> Any suggestion how to fix this? (For this patch, I just enabled
> pipe_decode again, so no Message-Id tag for it.)

In my opinion, the patch you mention is malformed.

I saved it to a local file with Thunderbird, saved the base64-encoded
body to a separate file, and decoded it with a naked "base64 --decode"
invocation. The result is a file with CRLF line terminators.

When someone sends a base64-encoded patch email, that's a statement
(again: IMO) that the patch conforms to the "canonical" checkout (=
working tree) line ending convention. For QEMU, I would think that said
convention dictates LF.

Note: I'm aware that with git, the "internal" representation for
newlines, and the "external" one, are different things. Dependent on
whether one is on Windows vs. Linux, git-checkout will produce CRLF vs.
LF in the working tree, as the "external" newline representation. What
I'm saying is that, if someone sends a base64-encoded patch, that's a
statement that their *external* newline representation matches that of
the people that they expect to apply the patch. Normally, external
representations (i.e., the local working trees' newline representations)
don't have to match each other -- but if a patch is sent with base64
Content-Transfer-Encoding, then I claim that they do.

IOW, I'd simply answer the patch in question with:

"""
Please resend the patch with one of the following options:
- use LF in your local working tree, and keep the base64 C-T-E, or
- keep CRLF in your local working tree, and send with 8bit C-T-E.
"""

As a practical result, if someone develops QEMU in a Windows
environment, they should only use 8bit C-T-E when posting patches.

Strictly my personal opinion, of course.

Thanks,
Laszlo