# [PATCH 3/3] git-merge-file: refuse to merge binary files 5 messages from 2007-06-05 to 2007-06-05. Participants: Johannes Schindelin, Linus Torvalds, Junio C Hamano. Thread: https://gitlist.dev/t/8431 ## Johannes Schindelin, 2007-06-05 02:37 Subject: [PATCH 3/3] git-merge-file: refuse to merge binary files Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.64.0706050337010.4046%40racer.site ``` Signed-off-by: Johannes Schindelin --- builtin-merge-file.c | 6 +++++- t/t6023-merge-file.sh | 5 +++++ 2 files changed, 10 insertions(+), 1 deletions(-) diff --git a/builtin-merge-file.c b/builtin-merge-file.c index 9135773..10ec63b 100644 --- a/builtin-merge-file.c +++ b/builtin-merge-file.c @@ -36,9 +36,13 @@ int cmd_merge_file(int argc, char **argv, char **envp) for (; i < 3; i++) names[i] = argv[i + 1]; - for (i = 0; i < 3; i++) + for (i = 0; i < 3; i++) { if (read_mmfile(mmfs + i, argv[i + 1])) return -1; + if (buffer_is_binary(mmfs[i].ptr, mmfs[i].size)) + return error("Cannot merge binary files: %s\n", + argv[i + 1]); + } ret = xdl_merge(mmfs + 1, mmfs + 0, names[0], mmfs + 2, names[2], &xpp, XDL_MERGE_ZEALOUS, &result); diff --git a/t/t6023-merge-file.sh b/t/t6023-merge-file.sh index c76fccf..1decbfb 100755 --- a/t/t6023-merge-file.sh +++ b/t/t6023-merge-file.sh @@ -134,5 +134,10 @@ EOF test_expect_success "expected conflict markers" "git diff expect out" +test_expect_success 'binary files cannot be merged' ' + ! git merge-file -p orig.txt ../test4012.png new1.txt 2> merge.err && + grep "Cannot merge binary files" merge.err +' + test_done -- 1.5.2.1.2626.ge1044-dirty ``` ## Linus Torvalds, 2007-06-05 04:00 Subject: Re: [PATCH 3/3] git-merge-file: refuse to merge binary files Message-ID: URL: https://gitlist.dev/e/alpine.LFD.0.98.0706042059420.23741%40woody.linux-foundation.org In-Reply-To: ``` I really think this would be better off using the crlf-like heuristics. It's entirely possible that the "NUL character in the first X bytes" heuristic is wrong for some cases, so I find it a bit nasty to hardcoding it as the only rule for "it must be binary" Linus ``` ## Johannes Schindelin, 2007-06-05 05:11 Subject: Re: [PATCH 3/3] git-merge-file: refuse to merge binary files Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.64.0706050607490.4046%40racer.site In-Reply-To: ``` Hi, On Mon, 4 Jun 2007, Linus Torvalds wrote: > I really think this would be better off using the crlf-like heuristics. I don't know. Given that it took quite a while to actually hit this bug... > It's entirely possible that the "NUL character in the first X bytes" > heuristic is wrong for some cases, so I find it a bit nasty to > hardcoding it as the only rule for "it must be binary" Yes, you're right. However, if you call git-merge-file, you have to be aware of what it does. It is a _standalone_ program, meant to make it easier to write scripts. Maybe it is not worth detecting binary files _at all_ in merge-file. Ciao, Dscho ``` ## Junio C Hamano, 2007-06-05 05:16 Subject: Re: [PATCH 3/3] git-merge-file: refuse to merge binary files Message-ID: <7vwsyjgeuz.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7vwsyjgeuz.fsf%40assigned-by-dhcp.cox.net In-Reply-To: ``` Linus Torvalds writes: > I really think this would be better off using the crlf-like heuristics. > > It's entirely possible that the "NUL character in the first X bytes" > heuristic is wrong for some cases, so I find it a bit nasty to hardcoding > it as the only rule for "it must be binary" > > Linus On the other hand that hardcoded fallback is consistent with diff, which is closely related to xdl_merge(). Attributes can be specified to override this anyway, so... ``` ## Junio C Hamano, 2007-06-05 05:16 Subject: Re: [PATCH 3/3] git-merge-file: refuse to merge binary files Message-ID: <7vr6orgeuy.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7vr6orgeuy.fsf%40assigned-by-dhcp.cox.net In-Reply-To: ``` Johannes Schindelin writes: > Signed-off-by: Johannes Schindelin > --- > builtin-merge-file.c | 6 +++++- > t/t6023-merge-file.sh | 5 +++++ > 2 files changed, 10 insertions(+), 1 deletions(-) > > diff --git a/builtin-merge-file.c b/builtin-merge-file.c > index 9135773..10ec63b 100644 > --- a/builtin-merge-file.c > +++ b/builtin-merge-file.c > @@ -36,9 +36,13 @@ int cmd_merge_file(int argc, char **argv, char **envp) > for (; i < 3; i++) > names[i] = argv[i + 1]; > > - for (i = 0; i < 3; i++) > + for (i = 0; i < 3; i++) { > if (read_mmfile(mmfs + i, argv[i + 1])) > return -1; > + if (buffer_is_binary(mmfs[i].ptr, mmfs[i].size)) > + return error("Cannot merge binary files: %s\n", > + argv[i + 1]); > + } > > ret = xdl_merge(mmfs + 1, mmfs + 0, names[0], mmfs + 2, names[2], > &xpp, XDL_MERGE_ZEALOUS, &result); Oh, my. I wonder if we can expose ll_merge() from merge-recursive to this part, instead of raw xdl_merge(). That would give you the custom merge driver the user may have specified in .gitattributes. ```