{"thread":{"id":"36593","subject":"[PATCH] git-p4: format-patch to diff-tree change breaks binary patches","startedAt":"2014-05-07T05:48:54Z","lastAt":"2014-05-07T17:27:14Z","messageCount":2,"participants":["Tolga Ceylan","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"240870","messageId":"20140507054854.GA3571@olive","threadId":"36593","inReplyTo":null,"subject":"[PATCH] git-p4: format-patch to diff-tree change breaks binary patches","fromName":"Tolga Ceylan","fromEmail":"tolga.ceylan@gmail.com","sentAt":"2014-05-07T05:48:54Z","receivedAt":"2014-05-07T05:48:54Z","isPatch":true,"sender":{"key":"tolga.ceylan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/6537562?v=4"},"body":"When applying binary patches a full index is required. format-patch\nalready handles this, but diff-tree needs '--full-index' argument\nto always output full index. When git-p4 runs git-apply to test\nthe patch, git-apply rejects the patch due to abbreviated blob\nobject names. This is the error message git-apply emits in this\ncase:\n\nerror: cannot apply binary patch to '<filename>' without full index line\nerror: <filename>: patch does not apply\n\nSigned-off-by: Tolga Ceylan <tolga.ceylan@gmail.com>\nAcked-by: Pete Wyckoff <pw@padd.com>\n---\n git-p4.py |    2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex cdfa2df..4ee6739 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -1311,7 +1311,7 @@ class P4Submit(Command, P4UserMap):\n             else:\n                 die(\"unknown modifier %s for %s\" % (modifier, path))\n \n-        diffcmd = \"git diff-tree -p \\\"%s\\\"\" % (id)\n+        diffcmd = \"git diff-tree --full-index -p \\\"%s\\\"\" % (id)\n         patchcmd = diffcmd + \" | git apply \"\n         tryPatchCmd = patchcmd + \"--check -\"\n         applyPatchCmd = patchcmd + \"--check --apply -\"\n-- \n1.7.9.5\n"},{"id":"240920","messageId":"xmqqtx91bitp.fsf@gitster.dls.corp.google.com","threadId":"36593","inReplyTo":"20140507054854.GA3571@olive","subject":"Re: [PATCH] git-p4: format-patch to diff-tree change breaks binary patches","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-05-07T17:27:14Z","receivedAt":"2014-05-07T17:27:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tolga Ceylan <tolga.ceylan@gmail.com> writes:\n\n> When applying binary patches a full index is required. format-patch\n> already handles this, but diff-tree needs '--full-index' argument\n> to always output full index. When git-p4 runs git-apply to test\n> the patch, git-apply rejects the patch due to abbreviated blob\n> object names. This is the error message git-apply emits in this\n> case:\n>\n> error: cannot apply binary patch to '<filename>' without full index line\n> error: <filename>: patch does not apply\n>\n> Signed-off-by: Tolga Ceylan <tolga.ceylan@gmail.com>\n> Acked-by: Pete Wyckoff <pw@padd.com>\n> ---\n\nBecause the original breakage was already in 1.9, not a regression\nbetween 1.9 and master, as the matter of principle our default is to\ndefer until 2.0 final to avoid risking unintended additional\nbreakages elsewhere.  But this fix is an obviously correct and\ntrivial single liner that were eyeballed by more than one person,\nand that affects only three calls to os.system(), and its\ncorrectness can be seen even without knowing p4 at all by somebody\nlike me ;-)\n\nSo let's queue it for 2.0.\n\nThanks.\n\n>  git-p4.py |    2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/git-p4.py b/git-p4.py\n> index cdfa2df..4ee6739 100755\n> --- a/git-p4.py\n> +++ b/git-p4.py\n> @@ -1311,7 +1311,7 @@ class P4Submit(Command, P4UserMap):\n>              else:\n>                  die(\"unknown modifier %s for %s\" % (modifier, path))\n>  \n> -        diffcmd = \"git diff-tree -p \\\"%s\\\"\" % (id)\n> +        diffcmd = \"git diff-tree --full-index -p \\\"%s\\\"\" % (id)\n>          patchcmd = diffcmd + \" | git apply \"\n>          tryPatchCmd = patchcmd + \"--check -\"\n>          applyPatchCmd = patchcmd + \"--check --apply -\"\n"}]}