Re: git diff --text does not work during rebase for binary files

From: Jeff King <peff@peff.net>
Date: Mar 26, 2021, 11:31 UTC
Message-ID: <YF3GDgYSo889TaRJ@coredump.intra.peff.net>
In-Reply-To: <3807b29f442ab7026418511f058ff9513ee200ec.camel@aegee.org>

On Mon, Mar 22, 2021 at 10:45:39AM +0200, Дилян Палаузов wrote:

Show 5 quoted lines

> And now `git diff` and `git diff --text` both print:
> 
> diff --cc a.txt
> index ee95164,d7db2f0..0000000
> Binary files differ

I think the "combined diff" code isn't ready to handle the "--text" flag.

An easier reproduction than rebase is to just do a merge with a conflict:

  git init
  printf 'base\0' >file && git add file && git commit -m base
  printf 'one\0' >file && git commit -am one
  git checkout -b side HEAD^
  printf 'two\0' >file && git commit -am two
  git merge master

Now "git diff --text" will try to show a combined diff, but it doesn't work. Likewise if you complete the merge:

  printf 'new\0' >file && git commit -a --no-edit

then "git show --text" will not show a binary combined diff, even though "git show --text HEAD^" will happily show a normal two-endpoint diff.

Curiously old versions of Git would show broken binary combined diffs, until 4d5f347199 (combine-diff: handle binary files as binary, 2011-05-23). So the binary detection there looks mostly sane, but it's just not respecting the "--text" flag.

Something like this helps a bit:

diff --git a/combine-diff.c b/combine-diff.c
index 06635f91bc..940c5c43ff 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -1128,6 +1128,8 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
 
 	if (textconv)
 		is_binary = 0;
+	else if (opt->flags.text)
+		is_binary = 0;
 	else if (userdiff->binary != -1)
 		is_binary = userdiff->binary;
 	else {

but I don't think that's the full story. It looks like the combined-diff
code is too eager to treat the content as NUL-terminated strings. A
regular diff shows the NULs:

  $ git show --format= --text HEAD^  | cat -A
  diff --git a/file b/file$
  index d249428..a9382c3 100644$
  --- a/file$
  +++ b/file$
  @@ -1 +1 @@$
  -base^@$
  \ No newline at end of file$
  +two^@$
  \ No newline at end of file$

but the combined diff doesn't:

  $ git show --format= --text HEAD | cat -A
  diff --cc file$
  index a9382c3,f33a432..c984a04$
  --- a/file$
  +++ b/file$
  @@@ -1,1 -1,1 +1,1 @@@$
  - two$
   -one$
  ++new$

So I suspect the code is simply not read to handle the NULs, and the
current behavior of ignoring --text is better than producing truncated
or bogus output until those deeper problems are fixed. We could issue a
warning that --text was ignored, I suppose, but that doesn't really
solve your problem. :)

-Peff