threads / patch / 36494

patchgit-p4: format-patch to diff-tree change breaks binary patches

Subject: [PATCH] git-p4: format-patch to diff-tree change breaks binary patches

## tl;dr

6 messages between Apr 25, 2014 and May 5, 2014. Diffs are folded; open one to read it.

replies: 5people: 2as markdown or json

Tolga Ceylan· Apr 25, 2014, 04:46 UTC · lore

When applying binary patches a full index is required. format-patch already handles this, but diff-tree needs '--full-index' argument to always output full index.

Signed-off-by: Tolga Ceylan <tolga.ceylan@gmail.com>
---
 git-p4.py |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Show changes to git-p4.py +1 −1
diff --git a/git-p4.py b/git-p4.py
index cdfa2df..4ee6739 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -1311,7 +1311,7 @@ class P4Submit(Command, P4UserMap):
             else:
                 die("unknown modifier %s for %s" % (modifier, path))
 
-        diffcmd = "git diff-tree -p \"%s\"" % (id)
+        diffcmd = "git diff-tree --full-index -p \"%s\"" % (id)
         patchcmd = diffcmd + " | git apply "
         tryPatchCmd = patchcmd + "--check -"
         applyPatchCmd = patchcmd + "--check --apply -"
-- 
1.7.9.5
Pete Wyckoff· Apr 26, 2014, 12:43 UTC · re: Tolga Ceylan · lore

Re: [PATCH] git-p4: format-patch to diff-tree change breaks binary patches

tolga.ceylan@gmail.com wrote on Thu, 24 Apr 2014 21:46 -0700:
Show 23 quoted lines
> When applying binary patches a full index is required. format-patch
> already handles this, but diff-tree needs '--full-index' argument
> to always output full index.
> 
> Signed-off-by: Tolga Ceylan <tolga.ceylan@gmail.com>
> ---
>  git-p4.py |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/git-p4.py b/git-p4.py
> index cdfa2df..4ee6739 100755
> --- a/git-p4.py
> +++ b/git-p4.py
> @@ -1311,7 +1311,7 @@ class P4Submit(Command, P4UserMap):
>              else:
>                  die("unknown modifier %s for %s" % (modifier, path))
>  
> -        diffcmd = "git diff-tree -p \"%s\"" % (id)
> +        diffcmd = "git diff-tree --full-index -p \"%s\"" % (id)
>          patchcmd = diffcmd + " | git apply "
>          tryPatchCmd = patchcmd + "--check -"
>          applyPatchCmd = patchcmd + "--check --apply -"
> -- 

This looks like a straightforward change, but can you give a bit more background on why a full index is required? Do you mean that "git apply" will reject a patch with abbreviated blob object names?

		-- Pete
tolga ceylan· Apr 26, 2014, 21:12 UTC · re: Pete Wyckoff · lore

Re: [PATCH] git-p4: format-patch to diff-tree change breaks binary patches

Yes, when git-p4 runs git-apply to test the patch, this fails due to abbreviated blob object names. I think git-apply requires full object names for binary patches.

On 04/26/2014 05:43 AM, Pete Wyckoff wrote:
Show 32 quoted lines
> tolga.ceylan@gmail.com wrote on Thu, 24 Apr 2014 21:46 -0700:
>> When applying binary patches a full index is required. format-patch
>> already handles this, but diff-tree needs '--full-index' argument
>> to always output full index.
>>
>> Signed-off-by: Tolga Ceylan <tolga.ceylan@gmail.com>
>> ---
>>   git-p4.py |    2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/git-p4.py b/git-p4.py
>> index cdfa2df..4ee6739 100755
>> --- a/git-p4.py
>> +++ b/git-p4.py
>> @@ -1311,7 +1311,7 @@ class P4Submit(Command, P4UserMap):
>>               else:
>>                   die("unknown modifier %s for %s" % (modifier, path))
>>
>> -        diffcmd = "git diff-tree -p \"%s\"" % (id)
>> +        diffcmd = "git diff-tree --full-index -p \"%s\"" % (id)
>>           patchcmd = diffcmd + " | git apply "
>>           tryPatchCmd = patchcmd + "--check -"
>>           applyPatchCmd = patchcmd + "--check --apply -"
>> --
>
> This looks like a straightforward change, but can you give a
> bit more background on why a full index is required?  Do you
> mean that "git apply" will reject a patch with abbreviated
> blob object names?
>
> 		-- Pete
>
tolga ceylan· Apr 26, 2014, 21:31 UTC · re: tolga ceylan · lore

Re: [PATCH] git-p4: format-patch to diff-tree change breaks binary patches

On 04/26/2014 02:12 PM, tolga ceylan wrote:
Show 11 quoted lines
> Yes, when git-p4 runs git-apply to test the patch, this fails
> due to abbreviated blob object names. I think git-apply requires
> full object names for binary patches.
>
>> This looks like a straightforward change, but can you give a
>> bit more background on why a full index is required?  Do you
>> mean that "git apply" will reject a patch with abbreviated
>> blob object names?
>>
>>         -- Pete
>>
This is the error message git-apply emits in this case:

error: cannot apply binary patch to '<filename>' without full index line error: <filename>: patch does not apply

Cheers, Tolga

tolga ceylan· May 3, 2014, 05:40 UTC · re: tolga ceylan · lore

Re: [PATCH] git-p4: format-patch to diff-tree change breaks binary patches

Show 8 quoted lines
>
> This is the error message git-apply emits in this case:
>
> error: cannot apply binary patch to '<filename>' without full index line
> error: <filename>: patch does not apply
>
> Cheers,
> Tolga

Any feedback is appreciated. Cheers, Tolga

Pete Wyckoff· May 5, 2014, 13:14 UTC · re: tolga ceylan · lore

Re: [PATCH] git-p4: format-patch to diff-tree change breaks binary patches

tolga.ceylan@gmail.com wrote on Fri, 02 May 2014 22:40 -0700:
Show 11 quoted lines
> 
> >
> >This is the error message git-apply emits in this case:
> >
> >error: cannot apply binary patch to '<filename>' without full index line
> >error: <filename>: patch does not apply
> >
> >Cheers,
> >Tolga
> 
> Any feedback is appreciated.

Sorry, travel delay. This explanation is pretty straight-forward, thanks.

Suggest you include it in the commit message along with the other text you had, and resend to the list, cc me and junio. Oh, and include an ack:

    Acked-by: Pete Wyckoff <pw@padd.com>

← back to recent threads