On Mon, Mar 22, 2021 at 10:45:39AM +0200, Дилян Палаузов wrote:
> 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
