threads / discuss / 21604

git am and CRLF files

Subject: git am and CRLF files

## tl;dr

7 messages between Nov 13, 2009 and Nov 30, 2009.

replies: 6people: 5as markdown or json

Stefan Naewe· Nov 13, 2009, 09:44 UTC · lore

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.
Stefan Naewe· Nov 16, 2009, 07:33 UTC · re: Stefan Naewe · lore

Re: git am and CRLF files

On 11/13/2009 10:44 AM, Stefan Naewe wrote:
Show 44 quoted lines
> 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.
Nanako Shiraishi· Nov 16, 2009, 10:50 UTC · re: Stefan Naewe · lore

Re: git am and CRLF files

Quoting Stefan Naewe <stefan.naewe@atlas-elektronik.com>
Show 11 quoted lines
>> 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/
Stefan Naewe· Nov 16, 2009, 11:15 UTC · re: Nanako Shiraishi · lore

Re: git am and CRLF files

On 11/16/2009 11:50 AM, Nanako Shiraishi wrote:
Show 26 quoted lines
> 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.
Erik Faye-Lund· Nov 16, 2009, 11:43 UTC · re: Nanako Shiraishi · lore

Re: git am and CRLF files

On Mon, Nov 16, 2009 at 11:50 AM, Nanako Shiraishi <nanako3@lavabit.com> wrote:
Show 19 quoted lines
> 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
Daniele Segato· Nov 30, 2009, 12:06 UTC · re: Erik Faye-Lund · lore

Re: git am and CRLF files

On Mon, Nov 16, 2009 at 12:43 PM, Erik Faye-Lund <kusmabite@googlemail.com> wrote:

Show 13 quoted lines
> 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)

Ismael Luceno· Nov 30, 2009, 12:50 UTC · re: Daniele Segato · lore

Re: git am and CRLF files

Daniele Segato escribió:
Show 26 quoted lines
> 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

← back to recent threads