Skip to content

Commit

Permalink
diff: handle diffstat of rewritten binary files
Browse files Browse the repository at this point in the history
The logic in builtin_diffstat assumes that a
complete_rewrite pair should have its lines counted. This is
nonsensical for binary files and leads to confusing things
like:

  $ git diff --stat --summary HEAD^ HEAD
   foo.rand |  Bin 4096 -> 4096 bytes
   1 files changed, 0 insertions(+), 0 deletions(-)

  $ git diff --stat --summary -B HEAD^ HEAD
   foo.rand |   34 +++++++++++++++-------------------
   1 files changed, 15 insertions(+), 19 deletions(-)
   rewrite foo.rand (100%)

So let's reorder the function to handle binary files first
(which from diffstat's perspective look like complete
rewrites anyway), then rewrites, then actual diffstats.

There are two bonus prizes to this reorder:

  1. It gets rid of a now-superfluous goto.

  2. The binary case is at the top, which means we can
     further optimize it in the next patch.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
  • Loading branch information
Jeff King authored and Junio C Hamano committed Feb 22, 2011
1 parent e923eae commit ded0abc
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 10 deletions.
24 changes: 14 additions & 10 deletions diff.c
Original file line number Diff line number Diff line change
Expand Up @@ -1811,34 +1811,38 @@ static void builtin_diffstat(const char *name_a, const char *name_b,
data->is_unmerged = 1;
return;
}
if (complete_rewrite) {

if (diff_filespec_is_binary(one) || diff_filespec_is_binary(two)) {
if (fill_mmfile(&mf1, one) < 0 || fill_mmfile(&mf2, two) < 0)
die("unable to read files to diff");
data->is_binary = 1;
data->added = mf2.size;
data->deleted = mf1.size;
}

else if (complete_rewrite) {
diff_populate_filespec(one, 0);
diff_populate_filespec(two, 0);
data->deleted = count_lines(one->data, one->size);
data->added = count_lines(two->data, two->size);
goto free_and_return;
}
if (fill_mmfile(&mf1, one) < 0 || fill_mmfile(&mf2, two) < 0)
die("unable to read files to diff");

if (diff_filespec_is_binary(one) || diff_filespec_is_binary(two)) {
data->is_binary = 1;
data->added = mf2.size;
data->deleted = mf1.size;
} else {
else {
/* Crazy xdl interfaces.. */
xpparam_t xpp;
xdemitconf_t xecfg;
xdemitcb_t ecb;

if (fill_mmfile(&mf1, one) < 0 || fill_mmfile(&mf2, two) < 0)
die("unable to read files to diff");

memset(&xpp, 0, sizeof(xpp));
memset(&xecfg, 0, sizeof(xecfg));
xpp.flags = XDF_NEED_MINIMAL | o->xdl_opts;
xdi_diff_outf(&mf1, &mf2, diffstat_consume, diffstat,
&xpp, &xecfg, &ecb);
}

free_and_return:
diff_free_filespec_data(one);
diff_free_filespec_data(two);
}
Expand Down
7 changes: 7 additions & 0 deletions t/t4031-diff-rewrite-binary.sh
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,13 @@ test_expect_success 'rewrite diff can show binary patch' '
grep "GIT binary patch" diff
'

test_expect_success 'rewrite diff --stat shows binary changes' '
git diff -B --stat --summary >diff &&
grep "Bin" diff &&
grep "0 insertions.*0 deletions" diff &&
grep " rewrite file" diff
'

{
echo "#!$SHELL_PATH"
cat <<'EOF'
Expand Down

0 comments on commit ded0abc

Please sign in to comment.