{"thread":{"id":"46495","subject":"core.autocrlf=true causes `git apply` to fail on patch generated with `git diff-index HEAD --patch`","startedAt":"2017-08-01T18:24:10Z","lastAt":"2017-08-19T11:28:16Z","messageCount":36,"participants":["Anthony Sottile","Torsten Bögershausen","tboegi@web.de","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"325393","messageId":"CA+dzEBmsgUjmf5fUmeiwS=Q81OgpL6K5p=8dBuTjuZ4XE1V5SA@mail.gmail.com","threadId":"46495","inReplyTo":null,"subject":"core.autocrlf=true causes `git apply` to fail on patch generated with `git diff-index HEAD --patch`","fromName":"Anthony Sottile","fromEmail":"asottile@umich.edu","sentAt":"2017-08-01T18:24:01Z","receivedAt":"2017-08-01T18:24:10Z","isPatch":false,"sender":{"key":"asottile@umich.edu","avatar":"https://avatars.githubusercontent.com/u/1810591?v=4"},"body":"Here's my minimal reproduction -- it's slightly far-fetched in that it\ninvolves *committing crlf* and\nthen using `autocrlf=true` (commit lf, check out crlf).\n\n```\n#!/bin/bash\nset -ex\n\nrm -rf foo\ngit init foo\ncd foo\n\n# Commit crlf into repository\ngit config --local core.autocrlf false\npython3 -c 'open(\"foo\", \"wb\").write(b\"1\\r\\n2\\r\\n\")'\ngit add foo\ngit commit -m \"Initial commit with crlf\"\n\n# Change whitespace mode to autocrlf, \"commit lf, checkout crlf\"\ngit config --local core.autocrlf true\npython3 -c 'open(\"foo\", \"wb\").write(b\"1\\r\\n2\\r\\n\\r\\n\\r\\n\\r\\n\")'\n\n# Generate a patch, check it out, restore it\ngit diff --ignore-submodules --binary --no-color --no-ext-diff > patch\npython3 -c 'print(open(\"patch\", \"rb\").read())'\ngit checkout -- .\n# I expect this to succeed, it fails\ngit apply patch\n```\n\nAnd here's the output:\n\n```\n+ rm -rf foo\n+ git init foo\nInitialized empty Git repository in /tmp/foo/.git/\n+ cd foo\n+ git config --local core.autocrlf false\n+ python3 -c 'open(\"foo\", \"wb\").write(b\"1\\r\\n2\\r\\n\")'\n+ git add foo\n+ git commit -m 'Initial commit with crlf'\n[master (root-commit) 02d3246] Initial commit with crlf\n 1 file changed, 2 insertions(+)\n create mode 100644 foo\n+ git config --local core.autocrlf true\n+ python3 -c 'open(\"foo\", \"wb\").write(b\"1\\r\\n2\\r\\n\\r\\n\\r\\n\\r\\n\")'\n+ git diff --ignore-submodules --binary --no-color --no-ext-diff\n+ python3 -c 'print(open(\"patch\", \"rb\").read())'\nb'diff --git a/foo b/foo\\nindex bd956ea..fbf7936 100644\\n---\na/foo\\n+++ b/foo\\n@@ -1,2 +1,5 @@\\n 1\\r\\n 2\\r\\n+\\r\\n+\\r\\n+\\r\\n'\n+ git checkout -- .\n+ git apply patch\npatch:8: trailing whitespace.\n\npatch:9: trailing whitespace.\n\npatch:10: trailing whitespace.\n\nerror: patch failed: foo:1\nerror: foo: patch does not apply\n```\n\nI also tried with `git apply --ignore-whitespace`, but this causes the\nline endings of the existing contents to be changed to *lf* (there may\nbe two bugs here?)\n\nThanks,\n\nAnthony\n"},{"id":"325403","messageId":"287407ac-b0d0-ef24-4950-0982a2db9bed@web.de","threadId":"46495","inReplyTo":"CA+dzEBmsgUjmf5fUmeiwS=Q81OgpL6K5p=8dBuTjuZ4XE1V5SA@mail.gmail.com","subject":"Re: core.autocrlf=true causes `git apply` to fail on patch generated with `git diff-index HEAD --patch`","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2017-08-01T20:47:20Z","receivedAt":"2017-08-01T20:47:44Z","isPatch":false,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"\n\nOn 08/01/2017 08:24 PM, Anthony Sottile wrote:\n> Here's my minimal reproduction -- it's slightly far-fetched in that it\n> involves*committing crlf* and\n> then using `autocrlf=true` (commit lf, check out crlf).\n>\n> ```\n> #!/bin/bash\n> set -ex\n>\n> rm -rf foo\n> git init foo\n> cd foo\n>\n> # Commit crlf into repository\n> git config --local core.autocrlf false\n> python3 -c 'open(\"foo\", \"wb\").write(b\"1\\r\\n2\\r\\n\")'\n> git add foo\n> git commit -m \"Initial commit with crlf\"\n>\n> # Change whitespace mode to autocrlf, \"commit lf, checkout crlf\"\n> git config --local core.autocrlf true\n> python3 -c 'open(\"foo\", \"wb\").write(b\"1\\r\\n2\\r\\n\\r\\n\\r\\n\\r\\n\")'\n>\n> # Generate a patch, check it out, restore it\n> git diff --ignore-submodules --binary --no-color --no-ext-diff > patch\n> python3 -c 'print(open(\"patch\", \"rb\").read())'\n> git checkout -- .\n> # I expect this to succeed, it fails\n> git apply patch\n> ```\n>\n> And here's the output:\n>\n> ```\n> + rm -rf foo\n> + git init foo\n> Initialized empty Git repository in/tmp/foo/.git/\n> + cd foo\n> + git config --local core.autocrlf false\n> + python3 -c 'open(\"foo\", \"wb\").write(b\"1\\r\\n2\\r\\n\")'\n> + git add foo\n> + git commit -m 'Initial commit with crlf'\n> [master (root-commit) 02d3246] Initial commit with crlf\n> 1 file changed, 2 insertions(+)\n> create mode 100644 foo\n> + git config --local core.autocrlf true\n> + python3 -c 'open(\"foo\", \"wb\").write(b\"1\\r\\n2\\r\\n\\r\\n\\r\\n\\r\\n\")'\n> + git diff --ignore-submodules --binary --no-color --no-ext-diff\n> + python3 -c 'print(open(\"patch\", \"rb\").read())'\n> b'diff --git a/foo b/foo\\nindex bd956ea..fbf7936 100644\\n---\n> a/foo\\n+++ b/foo\\n@@ -1,2 +1,5 @@\\n 1\\r\\n 2\\r\\n+\\r\\n+\\r\\n+\\r\\n'\n> + git checkout -- .\n> + git apply patch\n> patch:8: trailing whitespace.\n>\n> patch:9: trailing whitespace.\n>\n> patch:10: trailing whitespace.\n>\n> error: patch failed: foo:1\n> error: foo: patch does not apply\n> ```\n>\n> I also tried with `git apply --ignore-whitespace`, but this causes the\n> line endings of the existing contents to be changed to*lf* (there may\n> be two bugs here?)\n>\n> Thanks,\n>\n> Anthony\n\n\nI can reproduce you test case here.\n\nThe line\n\ngit apply patch\n\nwould succeed, if you temporally (for the runtime of the apply command) set\ncore.autocrlf to false:\n\ngit -c core.autocrlf=false apply patch\n\nSo this seems to be a bug (in a corner case ?):\n\nTypically repos which had been commited with CRLF should be normalized,\nwhich means that the CRLF in the repo are replaced by LF.\nSo you test script is a corner case, for which Git has not been designed,\nIt seems as if \"git apply\" gets things wrong here.\nEspecially, as the '\\r' is not a whitespace as a white space. but part\nof the line ending.\nSo in my understanding the \"--ignore-whitespace\" option shouldn't affect\nthe line endings at all.\n\nFixes are possible, does anyone have a clue, why the '\\r' is handled\nlike this in apply ?\n\nAnd out of interest: is this a real life problem ?\n\n\n\n \n\n\n"},{"id":"325406","messageId":"CA+dzEB=3OMw_YM4K_a8dyDG_FwGavU382stXrEOkbYoyM4DSZQ@mail.gmail.com","threadId":"46495","inReplyTo":"287407ac-b0d0-ef24-4950-0982a2db9bed@web.de","subject":"Re: core.autocrlf=true causes `git apply` to fail on patch generated with `git diff-index HEAD --patch`","fromName":"Anthony Sottile","fromEmail":"asottile@umich.edu","sentAt":"2017-08-01T20:58:32Z","receivedAt":"2017-08-01T20:59:16Z","isPatch":false,"sender":{"key":"asottile@umich.edu","avatar":"https://avatars.githubusercontent.com/u/1810591?v=4"},"body":"Here's where I'm hitting the problem described:\nhttps://github.com/pre-commit/pre-commit/issues/570\n\nNote that `git -c core.autocrlf=false` apply patch fixes this\nsituation, but breaks others.\n\nHere's a testcase where `git -c core.autocrlf=false apply patch`\ncauses a *different* patch failure:\n\n```\n#!/bin/bash\nset -ex\n\nrm -rf foo\ngit init foo\ncd foo\n\ngit config --local core.autocrlf true\n\n# Commit lf into repository\npython3 -c 'open(\"foo\", \"wb\").write(b\"1\\r\\n2\\r\\n\")'\ngit add foo\npython3 -c 'open(\"foo\", \"wb\").write(b\"3\\n4\\n\")'\n\n# Generate a patch, check it out, restore it\ngit diff --ignore-submodules --binary --no-color --no-ext-diff > patch\npython3 -c 'print(open(\"patch\", \"rb\").read())'\ngit checkout -- .\ngit -c core.autocrlf=false apply patch\n```\n\noutput:\n\n```\n+ rm -rf foo\n+ git init foo\nInitialized empty Git repository in /tmp/foo/.git/\n+ cd foo\n+ git config --local core.autocrlf true\n+ python3 -c 'open(\"foo\", \"wb\").write(b\"1\\r\\n2\\r\\n\")'\n+ git add foo\n+ python3 -c 'open(\"foo\", \"wb\").write(b\"3\\n4\\n\")'\n+ git diff --ignore-submodules --binary --no-color --no-ext-diff\nwarning: LF will be replaced by CRLF in foo.\nThe file will have its original line endings in your working directory.\n+ python3 -c 'print(open(\"patch\", \"rb\").read())'\nb'diff --git a/foo b/foo\\nindex 1191247..b944734 100644\\n---\na/foo\\n+++ b/foo\\n@@ -1,2 +1,2 @@\\n-1\\n-2\\n+3\\n+4\\n'\n+ git checkout -- .\n+ git -c core.autocrlf=false apply patch\nerror: patch failed: foo:1\n```\n\nMy current workaround is:\n- try `git apply patch`\n- try `git -c core.autocrlf=false apply patch`\n\nwhich seems to work pretty well.\n\nAnthony\n\n\nOn Tue, Aug 1, 2017 at 1:47 PM, Torsten Bögershausen wrote:\n>\n>\n> On 08/01/2017 08:24 PM, Anthony Sottile wrote:\n>>\n>> Here's my minimal reproduction -- it's slightly far-fetched in that it\n>> involves*committing crlf* and\n>>\n>> then using `autocrlf=true` (commit lf, check out crlf).\n>>\n>> ```\n>> #!/bin/bash\n>> set -ex\n>>\n>> rm -rf foo\n>> git init foo\n>> cd foo\n>>\n>> # Commit crlf into repository\n>> git config --local core.autocrlf false\n>> python3 -c 'open(\"foo\", \"wb\").write(b\"1\\r\\n2\\r\\n\")'\n>> git add foo\n>> git commit -m \"Initial commit with crlf\"\n>>\n>> # Change whitespace mode to autocrlf, \"commit lf, checkout crlf\"\n>> git config --local core.autocrlf true\n>> python3 -c 'open(\"foo\", \"wb\").write(b\"1\\r\\n2\\r\\n\\r\\n\\r\\n\\r\\n\")'\n>>\n>> # Generate a patch, check it out, restore it\n>> git diff --ignore-submodules --binary --no-color --no-ext-diff > patch\n>> python3 -c 'print(open(\"patch\", \"rb\").read())'\n>> git checkout -- .\n>> # I expect this to succeed, it fails\n>> git apply patch\n>> ```\n>>\n>> And here's the output:\n>>\n>> ```\n>> + rm -rf foo\n>> + git init foo\n>> Initialized empty Git repository in/tmp/foo/.git/\n>> + cd foo\n>> + git config --local core.autocrlf false\n>> + python3 -c 'open(\"foo\", \"wb\").write(b\"1\\r\\n2\\r\\n\")'\n>> + git add foo\n>> + git commit -m 'Initial commit with crlf'\n>> [master (root-commit) 02d3246] Initial commit with crlf\n>> 1 file changed, 2 insertions(+)\n>> create mode 100644 foo\n>> + git config --local core.autocrlf true\n>> + python3 -c 'open(\"foo\", \"wb\").write(b\"1\\r\\n2\\r\\n\\r\\n\\r\\n\\r\\n\")'\n>> + git diff --ignore-submodules --binary --no-color --no-ext-diff\n>> + python3 -c 'print(open(\"patch\", \"rb\").read())'\n>> b'diff --git a/foo b/foo\\nindex bd956ea..fbf7936 100644\\n---\n>> a/foo\\n+++ b/foo\\n@@ -1,2 +1,5 @@\\n 1\\r\\n 2\\r\\n+\\r\\n+\\r\\n+\\r\\n'\n>> + git checkout -- .\n>> + git apply patch\n>> patch:8: trailing whitespace.\n>>\n>> patch:9: trailing whitespace.\n>>\n>> patch:10: trailing whitespace.\n>>\n>> error: patch failed: foo:1\n>> error: foo: patch does not apply\n>> ```\n>>\n>> I also tried with `git apply --ignore-whitespace`, but this causes the\n>> line endings of the existing contents to be changed to*lf* (there may\n>> be two bugs here?)\n>>\n>> Thanks,\n>>\n>> Anthony\n>\n>\n>\n> I can reproduce you test case here.\n>\n> The line\n>\n> git apply patch\n>\n> would succeed, if you temporally (for the runtime of the apply command) set\n> core.autocrlf to false:\n>\n> git -c core.autocrlf=false apply patch\n>\n> So this seems to be a bug (in a corner case ?):\n>\n> Typically repos which had been commited with CRLF should be normalized,\n> which means that the CRLF in the repo are replaced by LF.\n> So you test script is a corner case, for which Git has not been designed,\n> It seems as if \"git apply\" gets things wrong here.\n> Especially, as the '\\r' is not a whitespace as a white space. but part\n> of the line ending.\n> So in my understanding the \"--ignore-whitespace\" option shouldn't affect\n> the line endings at all.\n>\n> Fixes are possible, does anyone have a clue, why the '\\r' is handled\n> like this in apply ?\n>\n> And out of interest: is this a real life problem ?\n>\n>\n>\n>\n>\n"},{"id":"325434","messageId":"cceaf377-73e0-e733-20ba-c4e547226382@web.de","threadId":"46495","inReplyTo":"CA+dzEB=3OMw_YM4K_a8dyDG_FwGavU382stXrEOkbYoyM4DSZQ@mail.gmail.com","subject":"Re: core.autocrlf=true causes `git apply` to fail on patch generated with `git diff-index HEAD --patch`","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2017-08-02T15:44:50Z","receivedAt":"2017-08-02T15:45:11Z","isPatch":false,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"\n\nOn 08/01/2017 10:58 PM, Anthony Sottile wrote:\n> Here's where I'm hitting the problem described:\n> https://github.com/pre-commit/pre-commit/issues/570\n>\n> Note that `git -c core.autocrlf=false` apply patch fixes this\n> situation, but breaks others.\n\n[]\nI wasn't thinking of that - and thanks for the detailed report.\nI seems as there are 3 things to be done:\n- Make a workaround in your scripts/tools. It seems as if that is \nalready done.\n- Fix Git.\n My very first investigation shows that a patch like this could fix \nthe problem:\n\ndiff --git a/apply.c b/apply.c\nindex f2d599141d..66b8387360 100644\n--- a/apply.c\n+++ b/apply.c\n@@ -2278,6 +2278,8 @@ static int read_old_data(struct stat *st, const \nchar *path, struct strbuf *buf)\n case S_IFREG:\n if (strbuf_read_file(buf, path, st->st_size) != \nst->st_size)\n return error(_(\"unable to open or read %s\"), path);\n+ if (would_convert_to_git(&the_index, path))\n+ read_cache();\n convert_to_git(&the_index, path, buf->buf, buf->len, \nbuf, 0);\n return 0;\n default:\n----------------\n I will probably do some more investigations if this is the core \nproblem, so it will take some days or weeks.\n\n- Convince people to normalize their repos and convert all CRLF in the \nrepo into LF.\n This may take even longer.\n\n\n"},{"id":"325478","messageId":"20170802204203.29484-1-tboegi@web.de","threadId":"46495","inReplyTo":"cceaf377-73e0-e733-20ba-c4e547226382@web.de","subject":"[PATCH v1 1/1] correct apply for files commited with CRLF","fromName":"","fromEmail":"tboegi@web.de","sentAt":"2017-08-02T20:42:03Z","receivedAt":"2017-08-02T20:42:19Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"From: Torsten Bögershausen \n\ngit apply does not find the source lines when files have CRLF in the index\nand core.autocrlf is true:\nThese files should not get the CRLF converted to LF. Because cmd_apply()\ndoes not load the index, this does not work, CRLF are converted into LF\nand apply fails.\n\nFix this in the spirit of commit a08feb8ef0b6,\n\"correct blame for files commited with CRLF\" by loading the index.\n\nAs an optimization, skip read_cache() when no conversion is specified\nfor this path.\n\nReported-by: Anthony Sottile \nSigned-off-by: Torsten Bögershausen \n---\n apply.c | 2 ++\n t/t0020-crlf.sh | 12 ++++++++++++\n 2 files changed, 14 insertions(+)\n\ndiff --git a/apply.c b/apply.c\nindex f2d599141d..66b8387360 100644\n--- a/apply.c\n+++ b/apply.c\n@@ -2278,6 +2278,8 @@ static int read_old_data(struct stat *st, const char *path, struct strbuf *buf)\n \tcase S_IFREG:\n \t\tif (strbuf_read_file(buf, path, st->st_size) != st->st_size)\n \t\t\treturn error(_(\"unable to open or read %s\"), path);\n+\t\tif (would_convert_to_git(&the_index, path))\n+\t\t\tread_cache();\n \t\tconvert_to_git(&the_index, path, buf->buf, buf->len, buf, 0);\n \t\treturn 0;\n \tdefault:\ndiff --git a/t/t0020-crlf.sh b/t/t0020-crlf.sh\nindex 71350e0657..6611f8a6f6 100755\n--- a/t/t0020-crlf.sh\n+++ b/t/t0020-crlf.sh\n@@ -386,4 +386,16 @@ test_expect_success 'New CRLF file gets LF in repo' '\n \ttest_cmp alllf alllf2\n '\n \n+test_expect_success 'CRLF in repo, apply with autocrlf=true' '\n+\tgit config core.autocrlf false &&\n+\tprintf \"1\\r\\n2\\r\\n\" >crlf &&\n+\tgit add crlf &&\n+\tgit commit -m \"commit crlf with crlf\" &&\n+\tgit config core.autocrlf true &&\n+\tprintf \"1\\r\\n2\\r\\n\\r\\n\\r\\n\\r\\n\" >crlf &&\n+\tgit diff >patch &&\n+\tgit checkout -- . &&\n+\tgit apply patch\n+'\n+\n test_done\n-- \n2.13.2.533.ge0aaa1b\n\n"},{"id":"325480","messageId":"xmqq4ltpsn42.fsf@gitster.mtv.corp.google.com","threadId":"46495","inReplyTo":"cceaf377-73e0-e733-20ba-c4e547226382@web.de","subject":"Re: core.autocrlf=true causes `git apply` to fail on patch generated with `git diff-index HEAD --patch`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-08-02T20:58:37Z","receivedAt":"2017-08-02T20:58:52Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Torsten Bögershausen writes:\n\n> My very first investigation shows that a patch like this could fix\n> the problem:\n>\n> diff --git a/apply.c b/apply.c\n> index f2d599141d..66b8387360 100644\n> --- a/apply.c\n> +++ b/apply.c\n> @@ -2278,6 +2278,8 @@ static int read_old_data(struct stat *st, const\n> char *path, struct strbuf *buf)\n> case S_IFREG:\n> if (strbuf_read_file(buf, path, st->st_size) !=\n> st->st_size)\n> return error(_(\"unable to open or read %s\"), path);\n> + if (would_convert_to_git(&the_index, path))\n> + read_cache();\n> convert_to_git(&the_index, path, buf->buf, buf->len,\n> buf, 0);\n\nSorry, but it is unclear why this is a \"fix\" to me. We may not even\nbe doing \"git apply --index\" or \"git apply --cached\" that care about\nthe state recorded in the index, but just the plain vanilla \"git apply\",\nwhich could even be outside any repository.\n"},{"id":"325481","messageId":"xmqqwp6lr7u0.fsf@gitster.mtv.corp.google.com","threadId":"46495","inReplyTo":"20170802204203.29484-1-tboegi@web.de","subject":"Re: [PATCH v1 1/1] correct apply for files commited with CRLF","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-08-02T21:13:59Z","receivedAt":"2017-08-02T21:14:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"tboegi@web.de writes:\n\n> From: Torsten Bögershausen \n>\n> git apply does not find the source lines when files have CRLF in the index\n> and core.autocrlf is true:\n> These files should not get the CRLF converted to LF. Because cmd_apply()\n> does not load the index, this does not work, CRLF are converted into LF\n> and apply fails.\n>\n> Fix this in the spirit of commit a08feb8ef0b6,\n> \"correct blame for files commited with CRLF\" by loading the index.\n>\n> As an optimization, skip read_cache() when no conversion is specified\n> for this path.\n\nWhat happens when the input to apply is a patch to create a new file\nand the patch uses CRLF line endings? In such a case, shouldn't\ncore.autocrlf=true cause the patch to be converted to LF and then\nsucceed in applying? An extra read_cache() would not help as there\nmust be no existing index entry for the path in such a case.\n\nIf you did \"rm .git/index\" after you did the \"git checkout -- .\" in\nyour test patch, \"git apply\" ought to succeed, as it is working as\na substitute for \"patch -p1\" and should not be consulting the index\nat all.\n\nI cannot shake this feeling that it is convert_to_git() that needs\nto be fixed so that it does not need to refer to the current\ncontents in the index. Why does it even need to look at the index?\nIf it is for the \"safe CRLF\" thing (which I would think is misguided),\nshouldn't it be checking the original file in the working tree, not\nthe index? After all, that is what we are patching, not the contents\nstored in the index.\n\n>\n> Reported-by: Anthony Sottile \n> Signed-off-by: Torsten Bögershausen \n> ---\n> apply.c | 2 ++\n> t/t0020-crlf.sh | 12 ++++++++++++\n> 2 files changed, 14 insertions(+)\n>\n> diff --git a/apply.c b/apply.c\n> index f2d599141d..66b8387360 100644\n> --- a/apply.c\n> +++ b/apply.c\n> @@ -2278,6 +2278,8 @@ static int read_old_data(struct stat *st, const char *path, struct strbuf *buf)\n> \tcase S_IFREG:\n> \t\tif (strbuf_read_file(buf, path, st->st_size) != st->st_size)\n> \t\t\treturn error(_(\"unable to open or read %s\"), path);\n> +\t\tif (would_convert_to_git(&the_index, path))\n> +\t\t\tread_cache();\n> \t\tconvert_to_git(&the_index, path, buf->buf, buf->len, buf, 0);\n> \t\treturn 0;\n> \tdefault:\n> diff --git a/t/t0020-crlf.sh b/t/t0020-crlf.sh\n> index 71350e0657..6611f8a6f6 100755\n> --- a/t/t0020-crlf.sh\n> +++ b/t/t0020-crlf.sh\n> @@ -386,4 +386,16 @@ test_expect_success 'New CRLF file gets LF in repo' '\n> \ttest_cmp alllf alllf2\n> '\n> \n> +test_expect_success 'CRLF in repo, apply with autocrlf=true' '\n> +\tgit config core.autocrlf false &&\n> +\tprintf \"1\\r\\n2\\r\\n\" >crlf &&\n> +\tgit add crlf &&\n> +\tgit commit -m \"commit crlf with crlf\" &&\n> +\tgit config core.autocrlf true &&\n> +\tprintf \"1\\r\\n2\\r\\n\\r\\n\\r\\n\\r\\n\" >crlf &&\n> +\tgit diff >patch &&\n> +\tgit checkout -- . &&\n> +\tgit apply patch\n> +'\n> +\n> test_done\n"},{"id":"325615","messageId":"191dc9ea-6ad3-f754-59da-2075a6fd581e@web.de","threadId":"46495","inReplyTo":"xmqqwp6lr7u0.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v1 1/1] correct apply for files commited with CRLF","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2017-08-04T17:31:42Z","receivedAt":"2017-08-04T17:32:16Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"\n\nOn 08/02/2017 11:13 PM, Junio C Hamano wrote:\n> tboegi@web.de writes:\n>\n>> From: Torsten Bögershausen \n>>\n>> git apply does not find the source lines when files have CRLF in the index\n>> and core.autocrlf is true:\n>> These files should not get the CRLF converted to LF. Because cmd_apply()\n>> does not load the index, this does not work, CRLF are converted into LF\n>> and apply fails.\n>>\n>> Fix this in the spirit of commit a08feb8ef0b6,\n>> \"correct blame for files commited with CRLF\" by loading the index.\n>>\n>> As an optimization, skip read_cache() when no conversion is specified\n>> for this path.\n> What happens when the input to apply is a patch to create a new file\n> and the patch uses CRLF line endings? In such a case, shouldn't\n> core.autocrlf=true cause the patch to be converted to LF and then\n> succeed in applying? An extra read_cache() would not help as there\n> must be no existing index entry for the path in such a case.\n>\n> If you did \"rm .git/index\" after you did the \"git checkout -- .\" in\n> your test patch, \"git apply\" ought to succeed, as it is working as\n> a substitute for \"patch -p1\" and should not be consulting the index\n> at all.\n>\n> I cannot shake this feeling that it is convert_to_git() that needs\n> to be fixed so that it does not need to refer to the current\n> contents in the index. Why does it even need to look at the index?\n> If it is for the \"safe CRLF\" thing (which I would think is misguided),\n> shouldn't it be checking the original file in the working tree, not\n> the index? After all, that is what we are patching, not the contents\n> stored in the index.\nThanks for the review.\nMy understanding is that there are different things possible with `git \napply`:\nworking on files in the index (\"cached\") files and \"worktree\" files.\n\nIf we work on files in the index, the line endings must match for\nthe patch to apply, the patch/apply machinery is not forgiving\nmismatching line endings. At least not by default.\nThere is one exception: Use \"blank-at-eol\" to declare the CR of\nthe CRLF as a whitespace, and then tell git apply to ignore white space:\n`git apply --ignore-whitespace`\nMy feeling is that this is not self-explaining, but this is a different \nstory.\n\nBack to the fix, the read_old_data() from below works on the working tree,\nyes, but after convert_to_git().\nAnd that is why we need the index, to fix this very case.\n\nappy.c:\nstatic int load_patch_target(struct apply_state *state,\n struct strbuf *buf,\n const struct cache_entry *ce,\n struct stat *st,\n const char *name,\n unsigned expected_mode)\n{\n if (state->cached || state->check_index) {\n if (read_file_or_gitlink(ce, buf))\n return error(_(\"failed to read %s\"), name);\n } else if (name) {\n if (S_ISGITLINK(expected_mode)) {\n if (ce)\n return read_file_or_gitlink(ce, buf);\n else\n return SUBMODULE_PATCH_WITHOUT_INDEX;\n } else if (has_symlink_leading_path(name, strlen(name))) {\n return error(_(\"reading from '%s' beyond a symbolic link\"), \nname);\n } else {\n if (read_old_data(st, name, buf))\n return error(_(\"failed to read %s\"), name);\n }\n }\n return 0;\n}\n\n"},{"id":"325616","messageId":"xmqqini3tdur.fsf@gitster.mtv.corp.google.com","threadId":"46495","inReplyTo":"191dc9ea-6ad3-f754-59da-2075a6fd581e@web.de","subject":"Re: [PATCH v1 1/1] correct apply for files commited with CRLF","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-08-04T17:57:48Z","receivedAt":"2017-08-04T17:58:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Torsten Bögershausen writes:\n\n> Back to the fix, the read_old_data() from below works on the working tree,\n> yes, but after convert_to_git().\n> And that is why we need the index, to fix this very case.\n\nBut \"git apply\" (without \"--cached\" or \"--index\") is to work on the\nworking tree file only. The target file of the patch that is going\nto be modified does not even have to be in the index, i.e. it is\nperfectly fine to:\n\n (1) create a file F, commit, then modify that file;\n\n (2) take two patches out of that repository you did (1);\n\n (3) go to a Git repository that does not yet know about file F;\n\n (4) run \"git apply\" to process the first patch you took in (2),\n which will create file F; then\n\n (5) run \"git apply\" to process the second patch on top, which will\n modify file F.\n\nStep (4) will probably not cause too much issue, as the only thing\nwe make sure is \"Because the patch creates F, F does not exist in\nthe directory\", which would be the case.\n\nNow, when trying to apply the second patch at step (5), we may need\nto adjust for the broken line-ending convention, but you do not have\nany entry in the index for F to rely on.\n\nThat is why I am saying convert_to_git() that checks the index\ncontent is a wrong thing to use in this codepath. \n\nIt is OK to use it for \"git add\" and friends, as the intent of the\n(I'd say \"broken\") \"safe CRLF\" mechanism is to take a hint from the\n\"previous\" state to see if CRLF in the \"new\" content should be\nmunged, and the \"previous\" in the context of running \"git add\" _is_\nwhat is in the index.\n\nThe \"previous\" in the context of running \"git apply\" (which does not\nlook at the index) is _not_ what is in the index, on the other hand.\nInstead of \"struct index_state *\", convert_to_git() needs to be\nfixed to take something else that can be queried for the \"previous\"\nversion to use as a hint, if \"safe CRLF\" wants to work correctly.\n"},{"id":"325619","messageId":"xmqq7eyjt9qt.fsf@gitster.mtv.corp.google.com","threadId":"46495","inReplyTo":"xmqqini3tdur.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v1 1/1] correct apply for files commited with CRLF","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-08-04T19:26:34Z","receivedAt":"2017-08-04T19:26:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> The \"previous\" in the context of running \"git apply\" (which does not\n> look at the index) is _not_ what is in the index, on the other hand.\n> Instead of \"struct index_state *\", convert_to_git() needs to be\n> fixed to take something else that can be queried for the \"previous\"\n> version to use as a hint, if \"safe CRLF\" wants to work correctly.\n\nI left it unsaid by mistake, but I think the right thing to use as\nthe \"previous\" to take hint from in the context of \"git apply\" is\nwhat is in the working tree, i.e. the result of applying patch in\nstep (4) to create a file F in the sample scenario. \n\nWhile applying patch in step (5), convert_to_git() should \"imagine\"\nadding the file F currently in the working tree (i.e. the result of\nstep (4)) to the index---if the resulting object in the index would\nhave CR, then the safe CRLF logic should refrain from doing CRLF->LF\nconversion. And it should do so without actually adding neither the\npreimage or the postimage to the index, of course.\n\nWhen we are doing \"git apply --index\", then we _require_ that the\nindexed contents and what is in the working tree matches before\napplying the patch, so it is perfectly fine to let convert_to_git()\nto look at the current index---that is the \"previous\" one we want to\ntake hint from while using the safe CRLF logic.\n\n"},{"id":"326188","messageId":"20170812053944.GA21232@tor.lan","threadId":"46495","inReplyTo":"xmqq4ltpsn42.fsf@gitster.mtv.corp.google.com","subject":"Re: core.autocrlf=true causes `git apply` to fail on patch generated with `git diff-index HEAD --patch`","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2017-08-12T05:45:16Z","receivedAt":"2017-08-12T05:45:29Z","isPatch":false,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"Test from mutt\n"},{"id":"326189","messageId":"20170812055335.GB21232@tor.lan","threadId":"46495","inReplyTo":"20170812053944.GA21232@tor.lan","subject":"Re: core.autocrlf=true causes `git apply` to fail on patch generated with `git diff-index HEAD --patch`","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2017-08-12T05:53:35Z","receivedAt":"2017-08-12T05:53:47Z","isPatch":false,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"\n>I left it unsaid by mistake, but I think the right thing to use as\n>the \"previous\" to take hint from in the context of \"git apply\" is\n>what is in the working tree, i.e. the result of applying patch in\n>step (4) to create a file F in the sample scenario.\n\n>While applying patch in step (5), convert_to_git() should \"imagine\"\n>adding the file F currently in the working tree (i.e. the result of\n>step (4)) to the index---if the resulting object in the index would\n>have CR, then the safe CRLF logic should refrain from doing CRLF->LF\n>conversion. And it should do so without actually adding neither the\n>preimage or the postimage to the index, of course.\n\n(Sorry for the test mail)\n\nWhat I wanted to say is that this long explanation convinced me to write\na patch and send it out the next days,\n\n>When we are doing \"git apply --index\", then we _require_ that the\n>indexed contents and what is in the working tree matches before\n>applying the patch, so it is perfectly fine to let convert_to_git()\n>to look at the current index---that is the \"previous\" one we want to\n>take hint from while using the safe CRLF logic.\n"},{"id":"326220","messageId":"20170812145623.31517-1-tboegi@web.de","threadId":"46495","inReplyTo":"xmqq4ltpsn42.fsf@gitster.mtv.corp.google.com","subject":"[PATCH/RFC] convert: Add SAFE_CRLF_KEEP_CRLF","fromName":"","fromEmail":"tboegi@web.de","sentAt":"2017-08-12T14:56:23Z","receivedAt":"2017-08-12T14:56:37Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"From: Torsten Bögershausen \n\nWhen convert_to_git() is called, the caller may want to keep CRLF\nto be kept as CRLF (and not converted into LF).\n\nThis will be used in the next commit, when apply works with files that have\nCRLF and patches are applied onto these files.\n\nAdd the new value \"SAFE_CRLF_KEEP_CRLF\" to safe_crlf.\n\nPrepare convert_to_git() to be able to run the clean filter,\nskip the CRLF conversion and run the ident filter.\n\nSigned-off-by: Torsten Bögershausen \n---\n convert.c | 10 ++++++----\n convert.h | 3 ++-\n 2 files changed, 8 insertions(+), 5 deletions(-)\n\ndiff --git a/convert.c b/convert.c\nindex deaf0ba7b3..040123b4fe 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -1104,10 +1104,12 @@ int convert_to_git(const struct index_state *istate,\n \t\tsrc = dst->buf;\n \t\tlen = dst->len;\n \t}\n-\tret |= crlf_to_git(istate, path, src, len, dst, ca.crlf_action, checksafe);\n-\tif (ret && dst) {\n-\t\tsrc = dst->buf;\n-\t\tlen = dst->len;\n+\tif (checksafe != SAFE_CRLF_KEEP_CRLF) {\n+\t\tret |= crlf_to_git(istate, path, src, len, dst, ca.crlf_action, checksafe);\n+\t\tif (ret && dst) {\n+\t\t\tsrc = dst->buf;\n+\t\t\tlen = dst->len;\n+\t\t}\n \t}\n \treturn ret | ident_to_git(path, src, len, dst, ca.ident);\n }\ndiff --git a/convert.h b/convert.h\nindex cecf59d1aa..cabd5ed6dd 100644\n--- a/convert.h\n+++ b/convert.h\n@@ -10,7 +10,8 @@ enum safe_crlf {\n \tSAFE_CRLF_FALSE = 0,\n \tSAFE_CRLF_FAIL = 1,\n \tSAFE_CRLF_WARN = 2,\n-\tSAFE_CRLF_RENORMALIZE = 3\n+\tSAFE_CRLF_RENORMALIZE = 3,\n+\tSAFE_CRLF_KEEP_CRLF = 4\n };\n \n extern enum safe_crlf safe_crlf;\n-- \n2.14.1.145.gb3622a4ee9\n\n"},{"id":"326221","messageId":"20170812145625.31560-1-tboegi@web.de","threadId":"46495","inReplyTo":"xmqq4ltpsn42.fsf@gitster.mtv.corp.google.com","subject":"[PATCH/RFC] File commited with CRLF should roundtrip diff and apply","fromName":"","fromEmail":"tboegi@web.de","sentAt":"2017-08-12T14:56:25Z","receivedAt":"2017-08-12T14:56:40Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"From: Torsten Bögershausen \n\nWhen a file had been commited with CRLF and core.autocrlf is true,\nthe following does not roundtrip, `git apply` fails:\n\nprintf \"Added line\\r\\n\" >>file &&\ngit diff >patch &&\ngit checkout -- . &&\ngit apply patch\n\nBefore applying the patch, the file from working tree is converted into the\nindex format (clean filter, CRLF conversion, ...)\nHere, when commited with CRLF, the line endings should not be converted.\n\nAnalyze the patch if there is any context line with CRLF,\nor if any line with CRLF is to be removed.\n\nIf yes, the new flag has_crlf is set in \"struct patch\", and two things\nwill happen:\n- read_old_data() will not convert CRLF into LF by calling\n convert_to_git(..., SAFE_CRLF_KEEP_CRLF);\n- The WS_CR_AT_EOL bit is set in the \"white space rule\",\n CRLF are no longer treated as white space.\n\nThanks to Junio C Hamano, his input became the base for t4140.\n\nReported-by: Anthony Sottile \nSigned-off-by: Torsten Bögershausen \n---\n apply.c | 37 ++++++++++++++++++++++++++++---------\n apply.h | 4 ++++\n t/t4140-apply-CRLF.sh | 46 ++++++++++++++++++++++++++++++++++++++++++++++\n 3 files changed, 78 insertions(+), 9 deletions(-)\n create mode 100755 t/t4140-apply-CRLF.sh\n\ndiff --git a/apply.c b/apply.c\nindex f2d599141d..63455cd65f 100644\n--- a/apply.c\n+++ b/apply.c\n@@ -220,6 +220,7 @@ struct patch {\n \tunsigned int recount:1;\n \tunsigned int conflicted_threeway:1;\n \tunsigned int direct_to_threeway:1;\n+\tunsigned int has_crlf:1;\n \tstruct fragment *fragments;\n \tchar *result;\n \tsize_t resultsize;\n@@ -1662,6 +1663,17 @@ static void check_whitespace(struct apply_state *state,\n \trecord_ws_error(state, result, line + 1, len - 2, state->linenr);\n }\n \n+/* Check if the patch has context lines with CRLF or\n+ the patch wants to remove lines with CRLF */\n+static void check_old_for_crlf(struct patch *patch, const char *line, int len)\n+{\n+\tif (len >= 2 && line[len-1] == '\\n' && line[len-2] == '\\r') {\n+\t\tpatch->ws_rule |= WS_CR_AT_EOL;\n+\t\tpatch->has_crlf = 1;\n+\t}\n+}\n+\n+\n /*\n * Parse a unified diff. Note that this really needs to parse each\n * fragment separately, since the only way to know the difference\n@@ -1712,11 +1724,13 @@ static int parse_fragment(struct apply_state *state,\n \t\t\tif (!deleted && !added)\n \t\t\t\tleading++;\n \t\t\ttrailing++;\n+\t\t\tcheck_old_for_crlf(patch, line, len);\n \t\t\tif (!state->apply_in_reverse &&\n \t\t\t state->ws_error_action == correct_ws_error)\n \t\t\t\tcheck_whitespace(state, line, len, patch->ws_rule);\n \t\t\tbreak;\n \t\tcase '-':\n+\t\t\tcheck_old_for_crlf(patch, line, len);\n \t\t\tif (state->apply_in_reverse &&\n \t\t\t state->ws_error_action != nowarn_ws_error)\n \t\t\t\tcheck_whitespace(state, line, len, patch->ws_rule);\n@@ -2268,8 +2282,10 @@ static void show_stats(struct apply_state *state, struct patch *patch)\n \t\tadd, pluses, del, minuses);\n }\n \n-static int read_old_data(struct stat *st, const char *path, struct strbuf *buf)\n+static int read_old_data(struct stat *st, const char *path, struct strbuf *buf, int flags)\n {\n+\tenum safe_crlf safe_crlf = flags & APPLY_FLAGS_CR_AT_EOL ?\n+\t\tSAFE_CRLF_KEEP_CRLF : SAFE_CRLF_FALSE;\n \tswitch (st->st_mode & S_IFMT) {\n \tcase S_IFLNK:\n \t\tif (strbuf_readlink(buf, path, st->st_size) < 0)\n@@ -2278,7 +2294,7 @@ static int read_old_data(struct stat *st, const char *path, struct strbuf *buf)\n \tcase S_IFREG:\n \t\tif (strbuf_read_file(buf, path, st->st_size) != st->st_size)\n \t\t\treturn error(_(\"unable to open or read %s\"), path);\n-\t\tconvert_to_git(&the_index, path, buf->buf, buf->len, buf, 0);\n+\t\tconvert_to_git(&the_index, path, buf->buf, buf->len, buf, safe_crlf);\n \t\treturn 0;\n \tdefault:\n \t\treturn -1;\n@@ -3385,7 +3401,8 @@ static int load_patch_target(struct apply_state *state,\n \t\t\t const struct cache_entry *ce,\n \t\t\t struct stat *st,\n \t\t\t const char *name,\n-\t\t\t unsigned expected_mode)\n+\t\t\t unsigned expected_mode,\n+\t\t\t int flags)\n {\n \tif (state->cached || state->check_index) {\n \t\tif (read_file_or_gitlink(ce, buf))\n@@ -3399,7 +3416,7 @@ static int load_patch_target(struct apply_state *state,\n \t\t} else if (has_symlink_leading_path(name, strlen(name))) {\n \t\t\treturn error(_(\"reading from '%s' beyond a symbolic link\"), name);\n \t\t} else {\n-\t\t\tif (read_old_data(st, name, buf))\n+\t\t\tif (read_old_data(st, name, buf, flags))\n \t\t\t\treturn error(_(\"failed to read %s\"), name);\n \t\t}\n \t}\n@@ -3423,6 +3440,7 @@ static int load_preimage(struct apply_state *state,\n \tchar *img;\n \tstruct patch *previous;\n \tint status;\n+\tint flags = patch->has_crlf ? APPLY_FLAGS_CR_AT_EOL : 0;\n \n \tprevious = previous_patch(state, patch, &status);\n \tif (status)\n@@ -3433,7 +3451,7 @@ static int load_preimage(struct apply_state *state,\n \t\tstrbuf_add(&buf, previous->result, previous->resultsize);\n \t} else {\n \t\tstatus = load_patch_target(state, &buf, ce, st,\n-\t\t\t\t\t patch->old_name, patch->old_mode);\n+\t\t\t\t\t patch->old_name, patch->old_mode, flags);\n \t\tif (status < 0)\n \t\t\treturn status;\n \t\telse if (status == SUBMODULE_PATCH_WITHOUT_INDEX) {\n@@ -3493,7 +3511,8 @@ static int three_way_merge(struct image *image,\n */\n static int load_current(struct apply_state *state,\n \t\t\tstruct image *image,\n-\t\t\tstruct patch *patch)\n+\t\t\tstruct patch *patch,\n+\t\t\tint flags)\n {\n \tstruct strbuf buf = STRBUF_INIT;\n \tint status, pos;\n@@ -3520,7 +3539,7 @@ static int load_current(struct apply_state *state,\n \tif (verify_index_match(ce, &st))\n \t\treturn error(_(\"%s: does not match index\"), name);\n \n-\tstatus = load_patch_target(state, &buf, ce, &st, name, mode);\n+\tstatus = load_patch_target(state, &buf, ce, &st, name, mode, flags);\n \tif (status < 0)\n \t\treturn status;\n \telse if (status)\n@@ -3571,7 +3590,8 @@ static int try_threeway(struct apply_state *state,\n \n \t/* our_oid is ours */\n \tif (patch->is_new) {\n-\t\tif (load_current(state, &tmp_image, patch))\n+\t\tint flags = 0;\n+\t\tif (load_current(state, &tmp_image, patch, flags))\n \t\t\treturn error(_(\"cannot read the current contents of '%s'\"),\n \t\t\t\t patch->new_name);\n \t} else {\n@@ -3617,7 +3637,6 @@ static int apply_data(struct apply_state *state, struct patch *patch,\n \t\t struct stat *st, const struct cache_entry *ce)\n {\n \tstruct image image;\n-\n \tif (load_preimage(state, &image, patch, st, ce) < 0)\n \t\treturn -1;\n \ndiff --git a/apply.h b/apply.h\nindex b3d6783d55..192140280f 100644\n--- a/apply.h\n+++ b/apply.h\n@@ -33,9 +33,13 @@ enum apply_verbosity {\n #define APPLY_SYMLINK_GOES_AWAY 01\n #define APPLY_SYMLINK_IN_RESULT 02\n \n+\n+#define APPLY_FLAGS_CR_AT_EOL (1<<0)\n+\n struct apply_state {\n \tconst char *prefix;\n \tint prefix_length;\n+\tint flags;\n \n \t/* These are lock_file related */\n \tstruct lock_file *lock_file;\ndiff --git a/t/t4140-apply-CRLF.sh b/t/t4140-apply-CRLF.sh\nnew file mode 100755\nindex 0000000000..fd528daabd\n--- /dev/null\n+++ b/t/t4140-apply-CRLF.sh\n@@ -0,0 +1,46 @@\n+#!/bin/sh\n+\n+test_description='CRLF diff and apply'\n+\n+. ./test-lib.sh\n+\n+test_expect_success setup '\n+\tmkdir upstream &&\n+\t(\n+\t\tcd upstream &&\n+\t\tgit init &&\n+\t\tgit config core.autocrlf false &&\n+\t\t>.gitignore &&\n+\t\tgit add . &&\n+\t\tgit commit -m gitignore &&\n+\t\tprintf \"F1\\r\\n\" >FileCRLF &&\n+\t\tgit add . &&\n+\t\tgit commit -m initial &&\n+\t\tgit diff HEAD^1 HEAD -- >../patch1\n+\t) &&\n+\tgit config core.autocrlf true\n+'\n+\n+test_expect_success 'apply patches Replace lines' '\n+\t(\n+\t\tcd upstream &&\n+\t\tprintf \"F11\\r\\nF12\\r\\n\" >FileCRLF &&\n+\t\tgit diff >../patch2Replace\n+\t) &&\n+\tgit apply patch1 &&\n+\tgit apply patch2Replace\n+'\n+\n+test_expect_success 'apply patches Add lines' '\n+\t(\n+\t\tcd upstream &&\n+\t\tgit checkout FileCRLF &&\n+\t\tprintf \"F2\\r\\n\" >>FileCRLF &&\n+\t\tgit diff >../patch2Add\n+\t) &&\n+\trm -f FileCRLF &&\n+\tgit apply patch1 &&\n+\tgit apply patch2Add\n+'\n+\n+test_done\n-- \n2.14.1.145.gb3622a4ee9\n\n"},{"id":"326237","messageId":"20170813085104.1688-1-tboegi@web.de","threadId":"46495","inReplyTo":"xmqq4ltpsn42.fsf@gitster.mtv.corp.google.com","subject":"[PATCH/RFC 1/2] convert: Add SAFE_CRLF_KEEP_CRLF","fromName":"","fromEmail":"tboegi@web.de","sentAt":"2017-08-13T08:51:04Z","receivedAt":"2017-08-13T08:51:21Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"From: Torsten Bögershausen \n\nWhen convert_to_git() is called, the caller may want to keep CRLF\nto be kept as CRLF (and not converted into LF).\n\nThis will be used in the next commit, when apply works with files that have\nCRLF and patches are applied onto these files.\n\nAdd the new value \"SAFE_CRLF_KEEP_CRLF\" to safe_crlf.\n\nPrepare convert_to_git() to be able to run the clean filter,\nskip the CRLF conversion and run the ident filter.\n\nSigned-off-by: Torsten Bögershausen \n---\nconvert.c | 10 ++++++----\n convert.h | 3 ++-\n 2 files changed, 8 insertions(+), 5 deletions(-)\n\ndiff --git a/convert.c b/convert.c\nindex deaf0ba7b3..040123b4fe 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -1104,10 +1104,12 @@ int convert_to_git(const struct index_state *istate,\n \t\tsrc = dst->buf;\n \t\tlen = dst->len;\n \t}\n-\tret |= crlf_to_git(istate, path, src, len, dst, ca.crlf_action, checksafe);\n-\tif (ret && dst) {\n-\t\tsrc = dst->buf;\n-\t\tlen = dst->len;\n+\tif (checksafe != SAFE_CRLF_KEEP_CRLF) {\n+\t\tret |= crlf_to_git(istate, path, src, len, dst, ca.crlf_action, checksafe);\n+\t\tif (ret && dst) {\n+\t\t\tsrc = dst->buf;\n+\t\t\tlen = dst->len;\n+\t\t}\n \t}\n \treturn ret | ident_to_git(path, src, len, dst, ca.ident);\n }\ndiff --git a/convert.h b/convert.h\nindex cecf59d1aa..cabd5ed6dd 100644\n--- a/convert.h\n+++ b/convert.h\n@@ -10,7 +10,8 @@ enum safe_crlf {\n \tSAFE_CRLF_FALSE = 0,\n \tSAFE_CRLF_FAIL = 1,\n \tSAFE_CRLF_WARN = 2,\n-\tSAFE_CRLF_RENORMALIZE = 3\n+\tSAFE_CRLF_RENORMALIZE = 3,\n+\tSAFE_CRLF_KEEP_CRLF = 4\n };\n \n extern enum safe_crlf safe_crlf;\n-- \n2.14.1.145.gb3622a4ee9\n\n"},{"id":"326238","messageId":"20170813085106.1731-1-tboegi@web.de","threadId":"46495","inReplyTo":"xmqq4ltpsn42.fsf@gitster.mtv.corp.google.com","subject":"[PATCH/RFC 2/2] File commited with CRLF should roundtrip diff and apply","fromName":"","fromEmail":"tboegi@web.de","sentAt":"2017-08-13T08:51:06Z","receivedAt":"2017-08-13T08:51:27Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"From: Torsten Bögershausen \n\nWhen a file had been commited with CRLF and core.autocrlf is true,\nthe following does not roundtrip, `git apply` fails:\n\nprintf \"Added line\\r\\n\" >>file &&\ngit diff >patch &&\ngit checkout -- . &&\ngit apply patch\n\nBefore applying the patch, the file from working tree is converted into the\nindex format (clean filter, CRLF conversion, ...)\nHere, when commited with CRLF, the line endings should not be converted.\n\nAnalyze the patch if there is any context line with CRLF,\nor if any line with CRLF is to be removed.\n\nIf yes, the new flag has_crlf is set in \"struct patch\", and two things\nwill happen:\n- read_old_data() will not convert CRLF into LF by calling\n convert_to_git(..., SAFE_CRLF_KEEP_CRLF);\n- The WS_CR_AT_EOL bit is set in the \"white space rule\",\n CRLF are no longer treated as white space.\n\nThanks to Junio C Hamano, his input became the base for t4140.\n\nReported-by: Anthony Sottile \nSigned-off-by: Torsten Bögershausen \n---\n\n\nThe last version did not pass t4124, fix this.\n\n\n\napply.c | 37 ++++++++++++++++++++++++++++---------\n apply.h | 4 ++++\n t/t4124-apply-ws-rule.sh | 3 +--\n t/t4140-apply-CRLF.sh | 46 ++++++++++++++++++++++++++++++++++++++++++++++\n 4 files changed, 79 insertions(+), 11 deletions(-)\n create mode 100755 t/t4140-apply-CRLF.sh\n\ndiff --git a/apply.c b/apply.c\nindex f2d599141d..63455cd65f 100644\n--- a/apply.c\n+++ b/apply.c\n@@ -220,6 +220,7 @@ struct patch {\n \tunsigned int recount:1;\n \tunsigned int conflicted_threeway:1;\n \tunsigned int direct_to_threeway:1;\n+\tunsigned int has_crlf:1;\n \tstruct fragment *fragments;\n \tchar *result;\n \tsize_t resultsize;\n@@ -1662,6 +1663,17 @@ static void check_whitespace(struct apply_state *state,\n \trecord_ws_error(state, result, line + 1, len - 2, state->linenr);\n }\n \n+/* Check if the patch has context lines with CRLF or\n+ the patch wants to remove lines with CRLF */\n+static void check_old_for_crlf(struct patch *patch, const char *line, int len)\n+{\n+\tif (len >= 2 && line[len-1] == '\\n' && line[len-2] == '\\r') {\n+\t\tpatch->ws_rule |= WS_CR_AT_EOL;\n+\t\tpatch->has_crlf = 1;\n+\t}\n+}\n+\n+\n /*\n * Parse a unified diff. Note that this really needs to parse each\n * fragment separately, since the only way to know the difference\n@@ -1712,11 +1724,13 @@ static int parse_fragment(struct apply_state *state,\n \t\t\tif (!deleted && !added)\n \t\t\t\tleading++;\n \t\t\ttrailing++;\n+\t\t\tcheck_old_for_crlf(patch, line, len);\n \t\t\tif (!state->apply_in_reverse &&\n \t\t\t state->ws_error_action == correct_ws_error)\n \t\t\t\tcheck_whitespace(state, line, len, patch->ws_rule);\n \t\t\tbreak;\n \t\tcase '-':\n+\t\t\tcheck_old_for_crlf(patch, line, len);\n \t\t\tif (state->apply_in_reverse &&\n \t\t\t state->ws_error_action != nowarn_ws_error)\n \t\t\t\tcheck_whitespace(state, line, len, patch->ws_rule);\n@@ -2268,8 +2282,10 @@ static void show_stats(struct apply_state *state, struct patch *patch)\n \t\tadd, pluses, del, minuses);\n }\n \n-static int read_old_data(struct stat *st, const char *path, struct strbuf *buf)\n+static int read_old_data(struct stat *st, const char *path, struct strbuf *buf, int flags)\n {\n+\tenum safe_crlf safe_crlf = flags & APPLY_FLAGS_CR_AT_EOL ?\n+\t\tSAFE_CRLF_KEEP_CRLF : SAFE_CRLF_FALSE;\n \tswitch (st->st_mode & S_IFMT) {\n \tcase S_IFLNK:\n \t\tif (strbuf_readlink(buf, path, st->st_size) < 0)\n@@ -2278,7 +2294,7 @@ static int read_old_data(struct stat *st, const char *path, struct strbuf *buf)\n \tcase S_IFREG:\n \t\tif (strbuf_read_file(buf, path, st->st_size) != st->st_size)\n \t\t\treturn error(_(\"unable to open or read %s\"), path);\n-\t\tconvert_to_git(&the_index, path, buf->buf, buf->len, buf, 0);\n+\t\tconvert_to_git(&the_index, path, buf->buf, buf->len, buf, safe_crlf);\n \t\treturn 0;\n \tdefault:\n \t\treturn -1;\n@@ -3385,7 +3401,8 @@ static int load_patch_target(struct apply_state *state,\n \t\t\t const struct cache_entry *ce,\n \t\t\t struct stat *st,\n \t\t\t const char *name,\n-\t\t\t unsigned expected_mode)\n+\t\t\t unsigned expected_mode,\n+\t\t\t int flags)\n {\n \tif (state->cached || state->check_index) {\n \t\tif (read_file_or_gitlink(ce, buf))\n@@ -3399,7 +3416,7 @@ static int load_patch_target(struct apply_state *state,\n \t\t} else if (has_symlink_leading_path(name, strlen(name))) {\n \t\t\treturn error(_(\"reading from '%s' beyond a symbolic link\"), name);\n \t\t} else {\n-\t\t\tif (read_old_data(st, name, buf))\n+\t\t\tif (read_old_data(st, name, buf, flags))\n \t\t\t\treturn error(_(\"failed to read %s\"), name);\n \t\t}\n \t}\n@@ -3423,6 +3440,7 @@ static int load_preimage(struct apply_state *state,\n \tchar *img;\n \tstruct patch *previous;\n \tint status;\n+\tint flags = patch->has_crlf ? APPLY_FLAGS_CR_AT_EOL : 0;\n \n \tprevious = previous_patch(state, patch, &status);\n \tif (status)\n@@ -3433,7 +3451,7 @@ static int load_preimage(struct apply_state *state,\n \t\tstrbuf_add(&buf, previous->result, previous->resultsize);\n \t} else {\n \t\tstatus = load_patch_target(state, &buf, ce, st,\n-\t\t\t\t\t patch->old_name, patch->old_mode);\n+\t\t\t\t\t patch->old_name, patch->old_mode, flags);\n \t\tif (status < 0)\n \t\t\treturn status;\n \t\telse if (status == SUBMODULE_PATCH_WITHOUT_INDEX) {\n@@ -3493,7 +3511,8 @@ static int three_way_merge(struct image *image,\n */\n static int load_current(struct apply_state *state,\n \t\t\tstruct image *image,\n-\t\t\tstruct patch *patch)\n+\t\t\tstruct patch *patch,\n+\t\t\tint flags)\n {\n \tstruct strbuf buf = STRBUF_INIT;\n \tint status, pos;\n@@ -3520,7 +3539,7 @@ static int load_current(struct apply_state *state,\n \tif (verify_index_match(ce, &st))\n \t\treturn error(_(\"%s: does not match index\"), name);\n \n-\tstatus = load_patch_target(state, &buf, ce, &st, name, mode);\n+\tstatus = load_patch_target(state, &buf, ce, &st, name, mode, flags);\n \tif (status < 0)\n \t\treturn status;\n \telse if (status)\n@@ -3571,7 +3590,8 @@ static int try_threeway(struct apply_state *state,\n \n \t/* our_oid is ours */\n \tif (patch->is_new) {\n-\t\tif (load_current(state, &tmp_image, patch))\n+\t\tint flags = 0;\n+\t\tif (load_current(state, &tmp_image, patch, flags))\n \t\t\treturn error(_(\"cannot read the current contents of '%s'\"),\n \t\t\t\t patch->new_name);\n \t} else {\n@@ -3617,7 +3637,6 @@ static int apply_data(struct apply_state *state, struct patch *patch,\n \t\t struct stat *st, const struct cache_entry *ce)\n {\n \tstruct image image;\n-\n \tif (load_preimage(state, &image, patch, st, ce) < 0)\n \t\treturn -1;\n \ndiff --git a/apply.h b/apply.h\nindex b3d6783d55..192140280f 100644\n--- a/apply.h\n+++ b/apply.h\n@@ -33,9 +33,13 @@ enum apply_verbosity {\n #define APPLY_SYMLINK_GOES_AWAY 01\n #define APPLY_SYMLINK_IN_RESULT 02\n \n+\n+#define APPLY_FLAGS_CR_AT_EOL (1<<0)\n+\n struct apply_state {\n \tconst char *prefix;\n \tint prefix_length;\n+\tint flags;\n \n \t/* These are lock_file related */\n \tstruct lock_file *lock_file;\ndiff --git a/t/t4124-apply-ws-rule.sh b/t/t4124-apply-ws-rule.sh\nindex d350065f25..4da6e6e894 100755\n--- a/t/t4124-apply-ws-rule.sh\n+++ b/t/t4124-apply-ws-rule.sh\n@@ -475,11 +475,10 @@ test_expect_success 'same, but with CR-LF line endings && cr-at-eol unset' '\n \tcp one save-one &&\n \tprintf \" \\r\\n\" >>one &&\n \tgit add one &&\n-\tcp one expect &&\n \tprintf \"d\\r\\n\" >>one &&\n+\tcp one expect &&\n \tgit diff -- one >patch &&\n \tmv save-one one &&\n-\techo d >>expect &&\n \n \tgit apply --ignore-space-change --whitespace=fix patch &&\n \ttest_cmp one expect\ndiff --git a/t/t4140-apply-CRLF.sh b/t/t4140-apply-CRLF.sh\nnew file mode 100755\nindex 0000000000..fd528daabd\n--- /dev/null\n+++ b/t/t4140-apply-CRLF.sh\n@@ -0,0 +1,46 @@\n+#!/bin/sh\n+\n+test_description='CRLF diff and apply'\n+\n+. ./test-lib.sh\n+\n+test_expect_success setup '\n+\tmkdir upstream &&\n+\t(\n+\t\tcd upstream &&\n+\t\tgit init &&\n+\t\tgit config core.autocrlf false &&\n+\t\t>.gitignore &&\n+\t\tgit add . &&\n+\t\tgit commit -m gitignore &&\n+\t\tprintf \"F1\\r\\n\" >FileCRLF &&\n+\t\tgit add . &&\n+\t\tgit commit -m initial &&\n+\t\tgit diff HEAD^1 HEAD -- >../patch1\n+\t) &&\n+\tgit config core.autocrlf true\n+'\n+\n+test_expect_success 'apply patches Replace lines' '\n+\t(\n+\t\tcd upstream &&\n+\t\tprintf \"F11\\r\\nF12\\r\\n\" >FileCRLF &&\n+\t\tgit diff >../patch2Replace\n+\t) &&\n+\tgit apply patch1 &&\n+\tgit apply patch2Replace\n+'\n+\n+test_expect_success 'apply patches Add lines' '\n+\t(\n+\t\tcd upstream &&\n+\t\tgit checkout FileCRLF &&\n+\t\tprintf \"F2\\r\\n\" >>FileCRLF &&\n+\t\tgit diff >../patch2Add\n+\t) &&\n+\trm -f FileCRLF &&\n+\tgit apply patch1 &&\n+\tgit apply patch2Add\n+'\n+\n+test_done\n-- \n2.14.1.145.gb3622a4ee9\n\n"},{"id":"326286","messageId":"xmqqa832vymw.fsf@gitster.mtv.corp.google.com","threadId":"46495","inReplyTo":"20170812145625.31560-1-tboegi@web.de","subject":"Re: [PATCH/RFC] File commited with CRLF should roundtrip diff and apply","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-08-14T17:37:43Z","receivedAt":"2017-08-14T17:37:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"tboegi@web.de writes:\n\n> From: Torsten Bögershausen \n>\n> When a file had been commited with CRLF and core.autocrlf is true,\n> the following does not roundtrip, `git apply` fails:\n>\n> printf \"Added line\\r\\n\" >>file &&\n> git diff >patch &&\n> git checkout -- . &&\n> git apply patch\n\nShould this tweak be in effect when we are paying attention to the\ncontents of the index? Or does it matter only when we are doing\n\"git apply\" without either \"--index\" or \"--cache\"? \n\nThe fact that the new flag that is passed from load_preimage() down\nto read_old_data(), the latter of which is only about the \"behave as\na better 'patch -p1', ignoring the index\" mode, hints that this\nshould not affect anything when we are paying attention to the\nindex, but then calling the load_patch_target() helper with a very\ngeneric CR_AT_EOL flag looks a bit misleading, by making it appear\nas if the caller is telling all three cases to do the funny CR\nbusiness. I wonder if we instead want to pass the \"patch\" structure\ndown the callchain and have _only_ read_old_data() pay attention to\nthe has_crlf bit in it---that way, it becomes more obvious what is\ngoing on.\n\nI also notice that read_old_data() still passes &the_index to\nconvert_to_git(). Can we fix convert_to_git() further to allow NULL\nas the istate parameter when we tell it not to look at the index ( I\nam reading your passing SAFE_CRLF_KEEP and SAFE_CRLF_FALSE as a way\nfor the caller to tell that the caller knows what it wants already\nand does not want covnert_to_git() to look at the index)?\n\nWith or without CR in the patch, I do not see any reason for the\ncodepath from read_old_data() down to the convert API to look at the\nindex, ever, as read_old_data() is called only when (!state->cached\n&& !state->check_index), i.e. when we are working as a better 'patch\n-p1' without even having to know that we are in a Git repository, by\nload_patch_target().\n\n> diff --git a/apply.c b/apply.c\n> index f2d599141d..63455cd65f 100644\n> --- a/apply.c\n> +++ b/apply.c\n> @@ -220,6 +220,7 @@ struct patch {\n> \tunsigned int recount:1;\n> \tunsigned int conflicted_threeway:1;\n> \tunsigned int direct_to_threeway:1;\n> +\tunsigned int has_crlf:1;\n> \tstruct fragment *fragments;\n> \tchar *result;\n> \tsize_t resultsize;\n> @@ -1662,6 +1663,17 @@ static void check_whitespace(struct apply_state *state,\n> \trecord_ws_error(state, result, line + 1, len - 2, state->linenr);\n> }\n> \n> +/* Check if the patch has context lines with CRLF or\n> + the patch wants to remove lines with CRLF */\n\nStyle.\n\n> +static void check_old_for_crlf(struct patch *patch, const char *line, int len)\n> +{\n> +\tif (len >= 2 && line[len-1] == '\\n' && line[len-2] == '\\r') {\n> +\t\tpatch->ws_rule |= WS_CR_AT_EOL;\n> +\t\tpatch->has_crlf = 1;\n> +\t}\n> +}\n\nI am wondering where the discrepancy between the names (old crlf\nhere, as opposed to \"struct patch\" that says \"has_crlf\" implying it\ndoes not care which side between old and new has one) comes from.\n\nIs the intention that you only care about what is expected of the\npreimage? \n\n> @@ -1712,11 +1724,13 @@ static int parse_fragment(struct apply_state *state,\n> \t\t\tif (!deleted && !added)\n> \t\t\t\tleading++;\n> \t\t\ttrailing++;\n> +\t\t\tcheck_old_for_crlf(patch, line, len);\n> \t\t\tif (!state->apply_in_reverse &&\n> \t\t\t state->ws_error_action == correct_ws_error)\n> \t\t\t\tcheck_whitespace(state, line, len, patch->ws_rule);\n> \t\t\tbreak;\n\nIf check_old is about \"we care about the lines that are supposed to\nmatch what currently exist in the target file to be patched\", then\nthe call to it also must pay attention to apply_in_reverse. What\nappears on '+' lines in a patch will become the \"expected old\ncontents the patched target is supposed to have\" when we are running\n\"apply -R\".\n\n> \t\tcase '-':\n> +\t\t\tcheck_old_for_crlf(patch, line, len);\n> \t\t\tif (state->apply_in_reverse &&\n> \t\t\t state->ws_error_action != nowarn_ws_error)\n> \t\t\t\tcheck_whitespace(state, line, len, patch->ws_rule);\n\nLikewise.\n\nThanks for working on this; unlike the previous one I commented, I\nthink this one I can sort of see how this is an approach to fix it.\n\n\n"},{"id":"326521","messageId":"xmqqwp63l63b.fsf@gitster.mtv.corp.google.com","threadId":"46495","inReplyTo":"20170813085106.1731-1-tboegi@web.de","subject":"[PATCH/FIXUP 3/2] apply: remove unused member apply_state.flags","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-08-16T18:28:56Z","receivedAt":"2017-08-16T18:29:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The previous step added the \"flags\" member to apply_state, but it is\nnever used. Remove it and move the bit assignment macro to apply.c\nas that is just a private implementation detail.\n\nSigned-off-by: Junio C Hamano \n---\n apply.c | 2 ++\n apply.h | 4 ----\n 2 files changed, 2 insertions(+), 4 deletions(-)\n\ndiff --git a/apply.c b/apply.c\nindex 63455cd65f..7663e63df7 100644\n--- a/apply.c\n+++ b/apply.c\n@@ -2282,6 +2282,8 @@ static void show_stats(struct apply_state *state, struct patch *patch)\n \t\tadd, pluses, del, minuses);\n }\n \n+#define APPLY_FLAGS_CR_AT_EOL (1<<0)\n+\n static int read_old_data(struct stat *st, const char *path, struct strbuf *buf, int flags)\n {\n \tenum safe_crlf safe_crlf = flags & APPLY_FLAGS_CR_AT_EOL ?\ndiff --git a/apply.h b/apply.h\nindex 192140280f..b3d6783d55 100644\n--- a/apply.h\n+++ b/apply.h\n@@ -33,13 +33,9 @@ enum apply_verbosity {\n #define APPLY_SYMLINK_GOES_AWAY 01\n #define APPLY_SYMLINK_IN_RESULT 02\n \n-\n-#define APPLY_FLAGS_CR_AT_EOL (1<<0)\n-\n struct apply_state {\n \tconst char *prefix;\n \tint prefix_length;\n-\tint flags;\n \n \t/* These are lock_file related */\n \tstruct lock_file *lock_file;\n-- \n2.14.1-331-g7631d96230\n\n"},{"id":"326522","messageId":"xmqq4lt7mko4.fsf@gitster.mtv.corp.google.com","threadId":"46495","inReplyTo":"20170813085106.1731-1-tboegi@web.de","subject":"Re: [PATCH/RFC 2/2] File commited with CRLF should roundtrip diff and apply","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-08-16T18:28:43Z","receivedAt":"2017-08-16T18:29:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"I'll be sending a few patches that apply on top of applying these\ntwo patches to show what I meant in my previous review comments.\nThe net change to apply.c, when you combine your 2/2 with these,\nwould become like the attached, which I think makes more sense.\n\nInstead of queuing a squashed result, I thought it may help to send\nthem as incremental fixes with their own justification.\n\ndiff --git a/apply.c b/apply.c\nindex f2d599141d..c06f7014a2 100644\n--- a/apply.c\n+++ b/apply.c\n@@ -220,6 +220,7 @@ struct patch {\n \tunsigned int recount:1;\n \tunsigned int conflicted_threeway:1;\n \tunsigned int direct_to_threeway:1;\n+\tunsigned int crlf_in_old:1;\n \tstruct fragment *fragments;\n \tchar *result;\n \tsize_t resultsize;\n@@ -1662,6 +1663,19 @@ static void check_whitespace(struct apply_state *state,\n \trecord_ws_error(state, result, line + 1, len - 2, state->linenr);\n }\n \n+/*\n+ * Check if the patch has context lines with CRLF or\n+ * the patch wants to remove lines with CRLF.\n+ */\n+static void check_old_for_crlf(struct patch *patch, const char *line, int len)\n+{\n+\tif (len >= 2 && line[len-1] == '\\n' && line[len-2] == '\\r') {\n+\t\tpatch->ws_rule |= WS_CR_AT_EOL;\n+\t\tpatch->crlf_in_old = 1;\n+\t}\n+}\n+\n+\n /*\n * Parse a unified diff. Note that this really needs to parse each\n * fragment separately, since the only way to know the difference\n@@ -1712,11 +1726,14 @@ static int parse_fragment(struct apply_state *state,\n \t\t\tif (!deleted && !added)\n \t\t\t\tleading++;\n \t\t\ttrailing++;\n+\t\t\tcheck_old_for_crlf(patch, line, len);\n \t\t\tif (!state->apply_in_reverse &&\n \t\t\t state->ws_error_action == correct_ws_error)\n \t\t\t\tcheck_whitespace(state, line, len, patch->ws_rule);\n \t\t\tbreak;\n \t\tcase '-':\n+\t\t\tif (!state->apply_in_reverse)\n+\t\t\t\tcheck_old_for_crlf(patch, line, len);\n \t\t\tif (state->apply_in_reverse &&\n \t\t\t state->ws_error_action != nowarn_ws_error)\n \t\t\t\tcheck_whitespace(state, line, len, patch->ws_rule);\n@@ -1725,6 +1742,8 @@ static int parse_fragment(struct apply_state *state,\n \t\t\ttrailing = 0;\n \t\t\tbreak;\n \t\tcase '+':\n+\t\t\tif (state->apply_in_reverse)\n+\t\t\t\tcheck_old_for_crlf(patch, line, len);\n \t\t\tif (!state->apply_in_reverse &&\n \t\t\t state->ws_error_action != nowarn_ws_error)\n \t\t\t\tcheck_whitespace(state, line, len, patch->ws_rule);\n@@ -2268,8 +2287,12 @@ static void show_stats(struct apply_state *state, struct patch *patch)\n \t\tadd, pluses, del, minuses);\n }\n \n-static int read_old_data(struct stat *st, const char *path, struct strbuf *buf)\n+static int read_old_data(struct stat *st, const char *path, struct strbuf *buf,\n+\t\t\t struct patch *patch)\n {\n+\tenum safe_crlf safe_crlf = (patch->crlf_in_old\n+\t\t\t\t ? SAFE_CRLF_KEEP_CRLF : SAFE_CRLF_FALSE);\n+\n \tswitch (st->st_mode & S_IFMT) {\n \tcase S_IFLNK:\n \t\tif (strbuf_readlink(buf, path, st->st_size) < 0)\n@@ -2278,7 +2301,7 @@ static int read_old_data(struct stat *st, const char *path, struct strbuf *buf)\n \tcase S_IFREG:\n \t\tif (strbuf_read_file(buf, path, st->st_size) != st->st_size)\n \t\t\treturn error(_(\"unable to open or read %s\"), path);\n-\t\tconvert_to_git(&the_index, path, buf->buf, buf->len, buf, 0);\n+\t\tconvert_to_git(&the_index, path, buf->buf, buf->len, buf, safe_crlf);\n \t\treturn 0;\n \tdefault:\n \t\treturn -1;\n@@ -3384,6 +3407,7 @@ static int load_patch_target(struct apply_state *state,\n \t\t\t struct strbuf *buf,\n \t\t\t const struct cache_entry *ce,\n \t\t\t struct stat *st,\n+\t\t\t struct patch *patch,\n \t\t\t const char *name,\n \t\t\t unsigned expected_mode)\n {\n@@ -3399,7 +3423,7 @@ static int load_patch_target(struct apply_state *state,\n \t\t} else if (has_symlink_leading_path(name, strlen(name))) {\n \t\t\treturn error(_(\"reading from '%s' beyond a symbolic link\"), name);\n \t\t} else {\n-\t\t\tif (read_old_data(st, name, buf))\n+\t\t\tif (read_old_data(st, name, buf, patch))\n \t\t\t\treturn error(_(\"failed to read %s\"), name);\n \t\t}\n \t}\n@@ -3432,7 +3456,7 @@ static int load_preimage(struct apply_state *state,\n \t\t/* We have a patched copy in memory; use that. */\n \t\tstrbuf_add(&buf, previous->result, previous->resultsize);\n \t} else {\n-\t\tstatus = load_patch_target(state, &buf, ce, st,\n+\t\tstatus = load_patch_target(state, &buf, ce, st, patch,\n \t\t\t\t\t patch->old_name, patch->old_mode);\n \t\tif (status < 0)\n \t\t\treturn status;\n@@ -3520,7 +3544,7 @@ static int load_current(struct apply_state *state,\n \tif (verify_index_match(ce, &st))\n \t\treturn error(_(\"%s: does not match index\"), name);\n \n-\tstatus = load_patch_target(state, &buf, ce, &st, name, mode);\n+\tstatus = load_patch_target(state, &buf, ce, &st, patch, name, mode);\n \tif (status < 0)\n \t\treturn status;\n \telse if (status)\n"},{"id":"326523","messageId":"xmqqpobvl627.fsf@gitster.mtv.corp.google.com","threadId":"46495","inReplyTo":"20170813085106.1731-1-tboegi@web.de","subject":"[PATCH/FIXUP 4/2] apply: only pay attention to CRLF in the preimage","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-08-16T18:29:36Z","receivedAt":"2017-08-16T18:29:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The newly added \"patch.has_crlf\" member wants to indicate if the\nincoming patch expects any CRLF line in the patch target, and\nparse_fragment() implements that logic for \"git apply\".\n\nRename the member to \"patch.crlf_in_old\" to clarify what it means,\nand fix the logic in parse_fragment() so that it also works correctly\nwhen running \"git apply -R\", where '+' lines correspond to the patch\ntarget.\n\nSigned-off-by: Junio C Hamano \n---\n\n There also is an obvious style fix for comment, but I didn't bother\n splitting it out to a separate step.\n\n apply.c | 17 +++++++++++------\n 1 file changed, 11 insertions(+), 6 deletions(-)\n\ndiff --git a/apply.c b/apply.c\nindex 7663e63df7..995973da3d 100644\n--- a/apply.c\n+++ b/apply.c\n@@ -220,7 +220,7 @@ struct patch {\n \tunsigned int recount:1;\n \tunsigned int conflicted_threeway:1;\n \tunsigned int direct_to_threeway:1;\n-\tunsigned int has_crlf:1;\n+\tunsigned int crlf_in_old:1;\n \tstruct fragment *fragments;\n \tchar *result;\n \tsize_t resultsize;\n@@ -1663,13 +1663,15 @@ static void check_whitespace(struct apply_state *state,\n \trecord_ws_error(state, result, line + 1, len - 2, state->linenr);\n }\n \n-/* Check if the patch has context lines with CRLF or\n- the patch wants to remove lines with CRLF */\n+/*\n+ * Check if the patch has context lines with CRLF or\n+ * the patch wants to remove lines with CRLF.\n+ */\n static void check_old_for_crlf(struct patch *patch, const char *line, int len)\n {\n \tif (len >= 2 && line[len-1] == '\\n' && line[len-2] == '\\r') {\n \t\tpatch->ws_rule |= WS_CR_AT_EOL;\n-\t\tpatch->has_crlf = 1;\n+\t\tpatch->crlf_in_old = 1;\n \t}\n }\n \n@@ -1730,7 +1732,8 @@ static int parse_fragment(struct apply_state *state,\n \t\t\t\tcheck_whitespace(state, line, len, patch->ws_rule);\n \t\t\tbreak;\n \t\tcase '-':\n-\t\t\tcheck_old_for_crlf(patch, line, len);\n+\t\t\tif (!state->apply_in_reverse)\n+\t\t\t\tcheck_old_for_crlf(patch, line, len);\n \t\t\tif (state->apply_in_reverse &&\n \t\t\t state->ws_error_action != nowarn_ws_error)\n \t\t\t\tcheck_whitespace(state, line, len, patch->ws_rule);\n@@ -1739,6 +1742,8 @@ static int parse_fragment(struct apply_state *state,\n \t\t\ttrailing = 0;\n \t\t\tbreak;\n \t\tcase '+':\n+\t\t\tif (state->apply_in_reverse)\n+\t\t\t\tcheck_old_for_crlf(patch, line, len);\n \t\t\tif (!state->apply_in_reverse &&\n \t\t\t state->ws_error_action != nowarn_ws_error)\n \t\t\t\tcheck_whitespace(state, line, len, patch->ws_rule);\n@@ -3442,7 +3447,7 @@ static int load_preimage(struct apply_state *state,\n \tchar *img;\n \tstruct patch *previous;\n \tint status;\n-\tint flags = patch->has_crlf ? APPLY_FLAGS_CR_AT_EOL : 0;\n+\tint flags = patch->crlf_in_old ? APPLY_FLAGS_CR_AT_EOL : 0;\n \n \tprevious = previous_patch(state, patch, &status);\n \tif (status)\n"},{"id":"326524","messageId":"xmqqinhnl61g.fsf@gitster.mtv.corp.google.com","threadId":"46495","inReplyTo":"20170813085106.1731-1-tboegi@web.de","subject":"[PATCH/FIXUP 5/2] apply: localize the CRLF business to read_old_data()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-08-16T18:30:03Z","receivedAt":"2017-08-16T18:30:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Previous changes passed a new APPLY_FLAGS_CR_AT_EOL option down from\nload_preimage() to read_old_data(), because the last function in\nthat callchain needs to decide how its call to convert_to_git()\nfunction is made on the data read from the working tree.\n\nThe load_preimage() function and its direct callees, however, are\nnot limited to the case where the patch is applied to the data in\nthe working tree (i.e. \"git apply\" that is working as a better\n\"patch -p1\"), unlike read_old_data(), which deals only with the\npatch target in the working tree. They are also responsible for\ndriving \"git apply --cached\" and \"git apply --index\", both of which\ntake the current index contents into account and do not need the new\nspecial-casing of CRLF. Exposing APPLY_FLAGS_CR_AT_EOL bit to them\nis misleading.\n\nInstead, just pass the \"struct patch\" down the same callchain, and\nhave read_old_data() look at its crlf_in_old member to make the\nnecessary decision.\n\nSigned-off-by: Junio C Hamano \n---\n\n This is what I care about the most in these fix-ups.\n\n apply.c | 30 ++++++++++++++----------------\n 1 file changed, 14 insertions(+), 16 deletions(-)\n\ndiff --git a/apply.c b/apply.c\nindex 995973da3d..c06f7014a2 100644\n--- a/apply.c\n+++ b/apply.c\n@@ -2287,12 +2287,12 @@ static void show_stats(struct apply_state *state, struct patch *patch)\n \t\tadd, pluses, del, minuses);\n }\n \n-#define APPLY_FLAGS_CR_AT_EOL (1<<0)\n-\n-static int read_old_data(struct stat *st, const char *path, struct strbuf *buf, int flags)\n+static int read_old_data(struct stat *st, const char *path, struct strbuf *buf,\n+\t\t\t struct patch *patch)\n {\n-\tenum safe_crlf safe_crlf = flags & APPLY_FLAGS_CR_AT_EOL ?\n-\t\tSAFE_CRLF_KEEP_CRLF : SAFE_CRLF_FALSE;\n+\tenum safe_crlf safe_crlf = (patch->crlf_in_old\n+\t\t\t\t ? SAFE_CRLF_KEEP_CRLF : SAFE_CRLF_FALSE);\n+\n \tswitch (st->st_mode & S_IFMT) {\n \tcase S_IFLNK:\n \t\tif (strbuf_readlink(buf, path, st->st_size) < 0)\n@@ -3407,9 +3407,9 @@ static int load_patch_target(struct apply_state *state,\n \t\t\t struct strbuf *buf,\n \t\t\t const struct cache_entry *ce,\n \t\t\t struct stat *st,\n+\t\t\t struct patch *patch,\n \t\t\t const char *name,\n-\t\t\t unsigned expected_mode,\n-\t\t\t int flags)\n+\t\t\t unsigned expected_mode)\n {\n \tif (state->cached || state->check_index) {\n \t\tif (read_file_or_gitlink(ce, buf))\n@@ -3423,7 +3423,7 @@ static int load_patch_target(struct apply_state *state,\n \t\t} else if (has_symlink_leading_path(name, strlen(name))) {\n \t\t\treturn error(_(\"reading from '%s' beyond a symbolic link\"), name);\n \t\t} else {\n-\t\t\tif (read_old_data(st, name, buf, flags))\n+\t\t\tif (read_old_data(st, name, buf, patch))\n \t\t\t\treturn error(_(\"failed to read %s\"), name);\n \t\t}\n \t}\n@@ -3447,7 +3447,6 @@ static int load_preimage(struct apply_state *state,\n \tchar *img;\n \tstruct patch *previous;\n \tint status;\n-\tint flags = patch->crlf_in_old ? APPLY_FLAGS_CR_AT_EOL : 0;\n \n \tprevious = previous_patch(state, patch, &status);\n \tif (status)\n@@ -3457,8 +3456,8 @@ static int load_preimage(struct apply_state *state,\n \t\t/* We have a patched copy in memory; use that. */\n \t\tstrbuf_add(&buf, previous->result, previous->resultsize);\n \t} else {\n-\t\tstatus = load_patch_target(state, &buf, ce, st,\n-\t\t\t\t\t patch->old_name, patch->old_mode, flags);\n+\t\tstatus = load_patch_target(state, &buf, ce, st, patch,\n+\t\t\t\t\t patch->old_name, patch->old_mode);\n \t\tif (status < 0)\n \t\t\treturn status;\n \t\telse if (status == SUBMODULE_PATCH_WITHOUT_INDEX) {\n@@ -3518,8 +3517,7 @@ static int three_way_merge(struct image *image,\n */\n static int load_current(struct apply_state *state,\n \t\t\tstruct image *image,\n-\t\t\tstruct patch *patch,\n-\t\t\tint flags)\n+\t\t\tstruct patch *patch)\n {\n \tstruct strbuf buf = STRBUF_INIT;\n \tint status, pos;\n@@ -3546,7 +3544,7 @@ static int load_current(struct apply_state *state,\n \tif (verify_index_match(ce, &st))\n \t\treturn error(_(\"%s: does not match index\"), name);\n \n-\tstatus = load_patch_target(state, &buf, ce, &st, name, mode, flags);\n+\tstatus = load_patch_target(state, &buf, ce, &st, patch, name, mode);\n \tif (status < 0)\n \t\treturn status;\n \telse if (status)\n@@ -3597,8 +3595,7 @@ static int try_threeway(struct apply_state *state,\n \n \t/* our_oid is ours */\n \tif (patch->is_new) {\n-\t\tint flags = 0;\n-\t\tif (load_current(state, &tmp_image, patch, flags))\n+\t\tif (load_current(state, &tmp_image, patch))\n \t\t\treturn error(_(\"cannot read the current contents of '%s'\"),\n \t\t\t\t patch->new_name);\n \t} else {\n@@ -3644,6 +3641,7 @@ static int apply_data(struct apply_state *state, struct patch *patch,\n \t\t struct stat *st, const struct cache_entry *ce)\n {\n \tstruct image image;\n+\n \tif (load_preimage(state, &image, patch, st, ce) < 0)\n \t\treturn -1;\n \n-- \n2.14.1-331-g7631d96230\n\n\n"},{"id":"326525","messageId":"xmqqbmnfl5tm.fsf@gitster.mtv.corp.google.com","threadId":"46495","inReplyTo":"20170813085106.1731-1-tboegi@web.de","subject":"[PATCH/FIXUP 6/2] apply: clarify read_old_data() is about no-index case","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-08-16T18:34:45Z","receivedAt":"2017-08-16T18:34:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"With the previous fixes to CRLF handling in place, read_old_data()\nknows what it wants convert_to_git() to do with respect to CRLF. In\nfact, this codepath is about applying a patch to a file in the\nfilesystem, which may not exist in the index, or may exist but may\nnot match what is recorded in the index, or in the extreme case, we\nmay not even be in a Git repository. If convert_to_git() peeked at\nthe index while doing its work, it *would* be a bug.\n\nPass NULL instead of &the_index to the function to make sure we\ncatch future bugs to clarify this.\n\nSigned-off-by: Junio C Hamano \n---\n\n * NOTE NOTE NOTE: This is not a part of the \"squashed diff\" I sent\n earlier, and with this applied, you will see failure in t0020.\n\n The breakage is because convert_to_git(), with your [PATCH 1/2],\n is not yet prepared to be told \"there is no need for safe-crlf\n processing, so do not even look at the index\". You either need\n to invent yet another flag similar to SAFE_CRLF_KEEP_CRLF that is\n different from SAFE_CRLF_FALSE to tell the machinery never to\n look at the index, or fix SAFE_CRLF_FALSE itself so that the\n index is not checked when the caller knows safe-crlf processing\n is not needed.\n\n apply.c | 10 +++++++++-\n 1 file changed, 9 insertions(+), 1 deletion(-)\n\ndiff --git a/apply.c b/apply.c\nindex c06f7014a2..ad58cd1c77 100644\n--- a/apply.c\n+++ b/apply.c\n@@ -2301,7 +2301,15 @@ static int read_old_data(struct stat *st, const char *path, struct strbuf *buf,\n \tcase S_IFREG:\n \t\tif (strbuf_read_file(buf, path, st->st_size) != st->st_size)\n \t\t\treturn error(_(\"unable to open or read %s\"), path);\n-\t\tconvert_to_git(&the_index, path, buf->buf, buf->len, buf, safe_crlf);\n+\t\t/*\n+\t\t * \"git apply\" without \"--index/--cached\" should never look\n+\t\t * at the index; the target file may not have been added to\n+\t\t * the index yet, and we may not even be in any Git repository.\n+\t\t * Pass NULL to convert_to_git() to stress this; the function\n+\t\t * should never look at the index when explicit crlf option\n+\t\t * is given.\n+\t\t */\n+\t\tconvert_to_git(NULL, path, buf->buf, buf->len, buf, safe_crlf);\n \t\treturn 0;\n \tdefault:\n \t\treturn -1;\n-- \n2.14.1-331-g7631d96230\n\n"},{"id":"326579","messageId":"20170817060630.28448-1-tboegi@web.de","threadId":"46495","inReplyTo":"xmqqa832vymw.fsf@gitster.mtv.corp.google.com","subject":"[PATCH v2 1/2] convert: Add SAFE_CRLF_KEEP_CRLF","fromName":"","fromEmail":"tboegi@web.de","sentAt":"2017-08-17T06:06:30Z","receivedAt":"2017-08-17T06:06:52Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"From: Torsten Bögershausen \n\nWhen convert_to_git() is called, the caller may want to keep CRLF\nto be kept as CRLF (and not converted into LF).\n\nThis will be used in the next commit, when apply works with files that have\nCRLF and patches are applied onto these files.\n\nAdd the new value \"SAFE_CRLF_KEEP_CRLF\" to safe_crlf.\n\nPrepare convert_to_git() to be able to run the clean filter,\nskip the CRLF conversion and run the ident filter.\n\nSigned-off-by: Torsten Bögershausen \n---\n convert.c | 10 ++++++----\n convert.h | 3 ++-\n 2 files changed, 8 insertions(+), 5 deletions(-)\n\ndiff --git a/convert.c b/convert.c\nindex deaf0ba7b3..040123b4fe 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -1104,10 +1104,12 @@ int convert_to_git(const struct index_state *istate,\n \t\tsrc = dst->buf;\n \t\tlen = dst->len;\n \t}\n-\tret |= crlf_to_git(istate, path, src, len, dst, ca.crlf_action, checksafe);\n-\tif (ret && dst) {\n-\t\tsrc = dst->buf;\n-\t\tlen = dst->len;\n+\tif (checksafe != SAFE_CRLF_KEEP_CRLF) {\n+\t\tret |= crlf_to_git(istate, path, src, len, dst, ca.crlf_action, checksafe);\n+\t\tif (ret && dst) {\n+\t\t\tsrc = dst->buf;\n+\t\t\tlen = dst->len;\n+\t\t}\n \t}\n \treturn ret | ident_to_git(path, src, len, dst, ca.ident);\n }\ndiff --git a/convert.h b/convert.h\nindex cecf59d1aa..cabd5ed6dd 100644\n--- a/convert.h\n+++ b/convert.h\n@@ -10,7 +10,8 @@ enum safe_crlf {\n \tSAFE_CRLF_FALSE = 0,\n \tSAFE_CRLF_FAIL = 1,\n \tSAFE_CRLF_WARN = 2,\n-\tSAFE_CRLF_RENORMALIZE = 3\n+\tSAFE_CRLF_RENORMALIZE = 3,\n+\tSAFE_CRLF_KEEP_CRLF = 4\n };\n \n extern enum safe_crlf safe_crlf;\n-- \n2.14.1.145.gb3622a4ee9\n\n"},{"id":"326580","messageId":"20170817060633.28491-1-tboegi@web.de","threadId":"46495","inReplyTo":"xmqqa832vymw.fsf@gitster.mtv.corp.google.com","subject":"[PATCH v2 2/2] File commited with CRLF should roundtrip diff and apply","fromName":"","fromEmail":"tboegi@web.de","sentAt":"2017-08-17T06:06:33Z","receivedAt":"2017-08-17T06:06:54Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"From: Torsten Bögershausen \n\nWhen a file had been commited with CRLF but now .gitattributes say\n\"* text=auto\" (or core.autocrlf is true),\nthe following does not roundtrip, `git apply` fails:\n\nprintf \"Added line\\r\\n\" >>file &&\ngit diff >patch &&\ngit checkout -- . &&\ngit apply patch\n\nBefore applying the patch, the file from working tree is converted into the\nindex format (clean filter, CRLF conversion, ...)\nHere, when commited with CRLF, the line endings should not be converted.\n\nNote that `git apply --index` or `git apply --cache` doesn't call\nconvert_to_git() because the source material is already in index format.\n\nAnalyze the patch if there is a) any context line with CRLF,\nor b) if any line with CRLF is to be removed.\nIn this case the patch file `patch` has mixed line endings, for a)\nit looks like this (ignore the $ at the begin of the line):\n\n$ diff --git a/one b/one\n$ index 533790e..c30dea8 100644\n$ --- a/one\n$ +++ b/one\n$ @@ -1 +1,2 @@\n$ a\\r\n$ +b\\r\n\nAnd for b) it looks like this:\n\n$ diff --git a/one b/one\n$ index 533790e..485540d 100644\n$ --- a/one\n$ +++ b/one\n$ @@ -1 +1 @@\n$ -a\\r\n$ +b\\r\n\nIf `git apply` detects that the patch itself has CRLF, (look at the line\n\" a\\r\" or \"-a\\r\" above), the new flag has_crlf is set in \"struct patch\"\nand two things will happen:\n- read_old_data() will not convert CRLF into LF by calling\n convert_to_git(..., SAFE_CRLF_KEEP_CRLF);\n- The WS_CR_AT_EOL bit is set in the \"white space rule\",\n CRLF are no longer treated as white space.\n\nThanks to Junio C Hamano, his input became the base for the changes in t4124.\nOne test case is split up into 3:\n- Detect the \" a\\r\" line in the patch\n- Detect the \"-a\\r\" line in the patch\n- Use LF in repo and CLRF in the worktree. (*)\n\n* This one proves that convert_to_git(&the_index,...) still needs to pass\nthe &index, otherwise Git will crash.\n\nReported-by: Anthony Sottile \nSigned-off-by: Torsten Bögershausen \n---\n apply.c | 28 +++++++++++++++++++++++-----\n t/t4124-apply-ws-rule.sh | 33 +++++++++++++++++++++++++++------\n 2 files changed, 50 insertions(+), 11 deletions(-)\n\ndiff --git a/apply.c b/apply.c\nindex f2d599141d..bebb176099 100644\n--- a/apply.c\n+++ b/apply.c\n@@ -220,6 +220,7 @@ struct patch {\n \tunsigned int recount:1;\n \tunsigned int conflicted_threeway:1;\n \tunsigned int direct_to_threeway:1;\n+\tunsigned int has_crlf:1;\n \tstruct fragment *fragments;\n \tchar *result;\n \tsize_t resultsize;\n@@ -1662,6 +1663,17 @@ static void check_whitespace(struct apply_state *state,\n \trecord_ws_error(state, result, line + 1, len - 2, state->linenr);\n }\n \n+/* Check if the patch has context lines with CRLF or\n+ the patch wants to remove lines with CRLF */\n+static void check_old_for_crlf(struct patch *patch, const char *line, int len)\n+{\n+\tif (len >= 2 && line[len-1] == '\\n' && line[len-2] == '\\r') {\n+\t\tpatch->ws_rule |= WS_CR_AT_EOL;\n+\t\tpatch->has_crlf = 1;\n+\t}\n+}\n+\n+\n /*\n * Parse a unified diff. Note that this really needs to parse each\n * fragment separately, since the only way to know the difference\n@@ -1712,11 +1724,13 @@ static int parse_fragment(struct apply_state *state,\n \t\t\tif (!deleted && !added)\n \t\t\t\tleading++;\n \t\t\ttrailing++;\n+\t\t\tcheck_old_for_crlf(patch, line, len);\n \t\t\tif (!state->apply_in_reverse &&\n \t\t\t state->ws_error_action == correct_ws_error)\n \t\t\t\tcheck_whitespace(state, line, len, patch->ws_rule);\n \t\t\tbreak;\n \t\tcase '-':\n+\t\t\tcheck_old_for_crlf(patch, line, len);\n \t\t\tif (state->apply_in_reverse &&\n \t\t\t state->ws_error_action != nowarn_ws_error)\n \t\t\t\tcheck_whitespace(state, line, len, patch->ws_rule);\n@@ -2268,8 +2282,11 @@ static void show_stats(struct apply_state *state, struct patch *patch)\n \t\tadd, pluses, del, minuses);\n }\n \n-static int read_old_data(struct stat *st, const char *path, struct strbuf *buf)\n+static int read_old_data(struct stat *st, struct patch *patch,\n+\t\t\t const char *path, struct strbuf *buf)\n {\n+\tenum safe_crlf safe_crlf = patch->has_crlf ?\n+\t\tSAFE_CRLF_KEEP_CRLF : SAFE_CRLF_FALSE;\n \tswitch (st->st_mode & S_IFMT) {\n \tcase S_IFLNK:\n \t\tif (strbuf_readlink(buf, path, st->st_size) < 0)\n@@ -2278,7 +2295,7 @@ static int read_old_data(struct stat *st, const char *path, struct strbuf *buf)\n \tcase S_IFREG:\n \t\tif (strbuf_read_file(buf, path, st->st_size) != st->st_size)\n \t\t\treturn error(_(\"unable to open or read %s\"), path);\n-\t\tconvert_to_git(&the_index, path, buf->buf, buf->len, buf, 0);\n+\t\tconvert_to_git(&the_index, path, buf->buf, buf->len, buf, safe_crlf);\n \t\treturn 0;\n \tdefault:\n \t\treturn -1;\n@@ -3384,6 +3401,7 @@ static int load_patch_target(struct apply_state *state,\n \t\t\t struct strbuf *buf,\n \t\t\t const struct cache_entry *ce,\n \t\t\t struct stat *st,\n+\t\t\t struct patch *patch,\n \t\t\t const char *name,\n \t\t\t unsigned expected_mode)\n {\n@@ -3399,7 +3417,7 @@ static int load_patch_target(struct apply_state *state,\n \t\t} else if (has_symlink_leading_path(name, strlen(name))) {\n \t\t\treturn error(_(\"reading from '%s' beyond a symbolic link\"), name);\n \t\t} else {\n-\t\t\tif (read_old_data(st, name, buf))\n+\t\t\tif (read_old_data(st, patch, name, buf))\n \t\t\t\treturn error(_(\"failed to read %s\"), name);\n \t\t}\n \t}\n@@ -3432,7 +3450,7 @@ static int load_preimage(struct apply_state *state,\n \t\t/* We have a patched copy in memory; use that. */\n \t\tstrbuf_add(&buf, previous->result, previous->resultsize);\n \t} else {\n-\t\tstatus = load_patch_target(state, &buf, ce, st,\n+\t\tstatus = load_patch_target(state, &buf, ce, st, patch,\n \t\t\t\t\t patch->old_name, patch->old_mode);\n \t\tif (status < 0)\n \t\t\treturn status;\n@@ -3520,7 +3538,7 @@ static int load_current(struct apply_state *state,\n \tif (verify_index_match(ce, &st))\n \t\treturn error(_(\"%s: does not match index\"), name);\n \n-\tstatus = load_patch_target(state, &buf, ce, &st, name, mode);\n+\tstatus = load_patch_target(state, &buf, ce, &st, patch, name, mode);\n \tif (status < 0)\n \t\treturn status;\n \telse if (status)\ndiff --git a/t/t4124-apply-ws-rule.sh b/t/t4124-apply-ws-rule.sh\nindex d350065f25..4fc27c51f7 100755\n--- a/t/t4124-apply-ws-rule.sh\n+++ b/t/t4124-apply-ws-rule.sh\n@@ -467,21 +467,42 @@ test_expect_success 'same, but with CR-LF line endings && cr-at-eol set' '\n \ttest_cmp one expect\n '\n \n-test_expect_success 'same, but with CR-LF line endings && cr-at-eol unset' '\n+test_expect_success 'CR-LF line endings && add line && text=auto' '\n \tgit config --unset core.whitespace &&\n \tprintf \"a\\r\\n\" >one &&\n+\tcp one save-one &&\n+\tgit add one &&\n \tprintf \"b\\r\\n\" >>one &&\n-\tprintf \"c\\r\\n\" >>one &&\n+\tcp one expect &&\n+\tgit diff -- one >patch &&\n+\tmv save-one one &&\n+\techo \"one text=auto\" >.gitattributes &&\n+\tgit apply patch &&\n+\ttest_cmp one expect\n+'\n+\n+test_expect_success 'CR-LF line endings && change line && text=auto' '\n+\tprintf \"a\\r\\n\" >one &&\n \tcp one save-one &&\n-\tprintf \" \\r\\n\" >>one &&\n \tgit add one &&\n+\tprintf \"b\\r\\n\" >one &&\n \tcp one expect &&\n-\tprintf \"d\\r\\n\" >>one &&\n \tgit diff -- one >patch &&\n \tmv save-one one &&\n-\techo d >>expect &&\n+\techo \"one text=auto\" >.gitattributes &&\n+\tgit apply patch &&\n+\ttest_cmp one expect\n+'\n \n-\tgit apply --ignore-space-change --whitespace=fix patch &&\n+test_expect_success 'LF in repo, CRLF in worktree && change line && text=auto' '\n+\tprintf \"a\\n\" >one &&\n+\tgit add one &&\n+\tprintf \"b\\r\\n\" >one &&\n+\tgit diff -- one >patch &&\n+\tprintf \"a\\r\\n\" >one &&\n+\techo \"one text=auto\" >.gitattributes &&\n+\tgit -c core.eol=CRLF apply patch &&\n+\tprintf \"b\\r\\n\" >expect &&\n \ttest_cmp one expect\n '\n \n-- \n2.14.1.145.gb3622a4ee9\n\n"},{"id":"326581","messageId":"20170817062450.GA28592@tor.lan","threadId":"46495","inReplyTo":"xmqqbmnfl5tm.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH/FIXUP 6/2] apply: clarify read_old_data() is about no-index case","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2017-08-17T06:24:50Z","receivedAt":"2017-08-17T06:25:10Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On Wed, Aug 16, 2017 at 11:34:45AM -0700, Junio C Hamano wrote:\n> With the previous fixes to CRLF handling in place, read_old_data()\n> knows what it wants convert_to_git() to do with respect to CRLF. In\n> fact, this codepath is about applying a patch to a file in the\n> filesystem, which may not exist in the index, or may exist but may\n> not match what is recorded in the index, or in the extreme case, we\n> may not even be in a Git repository. If convert_to_git() peeked at\n> the index while doing its work, it *would* be a bug.\n> \n> Pass NULL instead of &the_index to the function to make sure we\n> catch future bugs to clarify this.\n\nThanks for the work, and now our emails crossed.\n\nCalling convert_to_git(NULL,...) makes Git crash today, see t4124, my\nlatest version, \"LF in repo, CRLF in working tree)\nUnless we re-define the meaning of \"NULL\" into \"don't do CRLF conversions,\nlike SAFE_CRLF_KEEP_CRLF does.\nThe combination of convert_to_git(NULL,...,SAFE_CRLF_KEEP_CRLF) is OK,\nbut all others must supply an &index.\n\nAt a very first glance the end result may look like this:\n- Take my changes in convert.[ch]\n- Take your changes/commit in apply.c (except the NULL, see above)\n- Take my changes in t4124.\n\nI don't have time to look at this today or tomorrow,\nplease give a hint if you are working further.\n"},{"id":"326582","messageId":"xmqqk222itso.fsf@gitster.mtv.corp.google.com","threadId":"46495","inReplyTo":"20170817060633.28491-1-tboegi@web.de","subject":"Re: [PATCH v2 2/2] File commited with CRLF should roundtrip diff and apply","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-08-17T06:37:27Z","receivedAt":"2017-08-17T06:37:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"tboegi@web.de writes:\n\n> Analyze the patch if there is a) any context line with CRLF,\n> or b) if any line with CRLF is to be removed.\n> Thanks to Junio C Hamano, his input became the base for the changes in t4124.\n> One test case is split up into 3:\n> - Detect the \" a\\r\" line in the patch\n> - Detect the \"-a\\r\" line in the patch\n> - Use LF in repo and CLRF in the worktree. (*)\n>\n> * This one proves that convert_to_git(&the_index,...) still needs to pass\n> the &index, otherwise Git will crash.\n\nI do not understand why you think it proves anything like that.\nForget about \"in repo\" when you think about \"git apply\" without\n\"--index/--cache\". There is *NO* role the index should play in that\nmode.\n\n\"Otherwise Git will crash\" is true, because convert_to_git() tries\nto dereference the istate it is given to see if there is CR in the\nblob that happens to be in the index at the path.\n\nBut step back and *think*. It only proves that convert_to_git() you\nhave and/or the way read_old_data() you updated calls it after\napplying these two patches are still broken.\n\nThe \"blob that happens to be in the index at the path\" does *NOT*\nhave anything to do with the file in the working tree that you are\npatching. Such an index entry may not exist (and the code would see\nthat there is 0 CRs and 0 LFs---so what?), or it may have a blob\nfull of CRLF, or it may have a blob full of CR not LF, or any random\nthing that has NO relation to the file you are patching. Why should\nthat random blob (which may not even exist---we are discussing \"git\napply\" without \"--index/--cache\" here) affect the outcome? If it\ndoes not affect the outcome, why should convert_to_git() even look\nat it?\n\nIt shouldn't be calling down to \"has_cr_in_index(istate, path)\" that\nis called from crlf_to_git() in the first place. The check for CR\nwas done in the caller of convert_to_git(), i.e. read_old_data(),\nlong before convert_to_git() is called, and the caller knows if it\nis (or it is not) dealing with data that needs crlf conversion at\nthat point, based on the contents of the file being patched (which,\nagain, does not have any relation to the blob that may or may not\nhappen to be in the index). \n\nYour updated caller is already telling convert_to_git() codepath\nwhen it needs CRLF and it refrains from peeking in the index with\nSAFE_CRLF_KEEP_CRLF flag. The bug still in the code after applying\nthese two patches is that the other choice, i.e. SAFE_CRLF_FALSE,\nthat is passed from read_old_data() to convert_to_git() does *not*\nprevent the latter from peeking into the in-core index. \n\nAnd that is why \"Otherwise Git will crash\".\n"},{"id":"326585","messageId":"xmqqfucqisfg.fsf@gitster.mtv.corp.google.com","threadId":"46495","inReplyTo":"20170817062450.GA28592@tor.lan","subject":"Re: [PATCH/FIXUP 6/2] apply: clarify read_old_data() is about no-index case","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-08-17T07:06:59Z","receivedAt":"2017-08-17T07:07:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Torsten Bögershausen writes:\n\n> I don't have time to look at this today or tomorrow,\n> please give a hint if you are working further.\n\nIt is past my bedtime, and generally I prefer not to touch topics\nthat I know other people are willing to look into, especially when\nI know those \"other people\" are well informed and capable.\n\nThanks.\n"},{"id":"326586","messageId":"xmqqbmneis63.fsf@gitster.mtv.corp.google.com","threadId":"46495","inReplyTo":"20170817062450.GA28592@tor.lan","subject":"Re: [PATCH/FIXUP 6/2] apply: clarify read_old_data() is about no-index case","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-08-17T07:12:36Z","receivedAt":"2017-08-17T07:12:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Torsten Bögershausen writes:\n\n> Unless we re-define the meaning of \"NULL\" into \"don't do CRLF conversions,\n> like SAFE_CRLF_KEEP_CRLF does.\n\nMy preference is not to use NULL as any hint. Instead, the \"flag\"\nparameter we already pass to convert_to_git(), just like the updated\nread_old_data() uses SAFE_CRLF_KEEP_CRLF to tell it that it should\nnot disturb existing CRLF without looking at the istate, should be\nused to tell convert_to_git() to do the opposite, but do so without\nlooking at the istate.\n\nPerhaps SAFE_CRLF_FALSE should be such a flag. Or perhaps we need\nto invent another flag. I dunno.\n"},{"id":"326588","messageId":"20170817082435.GA7508@tor.lan","threadId":"46495","inReplyTo":"xmqqbmneis63.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH/FIXUP 6/2] apply: clarify read_old_data() is about no-index case","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2017-08-17T08:24:35Z","receivedAt":"2017-08-17T08:24:58Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On Thu, Aug 17, 2017 at 12:12:36AM -0700, Junio C Hamano wrote:\n> Torsten Bögershausen writes:\n> \n> > Unless we re-define the meaning of \"NULL\" into \"don't do CRLF conversions,\n> > like SAFE_CRLF_KEEP_CRLF does.\n> \n> My preference is not to use NULL as any hint. Instead, the \"flag\"\n> parameter we already pass to convert_to_git(), just like the updated\n> read_old_data() uses SAFE_CRLF_KEEP_CRLF to tell it that it should\n> not disturb existing CRLF without looking at the istate, should be\n> used to tell convert_to_git() to do the opposite, but do so without\n> looking at the istate.\n> \n> Perhaps SAFE_CRLF_FALSE should be such a flag. Or perhaps we need\n> to invent another flag. I dunno.\n\nOK, message taken, in short:\nI will come up with a new series in a couple of days - \nthanks for the input.\n"},{"id":"326607","messageId":"xmqq7ey2i0ly.fsf@gitster.mtv.corp.google.com","threadId":"46495","inReplyTo":"xmqqbmneis63.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH/FIXUP 6/2] apply: clarify read_old_data() is about no-index case","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-08-17T17:07:53Z","receivedAt":"2017-08-17T17:08:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Torsten Bögershausen writes:\n>\n>> Unless we re-define the meaning of \"NULL\" into \"don't do CRLF conversions,\n>> like SAFE_CRLF_KEEP_CRLF does.\n>\n> My preference is not to use NULL as any hint. Instead, the \"flag\"\n> parameter we already pass to convert_to_git(), just like the updated\n> read_old_data() uses SAFE_CRLF_KEEP_CRLF to tell it that it should\n> not disturb existing CRLF without looking at the istate, should be\n> used to tell convert_to_git() to do the opposite, but do so without\n> looking at the istate.\n>\n> Perhaps SAFE_CRLF_FALSE should be such a flag. Or perhaps we need\n> to invent another flag. I dunno.\n\nI grepped for SAFE_CRLF_FALSE and found only two callers that\nexplicitly pass it down the callchain, both of which essentially\nsays \"if we are writing the object out, use core.safecrlf, but if we\nare merely hashing, do SAFE_CRLF_FALSE thing\". \n\nI think their use case is quite similar to the codepath we are\ndiscussing---they have a data that come from the outside world, and\nthey know the index entry that happens to be at the path has nothing\nto do with the data they are asking convert_to_git() to massage\n(i.e. it is _wrong_ if the contents of the blob that happens to be\nin the index at the path affected the outcome of the conversion).\n\nSo I think the fix to convert_to_git() can just use SAFE_CRLF_FALSE\nas such an instruction that tells the function not do the \"safe\ncrlf\" thing, which looks at the contents in the index and decide if\nthe CRLFs in the contents being converted should be turned into LFs.\nAnd because the function is told not to look at the index, it should\nbe made safe to pass istate=NULL. There does not seem to be a need\nto invent another flag.\n\nThanks.\n\n\n\n"},{"id":"326653","messageId":"20170817214306.10683-1-tboegi@web.de","threadId":"46495","inReplyTo":"xmqqa832vymw.fsf@gitster.mtv.corp.google.com","subject":"[PATCH v3 1/2] convert: Add SAFE_CRLF_KEEP_CRLF","fromName":"","fromEmail":"tboegi@web.de","sentAt":"2017-08-17T21:43:06Z","receivedAt":"2017-08-17T21:43:22Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"From: Torsten Bögershausen \n\nWhen convert_to_git() is called, the caller may want to keep CRLF\nto be kept as CRLF (and not converted into LF).\n\nThis will be used in the next commit, when apply works with files that have\nCRLF and patches are applied onto these files.\n\nAdd the new value \"SAFE_CRLF_KEEP_CRLF\" to safe_crlf.\n\nPrepare convert_to_git() to be able to run the clean filter,\nskip the CRLF conversion and run the ident filter.\n\nSigned-off-by: Torsten Bögershausen \n---\n convert.c | 10 ++++++----\n convert.h | 3 ++-\n 2 files changed, 8 insertions(+), 5 deletions(-)\n\ndiff --git a/convert.c b/convert.c\nindex deaf0ba7b3..040123b4fe 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -1104,10 +1104,12 @@ int convert_to_git(const struct index_state *istate,\n \t\tsrc = dst->buf;\n \t\tlen = dst->len;\n \t}\n-\tret |= crlf_to_git(istate, path, src, len, dst, ca.crlf_action, checksafe);\n-\tif (ret && dst) {\n-\t\tsrc = dst->buf;\n-\t\tlen = dst->len;\n+\tif (checksafe != SAFE_CRLF_KEEP_CRLF) {\n+\t\tret |= crlf_to_git(istate, path, src, len, dst, ca.crlf_action, checksafe);\n+\t\tif (ret && dst) {\n+\t\t\tsrc = dst->buf;\n+\t\t\tlen = dst->len;\n+\t\t}\n \t}\n \treturn ret | ident_to_git(path, src, len, dst, ca.ident);\n }\ndiff --git a/convert.h b/convert.h\nindex cecf59d1aa..cabd5ed6dd 100644\n--- a/convert.h\n+++ b/convert.h\n@@ -10,7 +10,8 @@ enum safe_crlf {\n \tSAFE_CRLF_FALSE = 0,\n \tSAFE_CRLF_FAIL = 1,\n \tSAFE_CRLF_WARN = 2,\n-\tSAFE_CRLF_RENORMALIZE = 3\n+\tSAFE_CRLF_RENORMALIZE = 3,\n+\tSAFE_CRLF_KEEP_CRLF = 4\n };\n \n extern enum safe_crlf safe_crlf;\n-- \n2.14.1.145.gb3622a4ee9\n\n"},{"id":"326654","messageId":"20170817214308.10727-1-tboegi@web.de","threadId":"46495","inReplyTo":"xmqqa832vymw.fsf@gitster.mtv.corp.google.com","subject":"[PATCH v3 2/2] File commited with CRLF should roundtrip diff and apply","fromName":"","fromEmail":"tboegi@web.de","sentAt":"2017-08-17T21:43:08Z","receivedAt":"2017-08-17T21:43:23Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"From: Torsten Bögershausen \n\nWhen a file had been commited with CRLF but now .gitattributes say\n\"* text=auto\" (or core.autocrlf is true),\nthe following does not roundtrip, `git apply` fails:\n\nprintf \"Added line\\r\\n\" >>file &&\ngit diff >patch &&\ngit checkout -- . &&\ngit apply patch\n\nBefore applying the patch, the file from working tree is converted into the\nindex format (clean filter, CRLF conversion, ...)\nHere, when commited with CRLF, the line endings should not be converted.\n\nNote that `git apply --index` or `git apply --cache` doesn't call\nconvert_to_git() because the source material is already in index format.\n\nAnalyze the patch if there is a) any context line with CRLF,\nor b) if any line with CRLF is to be removed.\nIn this case the patch file `patch` has mixed line endings, for a)\nit looks like this (ignore the * at the begin of the line):\n\n* diff --git a/one b/one\n* index 533790e..c30dea8 100644\n* --- a/one\n* +++ b/one\n* @@ -1 +1,2 @@\n* a\\r\n* +b\\r\n\nAnd for b) it looks like this:\n\n* diff --git a/one b/one\n* index 533790e..485540d 100644\n* --- a/one\n* +++ b/one\n* @@ -1 +1 @@\n* -a\\r\n* +b\\r\n\nIf `git apply` detects that the patch itself has CRLF, (look at the line\n\" a\\r\" or \"-a\\r\" above), the new flag crlf_in_old is set in \"struct patch\"\nand two things will happen:\n- read_old_data() will not convert CRLF into LF by calling\n convert_to_git(..., SAFE_CRLF_KEEP_CRLF);\n- The WS_CR_AT_EOL bit is set in the \"white space rule\",\n CRLF are no longer treated as white space.\n\nThanks to Junio C Hamano, his input became the base for the changes in t4124.\nOne test case is split up into 3:\n- Detect the \" a\\r\" line in the patch\n- Detect the \"-a\\r\" line in the patch\n- Use LF in repo and CLRF in the worktree.\n\nReported-by: Anthony Sottile \nSigned-off-by: Torsten Bögershausen \n---\nChanges since v2:\n- Manually integrated all code changes from Junio\n (Thanks, I hope that I didn't miss something)\n- Having examples of \"git diff\" in the commit message confuses \"git apply\",\n so that all examples for git diff have a '*' at the beginnig of the line\n (V2 used '$' which is typically an example for a shell script)\n- The official version to apply the CRLF-rules without having an index is\n SAFE_CRLF_RENORMALIZE, that is already working today.\n- Now we have convert_to_git(NULL, ..., safe_crlf) with\n enum safe_crlf safe_crlf = patch->crlf_in_old ?\n SAFE_CRLF_KEEP_CRLF : SAFE_CRLF_RENORMALIZE;\n\napply.c | 40 +++++++++++++++++++++++++++++++++++-----\n t/t4124-apply-ws-rule.sh | 33 +++++++++++++++++++++++++++------\n 2 files changed, 62 insertions(+), 11 deletions(-)\n\ndiff --git a/apply.c b/apply.c\nindex f2d599141d..691f47c783 100644\n--- a/apply.c\n+++ b/apply.c\n@@ -220,6 +220,7 @@ struct patch {\n \tunsigned int recount:1;\n \tunsigned int conflicted_threeway:1;\n \tunsigned int direct_to_threeway:1;\n+\tunsigned int crlf_in_old:1;\n \tstruct fragment *fragments;\n \tchar *result;\n \tsize_t resultsize;\n@@ -1662,6 +1663,19 @@ static void check_whitespace(struct apply_state *state,\n \trecord_ws_error(state, result, line + 1, len - 2, state->linenr);\n }\n \n+/*\n+ * Check if the patch has context lines with CRLF or\n+ * the patch wants to remove lines with CRLF.\n+ */\n+static void check_old_for_crlf(struct patch *patch, const char *line, int len)\n+{\n+\tif (len >= 2 && line[len-1] == '\\n' && line[len-2] == '\\r') {\n+\t\tpatch->ws_rule |= WS_CR_AT_EOL;\n+\t\tpatch->crlf_in_old = 1;\n+\t}\n+}\n+\n+\n /*\n * Parse a unified diff. Note that this really needs to parse each\n * fragment separately, since the only way to know the difference\n@@ -1712,11 +1726,15 @@ static int parse_fragment(struct apply_state *state,\n \t\t\tif (!deleted && !added)\n \t\t\t\tleading++;\n \t\t\ttrailing++;\n+\t\t\tif (!state->apply_in_reverse)\n+\t\t\t\tcheck_old_for_crlf(patch, line, len);\n \t\t\tif (!state->apply_in_reverse &&\n \t\t\t state->ws_error_action == correct_ws_error)\n \t\t\t\tcheck_whitespace(state, line, len, patch->ws_rule);\n \t\t\tbreak;\n \t\tcase '-':\n+\t\t\tif (!state->apply_in_reverse)\n+\t\t\t\tcheck_old_for_crlf(patch, line, len);\n \t\t\tif (state->apply_in_reverse &&\n \t\t\t state->ws_error_action != nowarn_ws_error)\n \t\t\t\tcheck_whitespace(state, line, len, patch->ws_rule);\n@@ -2268,8 +2286,11 @@ static void show_stats(struct apply_state *state, struct patch *patch)\n \t\tadd, pluses, del, minuses);\n }\n \n-static int read_old_data(struct stat *st, const char *path, struct strbuf *buf)\n+static int read_old_data(struct stat *st, struct patch *patch,\n+\t\t\t const char *path, struct strbuf *buf)\n {\n+\tenum safe_crlf safe_crlf = patch->crlf_in_old ?\n+\t\tSAFE_CRLF_KEEP_CRLF : SAFE_CRLF_RENORMALIZE;\n \tswitch (st->st_mode & S_IFMT) {\n \tcase S_IFLNK:\n \t\tif (strbuf_readlink(buf, path, st->st_size) < 0)\n@@ -2278,7 +2299,15 @@ static int read_old_data(struct stat *st, const char *path, struct strbuf *buf)\n \tcase S_IFREG:\n \t\tif (strbuf_read_file(buf, path, st->st_size) != st->st_size)\n \t\t\treturn error(_(\"unable to open or read %s\"), path);\n-\t\tconvert_to_git(&the_index, path, buf->buf, buf->len, buf, 0);\n+\t /*\n+\t\t* \"git apply\" without \"--index/--cached\" should never look\n+\t\t* at the index; the target file may not have been added to\n+\t\t* the index yet, and we may not even be in any Git repository.\n+\t\t* Pass NULL to convert_to_git() to stress this; the function\n+\t\t* should never look at the index when explicit crlf option\n+\t\t* is given.\n+\t\t*/\n+\t\tconvert_to_git(NULL, path, buf->buf, buf->len, buf, safe_crlf);\n \t\treturn 0;\n \tdefault:\n \t\treturn -1;\n@@ -3384,6 +3413,7 @@ static int load_patch_target(struct apply_state *state,\n \t\t\t struct strbuf *buf,\n \t\t\t const struct cache_entry *ce,\n \t\t\t struct stat *st,\n+\t\t\t struct patch *patch,\n \t\t\t const char *name,\n \t\t\t unsigned expected_mode)\n {\n@@ -3399,7 +3429,7 @@ static int load_patch_target(struct apply_state *state,\n \t\t} else if (has_symlink_leading_path(name, strlen(name))) {\n \t\t\treturn error(_(\"reading from '%s' beyond a symbolic link\"), name);\n \t\t} else {\n-\t\t\tif (read_old_data(st, name, buf))\n+\t\t\tif (read_old_data(st, patch, name, buf))\n \t\t\t\treturn error(_(\"failed to read %s\"), name);\n \t\t}\n \t}\n@@ -3432,7 +3462,7 @@ static int load_preimage(struct apply_state *state,\n \t\t/* We have a patched copy in memory; use that. */\n \t\tstrbuf_add(&buf, previous->result, previous->resultsize);\n \t} else {\n-\t\tstatus = load_patch_target(state, &buf, ce, st,\n+\t\tstatus = load_patch_target(state, &buf, ce, st, patch,\n \t\t\t\t\t patch->old_name, patch->old_mode);\n \t\tif (status < 0)\n \t\t\treturn status;\n@@ -3520,7 +3550,7 @@ static int load_current(struct apply_state *state,\n \tif (verify_index_match(ce, &st))\n \t\treturn error(_(\"%s: does not match index\"), name);\n \n-\tstatus = load_patch_target(state, &buf, ce, &st, name, mode);\n+\tstatus = load_patch_target(state, &buf, ce, &st, patch, name, mode);\n \tif (status < 0)\n \t\treturn status;\n \telse if (status)\ndiff --git a/t/t4124-apply-ws-rule.sh b/t/t4124-apply-ws-rule.sh\nindex d350065f25..4fc27c51f7 100755\n--- a/t/t4124-apply-ws-rule.sh\n+++ b/t/t4124-apply-ws-rule.sh\n@@ -467,21 +467,42 @@ test_expect_success 'same, but with CR-LF line endings && cr-at-eol set' '\n \ttest_cmp one expect\n '\n \n-test_expect_success 'same, but with CR-LF line endings && cr-at-eol unset' '\n+test_expect_success 'CR-LF line endings && add line && text=auto' '\n \tgit config --unset core.whitespace &&\n \tprintf \"a\\r\\n\" >one &&\n+\tcp one save-one &&\n+\tgit add one &&\n \tprintf \"b\\r\\n\" >>one &&\n-\tprintf \"c\\r\\n\" >>one &&\n+\tcp one expect &&\n+\tgit diff -- one >patch &&\n+\tmv save-one one &&\n+\techo \"one text=auto\" >.gitattributes &&\n+\tgit apply patch &&\n+\ttest_cmp one expect\n+'\n+\n+test_expect_success 'CR-LF line endings && change line && text=auto' '\n+\tprintf \"a\\r\\n\" >one &&\n \tcp one save-one &&\n-\tprintf \" \\r\\n\" >>one &&\n \tgit add one &&\n+\tprintf \"b\\r\\n\" >one &&\n \tcp one expect &&\n-\tprintf \"d\\r\\n\" >>one &&\n \tgit diff -- one >patch &&\n \tmv save-one one &&\n-\techo d >>expect &&\n+\techo \"one text=auto\" >.gitattributes &&\n+\tgit apply patch &&\n+\ttest_cmp one expect\n+'\n \n-\tgit apply --ignore-space-change --whitespace=fix patch &&\n+test_expect_success 'LF in repo, CRLF in worktree && change line && text=auto' '\n+\tprintf \"a\\n\" >one &&\n+\tgit add one &&\n+\tprintf \"b\\r\\n\" >one &&\n+\tgit diff -- one >patch &&\n+\tprintf \"a\\r\\n\" >one &&\n+\techo \"one text=auto\" >.gitattributes &&\n+\tgit -c core.eol=CRLF apply patch &&\n+\tprintf \"b\\r\\n\" >expect &&\n \ttest_cmp one expect\n '\n \n-- \n2.14.1.145.gb3622a4ee9\n\n"},{"id":"326657","messageId":"xmqqlgmheskc.fsf@gitster.mtv.corp.google.com","threadId":"46495","inReplyTo":"20170817214308.10727-1-tboegi@web.de","subject":"Re: [PATCH v3 2/2] File commited with CRLF should roundtrip diff and apply","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-08-17T22:29:55Z","receivedAt":"2017-08-17T22:30:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"tboegi@web.de writes:\n\n> Changes since v2:\n> - Manually integrated all code changes from Junio\n> (Thanks, I hope that I didn't miss something)\n\nI suspect that \"apply -R makes '+' preimage\" change is not here.\n\n> - Having examples of \"git diff\" in the commit message confuses \"git apply\",\n> so that all examples for git diff have a '*' at the beginnig of the line\n> (V2 used '$' which is typically an example for a shell script)\n\nJust FYI we tend to just indent them further, just like any\ndisplayed material in the proposed log message.\n\n> - The official version to apply the CRLF-rules without having an index is\n> SAFE_CRLF_RENORMALIZE, that is already working today.\n\nAh, good find. I forgot about that thing you added some time ago.\n\n"},{"id":"326658","messageId":"xmqqh8x5esaj.fsf@gitster.mtv.corp.google.com","threadId":"46495","inReplyTo":"20170817214308.10727-1-tboegi@web.de","subject":"Re: [PATCH v3 2/2] File commited with CRLF should roundtrip diff and apply","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-08-17T22:35:48Z","receivedAt":"2017-08-17T22:36:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"tboegi@web.de writes:\n\n> @@ -1712,11 +1726,15 @@ static int parse_fragment(struct apply_state *state,\n> \t\t\tif (!deleted && !added)\n> \t\t\t\tleading++;\n> \t\t\ttrailing++;\n> +\t\t\tif (!state->apply_in_reverse)\n> +\t\t\t\tcheck_old_for_crlf(patch, line, len);\n> \t\t\tif (!state->apply_in_reverse &&\n> \t\t\t state->ws_error_action == correct_ws_error)\n> \t\t\t\tcheck_whitespace(state, line, len, patch->ws_rule);\n> \t\t\tbreak;\n\nThis one is wrong. You are looking at \" \" (common context) and you\nshould unconditionally call check_old for them.\n\n\n> \t\tcase '-':\n> +\t\t\tif (!state->apply_in_reverse)\n> +\t\t\t\tcheck_old_for_crlf(patch, line, len);\n\nThis is correct.\n\nThere is \"case '+':\" below here you did not touch. There should be\n\n\t\tcase '+':\n\t+\t\tif (state->apply_in_reverse)\n\t+\t\t\tcheck_old_for_crlf(...);\n\nthere. Note that we call check_old() only when applying in reverse.\n\n> @@ -2268,8 +2286,11 @@ static void show_stats(struct apply_state *state, struct patch *patch)\n> \t\tadd, pluses, del, minuses);\n> }\n> \n> -static int read_old_data(struct stat *st, const char *path, struct strbuf *buf)\n> +static int read_old_data(struct stat *st, struct patch *patch,\n> +\t\t\t const char *path, struct strbuf *buf)\n\nThe order of argument to have the patch structure earlier is\ndifferent from my version; what we see here looks much better to me.\n\n> {\n> +\tenum safe_crlf safe_crlf = patch->crlf_in_old ?\n> +\t\tSAFE_CRLF_KEEP_CRLF : SAFE_CRLF_RENORMALIZE;\n> \tswitch (st->st_mode & S_IFMT) {\n> \tcase S_IFLNK:\n> \t\tif (strbuf_readlink(buf, path, st->st_size) < 0)\n> @@ -2278,7 +2299,15 @@ static int read_old_data(struct stat *st, const char *path, struct strbuf *buf)\n> \tcase S_IFREG:\n> \t\tif (strbuf_read_file(buf, path, st->st_size) != st->st_size)\n> \t\t\treturn error(_(\"unable to open or read %s\"), path);\n> -\t\tconvert_to_git(&the_index, path, buf->buf, buf->len, buf, 0);\n> +\t /*\n> +\t\t* \"git apply\" without \"--index/--cached\" should never look\n> +\t\t* at the index; the target file may not have been added to\n> +\t\t* the index yet, and we may not even be in any Git repository.\n> +\t\t* Pass NULL to convert_to_git() to stress this; the function\n> +\t\t* should never look at the index when explicit crlf option\n> +\t\t* is given.\n> +\t\t*/\n> +\t\tconvert_to_git(NULL, path, buf->buf, buf->len, buf, safe_crlf);\n\nThis comment is somewhat strangly indented. I thought opening \"/*\"\nalighs with the usual tab stop.\n\nThanks.\n"},{"id":"326767","messageId":"20170819112759.18521-1-tboegi@web.de","threadId":"46495","inReplyTo":"xmqqa832vymw.fsf@gitster.mtv.corp.google.com","subject":"[PATCH v4 1/2] convert: Add SAFE_CRLF_KEEP_CRLF","fromName":"","fromEmail":"tboegi@web.de","sentAt":"2017-08-19T11:27:59Z","receivedAt":"2017-08-19T11:28:12Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"From: Torsten Bögershausen \n\nWhen convert_to_git() is called, the caller may want to keep CRLF\nto be kept as CRLF (and not converted into LF).\n\nThis will be used in the next commit, when apply works with files that have\nCRLF and patches are applied onto these files.\n\nAdd the new value \"SAFE_CRLF_KEEP_CRLF\" to safe_crlf.\n\nPrepare convert_to_git() to be able to run the clean filter,\nskip the CRLF conversion and run the ident filter.\n\nSigned-off-by: Torsten Bögershausen \n---\n convert.c | 10 ++++++----\n convert.h | 3 ++-\n 2 files changed, 8 insertions(+), 5 deletions(-)\n\ndiff --git a/convert.c b/convert.c\nindex deaf0ba7b3..040123b4fe 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -1104,10 +1104,12 @@ int convert_to_git(const struct index_state *istate,\n \t\tsrc = dst->buf;\n \t\tlen = dst->len;\n \t}\n-\tret |= crlf_to_git(istate, path, src, len, dst, ca.crlf_action, checksafe);\n-\tif (ret && dst) {\n-\t\tsrc = dst->buf;\n-\t\tlen = dst->len;\n+\tif (checksafe != SAFE_CRLF_KEEP_CRLF) {\n+\t\tret |= crlf_to_git(istate, path, src, len, dst, ca.crlf_action, checksafe);\n+\t\tif (ret && dst) {\n+\t\t\tsrc = dst->buf;\n+\t\t\tlen = dst->len;\n+\t\t}\n \t}\n \treturn ret | ident_to_git(path, src, len, dst, ca.ident);\n }\ndiff --git a/convert.h b/convert.h\nindex cecf59d1aa..cabd5ed6dd 100644\n--- a/convert.h\n+++ b/convert.h\n@@ -10,7 +10,8 @@ enum safe_crlf {\n \tSAFE_CRLF_FALSE = 0,\n \tSAFE_CRLF_FAIL = 1,\n \tSAFE_CRLF_WARN = 2,\n-\tSAFE_CRLF_RENORMALIZE = 3\n+\tSAFE_CRLF_RENORMALIZE = 3,\n+\tSAFE_CRLF_KEEP_CRLF = 4\n };\n \n extern enum safe_crlf safe_crlf;\n-- \n2.14.0.rc1.15.gd40c2d4e85.dirty\n\n"},{"id":"326768","messageId":"20170819112801.18564-1-tboegi@web.de","threadId":"46495","inReplyTo":"xmqqa832vymw.fsf@gitster.mtv.corp.google.com","subject":"[PATCH v4 2/2] File commited with CRLF should roundtrip diff and apply","fromName":"","fromEmail":"tboegi@web.de","sentAt":"2017-08-19T11:28:01Z","receivedAt":"2017-08-19T11:28:16Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"From: Torsten Bögershausen \n\nWhen a file had been commited with CRLF but now .gitattributes say\n\"* text=auto\" (or core.autocrlf is true),\nthe following does not roundtrip, `git apply` fails:\n\nprintf \"Added line\\r\\n\" >>file &&\ngit diff >patch &&\ngit checkout -- . &&\ngit apply patch\n\nBefore applying the patch, the file from working tree is converted into the\nindex format (clean filter, CRLF conversion, ...)\nHere, when commited with CRLF, the line endings should not be converted.\n\nNote that `git apply --index` or `git apply --cache` doesn't call\nconvert_to_git() because the source material is already in index format.\n\nAnalyze the patch if there is a) any context line with CRLF,\nor b) if any line with CRLF is to be removed.\nIn this case the patch file `patch` has mixed line endings, for a)\nit looks like this:\n\n diff --git a/one b/one\n index 533790e..c30dea8 100644\n --- a/one\n +++ b/one\n @@ -1 +1,2 @@\n a\\r\n +b\\r\n\nAnd for b) it looks like this:\n\n diff --git a/one b/one\n index 533790e..485540d 100644\n --- a/one\n +++ b/one\n @@ -1 +1 @@\n -a\\r\n +b\\r\n\nIf `git apply` detects that the patch itself has CRLF, (look at the line\n\" a\\r\" or \"-a\\r\" above), the new flag crlf_in_old is set in \"struct patch\"\nand two things will happen:\n- read_old_data() will not convert CRLF into LF by calling\n convert_to_git(..., SAFE_CRLF_KEEP_CRLF);\n- The WS_CR_AT_EOL bit is set in the \"white space rule\",\n CRLF are no longer treated as white space.\n\nWhile at there, make clear that read_old_data() in apply.c\nknows what it wants convert_to_git() to do with respect to CRLF. In\nfact, this codepath is about applying a patch to a file in the\nfilesystem, which may not exist in the index, or may exist but may\nnot match what is recorded in the index, or in the extreme case, we\nmay not even be in a Git repository. If convert_to_git() peeked at\nthe index while doing its work, it *would* be a bug.\n\nPass NULL instead of &the_index to convert_to_git() to make sure we\ncatch future bugs to clarify this.\n\nUpdate the test in t4124: split one test case into 3:\n- Detect the \" a\\r\" line in the patch\n- Detect the \"-a\\r\" line in the patch\n- Use LF in repo and CLRF in the worktree.\n\nReported-by: Anthony Sottile \nHelped-by: Junio C Hamano \nSigned-off-by: Torsten Bögershausen \n---\n\nChanges since v3:\n- took apply.c from junio/tb/apply-with-crlf\n- Remove the leading asterix in the commit message, at the place\n where the \"git diff\" is cited.\n- Mention \"Pass NULL instead of &the_index to convert_to_git()\"\n\napply.c | 41 ++++++++++++++++++++++++++++++++++++-----\n t/t4124-apply-ws-rule.sh | 33 +++++++++++++++++++++++++++------\n 2 files changed, 63 insertions(+), 11 deletions(-)\n\ndiff --git a/apply.c b/apply.c\nindex f2d599141d..66c68f193a 100644\n--- a/apply.c\n+++ b/apply.c\n@@ -220,6 +220,7 @@ struct patch {\n \tunsigned int recount:1;\n \tunsigned int conflicted_threeway:1;\n \tunsigned int direct_to_threeway:1;\n+\tunsigned int crlf_in_old:1;\n \tstruct fragment *fragments;\n \tchar *result;\n \tsize_t resultsize;\n@@ -1662,6 +1663,19 @@ static void check_whitespace(struct apply_state *state,\n \trecord_ws_error(state, result, line + 1, len - 2, state->linenr);\n }\n \n+/*\n+ * Check if the patch has context lines with CRLF or\n+ * the patch wants to remove lines with CRLF.\n+ */\n+static void check_old_for_crlf(struct patch *patch, const char *line, int len)\n+{\n+\tif (len >= 2 && line[len-1] == '\\n' && line[len-2] == '\\r') {\n+\t\tpatch->ws_rule |= WS_CR_AT_EOL;\n+\t\tpatch->crlf_in_old = 1;\n+\t}\n+}\n+\n+\n /*\n * Parse a unified diff. Note that this really needs to parse each\n * fragment separately, since the only way to know the difference\n@@ -1712,11 +1726,14 @@ static int parse_fragment(struct apply_state *state,\n \t\t\tif (!deleted && !added)\n \t\t\t\tleading++;\n \t\t\ttrailing++;\n+\t\t\tcheck_old_for_crlf(patch, line, len);\n \t\t\tif (!state->apply_in_reverse &&\n \t\t\t state->ws_error_action == correct_ws_error)\n \t\t\t\tcheck_whitespace(state, line, len, patch->ws_rule);\n \t\t\tbreak;\n \t\tcase '-':\n+\t\t\tif (!state->apply_in_reverse)\n+\t\t\t\tcheck_old_for_crlf(patch, line, len);\n \t\t\tif (state->apply_in_reverse &&\n \t\t\t state->ws_error_action != nowarn_ws_error)\n \t\t\t\tcheck_whitespace(state, line, len, patch->ws_rule);\n@@ -1725,6 +1742,8 @@ static int parse_fragment(struct apply_state *state,\n \t\t\ttrailing = 0;\n \t\t\tbreak;\n \t\tcase '+':\n+\t\t\tif (state->apply_in_reverse)\n+\t\t\t\tcheck_old_for_crlf(patch, line, len);\n \t\t\tif (!state->apply_in_reverse &&\n \t\t\t state->ws_error_action != nowarn_ws_error)\n \t\t\t\tcheck_whitespace(state, line, len, patch->ws_rule);\n@@ -2268,8 +2287,11 @@ static void show_stats(struct apply_state *state, struct patch *patch)\n \t\tadd, pluses, del, minuses);\n }\n \n-static int read_old_data(struct stat *st, const char *path, struct strbuf *buf)\n+static int read_old_data(struct stat *st, struct patch *patch,\n+\t\t\t const char *path, struct strbuf *buf)\n {\n+\tenum safe_crlf safe_crlf = patch->crlf_in_old ?\n+\t\tSAFE_CRLF_KEEP_CRLF : SAFE_CRLF_RENORMALIZE;\n \tswitch (st->st_mode & S_IFMT) {\n \tcase S_IFLNK:\n \t\tif (strbuf_readlink(buf, path, st->st_size) < 0)\n@@ -2278,7 +2300,15 @@ static int read_old_data(struct stat *st, const char *path, struct strbuf *buf)\n \tcase S_IFREG:\n \t\tif (strbuf_read_file(buf, path, st->st_size) != st->st_size)\n \t\t\treturn error(_(\"unable to open or read %s\"), path);\n-\t\tconvert_to_git(&the_index, path, buf->buf, buf->len, buf, 0);\n+\t\t/*\n+\t\t * \"git apply\" without \"--index/--cached\" should never look\n+\t\t * at the index; the target file may not have been added to\n+\t\t * the index yet, and we may not even be in any Git repository.\n+\t\t * Pass NULL to convert_to_git() to stress this; the function\n+\t\t * should never look at the index when explicit crlf option\n+\t\t * is given.\n+\t\t */\n+\t\tconvert_to_git(NULL, path, buf->buf, buf->len, buf, safe_crlf);\n \t\treturn 0;\n \tdefault:\n \t\treturn -1;\n@@ -3384,6 +3414,7 @@ static int load_patch_target(struct apply_state *state,\n \t\t\t struct strbuf *buf,\n \t\t\t const struct cache_entry *ce,\n \t\t\t struct stat *st,\n+\t\t\t struct patch *patch,\n \t\t\t const char *name,\n \t\t\t unsigned expected_mode)\n {\n@@ -3399,7 +3430,7 @@ static int load_patch_target(struct apply_state *state,\n \t\t} else if (has_symlink_leading_path(name, strlen(name))) {\n \t\t\treturn error(_(\"reading from '%s' beyond a symbolic link\"), name);\n \t\t} else {\n-\t\t\tif (read_old_data(st, name, buf))\n+\t\t\tif (read_old_data(st, patch, name, buf))\n \t\t\t\treturn error(_(\"failed to read %s\"), name);\n \t\t}\n \t}\n@@ -3432,7 +3463,7 @@ static int load_preimage(struct apply_state *state,\n \t\t/* We have a patched copy in memory; use that. */\n \t\tstrbuf_add(&buf, previous->result, previous->resultsize);\n \t} else {\n-\t\tstatus = load_patch_target(state, &buf, ce, st,\n+\t\tstatus = load_patch_target(state, &buf, ce, st, patch,\n \t\t\t\t\t patch->old_name, patch->old_mode);\n \t\tif (status < 0)\n \t\t\treturn status;\n@@ -3520,7 +3551,7 @@ static int load_current(struct apply_state *state,\n \tif (verify_index_match(ce, &st))\n \t\treturn error(_(\"%s: does not match index\"), name);\n \n-\tstatus = load_patch_target(state, &buf, ce, &st, name, mode);\n+\tstatus = load_patch_target(state, &buf, ce, &st, patch, name, mode);\n \tif (status < 0)\n \t\treturn status;\n \telse if (status)\ndiff --git a/t/t4124-apply-ws-rule.sh b/t/t4124-apply-ws-rule.sh\nindex d350065f25..4fc27c51f7 100755\n--- a/t/t4124-apply-ws-rule.sh\n+++ b/t/t4124-apply-ws-rule.sh\n@@ -467,21 +467,42 @@ test_expect_success 'same, but with CR-LF line endings && cr-at-eol set' '\n \ttest_cmp one expect\n '\n \n-test_expect_success 'same, but with CR-LF line endings && cr-at-eol unset' '\n+test_expect_success 'CR-LF line endings && add line && text=auto' '\n \tgit config --unset core.whitespace &&\n \tprintf \"a\\r\\n\" >one &&\n+\tcp one save-one &&\n+\tgit add one &&\n \tprintf \"b\\r\\n\" >>one &&\n-\tprintf \"c\\r\\n\" >>one &&\n+\tcp one expect &&\n+\tgit diff -- one >patch &&\n+\tmv save-one one &&\n+\techo \"one text=auto\" >.gitattributes &&\n+\tgit apply patch &&\n+\ttest_cmp one expect\n+'\n+\n+test_expect_success 'CR-LF line endings && change line && text=auto' '\n+\tprintf \"a\\r\\n\" >one &&\n \tcp one save-one &&\n-\tprintf \" \\r\\n\" >>one &&\n \tgit add one &&\n+\tprintf \"b\\r\\n\" >one &&\n \tcp one expect &&\n-\tprintf \"d\\r\\n\" >>one &&\n \tgit diff -- one >patch &&\n \tmv save-one one &&\n-\techo d >>expect &&\n+\techo \"one text=auto\" >.gitattributes &&\n+\tgit apply patch &&\n+\ttest_cmp one expect\n+'\n \n-\tgit apply --ignore-space-change --whitespace=fix patch &&\n+test_expect_success 'LF in repo, CRLF in worktree && change line && text=auto' '\n+\tprintf \"a\\n\" >one &&\n+\tgit add one &&\n+\tprintf \"b\\r\\n\" >one &&\n+\tgit diff -- one >patch &&\n+\tprintf \"a\\r\\n\" >one &&\n+\techo \"one text=auto\" >.gitattributes &&\n+\tgit -c core.eol=CRLF apply patch &&\n+\tprintf \"b\\r\\n\" >expect &&\n \ttest_cmp one expect\n '\n \n-- \n2.14.0.rc1.15.gd40c2d4e85.dirty\n\n"}]}