diffstat summary mode change bug
To
Stefan Beller
Junio C Hamano
Cc
Git Mailing List
From
Linus Torvalds
Date
2017-09-27 18:15:04 UTC
Current git shows file-mode changes incorrectly in the diffstat
summary, as I just noted from a pull request I did on the kernel.

The pull request *should* have resulted in a summary like this:

   ...
   21 files changed, 247 insertions(+), 67 deletions(-)
   mode change 100644 => 100755 tools/testing/selftests/memfd/run_tests.sh
   create mode 100644 tools/testing/selftests/net/reuseaddr_conflict.c

but instead that "mode change" line didn't have a newline at the end, and I got

   ...
   21 files changed, 247 insertions(+), 67 deletions(-)
   mode change 100644 => 100755
tools/testing/selftests/memfd/run_tests.sh create mode 100644
tools/testing/selftests/net/reuseaddr_conflict.c

(ok, linewrapping in this email may make that look wrong - but the
"mode change" land the "create mode" are both on the same line.

Bisecting it got me:

  146fdb0dfe445464fa438f3835557c58a01d85d7 is the first bad commit
  commit 146fdb0dfe445464fa438f3835557c58a01d85d7
  Author: Stefan Beller <sbeller@google.com>
  Date:   Thu Jun 29 17:07:05 2017 -0700

      diff.c: emit_diff_symbol learns about DIFF_SYMBOL_SUMMARY

      Signed-off-by: Stefan Beller <sbeller@google.com>
      Signed-off-by: Junio C Hamano <gitster@pobox.com>

and the reason seems to be that the '\n' at the end got dropped as the
old code was very confusing (the old code had two different '\n' cases
for the "show filename or not").

I think the right fix is this whitespace-damaged trivial one-liner:

  diff --git a/diff.c b/diff.c
  index 3c6a3e0fa..653bb2e72 100644
  --- a/diff.c
  +++ b/diff.c
  @@ -5272,6 +5272,7 @@ static void show_mode_change(struct
diff_options *opt, struct diff_filepair *p,
                          strbuf_addch(&sb, ' ');
                          quote_c_style(p->two->path, &sb, NULL, 0);
                  }
  +               strbuf_addch(&sb, '\n');
                  emit_diff_symbol(opt, DIFF_SYMBOL_SUMMARY,
                                   sb.buf, sb.len, 0);
                  strbuf_release(&sb);

but somebody should double-check that.

                        Linus