git rm / format-patch / am fails on my file: patch does not apply

Post by Ken Tanzer
ASCII text, with very long lines, with CRLF, CR, LF line terminators

I am not very much surprised if such a file misbehaves, because the
"format-patch | am" pipeline is designed to be used on patches that
can be transferred in plain-text e-mail safely. Long lines should
probably be OK, but mixed CRLF, CR and LF may be problematic.

Having said that...

Post by Ken Tanzer
I've confirmed with the following test case on three machines, so it
mkdir temp_test_case
cd temp_test_case
git init
# my file. Sorry--couldn't find a saner link!
wget -O jquery-ui-1.8.custom.min.js
http://sourceforge.net/p/agency/code/ci/9358ea4dbe8e1540ec0b8bebfc7770f1bf8be0ec/tree/jquery-ui-1.8.custom.min.js?format=raw
git add jquery-ui-1.8.custom.min.js
git commit -m 'Adding jquery-ui'
git rm jquery-ui-1.8.custom.min.js
git commit -m 'Removing jquery-ui'
git format-patch HEAD~1
git reset --hard HEAD~1
git am 0001*

... this does not break at all for me.

Stefan Beller

2013-11-11 19:15:12 UTC

Post by Ken Tanzer
ASCII text, with very long lines, with CRLF, CR, LF line terminators

... this does not break at all for me.
--

Here it breaks, though it doesn't break when using
git am --ignore-whitespace 0001*

I do have this global config
core.safecrlf=warn
regarding line endings.

I was using 1.8.5.rc1.17.g0ecd94d

Trying to understand the problem,
Stefan

Junio C Hamano

2013-11-11 19:37:02 UTC

Post by Stefan Beller
I do have this global config
core.safecrlf=warn
regarding line endings.

Oh, that sounds very suspicious. If the payload has CRLF, CR and LF
mixed, that would immediately violate safecrlf, so failing the
application sounds like the right thing to do.

Post by Stefan Beller
I was using 1.8.5.rc1.17.g0ecd94d
Trying to understand the problem,

Likewise. Thanks for chiming in.

Stefan Beller

2013-11-11 19:43:43 UTC

Post by Stefan Beller
I do have this global config
core.safecrlf=warn
regarding line endings.

Oh, that sounds very suspicious. If the payload has CRLF, CR and LF
mixed, that would immediately violate safecrlf, so failing the
application sounds like the right thing to do.

Not having read the man page, but copied this global config
from some '1000 git tips in 5 minutes' years ago,
I do expect a setting set to "warn" to actually not change the
program flow except some inserted prints with warnings.

Post by Stefan Beller
I was using 1.8.5.rc1.17.g0ecd94d
Trying to understand the problem,

Likewise. Thanks for chiming in.

Looking at the file we have a mixture of LF/CR, LF only and CR only
ending the lines. The formatted patch does have the same file endings
on the respective line of that file here.

I was trying to change just one line ending to 2 lines (LF CR -> LF LF)
with a hex editor, so there it becomes a smaller (and more debugable )
patch. I also tried LF -> CR and CR -> LF, but none of these small
changes seem to work.

Stefan

Stefan Beller

2013-11-11 20:39:42 UTC

Post by Stefan Beller

Post by Stefan Beller
I do have this global config
core.safecrlf=warn
regarding line endings.

Oh, that sounds very suspicious. If the payload has CRLF, CR and LF
mixed, that would immediately violate safecrlf, so failing the
application sounds like the right thing to do.

Post by Stefan Beller
I was using 1.8.5.rc1.17.g0ecd94d
Trying to understand the problem,

Likewise. Thanks for chiming in.

Looking at the file we have a mixture of LF/CR, LF only and CR only
ending the lines. The formatted patch does have the same file endings
on the respective line of that file here.
I was trying to change just one line ending to 2 lines (LF CR -> LF LF)
with a hex editor, so there it becomes a smaller (and more debugable )
patch. I also tried LF -> CR and CR -> LF, but none of these small
changes seem to work.
Stefan

Just having looked at the CRLF conversion code of git,
and then at the file again. This file seems to be a special nasty kind,
as the number of LFs is equal to the number of CRs in the file
(373 of 0x0a and 0x0d each), but only 343 occurences of LF strictly
following a CR, so there are 30 CRs and 30 LFs.

This shouldn't be as problematic as my first thought was, there is
never a direct comparison of stats.cr to stats.lf in convert.c.
The CR and LF count are only compared to either the crlf count or 0.

Stefan

Ken Tanzer

2013-11-13 00:26:49 UTC

Post by Junio C Hamano
I am not very much surprised if such a file misbehaves, because the
"format-patch | am" pipeline is designed to be used on patches that
can be transferred in plain-text e-mail safely. Long lines should
probably be OK, but mixed CRLF, CR and LF may be problematic.

I'm not sure I understand this comment. format-patch seems to work
fine on binary files. So if it can handle any random collection of
bytes, why not text files with (admittedly funky) CRs and LFs?

Cheers,
Ken

Post by Ken Tanzer
ASCII text, with very long lines, with CRLF, CR, LF line terminators

... this does not break at all for me.

--
AGENCY Software
A data system that puts you in control
100% Free Software
http://agency-software.org/
***@agency-software.org
(253) 245-3801

Subscribe to the mailing list to
learn more about AGENCY or
follow the discussion.

Junio C Hamano

2013-11-13 17:04:47 UTC

I'm not sure I understand this comment. format-patch seems to work
fine on binary files.

Yes.

The problematic is when you tell it to process text files, taking
into account various common text breakages may be introduced to the
payload, and line-end conversion is among the operations that the
user may be telling it to munge the perfectly-fine input, in an
attempt to compensate.

Philip Oakley

2013-11-13 23:12:32 UTC

From: "Junio C Hamano" <***@pobox.com>
To: "Ken Tanzer" <***@gmail.com>
Sent: Wednesday, November 13, 2013 5:04 PM

I'm not sure I understand this comment. format-patch seems to work
fine on binary files.

Yes.
The problematic is when you tell it to process text files, taking
into account various common text breakages may be introduced to the
payload, and line-end conversion is among the operations that the
user may be telling it to munge the perfectly-fine input, in an
attempt to compensate.

Post by Junio C Hamano
I do have this global config
core.safecrlf=warn
regarding line endings.

Ken,

Do you have one of these line ending conversion settings active in your
config file?

It sounds like the 'git am' script may be silencing the warnings too
aggressively if that is the case.

Philip

Ken Tanzer

2013-11-14 07:26:35 UTC

Hi Philip. I don't have any setting like that in either my
~/.gitconfig, or in the .git/config files. I really haven't tweaked
my git config at all.

This is a typical .git/config file:

[core]
repositoryformatversion = 0
filemode = true
bare = false
logallrefupdates = true
[remote "origin"]
url = ssh://***@git.code.sf.net/p/agency/code
fetch = +refs/heads/*:refs/remotes/origin/*
[branch "master"]
remote = origin
merge = refs/heads/master

My ~/.gitconfig just has name and email settings in it.

Ken

Post by Philip Oakley
Sent: Wednesday, November 13, 2013 5:04 PM

I'm not sure I understand this comment. format-patch seems to work
fine on binary files.

Yes.
The problematic is when you tell it to process text files, taking
into account various common text breakages may be introduced to the
payload, and line-end conversion is among the operations that the
user may be telling it to munge the perfectly-fine input, in an
attempt to compensate.