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

[Torsten Bögershausen]

>I left it unsaid by mistake, but I think the right thing to use as
>the "previous" to take hint from in the context of "git apply" is
>what is in the working tree, i.e. the result of applying patch in
>step (4) to create a file F in the sample scenario.

>While applying patch in step (5), convert_to_git() should "imagine"
>adding the file F currently in the working tree (i.e. the result of
>step (4)) to the index---if the resulting object in the index would
>have CR, then the safe CRLF logic should refrain from doing CRLF->LF
>conversion.  And it should do so without actually adding neither the
>preimage or the postimage to the index, of course.

(Sorry for the test mail)

What I wanted to say is that this long explanation convinced me to write
a patch and send it out the next days,

>When we are doing "git apply --index", then we _require_ that the
>indexed contents and what is in the working tree matches before
>applying the patch, so it is perfectly fine to let convert_to_git()
>to look at the current index---that is the "previous" one we want to
>take hint from while using the safe CRLF logic.

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

[Torsten Bögershausen]

Test from mutt

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

[Junio C Hamano]

Torsten Bögershausen  writes:

>   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(_index, path))
> +   read_cache();
> convert_to_git(_index, path, buf->buf, buf->len,
> buf, 0);

Sorry, but it is unclear why this is a "fix" to me.  We may not even
be doing "git apply --index" or "git apply --cached" that care about
the state recorded in the index, but just the plain vanilla "git apply",
which could even be outside any repository.

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

[Torsten Bögershausen]

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(_index, path))
+   read_cache();
convert_to_git(_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.

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

[Anthony Sottile]

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  wrote:
>
>
> 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 ?
>
>
>
>
>

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

[Torsten Bögershausen]

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 ?



 

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

[Anthony Sottile]

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