{"thread":{"id":"8431","subject":"[PATCH 3/3] git-merge-file: refuse to merge binary files","startedAt":"2007-06-05T02:37:13Z","lastAt":"2007-06-05T05:16:53Z","messageCount":5,"participants":["Johannes Schindelin","Linus Torvalds","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"44022","messageId":"Pine.LNX.4.64.0706050337010.4046@racer.site","threadId":"8431","inReplyTo":null,"subject":"[PATCH 3/3] git-merge-file: refuse to merge binary files","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-06-05T02:37:13Z","receivedAt":"2007-06-05T02:37:13Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nSigned-off-by: Johannes Schindelin \n---\n builtin-merge-file.c | 6 +++++-\n t/t6023-merge-file.sh | 5 +++++\n 2 files changed, 10 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin-merge-file.c b/builtin-merge-file.c\nindex 9135773..10ec63b 100644\n--- a/builtin-merge-file.c\n+++ b/builtin-merge-file.c\n@@ -36,9 +36,13 @@ int cmd_merge_file(int argc, char **argv, char **envp)\n \tfor (; i < 3; i++)\n \t\tnames[i] = argv[i + 1];\n \n-\tfor (i = 0; i < 3; i++)\n+\tfor (i = 0; i < 3; i++) {\n \t\tif (read_mmfile(mmfs + i, argv[i + 1]))\n \t\t\treturn -1;\n+\t\tif (buffer_is_binary(mmfs[i].ptr, mmfs[i].size))\n+\t\t\treturn error(\"Cannot merge binary files: %s\\n\",\n+\t\t\t\t\targv[i + 1]);\n+\t}\n \n \tret = xdl_merge(mmfs + 1, mmfs + 0, names[0], mmfs + 2, names[2],\n \t\t\t&xpp, XDL_MERGE_ZEALOUS, &result);\ndiff --git a/t/t6023-merge-file.sh b/t/t6023-merge-file.sh\nindex c76fccf..1decbfb 100755\n--- a/t/t6023-merge-file.sh\n+++ b/t/t6023-merge-file.sh\n@@ -134,5 +134,10 @@ EOF\n \n test_expect_success \"expected conflict markers\" \"git diff expect out\"\n \n+test_expect_success 'binary files cannot be merged' '\n+\t! git merge-file -p orig.txt ../test4012.png new1.txt 2> merge.err &&\n+\tgrep \"Cannot merge binary files\" merge.err\n+'\n+\n test_done\n \n-- \n1.5.2.1.2626.ge1044-dirty\n"},{"id":"44028","messageId":"alpine.LFD.0.98.0706042059420.23741@woody.linux-foundation.org","threadId":"8431","inReplyTo":"Pine.LNX.4.64.0706050337010.4046@racer.site","subject":"Re: [PATCH 3/3] git-merge-file: refuse to merge binary files","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-06-05T04:00:46Z","receivedAt":"2007-06-05T04:00:46Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nI really think this would be better off using the crlf-like heuristics.\n\nIt's entirely possible that the \"NUL character in the first X bytes\" \nheuristic is wrong for some cases, so I find it a bit nasty to hardcoding \nit as the only rule for \"it must be binary\"\n\n\t\tLinus\n"},{"id":"44035","messageId":"Pine.LNX.4.64.0706050607490.4046@racer.site","threadId":"8431","inReplyTo":"alpine.LFD.0.98.0706042059420.23741@woody.linux-foundation.org","subject":"Re: [PATCH 3/3] git-merge-file: refuse to merge binary files","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-06-05T05:11:33Z","receivedAt":"2007-06-05T05:11:33Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 4 Jun 2007, Linus Torvalds wrote:\n\n> I really think this would be better off using the crlf-like heuristics.\n\nI don't know. Given that it took quite a while to actually hit this bug...\n\n> It's entirely possible that the \"NUL character in the first X bytes\" \n> heuristic is wrong for some cases, so I find it a bit nasty to \n> hardcoding it as the only rule for \"it must be binary\"\n\nYes, you're right.\n\nHowever, if you call git-merge-file, you have to be aware of what it does. \nIt is a _standalone_ program, meant to make it easier to write scripts. \nMaybe it is not worth detecting binary files _at all_ in merge-file.\n\nCiao,\nDscho\n"},{"id":"44036","messageId":"7vwsyjgeuz.fsf@assigned-by-dhcp.cox.net","threadId":"8431","inReplyTo":"alpine.LFD.0.98.0706042059420.23741@woody.linux-foundation.org","subject":"Re: [PATCH 3/3] git-merge-file: refuse to merge binary files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-06-05T05:16:52Z","receivedAt":"2007-06-05T05:16:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds writes:\n\n> I really think this would be better off using the crlf-like heuristics.\n>\n> It's entirely possible that the \"NUL character in the first X bytes\" \n> heuristic is wrong for some cases, so I find it a bit nasty to hardcoding \n> it as the only rule for \"it must be binary\"\n>\n> \t\tLinus\n\nOn the other hand that hardcoded fallback is consistent with\ndiff, which is closely related to xdl_merge(). Attributes can\nbe specified to override this anyway, so...\n"},{"id":"44037","messageId":"7vr6orgeuy.fsf@assigned-by-dhcp.cox.net","threadId":"8431","inReplyTo":"Pine.LNX.4.64.0706050337010.4046@racer.site","subject":"Re: [PATCH 3/3] git-merge-file: refuse to merge binary files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-06-05T05:16:53Z","receivedAt":"2007-06-05T05:16:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> Signed-off-by: Johannes Schindelin \n> ---\n> builtin-merge-file.c | 6 +++++-\n> t/t6023-merge-file.sh | 5 +++++\n> 2 files changed, 10 insertions(+), 1 deletions(-)\n>\n> diff --git a/builtin-merge-file.c b/builtin-merge-file.c\n> index 9135773..10ec63b 100644\n> --- a/builtin-merge-file.c\n> +++ b/builtin-merge-file.c\n> @@ -36,9 +36,13 @@ int cmd_merge_file(int argc, char **argv, char **envp)\n> \tfor (; i < 3; i++)\n> \t\tnames[i] = argv[i + 1];\n> \n> -\tfor (i = 0; i < 3; i++)\n> +\tfor (i = 0; i < 3; i++) {\n> \t\tif (read_mmfile(mmfs + i, argv[i + 1]))\n> \t\t\treturn -1;\n> +\t\tif (buffer_is_binary(mmfs[i].ptr, mmfs[i].size))\n> +\t\t\treturn error(\"Cannot merge binary files: %s\\n\",\n> +\t\t\t\t\targv[i + 1]);\n> +\t}\n> \n> \tret = xdl_merge(mmfs + 1, mmfs + 0, names[0], mmfs + 2, names[2],\n> \t\t\t&xpp, XDL_MERGE_ZEALOUS, &result);\n\nOh, my.\n\nI wonder if we can expose ll_merge() from merge-recursive to\nthis part, instead of raw xdl_merge(). That would give you the\ncustom merge driver the user may have specified in\n.gitattributes.\n"}]}