git am and CRLF files

git am and CRLF files

[Stefan Naewe]

Hi there.
I have:

$ git version
git version 1.6.5.1.1367.gcd48

$ git config --get core.autocrlf
false

A repository with some UNIX (LF) and some Windows (CRLF) files.
(and no: I will not change the files. My editors handle CRLF and LF correctly)

My problem:

'git am' can't handle changes in CRLF files because the patch
gets converted (by git mailsplit) to contain only LF.

Which is wrong IMHO.

git-am on my msysgit version looks like this (lines: 214++)

<---------->
split_patches () {
	case "$patch_format" in
	mbox)
		case "$rebasing" in
		'')
			keep_cr= ;;
		?*)
			keep_cr=--keep-cr ;;
		esac
		git mailsplit -d"$prec" -o"$dotest" -b $keep_cr -- "$@" > "$dotest/last" ||
		clean_abort
		;;
<---------->

The '--keep-cr' flags is passed to git mailsplit when git am is in 'rebasing' mode.
By looking through git-am I found that I can pass "--rebasing" to git am to get my
patch applied correctly.
But why is git am behaving that way ?

Puzzled,

Stefan
-- 
----------------------------------------------------------------
/dev/random says: I'm dangerous when I know what I'm doing.

Re: git am and CRLF files

[Stefan Naewe]

On 11/13/2009 10:44 AM, Stefan Naewe wrote:
> Hi there.
> I have:
> 
> $ git version
> git version 1.6.5.1.1367.gcd48
> 
> $ git config --get core.autocrlf
> false
> 
> A repository with some UNIX (LF) and some Windows (CRLF) files.
> (and no: I will not change the files. My editors handle CRLF and LF correctly)
> 
> My problem:
> 
> 'git am' can't handle changes in CRLF files because the patch
> gets converted (by git mailsplit) to contain only LF.
> 
> Which is wrong IMHO.
> 
> git-am on my msysgit version looks like this (lines: 214++)
> 
> <---------->
> split_patches () {
> 	case "$patch_format" in
> 	mbox)
> 		case "$rebasing" in
> 		'')
> 			keep_cr= ;;
> 		?*)
> 			keep_cr=--keep-cr ;;
> 		esac
> 		git mailsplit -d"$prec" -o"$dotest" -b $keep_cr -- "$@" > "$dotest/last" ||
> 		clean_abort
> 		;;
> <---------->
> 
> The '--keep-cr' flags is passed to git mailsplit when git am is in 'rebasing' mode.
> By looking through git-am I found that I can pass "--rebasing" to git am to get my
> patch applied correctly.
> But why is git am behaving that way ?
> 
> Puzzled,
> 
> Stefan

Does anyone have any comment on this ?

Regards,

Stefan
-- 
----------------------------------------------------------------
/dev/random says: Microsoft Windows... a virus with mouse support.

Re: git am and CRLF files

[Nanako Shiraishi]

Quoting Stefan Naewe <stefan.naewe@atlas-elektronik.com>

>> A repository with some UNIX (LF) and some Windows (CRLF) files.
>> (and no: I will not change the files. My editors handle CRLF and LF correctly)
>> 
>> My problem:
>> 
>> 'git am' can't handle changes in CRLF files because the patch
>> gets converted (by git mailsplit) to contain only LF.
>
>> Stefan
>
> Does anyone have any comment on this ?

This was done very much on purpose.

The "am" command is meant to handle e-mailed patches, and traditionally
mails are known to clobber carriage returns.

See commit c2ca1d79dbd54b06a05e5d14a897699e59dc9f9f

    Allow mailsplit (and hence git-am) to handle mails with CRLF line-endings
    
    It is not that uncommon to have mails with DOS line-ending, notably
    Thunderbird and web mailers like Gmail (when saving what they call
    "original" message).  So modify mailsplit to convert CRLF line-endings to
    just LF.

-- 
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/

Re: git am and CRLF files

[Stefan Naewe]

On 11/16/2009 11:50 AM, Nanako Shiraishi wrote:
> Quoting Stefan Naewe <stefan.naewe@atlas-elektronik.com>
> 
>>> A repository with some UNIX (LF) and some Windows (CRLF) files.
>>> (and no: I will not change the files. My editors handle CRLF and LF correctly)
>>>
>>> My problem:
>>>
>>> 'git am' can't handle changes in CRLF files because the patch
>>> gets converted (by git mailsplit) to contain only LF.
>>> Stefan
>> Does anyone have any comment on this ?
> 
> This was done very much on purpose.
> 
> The "am" command is meant to handle e-mailed patches, and traditionally
> mails are known to clobber carriage returns.
> 
> See commit c2ca1d79dbd54b06a05e5d14a897699e59dc9f9f
> 
>     Allow mailsplit (and hence git-am) to handle mails with CRLF line-endings
>     
>     It is not that uncommon to have mails with DOS line-ending, notably
>     Thunderbird and web mailers like Gmail (when saving what they call
>     "original" message).  So modify mailsplit to convert CRLF line-endings to
>     just LF.
> 

I've noticed that.
But converting everything just breaks git am for CRLF files, doesn't it ?
Wouldn't it be possible (and sensible) to not convert the diff text, but
only the rest (mail text, headers, etc.) ?

Regards,

Stefan
-- 
----------------------------------------------------------------
/dev/random says: Change is inevitable, except from a vending machine.

Re: git am and CRLF files

[Erik Faye-Lund]

On Mon, Nov 16, 2009 at 11:50 AM, Nanako Shiraishi <nanako3@lavabit.com> wrote:
> Quoting Stefan Naewe <stefan.naewe@atlas-elektronik.com>
>
>>> A repository with some UNIX (LF) and some Windows (CRLF) files.
>>> (and no: I will not change the files. My editors handle CRLF and LF correctly)
>>>
>>> My problem:
>>>
>>> 'git am' can't handle changes in CRLF files because the patch
>>> gets converted (by git mailsplit) to contain only LF.
>>
>>> Stefan
>>
>> Does anyone have any comment on this ?
>
> This was done very much on purpose.
>
> The "am" command is meant to handle e-mailed patches, and traditionally
> mails are known to clobber carriage returns.
>

According to RFC 5322, email messages use CRLF as the
newline-sequence. In order to be able to distinguish between CRLF and
LF in an e-mail patch, the message needs to be use some
transfer-encoding that preserves newline style (like base64).

Perhaps this would be better fixed by having format-patch (or prehaps
the MUA ?) base64-encode the message body if the file contains
non-LF-newlines, and normalizing CRLF to LF before transport-decoding?
Or does some MUAs transport-decode before storing the message to disk?

I realize this might make it a bit tricky to review patches that
contains CRLF-newlines before mailing them out, but perhaps inspecting
the format-patch output is the wrong place to do this?

-- 
Erik "kusma" Faye-Lund

Re: git am and CRLF files

[Daniele Segato]

On Mon, Nov 16, 2009 at 12:43 PM, Erik Faye-Lund
<kusmabite@googlemail.com> wrote:
> According to RFC 5322, email messages use CRLF as the
> newline-sequence. In order to be able to distinguish between CRLF and
> LF in an e-mail patch, the message needs to be use some
> transfer-encoding that preserves newline style (like base64).
>
> Perhaps this would be better fixed by having format-patch (or prehaps
> the MUA ?) base64-encode the message body if the file contains
> non-LF-newlines, and normalizing CRLF to LF before transport-decoding?
> Or does some MUAs transport-decode before storing the message to disk?
>
> I realize this might make it a bit tricky to review patches that
> contains CRLF-newlines before mailing them out, but perhaps inspecting
> the format-patch output is the wrong place to do this?


why don't adding that information in the mail header?
or may be made format-patch create a "comment line" with that information?

if that line is missing it could keep the default behavior (what it
did until now)

Re: git am and CRLF files

[Ismael Luceno]

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

Daniele Segato escribió:
> On Mon, Nov 16, 2009 at 12:43 PM, Erik Faye-Lund
> <kusmabite@googlemail.com> wrote:
>> According to RFC 5322, email messages use CRLF as the
>> newline-sequence. In order to be able to distinguish between CRLF and
>> LF in an e-mail patch, the message needs to be use some
>> transfer-encoding that preserves newline style (like base64).
>>
>> Perhaps this would be better fixed by having format-patch (or prehaps
>> the MUA ?) base64-encode the message body if the file contains
>> non-LF-newlines, and normalizing CRLF to LF before transport-decoding?
>> Or does some MUAs transport-decode before storing the message to disk?
>>
>> I realize this might make it a bit tricky to review patches that
>> contains CRLF-newlines before mailing them out, but perhaps inspecting
>> the format-patch output is the wrong place to do this?
> 
> 
> why don't adding that information in the mail header?
> or may be made format-patch create a "comment line" with that information?
> 
> if that line is missing it could keep the default behavior (what it
> did until now)
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

It would make more sense to simply use MIME attachments...

-- 
Ismael Luceno


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]