{"thread":{"id":"4300","subject":"git-apply can't apply patches to CRLF-files","startedAt":"2006-05-26T16:00:42Z","lastAt":"2006-05-27T18:25:45Z","messageCount":5,"participants":["Salikh Zakirov","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"20759","messageId":"4477262A.5000301@Intel.com","threadId":"4300","inReplyTo":null,"subject":"git-apply can't apply patches to CRLF-files","fromName":"Salikh Zakirov","fromEmail":"salikh.zakirov@intel.com","sentAt":"2006-05-26T16:00:42Z","receivedAt":"2006-05-26T16:00:42Z","isPatch":false,"sender":{"key":"salikh.zakirov@gmail.com","avatar":null},"body":"Hello, \n\ngit-apply can't apply the patch to file with windows-style CRLF line endings,\neven if the patch was generated by git-format-patch.\n\nIs this a bug or known deficiency?\n\nThe following script reproduces the problem\n---------\n#!/bin/sh\nset -e\nmkdir trash\ncd trash\ngit init-db\necho \"abc\" > a\nunix2dos a\ngit add a\ngit commit -m \"a added\" a\necho \"cde\" >> a\nunix2dos a\ngit commit -m \"a modified\" a\ngit format-patch HEAD^\ngit reset --hard HEAD^\ngit am 0001*.txt\n---------\n\nThe resulting output is\n---------\n$ ./test\ndefaulting to local storage area\na: done.\nCommitting initial tree 357c56061b96c1548b15168bc0d02e8d1a319e0b\na: done.\n0001-a-modified.txt\n\nApplying 'a modified'\n\nerror: patch failed: a:1\nerror: a: patch does not apply\nPatch failed at 0001.\nWhen you have resolved this problem run \"git-am --resolved\".\nIf you would prefer to skip this patch, instead run \"git-am --skip\".\n---------\n\nIf I remove unix2dos calls and so the file has normal unix LF line endings,\nthen the result is correct as expected\n\n---------\n$ ./test\ndefaulting to local storage area\nCommitting initial tree 6afc8719a182fed19980da0e53d13fba1f94dd3f\n0001-a-modified.txt\n\nApplying 'a modified'\n\nWrote tree 49f5181a399bbcaac1da3bf693c466a281c4a255\nCommitted: 2b0a2936d0a65b3511882b8e88586ab054dd15b2\n---------\n"},{"id":"20768","messageId":"7virnsk6fe.fsf@assigned-by-dhcp.cox.net","threadId":"4300","inReplyTo":"4477262A.5000301@Intel.com","subject":"Re: git-apply can't apply patches to CRLF-files","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-05-26T18:05:41Z","receivedAt":"2006-05-26T18:05:41Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Salikh Zakirov <Salikh.Zakirov@Intel.com> writes:\n\n> git-apply can't apply the patch to file with windows-style CRLF line endings,\n> even if the patch was generated by git-format-patch.\n\nI do not think that is the case.\n\n> Is this a bug or known deficiency?\n\nThis particular reproduction recipe looks like a PEBCAK; it does\nnot reproduce for me, but I do not have/use unix2dos so I did\nDOSsy line endings a bit differently.\n\n\tgit init-db\n\techo 'abc@' | tr '[@]' '[\\015]' >a\n        git add a\n        git commit -m initial\n\techo 'def@' | tr '[@]' '[\\015]' >>a\n        git commit -a -m second\n        git format-patch HEAD^\n\tgit reset --hard HEAD^\n        git am 0*.txt\n\n> The following script reproduces the problem\n> ---------\n> #!/bin/sh\n> set -e\n> mkdir trash\n> cd trash\n> git init-db\n> echo \"abc\" > a\n> unix2dos a\n> git add a\n> git commit -m \"a added\" a\n> echo \"cde\" >> a\n> unix2dos a\n\nHere the first line of a ends with \\r\\n already end the second\nline ends with a \\n.  Does running unix2dos on that do a\nsensible thing on the first line?  Compare it with my above\nrecipe which appends DOSsy line at the end of the file.\n\nHaving said that, CRLF is unsafe for E-mail transfers anyway, so\nI think we would need a special option to tell git-apply that it\nshould match '\\n' that appears in the patch with '\\r\\n' in the\nfile being patched.  But I do not think that has anything to do\nwith the breakage you saw in your reproduction recipe.\n"},{"id":"20806","messageId":"44789309.1030002@Intel.com","threadId":"4300","inReplyTo":"7virnsk6fe.fsf@assigned-by-dhcp.cox.net","subject":"[PATCH] Fixed Cygwin CR-munging problem in mailsplit","fromName":"Salikh Zakirov","fromEmail":"salikh.zakirov@intel.com","sentAt":"2006-05-27T17:57:29Z","receivedAt":"2006-05-27T17:57:29Z","isPatch":true,"sender":{"key":"salikh.zakirov@gmail.com","avatar":null},"body":"\nDo not open mailbox file as fopen(..., \"rt\")\nas this strips CR characters from the diff,\nthus breaking the patch context for changes \nin CRLF files.\n\nSigned-off-by: Salikh Zakirov <Salikh.Zakirov@Intel.com>\n\n---\n\nJunio C Hamano wrote:\n> \n> \tgit init-db\n> \techo 'abc@' | tr '[@]' '[\\015]' >a\n>         git add a\n>         git commit -m initial\n> \techo 'def@' | tr '[@]' '[\\015]' >>a\n>         git commit -a -m second\n>         git format-patch HEAD^\n> \tgit reset --hard HEAD^\n>         git am 0*.txt\n> \n\nThis reproduction scenario results in exactly the same problem.\nThe problem is observed on Cygwin.\nMy initial evaluation of the problem turned out to be completely bogus.\n\nI've tracked the problem down to the fopen(file, \"rt\") in mailsplit.c,\nwhich then truncates the CR character from the patch file.\nThis changes the patch context lines and it no longer applies.\nChanging it to fopen(file, \"r\") fixes the problem.\n\n> Having said that, CRLF is unsafe for E-mail transfers anyway, so\n> I think we would need a special option to tell git-apply that it\n> should match '\\n' that appears in the patch with '\\r\\n' in the\n> file being patched.  But I do not think that has anything to do\n> with the breakage you saw in your reproduction recipe.\n\nMy use case does not involve e-mail transfers at all.\nI'm using git-format-patch and git-am to rewrite the\npatch sequence with different commit messages. \n\nUnfortunately, some of my fellow developers are not quite\ncareful, and occasionally some of the source files acquire\nCR characters, sometimes in several lines only.\n\nfd405a0843f3efd474bc7897b06d813d6498fbf4\n mailsplit.c |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\nfd405a0843f3efd474bc7897b06d813d6498fbf4\ndiff --git mailsplit.c mailsplit.c\nindex c529e2d..70a569c 100644\n--- mailsplit.c\n+++ mailsplit.c\n@@ -162,7 +162,7 @@ int main(int argc, const char **argv)\n \n \twhile (*argp) {\n \t\tconst char *file = *argp++;\n-\t\tFILE *f = !strcmp(file, \"-\") ? stdin : fopen(file, \"rt\");\n+\t\tFILE *f = !strcmp(file, \"-\") ? stdin : fopen(file, \"r\");\n \t\tint file_done = 0;\n \n \t\tif ( !f )\n-- \n1.3.3.gfd40\n"},{"id":"20808","messageId":"7vfyivfhw4.fsf@assigned-by-dhcp.cox.net","threadId":"4300","inReplyTo":"44789309.1030002@Intel.com","subject":"Re: [PATCH] Fixed Cygwin CR-munging problem in mailsplit","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-05-27T18:21:31Z","receivedAt":"2006-05-27T18:21:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Salikh Zakirov <Salikh.Zakirov@Intel.com> writes:\n\n> Do not open mailbox file as fopen(..., \"rt\")\n> as this strips CR characters from the diff,\n> thus breaking the patch context for changes \n> in CRLF files.\n>\n> Signed-off-by: Salikh Zakirov <Salikh.Zakirov@Intel.com>\n>\n> ---\n> fd405a0843f3efd474bc7897b06d813d6498fbf4\n> diff --git mailsplit.c mailsplit.c\n> index c529e2d..70a569c 100644\n> --- mailsplit.c\n> +++ mailsplit.c\n> @@ -162,7 +162,7 @@ int main(int argc, const char **argv)\n>  \n>  \twhile (*argp) {\n>  \t\tconst char *file = *argp++;\n> -\t\tFILE *f = !strcmp(file, \"-\") ? stdin : fopen(file, \"rt\");\n> +\t\tFILE *f = !strcmp(file, \"-\") ? stdin : fopen(file, \"r\");\n>  \t\tint file_done = 0;\n>  \n>  \t\tif ( !f )\n\nI personally think this is a right change.  Provided if MTAs on\nthe path between patch originator and you are not broken and\nyour MUA saved the message with CR/LF distinction in the\ncontents intact, this should do more right thing.\n\nI see broken patches every once in a while, but when they are\nmangled by the mailpath, CRLF is the least of the problem; they\nhave other whitespace breakage that makes them unapplicable\nanyway.\n\nHaving said that, however, that historically used to be a big IF\nwith capital letters.\n\n\nI have a feeling that Linus did this on purpose.  For the\nprojects we originally cared about, a patch to introduce CRLF\nin the tracked content was a broken patch 100% of the time (not\n99%), and most likely caused by a breakage somewhere on the\nmailpath.  At least in the original git context, protecting\nUNIX/POSIX people from broken MTA/MUA counted far more than\ncatering to people who deals with DOSsy contents.\n\nSo I am slightly in favor of the change, but just barely.\n"},{"id":"20809","messageId":"7vbqtjfhp2.fsf@assigned-by-dhcp.cox.net","threadId":"4300","inReplyTo":"7vfyivfhw4.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Fixed Cygwin CR-munging problem in mailsplit","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-05-27T18:25:45Z","receivedAt":"2006-05-27T18:25:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <junkio@cox.net> writes:\n\n> I personally think this is a right change.  Provided if MTAs on\n> the path between patch originator and you are not broken and\n> your MUA saved the message with CR/LF distinction in the\n> contents intact, this should do more right thing.\n>\n> I see broken patches every once in a while, but when they are\n> mangled by the mailpath, CRLF is the least of the problem; they\n> have other whitespace breakage that makes them unapplicable\n> anyway.\n>\n> Having said that, however, that historically used to be a big IF\n> with capital letters.\n>\n>\n> I have a feeling that Linus did this on purpose.  For the\n\nHeh, I had a trailing CR after the \"with capital letters.\" and\none blank line between paragraphs, but I now see two blank lines\nthere.  So even in this modern day, preserving CRLF is not\nsomething that happens by default -- you would need to make sure\nthat everybody on your mailpath to the recipient is set up the\nright way.\n\n> So I am slightly in favor of the change, but just barely.\n\nSo now I am less in favor of the change than when I wrote that\nresponse.\n"}]}