{"thread":{"id":"25829","subject":"[PATCH] git apply: binary files differ can be applied with -pN (N>1).","startedAt":"2010-11-24T05:20:46Z","lastAt":"2010-11-25T02:52:53Z","messageCount":5,"participants":["Jiang Xin","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"156468","messageId":"4CECA0AE.90505@gmail.com","threadId":"25829","inReplyTo":null,"subject":"[PATCH] git apply: binary files differ can be applied with -pN (N>1).","fromName":"Jiang Xin","fromEmail":"worldhello.net@gmail.com","sentAt":"2010-11-24T05:20:46Z","receivedAt":"2010-11-24T05:20:46Z","isPatch":true,"sender":{"key":"worldhello.net@gmail.com","avatar":"https://avatars.githubusercontent.com/u/183860?v=4"},"body":"When patch file generated against two non-git directories using\n'git diff --binary --no-index' without '--no-prefix', the patch\nfile has patch level greater then 1, and should be applied with\n'-p2' option. But it does not work if there are binary differ\nin the patch file, it is because in one case the patch level is\nnot properly handled.\n\nSigned-off-by: Jiang Xin <jiangxin@ossxp.com>\n---\n builtin/apply.c |    3 ++-\n 1 files changed, 2 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin/apply.c b/builtin/apply.c\nindex 23c18c5..d603e37 100644\n--- a/builtin/apply.c\n+++ b/builtin/apply.c\n@@ -1126,6 +1126,7 @@ static char *git_header_name(char *line, int llen)\n \t * form.\n \t */\n \tfor (len = 0 ; ; len++) {\n+\t\tint nslash = p_value;\n \t\tswitch (name[len]) {\n \t\tdefault:\n \t\t\tcontinue;\n@@ -1137,7 +1138,7 @@ static char *git_header_name(char *line, int llen)\n \t\t\t\tchar c = *second++;\n \t\t\t\tif (c == '\\n')\n \t\t\t\t\treturn NULL;\n-\t\t\t\tif (c == '/')\n+\t\t\t\tif (c == '/' && --nslash <= 0)\n \t\t\t\t\tbreak;\n \t\t\t}\n \t\t\tif (second[len] == '\\n' && !memcmp(name, second, len)) {\n-- \n1.7.3.2.245.g03276.dirty\n"},{"id":"156470","messageId":"4CECB673.9090809@gmail.com","threadId":"25829","inReplyTo":"4CECA0AE.90505@gmail.com","subject":"Re: [PATCH] git apply: binary files differ can be applied with -pN (N>1).","fromName":"Jiang Xin","fromEmail":"worldhello.net@gmail.com","sentAt":"2010-11-24T06:53:39Z","receivedAt":"2010-11-24T06:53:39Z","isPatch":true,"sender":{"key":"worldhello.net@gmail.com","avatar":"https://avatars.githubusercontent.com/u/183860?v=4"},"body":"How to reproduce the bug:\n\n1. Generate a patch file against two directory with binary diff.\n================================================================\n\n$ mkdir hello-1.0 hello-2.0\n\n$ dd if=/bin/ls of=hello-1.0/binary.dat count=1 bs=32\n1+0 records in\n1+0 records out\n32 bytes (32 B) copied, 0.00121276 s, 26.4 kB/s\n\n$ dd if=/bin/ls of=hello-2.0/binary.dat count=1 bs=64\n1+0 records in\n1+0 records out\n64 bytes (64 B) copied, 0.00030288 s, 211 kB/s\n\n$ git diff --no-index --binary hello-1.0 hello-2.0 > patch.txt\n\n2. The patch file has patch level 2\n===================================\n\n$ cat patch.txt \ndiff --git a/hello-1.0/binary.dat b/hello-2.0/binary.dat\nindex dc2e37f81e0fa88308bec48cd5195b6542e61a20..bf948689934caf2d874ff8168cb716fbc2a127c3 100644\nGIT binary patch\ndelta 37\nhcmY#zn4qBGzyJX+<}pH93=9qo77QFfQiegA0RUZd1MdI;\n\ndelta 4\nLcmZ=zn4kav0;B;E\n\n3. Apply the patch failed\n=========================\n\n$ cd hello-1.0\n\n$ git apply --check -p2 ../patch.txt\nfatal: git diff header lacks filename information when removing 2 leading pathname components (line 3)\n\n\n于 2010年11月24日 13:20, Jiang Xin 写道:\n> When patch file generated against two non-git directories using\n> 'git diff --binary --no-index' without '--no-prefix', the patch\n> file has patch level greater then 1, and should be applied with\n> '-p2' option. But it does not work if there are binary differ\n> in the patch file, it is because in one case the patch level is\n> not properly handled.\n> \n> Signed-off-by: Jiang Xin <jiangxin@ossxp.com>\n> ---\n>  builtin/apply.c |    3 ++-\n>  1 files changed, 2 insertions(+), 1 deletions(-)\n> \n> diff --git a/builtin/apply.c b/builtin/apply.c\n> index 23c18c5..d603e37 100644\n> --- a/builtin/apply.c\n> +++ b/builtin/apply.c\n> @@ -1126,6 +1126,7 @@ static char *git_header_name(char *line, int llen)\n>  \t * form.\n>  \t */\n>  \tfor (len = 0 ; ; len++) {\n> +\t\tint nslash = p_value;\n>  \t\tswitch (name[len]) {\n>  \t\tdefault:\n>  \t\t\tcontinue;\n> @@ -1137,7 +1138,7 @@ static char *git_header_name(char *line, int llen)\n>  \t\t\t\tchar c = *second++;\n>  \t\t\t\tif (c == '\\n')\n>  \t\t\t\t\treturn NULL;\n> -\t\t\t\tif (c == '/')\n> +\t\t\t\tif (c == '/' && --nslash <= 0)\n>  \t\t\t\t\tbreak;\n>  \t\t\t}\n>  \t\t\tif (second[len] == '\\n' && !memcmp(name, second, len)) {\n\n\n-- \n蒋鑫\n\n北京群英汇信息技术有限公司\n邮件: worldhello.net@gmail.com\n网址: http://www.ossxp.com/\n　　  http://blog.ossxp.com/\n电话: 010-51262007, 13910430470\n传真: 010-51262007\n"},{"id":"156489","messageId":"7vlj4itz8j.fsf@alter.siamese.dyndns.org","threadId":"25829","inReplyTo":"4CECA0AE.90505@gmail.com","subject":"Re: [PATCH] git apply: binary files differ can be applied with -pN (N>1).","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-11-24T17:20:28Z","receivedAt":"2010-11-24T17:20:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jiang Xin <worldhello.net@gmail.com> writes:\n\n> When patch file generated against two non-git directories using\n> 'git diff --binary --no-index' without '--no-prefix', the patch\n> file has patch level greater then 1, and should be applied with\n> '-p2' option. But it does not work if there are binary differ\n> in the patch file, it is because in one case the patch level is\n> not properly handled.\n>\n> Signed-off-by: Jiang Xin <jiangxin@ossxp.com>\n\nCan you please add a testcase to protect your fix from getting broken by\nlater changes by other people, perhaps to t/t4120?\n\nBy the way, this codepath is shared by all forms of patches \"diff --git\"\nheader, not just binary.  Do you see a similar breakage with --no-prefix\npatches that are not binary, and if not why?\n\n> ---\n>  builtin/apply.c |    3 ++-\n>  1 files changed, 2 insertions(+), 1 deletions(-)\n>\n> diff --git a/builtin/apply.c b/builtin/apply.c\n> index 23c18c5..d603e37 100644\n> --- a/builtin/apply.c\n> +++ b/builtin/apply.c\n> @@ -1126,6 +1126,7 @@ static char *git_header_name(char *line, int llen)\n>  \t * form.\n>  \t */\n>  \tfor (len = 0 ; ; len++) {\n> +\t\tint nslash = p_value;\n>  \t\tswitch (name[len]) {\n>  \t\tdefault:\n>  \t\t\tcontinue;\n> @@ -1137,7 +1138,7 @@ static char *git_header_name(char *line, int llen)\n>  \t\t\t\tchar c = *second++;\n>  \t\t\t\tif (c == '\\n')\n>  \t\t\t\t\treturn NULL;\n> -\t\t\t\tif (c == '/')\n> +\t\t\t\tif (c == '/' && --nslash <= 0)\n>  \t\t\t\t\tbreak;\n>  \t\t\t}\n>  \t\t\tif (second[len] == '\\n' && !memcmp(name, second, len)) {\n> -- \n> 1.7.3.2.245.g03276.dirty\n"},{"id":"156515","messageId":"4CEDC011.1050503@gmail.com","threadId":"25829","inReplyTo":"7vlj4itz8j.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] git apply: binary files differ can be applied with -pN (N>1).","fromName":"Jiang Xin","fromEmail":"worldhello.net@gmail.com","sentAt":"2010-11-25T01:46:57Z","receivedAt":"2010-11-25T01:46:57Z","isPatch":true,"sender":{"key":"worldhello.net@gmail.com","avatar":"https://avatars.githubusercontent.com/u/183860?v=4"},"body":"\n于 2010年11月25日 01:20, Junio C Hamano 写道:\n> Jiang Xin <worldhello.net@gmail.com> writes:\n> \n>> When patch file generated against two non-git directories using\n>> 'git diff --binary --no-index' without '--no-prefix', the patch\n>> file has patch level greater then 1, and should be applied with\n>> '-p2' option. But it does not work if there are binary differ\n>> in the patch file, it is because in one case the patch level is\n>> not properly handled.\n>>\n>> Signed-off-by: Jiang Xin <jiangxin@ossxp.com>\n> \n> Can you please add a testcase to protect your fix from getting broken by\n> later changes by other people, perhaps to t/t4120?\n> \n\nYes, I add a testcase in t/t4120, and I will send it as a new PATCH.\nThe new testcase is like following:\n\n+test_expect_success 'apply git diff with -p2 and use default name from header' '\n+       sed -e \"/^\\(---\\|+++\\) / d\" patch.file > patch.newheader &&\n+       cp file1.saved file1 &&\n+       git apply -p2 patch.newheader\n+'\n+\n\n> By the way, this codepath is shared by all forms of patches \"diff --git\"\n> header, not just binary.  Do you see a similar breakage with --no-prefix\n> patches that are not binary, and if not why?\n> \n\nThe breakage will appear under these circumstances:\n* the patch is in git style: header with \"diff --git\".\n* the header does not contain '--- path/to/old' and '+++ path/to/new'.\n* has a patch level greater than 1.\n\nWhen I do `git diff` against binary files, I find the patch header does not has\n'--- path/to/old' and '+++ path/to/new'. May be there exist other cases, but\nI'm not sure.\n\n>> ---\n>>  builtin/apply.c |    3 ++-\n>>  1 files changed, 2 insertions(+), 1 deletions(-)\n>>\n>> diff --git a/builtin/apply.c b/builtin/apply.c\n>> index 23c18c5..d603e37 100644\n>> --- a/builtin/apply.c\n>> +++ b/builtin/apply.c\n>> @@ -1126,6 +1126,7 @@ static char *git_header_name(char *line, int llen)\n>>  \t * form.\n>>  \t */\n>>  \tfor (len = 0 ; ; len++) {\n>> +\t\tint nslash = p_value;\n>>  \t\tswitch (name[len]) {\n>>  \t\tdefault:\n>>  \t\t\tcontinue;\n>> @@ -1137,7 +1138,7 @@ static char *git_header_name(char *line, int llen)\n>>  \t\t\t\tchar c = *second++;\n>>  \t\t\t\tif (c == '\\n')\n>>  \t\t\t\t\treturn NULL;\n>> -\t\t\t\tif (c == '/')\n>> +\t\t\t\tif (c == '/' && --nslash <= 0)\n>>  \t\t\t\t\tbreak;\n>>  \t\t\t}\n>>  \t\t\tif (second[len] == '\\n' && !memcmp(name, second, len)) {\n>> -- \n>> 1.7.3.2.245.g03276.dirty\n\n\n-- \nJiang Xin\n"},{"id":"156517","messageId":"4CEDCF85.1010806@gmail.com","threadId":"25829","inReplyTo":"4CEDC011.1050503@gmail.com","subject":"Re: [PATCH] git apply: binary files differ can be applied with -pN (N>1).","fromName":"Jiang Xin","fromEmail":"worldhello.net@gmail.com","sentAt":"2010-11-25T02:52:53Z","receivedAt":"2010-11-25T02:52:53Z","isPatch":true,"sender":{"key":"worldhello.net@gmail.com","avatar":"https://avatars.githubusercontent.com/u/183860?v=4"},"body":"> 于 2010年11月25日 01:20, Junio C Hamano 写道:\n> By the way, this codepath is shared by all forms of patches \"diff --git\"\n> header, not just binary.  Do you see a similar breakage with --no-prefix\n> patches that are not binary, and if not why?\n\nSome cases a git diff without \"--- path/to/old\" and \"+++ path/to/new\",\nwill make current git apply -p2 broken.\n\n1. Prue file mode changes:\n\n    diff --git a/sub1/file1 b/sub2/file1\n    old mode 100644\n    new mode 100755\n\n2. Remove empty file:\n\n    diff --git a/sub1/file1 b/sub1/file1\n    deleted file mode 100644\n    index e69de29..0000000\n\n3. Add empty file:\n\n    diff --git a/sub2/file1 b/sub2/file1\n    new file mode 100644\n    index 0000000..e69de29\n\n4. New binary file:\n\n    diff --git a/sub2/file1 b/sub2/file1\n    new file mode 100644\n    index 0000000000000000000000000000000000000000..4dd0c65a410e8aea1608d6b2dd70e74def0012c5\n    GIT binary patch\n    literal 8\n    Pcmb<-^>JfjWMlvU2^Ilc\n    \n    literal 0\n    HcmV?d00001\n    \n5. Modify binary file:\n    diff --git a/sub1/file1 b/sub2/file1\n    index f96ec1a8358556543a73b3fb93a0d1c2aad8432a..4dd0c65a410e8aea1608d6b2dd70e74def0012c5 100644\n    GIT binary patch\n    literal 8\n    Pcmb<-^>JfjWMlvU2^Ilc\n\n    literal 7\n    Ocmb<-^>JfjWCQ>Qy8&AO\n    \n\nI think all these cases can use this testcase in t/t4120-apply-popt.sh:\n\n    +test_expect_success 'apply git diff with -p2 and use default name from header' '\n    +       sed -e \"/^\\(---\\|+++\\) / d\" patch.file > patch.newheader &&\n    +       cp file1.saved file1 &&\n    +       git apply -p2 patch.newheader\n    +'\n\n\n于 2010年11月25日 09:46, Jiang Xin 写道:\n> \n> 于 2010年11月25日 01:20, Junio C Hamano 写道:\n>> Jiang Xin <worldhello.net@gmail.com> writes:\n>>\n>>> When patch file generated against two non-git directories using\n>>> 'git diff --binary --no-index' without '--no-prefix', the patch\n>>> file has patch level greater then 1, and should be applied with\n>>> '-p2' option. But it does not work if there are binary differ\n>>> in the patch file, it is because in one case the patch level is\n>>> not properly handled.\n>>>\n>>> Signed-off-by: Jiang Xin <jiangxin@ossxp.com>\n>>\n>> Can you please add a testcase to protect your fix from getting broken by\n>> later changes by other people, perhaps to t/t4120?\n>>\n> \n> Yes, I add a testcase in t/t4120, and I will send it as a new PATCH.\n> The new testcase is like following:\n> \n> +test_expect_success 'apply git diff with -p2 and use default name from header' '\n> +       sed -e \"/^\\(---\\|+++\\) / d\" patch.file > patch.newheader &&\n> +       cp file1.saved file1 &&\n> +       git apply -p2 patch.newheader\n> +'\n> +\n> \n>> By the way, this codepath is shared by all forms of patches \"diff --git\"\n>> header, not just binary.  Do you see a similar breakage with --no-prefix\n>> patches that are not binary, and if not why?\n>>\n> \n> The breakage will appear under these circumstances:\n> * the patch is in git style: header with \"diff --git\".\n> * the header does not contain '--- path/to/old' and '+++ path/to/new'.\n> * has a patch level greater than 1.\n> \n> When I do `git diff` against binary files, I find the patch header does not has\n> '--- path/to/old' and '+++ path/to/new'. May be there exist other cases, but\n> I'm not sure.\n> \n>>> ---\n>>>  builtin/apply.c |    3 ++-\n>>>  1 files changed, 2 insertions(+), 1 deletions(-)\n>>>\n>>> diff --git a/builtin/apply.c b/builtin/apply.c\n>>> index 23c18c5..d603e37 100644\n>>> --- a/builtin/apply.c\n>>> +++ b/builtin/apply.c\n>>> @@ -1126,6 +1126,7 @@ static char *git_header_name(char *line, int llen)\n>>>  \t * form.\n>>>  \t */\n>>>  \tfor (len = 0 ; ; len++) {\n>>> +\t\tint nslash = p_value;\n>>>  \t\tswitch (name[len]) {\n>>>  \t\tdefault:\n>>>  \t\t\tcontinue;\n>>> @@ -1137,7 +1138,7 @@ static char *git_header_name(char *line, int llen)\n>>>  \t\t\t\tchar c = *second++;\n>>>  \t\t\t\tif (c == '\\n')\n>>>  \t\t\t\t\treturn NULL;\n>>> -\t\t\t\tif (c == '/')\n>>> +\t\t\t\tif (c == '/' && --nslash <= 0)\n>>>  \t\t\t\t\tbreak;\n>>>  \t\t\t}\n>>>  \t\t\tif (second[len] == '\\n' && !memcmp(name, second, len)) {\n>>> -- \n>>> 1.7.3.2.245.g03276.dirty\n> \n> \n"}]}