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

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

[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)) {
-- 
1.7.3.2.245.g03276.dirty

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

[Jiang Xin]

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 
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

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

[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?

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?

> ---
>  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

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

[Jiang Xin]

于 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


-- 
Jiang Xin

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

[Jiang Xin]

> 于 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 写道:
> 
> 于 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
> 
>