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

Tolga Ceylan· May 7, 2014, 05:48 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. When git-p4 runs git-apply to test the patch, git-apply rejects the patch due to abbreviated blob object names. 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

Signed-off-by: Tolga Ceylan <tolga.ceylan@gmail.com>
Acked-by: Pete Wyckoff <pw@padd.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

Junio C Hamano· May 7, 2014, 17:27 UTC · re: Tolga Ceylan · lore

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

Tolga Ceylan <tolga.ceylan@gmail.com> writes:

Show 13 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. When git-p4 runs git-apply to test
> the patch, git-apply rejects the patch due to abbreviated blob
> object names. 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
>
> Signed-off-by: Tolga Ceylan <tolga.ceylan@gmail.com>
> Acked-by: Pete Wyckoff <pw@padd.com>
> ---

Because the original breakage was already in 1.9, not a regression between 1.9 and master, as the matter of principle our default is to defer until 2.0 final to avoid risking unintended additional breakages elsewhere. But this fix is an obviously correct and trivial single liner that were eyeballed by more than one person, and that affects only three calls to os.system(), and its correctness can be seen even without knowing p4 at all by somebody like me ;-)

So let's queue it for 2.0.

Thanks.

Show 16 quoted lines

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

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