threads / patch / 25829

patchgit apply: binary files differ can be applied with -pN (N>1).

Subject: [PATCH] git apply: binary files differ can be applied with -pN (N>1).

## tl;dr

5 messages between Nov 24, 2010 and Nov 25, 2010. Diffs are folded; open one to read it.

replies: 4people: 2as markdown or json

Jiang Xin· Nov 24, 2010, 05:20 UTC · lore

When patch file generated against two non-git directories using 'git diff --binary --no-index' without '--no-prefix', the patch file has patch level greater then 1, and should be applied with '-p2' option. But it does not work if there are binary differ in the patch file, it is because in one case the patch level is not properly handled.

Signed-off-by: Jiang Xin <jiangxin@ossxp.com>
---
 builtin/apply.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)
Show changes to builtin/apply.c +2 −1
diff --git a/builtin/apply.c b/builtin/apply.c
index 23c18c5..d603e37 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -1126,6 +1126,7 @@ static char *git_header_name(char *line, int llen)
 	 * form.
 	 */
 	for (len = 0 ; ; len++) {
+		int nslash = p_value;
 		switch (name[len]) {
 		default:
 			continue;
@@ -1137,7 +1138,7 @@ static char *git_header_name(char *line, int llen)
 				char c = *second++;
 				if (c == '\n')
 					return NULL;
-				if (c == '/')
+				if (c == '/' && --nslash <= 0)
 					break;
 			}
 			if (second[len] == '\n' && !memcmp(name, second, len)) {
-- 
1.7.3.2.245.g03276.dirty
Jiang Xin· Nov 24, 2010, 06:53 UTC · re: Jiang Xin · lore

Re: [PATCH] git apply: binary files differ can be applied with -pN (N>1).

How to reproduce the bug:
1. Generate a patch file against two directory with binary diff.
================================================================
$ mkdir hello-1.0 hello-2.0

$ dd if=/bin/ls of=hello-1.0/binary.dat count=1 bs=32 1+0 records in 1+0 records out 32 bytes (32 B) copied, 0.00121276 s, 26.4 kB/s

$ dd if=/bin/ls of=hello-2.0/binary.dat count=1 bs=64 1+0 records in 1+0 records out 64 bytes (64 B) copied, 0.00030288 s, 211 kB/s

$ git diff --no-index --binary hello-1.0 hello-2.0 > patch.txt
2. The patch file has patch level 2
===================================
$ cat patch.txt 
Show changes to hello-1.0/binary.dat +0 −0
diff --git a/hello-1.0/binary.dat b/hello-2.0/binary.dat
index dc2e37f81e0fa88308bec48cd5195b6542e61a20..bf948689934caf2d874ff8168cb716fbc2a127c3 100644
GIT binary patch
delta 37
hcmY#zn4qBGzyJX+<}pH93=9qo77QFfQiegA0RUZd1MdI;

delta 4
LcmZ=zn4kav0;B;E

3. Apply the patch failed
=========================

$ cd hello-1.0

$ git apply --check -p2 ../patch.txt
fatal: git diff header lacks filename information when removing 2 leading pathname components (line 3)


于 2010年11月24日 13:20, Jiang Xin 写道:
> When patch file generated against two non-git directories using
> 'git diff --binary --no-index' without '--no-prefix', the patch
> file has patch level greater then 1, and should be applied with
> '-p2' option. But it does not work if there are binary differ
> in the patch file, it is because in one case the patch level is
> not properly handled.
> 
> Signed-off-by: Jiang Xin <jiangxin@ossxp.com>
> ---
>  builtin/apply.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/builtin/apply.c b/builtin/apply.c
> index 23c18c5..d603e37 100644
> --- a/builtin/apply.c
> +++ b/builtin/apply.c
> @@ -1126,6 +1126,7 @@ static char *git_header_name(char *line, int llen)
>  	 * form.
>  	 */
>  	for (len = 0 ; ; len++) {
> +		int nslash = p_value;
>  		switch (name[len]) {
>  		default:
>  			continue;
> @@ -1137,7 +1138,7 @@ static char *git_header_name(char *line, int llen)
>  				char c = *second++;
>  				if (c == '\n')
>  					return NULL;
> -				if (c == '/')
> +				if (c == '/' && --nslash <= 0)
>  					break;
>  			}
>  			if (second[len] == '\n' && !memcmp(name, second, len)) {
-- 
蒋鑫

北京群英汇信息技术有限公司
邮件: worldhello.net@gmail.com
网址: http://www.ossxp.com/
    http://blog.ossxp.com/
电话: 010-51262007, 13910430470
传真: 010-51262007
Junio C Hamano· Nov 24, 2010, 17:20 UTC · re: Jiang Xin · lore

Re: [PATCH] git apply: binary files differ can be applied with -pN (N>1).

Jiang Xin <worldhello.net@gmail.com> writes:
Show 8 quoted lines
> When patch file generated against two non-git directories using
> 'git diff --binary --no-index' without '--no-prefix', the patch
> file has patch level greater then 1, and should be applied with
> '-p2' option. But it does not work if there are binary differ
> in the patch file, it is because in one case the patch level is
> not properly handled.
>
> Signed-off-by: Jiang Xin <jiangxin@ossxp.com>

Can you please add a testcase to protect your fix from getting broken by later changes by other people, perhaps to t/t4120?

By the way, this codepath is shared by all forms of patches "diff --git" header, not just binary. Do you see a similar breakage with --no-prefix patches that are not binary, and if not why?

Show 27 quoted lines
> ---
>  builtin/apply.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/builtin/apply.c b/builtin/apply.c
> index 23c18c5..d603e37 100644
> --- a/builtin/apply.c
> +++ b/builtin/apply.c
> @@ -1126,6 +1126,7 @@ static char *git_header_name(char *line, int llen)
>  	 * form.
>  	 */
>  	for (len = 0 ; ; len++) {
> +		int nslash = p_value;
>  		switch (name[len]) {
>  		default:
>  			continue;
> @@ -1137,7 +1138,7 @@ static char *git_header_name(char *line, int llen)
>  				char c = *second++;
>  				if (c == '\n')
>  					return NULL;
> -				if (c == '/')
> +				if (c == '/' && --nslash <= 0)
>  					break;
>  			}
>  			if (second[len] == '\n' && !memcmp(name, second, len)) {
> -- 
> 1.7.3.2.245.g03276.dirty
Jiang Xin· Nov 25, 2010, 01:46 UTC · re: Junio C Hamano · lore

Re: [PATCH] git apply: binary files differ can be applied with -pN (N>1).

于 2010年11月25日 01:20, Junio C Hamano 写道:
Show 14 quoted lines
> Jiang Xin <worldhello.net@gmail.com> writes:
> 
>> When patch file generated against two non-git directories using
>> 'git diff --binary --no-index' without '--no-prefix', the patch
>> file has patch level greater then 1, and should be applied with
>> '-p2' option. But it does not work if there are binary differ
>> in the patch file, it is because in one case the patch level is
>> not properly handled.
>>
>> Signed-off-by: Jiang Xin <jiangxin@ossxp.com>
> 
> Can you please add a testcase to protect your fix from getting broken by
> later changes by other people, perhaps to t/t4120?
> 

Yes, I add a testcase in t/t4120, and I will send it as a new PATCH. The new testcase is like following:

+test_expect_success 'apply git diff with -p2 and use default name from header' '
+       sed -e "/^\(---\|+++\) / d" patch.file > patch.newheader &&
+       cp file1.saved file1 &&
+       git apply -p2 patch.newheader
+'
+
> By the way, this codepath is shared by all forms of patches "diff --git"
> header, not just binary.  Do you see a similar breakage with --no-prefix
> patches that are not binary, and if not why?
> 
The breakage will appear under these circumstances:
* the patch is in git style: header with "diff --git".
* the header does not contain '--- path/to/old' and '+++ path/to/new'.
* has a patch level greater than 1.

When I do `git diff` against binary files, I find the patch header does not has '--- path/to/old' and '+++ path/to/new'. May be there exist other cases, but I'm not sure.

Show 27 quoted lines
>> ---
>>  builtin/apply.c |    3 ++-
>>  1 files changed, 2 insertions(+), 1 deletions(-)
>>
>> diff --git a/builtin/apply.c b/builtin/apply.c
>> index 23c18c5..d603e37 100644
>> --- a/builtin/apply.c
>> +++ b/builtin/apply.c
>> @@ -1126,6 +1126,7 @@ static char *git_header_name(char *line, int llen)
>>  	 * form.
>>  	 */
>>  	for (len = 0 ; ; len++) {
>> +		int nslash = p_value;
>>  		switch (name[len]) {
>>  		default:
>>  			continue;
>> @@ -1137,7 +1138,7 @@ static char *git_header_name(char *line, int llen)
>>  				char c = *second++;
>>  				if (c == '\n')
>>  					return NULL;
>> -				if (c == '/')
>> +				if (c == '/' && --nslash <= 0)
>>  					break;
>>  			}
>>  			if (second[len] == '\n' && !memcmp(name, second, len)) {
>> -- 
>> 1.7.3.2.245.g03276.dirty
-- 
Jiang Xin
Jiang Xin· Nov 25, 2010, 02:52 UTC · re: Jiang Xin · lore

Re: [PATCH] git apply: binary files differ can be applied with -pN (N>1).

> 于 2010年11月25日 01:20, Junio C Hamano 写道:
> By the way, this codepath is shared by all forms of patches "diff --git"
> header, not just binary.  Do you see a similar breakage with --no-prefix
> patches that are not binary, and if not why?

Some cases a git diff without "--- path/to/old" and "+++ path/to/new", will make current git apply -p2 broken.

1. Prue file mode changes:
    diff --git a/sub1/file1 b/sub2/file1
    old mode 100644
    new mode 100755
2. Remove empty file:
    diff --git a/sub1/file1 b/sub1/file1
    deleted file mode 100644
    index e69de29..0000000
3. Add empty file:
    diff --git a/sub2/file1 b/sub2/file1
    new file mode 100644
    index 0000000..e69de29
4. New binary file:
    diff --git a/sub2/file1 b/sub2/file1
    new file mode 100644
    index 0000000000000000000000000000000000000000..4dd0c65a410e8aea1608d6b2dd70e74def0012c5
    GIT binary patch
    literal 8
    Pcmb<-^>JfjWMlvU2^Ilc
    
    literal 0
    HcmV?d00001
    
5. Modify binary file:
    diff --git a/sub1/file1 b/sub2/file1
    index f96ec1a8358556543a73b3fb93a0d1c2aad8432a..4dd0c65a410e8aea1608d6b2dd70e74def0012c5 100644
    GIT binary patch
    literal 8
    Pcmb<-^>JfjWMlvU2^Ilc
    literal 7
    Ocmb<-^>JfjWCQ>Qy8&AO
    
I think all these cases can use this testcase in t/t4120-apply-popt.sh:
    +test_expect_success 'apply git diff with -p2 and use default name from header' '
    +       sed -e "/^\(---\|+++\) / d" patch.file > patch.newheader &&
    +       cp file1.saved file1 &&
    +       git apply -p2 patch.newheader
    +'
于 2010年11月25日 09:46, Jiang Xin 写道:
Show 70 quoted lines
> 
> 于 2010年11月25日 01:20, Junio C Hamano 写道:
>> Jiang Xin <worldhello.net@gmail.com> writes:
>>
>>> When patch file generated against two non-git directories using
>>> 'git diff --binary --no-index' without '--no-prefix', the patch
>>> file has patch level greater then 1, and should be applied with
>>> '-p2' option. But it does not work if there are binary differ
>>> in the patch file, it is because in one case the patch level is
>>> not properly handled.
>>>
>>> Signed-off-by: Jiang Xin <jiangxin@ossxp.com>
>>
>> Can you please add a testcase to protect your fix from getting broken by
>> later changes by other people, perhaps to t/t4120?
>>
> 
> Yes, I add a testcase in t/t4120, and I will send it as a new PATCH.
> The new testcase is like following:
> 
> +test_expect_success 'apply git diff with -p2 and use default name from header' '
> +       sed -e "/^\(---\|+++\) / d" patch.file > patch.newheader &&
> +       cp file1.saved file1 &&
> +       git apply -p2 patch.newheader
> +'
> +
> 
>> By the way, this codepath is shared by all forms of patches "diff --git"
>> header, not just binary.  Do you see a similar breakage with --no-prefix
>> patches that are not binary, and if not why?
>>
> 
> The breakage will appear under these circumstances:
> * the patch is in git style: header with "diff --git".
> * the header does not contain '--- path/to/old' and '+++ path/to/new'.
> * has a patch level greater than 1.
> 
> When I do `git diff` against binary files, I find the patch header does not has
> '--- path/to/old' and '+++ path/to/new'. May be there exist other cases, but
> I'm not sure.
> 
>>> ---
>>>  builtin/apply.c |    3 ++-
>>>  1 files changed, 2 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/builtin/apply.c b/builtin/apply.c
>>> index 23c18c5..d603e37 100644
>>> --- a/builtin/apply.c
>>> +++ b/builtin/apply.c
>>> @@ -1126,6 +1126,7 @@ static char *git_header_name(char *line, int llen)
>>>  	 * form.
>>>  	 */
>>>  	for (len = 0 ; ; len++) {
>>> +		int nslash = p_value;
>>>  		switch (name[len]) {
>>>  		default:
>>>  			continue;
>>> @@ -1137,7 +1138,7 @@ static char *git_header_name(char *line, int llen)
>>>  				char c = *second++;
>>>  				if (c == '\n')
>>>  					return NULL;
>>> -				if (c == '/')
>>> +				if (c == '/' && --nslash <= 0)
>>>  					break;
>>>  			}
>>>  			if (second[len] == '\n' && !memcmp(name, second, len)) {
>>> -- 
>>> 1.7.3.2.245.g03276.dirty
> 
> 

← back to recent threads