{"thread":{"id":"15792","subject":"[PATCH] fix bogus \"diff --git\" header from \"diff --no-index\"","startedAt":"2008-10-05T19:35:15Z","lastAt":"2008-10-05T20:47:47Z","messageCount":3,"participants":["Jeff King","Linus Torvalds","Imre Deak"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"92351","messageId":"20081005193515.GA29022@coredump.intra.peff.net","threadId":"15792","inReplyTo":null,"subject":"[PATCH] fix bogus \"diff --git\" header from \"diff --no-index\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-10-05T19:35:15Z","receivedAt":"2008-10-05T19:35:15Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"From: Linus Torvalds \n\nWhen \"git diff --no-index\" is given an absolute pathname, it\nwould generate a diff header with the absolute path\nprepended by the prefix, like:\n\n diff --git a/dev/null b/foo\n\nNot only is this nonsensical, and not only does it violate\nthe description of diffs given in git-diff(1), but it would\nproduce broken binary diffs. Unlike text diffs, the binary\ndiffs don't contain the filenames anywhere else, and so \"git\napply\" relies on this header to figure out the filename.\n\nThis patch just refuses to use an invalid name for anything\nvisible in the diff.\n\nNow, this fixes the \"git diff --no-index --binary a\n/dev/null\" kind of case (and we'll end up using \"a\" as the\nbasename), but some other insane cases are impossible to\nhandle. If you do\n\n\tgit diff --no-index --binary a /bin/echo\n\nyou'll still get a patch like\n\n\tdiff --git a/a b/bin/echo\n\told mode 100644\n\tnew mode 100755\n\tindex ...\n\nand \"git apply\" will refuse to apply it for a couple of\nreasons, and the diff is simply bogus.\n\nAnd that, btw, is no longer a bug, I think. It's impossible\nto know whethe the user meant for the patch to be a rename\nor not. And as such, refusing to apply it because you don't\nknow what name you should use is probably _exactly_ the\nright thing to do!\n\nOriginal problem reported by Imre Deak. Test script and problem\ndescription by Jeff King.\n\nSigned-off-by: Jeff King \n---\n diff.c | 4 ++++\n t/t4012-diff-binary.sh | 21 +++++++++++++++++++++\n 2 files changed, 25 insertions(+), 0 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex 4e4e439..02e948c 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -1509,6 +1509,10 @@ static void builtin_diff(const char *name_a,\n \t\tb_prefix = o->b_prefix;\n \t}\n \n+\t/* Never use a non-valid filename anywhere if at all possible */\n+\tname_a = DIFF_FILE_VALID(one) ? name_a : name_b;\n+\tname_b = DIFF_FILE_VALID(two) ? name_b : name_a;\n+\n \ta_one = quote_two(a_prefix, name_a + (*name_a == '/'));\n \tb_two = quote_two(b_prefix, name_b + (*name_b == '/'));\n \tlbl[0] = DIFF_FILE_VALID(one) ? a_one : \"/dev/null\";\ndiff --git a/t/t4012-diff-binary.sh b/t/t4012-diff-binary.sh\nindex b8ec6e9..421f4bb 100755\n--- a/t/t4012-diff-binary.sh\n+++ b/t/t4012-diff-binary.sh\n@@ -77,4 +77,25 @@ test_expect_success 'apply binary patch' \\\n \t tree1=`git write-tree` &&\n \t test \"$tree1\" = \"$tree0\"'\n \n+q_to_nul() {\n+\tperl -pe 'y/Q/\\000/'\n+}\n+\n+nul_to_q() {\n+\tperl -pe 'y/\\000/Q/'\n+}\n+\n+test_expect_success 'diff --no-index with binary creation' '\n+\techo Q | q_to_nul >binary &&\n+\t(:# hide error code from diff, which just indicates differences\n+\t git diff --binary --no-index /dev/null binary >current ||\n+\t true\n+\t) &&\n+\trm binary &&\n+\tgit apply --binary expected &&\n+\tnul_to_q actual &&\n+\ttest_cmp expected actual\n+'\n+\n test_done\n-- \n1.6.0.2.638.g50eb2\n"},{"id":"92358","messageId":"alpine.LFD.2.00.0810051333110.3208@nehalem.linux-foundation.org","threadId":"15792","inReplyTo":"20081005193515.GA29022@coredump.intra.peff.net","subject":"Re: [PATCH] fix bogus \"diff --git\" header from \"diff --no-index\"","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2008-10-05T20:40:24Z","receivedAt":"2008-10-05T20:40:24Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Sun, 5 Oct 2008, Jeff King wrote:\n> From: Linus Torvalds \n> \n> Signed-off-by: Jeff King \n\nTests and explanation looks fine, so you can add my Signed-off-by: too..\n\n\tSigned-off-by: Linus Torvalds \n\nThanks,\n\n\t\tLinus\n"},{"id":"92359","messageId":"500f3d130810051347g4033c86euf683d0e29cfb9f43@mail.gmail.com","threadId":"15792","inReplyTo":"alpine.LFD.2.00.0810051333110.3208@nehalem.linux-foundation.org","subject":"Re: [PATCH] fix bogus \"diff --git\" header from \"diff --no-index\"","fromName":"Imre Deak","fromEmail":"imre.deak@gmail.com","sentAt":"2008-10-05T20:47:47Z","receivedAt":"2008-10-05T20:47:47Z","isPatch":true,"sender":{"key":"imre.deak@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1049948?v=4"},"body":"This solved my problem, thanks for the fast fix!\n\n--Imre\n\nOn Sun, Oct 5, 2008 at 11:40 PM, Linus Torvalds\n wrote:\n>\n>\n> On Sun, 5 Oct 2008, Jeff King wrote:\n>> From: Linus Torvalds \n>>\n>> Signed-off-by: Jeff King \n>\n> Tests and explanation looks fine, so you can add my Signed-off-by: too..\n>\n> Signed-off-by: Linus Torvalds \n>\n> Thanks,\n>\n> Linus\n>\n"}]}