All of lore.kernel.org
 help / color / mirror / Atom feed
From: Anthony Sottile <asottile@umich.edu>
To: "Torsten Bögershausen" <tboegi@web.de>
Cc: Git Mailing List <git@vger.kernel.org>
Subject: Re: core.autocrlf=true causes `git apply` to fail on patch generated with `git diff-index HEAD --patch`
Date: Tue, 1 Aug 2017 13:58:32 -0700	[thread overview]
Message-ID: <CA+dzEB=3OMw_YM4K_a8dyDG_FwGavU382stXrEOkbYoyM4DSZQ@mail.gmail.com> (raw)
In-Reply-To: <287407ac-b0d0-ef24-4950-0982a2db9bed@web.de>

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 <tboegi@web.de> 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 ?
>
>
>
>
>

  reply	other threads:[~2017-08-01 20:59 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-08-01 18:24 core.autocrlf=true causes `git apply` to fail on patch generated with `git diff-index HEAD --patch` Anthony Sottile
2017-08-01 20:47 ` Torsten Bögershausen
2017-08-01 20:58   ` Anthony Sottile [this message]
2017-08-02 15:44     ` Torsten Bögershausen
2017-08-02 20:42       ` [PATCH v1 1/1] correct apply for files commited with CRLF tboegi
2017-08-02 21:13         ` Junio C Hamano
2017-08-04 17:31           ` Torsten Bögershausen
2017-08-04 17:57             ` Junio C Hamano
2017-08-04 19:26               ` Junio C Hamano
2017-08-02 20:58       ` core.autocrlf=true causes `git apply` to fail on patch generated with `git diff-index HEAD --patch` Junio C Hamano
2017-08-12  5:45         ` Torsten Bögershausen
2017-08-12  5:53           ` Torsten Bögershausen
2017-08-12 14:56         ` [PATCH/RFC] convert: Add SAFE_CRLF_KEEP_CRLF tboegi
2017-08-12 14:56         ` [PATCH/RFC] File commited with CRLF should roundtrip diff and apply tboegi
2017-08-14 17:37           ` Junio C Hamano
2017-08-17  6:06             ` [PATCH v2 1/2] convert: Add SAFE_CRLF_KEEP_CRLF tboegi
2017-08-17  6:06             ` [PATCH v2 2/2] File commited with CRLF should roundtrip diff and apply tboegi
2017-08-17  6:37               ` Junio C Hamano
2017-08-17 21:43             ` [PATCH v3 1/2] convert: Add SAFE_CRLF_KEEP_CRLF tboegi
2017-08-17 21:43             ` [PATCH v3 2/2] File commited with CRLF should roundtrip diff and apply tboegi
2017-08-17 22:29               ` Junio C Hamano
2017-08-17 22:35               ` Junio C Hamano
2017-08-19 11:27             ` [PATCH v4 1/2] convert: Add SAFE_CRLF_KEEP_CRLF tboegi
2017-08-19 11:28             ` [PATCH v4 2/2] File commited with CRLF should roundtrip diff and apply tboegi
2017-08-13  8:51         ` [PATCH/RFC 1/2] convert: Add SAFE_CRLF_KEEP_CRLF tboegi
2017-08-13  8:51         ` [PATCH/RFC 2/2] File commited with CRLF should roundtrip diff and apply tboegi
2017-08-16 18:28           ` Junio C Hamano
2017-08-16 18:28           ` [PATCH/FIXUP 3/2] apply: remove unused member apply_state.flags Junio C Hamano
2017-08-16 18:29           ` [PATCH/FIXUP 4/2] apply: only pay attention to CRLF in the preimage Junio C Hamano
2017-08-16 18:30           ` [PATCH/FIXUP 5/2] apply: localize the CRLF business to read_old_data() Junio C Hamano
2017-08-16 18:34           ` [PATCH/FIXUP 6/2] apply: clarify read_old_data() is about no-index case Junio C Hamano
2017-08-17  6:24             ` Torsten Bögershausen
2017-08-17  7:06               ` Junio C Hamano
2017-08-17  7:12               ` Junio C Hamano
2017-08-17  8:24                 ` Torsten Bögershausen
2017-08-17 17:07                 ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CA+dzEB=3OMw_YM4K_a8dyDG_FwGavU382stXrEOkbYoyM4DSZQ@mail.gmail.com' \
    --to=asottile@umich.edu \
    --cc=git@vger.kernel.org \
    --cc=tboegi@web.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.