core.autocrlf=true causes `git apply` to fail on patch generated with `git diff-index HEAD --patch`

Here's my minimal reproduction -- it's slightly far-fetched in that it
involves *committing crlf* and
then using `autocrlf=true` (commit lf, check out crlf).

#!/bin/bash
set -ex

rm -rf foo
git init foo
cd foo

# Commit crlf into repository
git config --local core.autocrlf false
python3 -c 'open("foo", "wb").write(b"1\r\n2\r\n")'
git add foo
git commit -m "Initial commit with crlf"

# Change whitespace mode to autocrlf, "commit lf, checkout crlf"
git config --local core.autocrlf true
python3 -c 'open("foo", "wb").write(b"1\r\n2\r\n\r\n\r\n\r\n")'

# Generate a patch, check it out, restore it
git diff --ignore-submodules --binary --no-color --no-ext-diff > patch
python3 -c 'print(open("patch", "rb").read())'
git checkout -- .
# I expect this to succeed, it fails
git apply patch

And here's the output:

+ rm -rf foo
+ git init foo
Initialized empty Git repository in /tmp/foo/.git/
+ cd foo
+ git config --local core.autocrlf false
+ python3 -c 'open("foo", "wb").write(b"1\r\n2\r\n")'
+ git add foo
+ git commit -m 'Initial commit with crlf'
[master (root-commit) 02d3246] Initial commit with crlf
 1 file changed, 2 insertions(+)
 create mode 100644 foo
+ git config --local core.autocrlf true
+ python3 -c 'open("foo", "wb").write(b"1\r\n2\r\n\r\n\r\n\r\n")'
+ git diff --ignore-submodules --binary --no-color --no-ext-diff
+ python3 -c 'print(open("patch", "rb").read())'
b'diff --git a/foo b/foo\nindex bd956ea..fbf7936 100644\n---
a/foo\n+++ b/foo\n@@ -1,2 +1,5 @@\n 1\r\n 2\r\n+\r\n+\r\n+\r\n'
+ git checkout -- .
+ git apply patch
patch:8: trailing whitespace.

patch:9: trailing whitespace.

patch:10: trailing whitespace.

error: patch failed: foo:1
error: foo: patch does not apply

I also tried with `git apply --ignore-whitespace`, but this causes the
line endings of the existing contents to be changed to *lf* (there may
be two bugs here?)

Thanks,

Anthony

On 08/01/2017 08:24 PM, Anthony Sottile wrote:

Here's my minimal reproduction -- it's slightly far-fetched in that it
involves*committing crlf*  and
then using `autocrlf=true` (commit lf, check out crlf).

#!/bin/bash
set -ex

rm -rf foo
git init foo
cd foo

# Commit crlf into repository
git config --local core.autocrlf false
python3 -c 'open("foo", "wb").write(b"1\r\n2\r\n")'
git add foo
git commit -m "Initial commit with crlf"

# Change whitespace mode to autocrlf, "commit lf, checkout crlf"
git config --local core.autocrlf true
python3 -c 'open("foo", "wb").write(b"1\r\n2\r\n\r\n\r\n\r\n")'

# Generate a patch, check it out, restore it
git diff --ignore-submodules --binary --no-color --no-ext-diff > patch
python3 -c 'print(open("patch", "rb").read())'
git checkout -- .
# I expect this to succeed, it fails
git apply patch

And here's the output:

+ rm -rf foo
+ git init foo
Initialized empty Git repository in/tmp/foo/.git/
+ cd foo
+ git config --local core.autocrlf false
+ python3 -c 'open("foo", "wb").write(b"1\r\n2\r\n")'
+ git add foo
+ git commit -m 'Initial commit with crlf'
[master (root-commit) 02d3246] Initial commit with crlf
  1 file changed, 2 insertions(+)
  create mode 100644 foo
+ git config --local core.autocrlf true
+ python3 -c 'open("foo", "wb").write(b"1\r\n2\r\n\r\n\r\n\r\n")'
+ git diff --ignore-submodules --binary --no-color --no-ext-diff
+ python3 -c 'print(open("patch", "rb").read())'
b'diff --git a/foo b/foo\nindex bd956ea..fbf7936 100644\n---
a/foo\n+++ b/foo\n@@ -1,2 +1,5 @@\n 1\r\n 2\r\n+\r\n+\r\n+\r\n'
+ git checkout -- .
+ git apply patch
patch:8: trailing whitespace.

patch:9: trailing whitespace.

patch:10: trailing whitespace.

error: patch failed: foo:1
error: foo: patch does not apply

I also tried with `git apply --ignore-whitespace`, but this causes the
line endings of the existing contents to be changed to*lf*  (there may
be two bugs here?)

Thanks,

Anthony

I can reproduce you test case here.

The line

git apply patch

would succeed, if you temporally (for the runtime of the apply command) set
core.autocrlf to false:

git -c core.autocrlf=false apply patch

So this seems to be a bug (in a corner case ?):

Typically repos which had been commited with CRLF should be normalized,
which means that the CRLF in the repo are replaced by LF.
So you test script is a corner case, for which Git has not been designed,
It seems as if "git apply" gets things wrong here.
Especially, as the '\r' is not a whitespace as a white space. but part
of the line ending.
So in my understanding the "--ignore-whitespace" option shouldn't affect
the line endings at all.

Fixes are possible, does anyone have a clue, why the '\r' is handled
like this in apply ?

And out of interest: is this a real life problem ?



  

Here's where I'm hitting the problem described:
https://github.com/pre-commit/pre-commit/issues/570

Note that `git -c core.autocrlf=false` apply patch fixes this
situation, but breaks others.

Here's a testcase where `git -c core.autocrlf=false apply patch`
causes a *different* patch failure:

#!/bin/bash
set -ex

rm -rf foo
git init foo
cd foo

git config --local core.autocrlf true

# Commit lf into repository
python3 -c 'open("foo", "wb").write(b"1\r\n2\r\n")'
git add foo
python3 -c 'open("foo", "wb").write(b"3\n4\n")'

# Generate a patch, check it out, restore it
git diff --ignore-submodules --binary --no-color --no-ext-diff > patch
python3 -c 'print(open("patch", "rb").read())'
git checkout -- .
git -c core.autocrlf=false apply patch

output:

+ rm -rf foo
+ git init foo
Initialized empty Git repository in /tmp/foo/.git/
+ cd foo
+ git config --local core.autocrlf true
+ python3 -c 'open("foo", "wb").write(b"1\r\n2\r\n")'
+ git add foo
+ python3 -c 'open("foo", "wb").write(b"3\n4\n")'
+ git diff --ignore-submodules --binary --no-color --no-ext-diff
warning: LF will be replaced by CRLF in foo.
The file will have its original line endings in your working directory.
+ python3 -c 'print(open("patch", "rb").read())'
b'diff --git a/foo b/foo\nindex 1191247..b944734 100644\n---
a/foo\n+++ b/foo\n@@ -1,2 +1,2 @@\n-1\n-2\n+3\n+4\n'
+ git checkout -- .
+ git -c core.autocrlf=false apply patch
error: patch failed: foo:1

My current workaround is:
- try `git apply patch`
- try `git -c core.autocrlf=false apply patch`

which seems to work pretty well.

Anthony


On Tue, Aug 1, 2017 at 1:47 PM, Torsten Bögershausen [off-list ref] wrote:

On 08/01/2017 08:24 PM, Anthony Sottile wrote:

quoted

Here's my minimal reproduction -- it's slightly far-fetched in that it
involves*committing crlf*  and

then using `autocrlf=true` (commit lf, check out crlf).

#!/bin/bash
set -ex

rm -rf foo
git init foo
cd foo

# Commit crlf into repository
git config --local core.autocrlf false
python3 -c 'open("foo", "wb").write(b"1\r\n2\r\n")'
git add foo
git commit -m "Initial commit with crlf"

# Change whitespace mode to autocrlf, "commit lf, checkout crlf"
git config --local core.autocrlf true
python3 -c 'open("foo", "wb").write(b"1\r\n2\r\n\r\n\r\n\r\n")'

# Generate a patch, check it out, restore it
git diff --ignore-submodules --binary --no-color --no-ext-diff > patch
python3 -c 'print(open("patch", "rb").read())'
git checkout -- .
# I expect this to succeed, it fails
git apply patch

And here's the output:

+ rm -rf foo
+ git init foo
Initialized empty Git repository in/tmp/foo/.git/
+ cd foo
+ git config --local core.autocrlf false
+ python3 -c 'open("foo", "wb").write(b"1\r\n2\r\n")'
+ git add foo
+ git commit -m 'Initial commit with crlf'
[master (root-commit) 02d3246] Initial commit with crlf
  1 file changed, 2 insertions(+)
  create mode 100644 foo
+ git config --local core.autocrlf true
+ python3 -c 'open("foo", "wb").write(b"1\r\n2\r\n\r\n\r\n\r\n")'
+ git diff --ignore-submodules --binary --no-color --no-ext-diff
+ python3 -c 'print(open("patch", "rb").read())'
b'diff --git a/foo b/foo\nindex bd956ea..fbf7936 100644\n---
a/foo\n+++ b/foo\n@@ -1,2 +1,5 @@\n 1\r\n 2\r\n+\r\n+\r\n+\r\n'
+ git checkout -- .
+ git apply patch
patch:8: trailing whitespace.

patch:9: trailing whitespace.

patch:10: trailing whitespace.

error: patch failed: foo:1
error: foo: patch does not apply

I also tried with `git apply --ignore-whitespace`, but this causes the
line endings of the existing contents to be changed to*lf*  (there may
be two bugs here?)

Thanks,

Anthony

I can reproduce you test case here.

The line

git apply patch

would succeed, if you temporally (for the runtime of the apply command) set
core.autocrlf to false:

git -c core.autocrlf=false apply patch

So this seems to be a bug (in a corner case ?):

Typically repos which had been commited with CRLF should be normalized,
which means that the CRLF in the repo are replaced by LF.
So you test script is a corner case, for which Git has not been designed,
It seems as if "git apply" gets things wrong here.
Especially, as the '\r' is not a whitespace as a white space. but part
of the line ending.
So in my understanding the "--ignore-whitespace" option shouldn't affect
the line endings at all.

Fixes are possible, does anyone have a clue, why the '\r' is handled
like this in apply ?

And out of interest: is this a real life problem ?

On 08/01/2017 10:58 PM, Anthony Sottile wrote:

Here's where I'm hitting the problem described:
https://github.com/pre-commit/pre-commit/issues/570

Note that `git -c core.autocrlf=false` apply patch fixes this
situation, but breaks others.

[]
I wasn't thinking of that - and thanks for the detailed report.
I seems as there are 3 things to be done:
- Make a workaround in your scripts/tools. It seems as if that is 
already done.
- Fix Git.
   My very first investigation shows that a patch like this could fix 
the problem:

diff --git a/apply.c b/apply.c
index f2d599141d..66b8387360 100644
--- a/apply.c
+++ b/apply.c

@@ -2278,6 +2278,8 @@ static int read_old_data(struct stat *st, const 

char *path, struct strbuf *buf)
         case S_IFREG:
                 if (strbuf_read_file(buf, path, st->st_size) != 
st->st_size)
                         return error(_("unable to open or read %s"), path);
+               if (would_convert_to_git(&the_index, path))
+                       read_cache();
                 convert_to_git(&the_index, path, buf->buf, buf->len, 
buf, 0);
                 return 0;
         default:
----------------
   I will probably do some more investigations if this is the core 
problem, so it will take some days or weeks.

- Convince people to normalize their repos and convert all CRLF in the 
repo into LF.
    This may take even longer.

From: Torsten Bögershausen <redacted>

git apply does not find the source lines when files have CRLF in the index
and core.autocrlf is true:
These files should not get the CRLF converted to LF. Because cmd_apply()
does not load the index, this does not work, CRLF are converted into LF
and apply fails.

Fix this in the spirit of commit a08feb8ef0b6,
"correct blame for files commited with CRLF" by loading the index.

As an optimization, skip read_cache() when no conversion is specified
for this path.

Reported-by: Anthony Sottile <redacted>
Signed-off-by: Torsten Bögershausen <redacted>
---
 apply.c         |  2 ++
 t/t0020-crlf.sh | 12 ++++++++++++
 2 files changed, 14 insertions(+)

diff --git a/apply.c b/apply.c
index f2d599141d..66b8387360 100644
--- a/apply.c
+++ b/apply.c

@@ -2278,6 +2278,8 @@ static int read_old_data(struct stat *st, const char *path, struct strbuf *buf)
 	case S_IFREG:
 		if (strbuf_read_file(buf, path, st->st_size) != st->st_size)
 			return error(_("unable to open or read %s"), path);
+		if (would_convert_to_git(&the_index, path))
+			read_cache();
 		convert_to_git(&the_index, path, buf->buf, buf->len, buf, 0);
 		return 0;
 	default:

diff --git a/t/t0020-crlf.sh b/t/t0020-crlf.sh
index 71350e0657..6611f8a6f6 100755
--- a/t/t0020-crlf.sh
+++ b/t/t0020-crlf.sh

@@ -386,4 +386,16 @@ test_expect_success 'New CRLF file gets LF in repo' '
 	test_cmp alllf alllf2
 '
 
+test_expect_success 'CRLF in repo, apply with autocrlf=true' '
+	git config core.autocrlf false &&
+	printf "1\r\n2\r\n" >crlf &&
+	git add crlf &&
+	git commit -m "commit crlf with crlf" &&
+	git config core.autocrlf true &&
+	printf "1\r\n2\r\n\r\n\r\n\r\n" >crlf &&
+	git diff >patch &&
+	git checkout -- . &&
+	git apply patch
+'
+
 test_done

-- 
2.13.2.533.ge0aaa1b