Skip to content

Commit cdb58cf

Browse files
committed
diff: consult diff process for --stat counts
builtin_diff() already consults a configured diff.<driver>.process: a file the tool reports as equivalent emits no patch, and otherwise the tool's hunks drive the output. builtin_diffstat() ran its own xdiff and ignored the process, so "git diff --stat" still counted a byte-level change for a file that "git diff" showed as unchanged. Consult diff_process_fill_hunks() before the stat xdiff, mirroring the blame integration. On DIFF_PROCESS_EQUIVALENT, skip the xdiff so the file keeps its zero inserted and deleted counts and the existing "nothing changed" pruning drops it, matching the empty patch. Otherwise the tool's hunks, or the builtin fallback, feed the counts through the shared xpparam_t. Like the builtin summary path, builtin_diffstat() does not apply textconv, so the process is consulted on the raw blob content here, unlike builtin_diff() which sends textconv'd content. This keeps "git diff --stat" counting raw lines as it does today; the asymmetry between patch output and summary counts under textconv predates this change. Move the summary formats out of the "not yet wired" group of the "Which features consult the diff process" documentation and into the list of features that use the tool's hunks, noting the raw, non-textconv content they receive. Add tests covering counts that come from the tool's hunks (--numstat), an equivalent file producing no stat line, and the raw, non-textconv content the tool receives for --stat. Document that the line-counting --dirstat=lines follows these counts while the default --dirstat does not, and that summary formats and blame (only under --textconv) differ from patch output in whether they textconv the content the tool sees. Extend the tests to cover --shortstat, --stat --exit-code, a multi-file mix of equivalent and changed files, and a mode-only change. Signed-off-by: Michael Montalbo <mmontalbo@gmail.com>
1 parent dda47f6 commit cdb58cf

3 files changed

Lines changed: 167 additions & 11 deletions

File tree

Documentation/gitattributes.adoc

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -872,7 +872,10 @@ a flush packet, followed by the old and new file content as packetized
872872
data, each terminated with a flush packet. The pathname is relative
873873
to the repository root. When `diff.<name>.textconv` is also set,
874874
the tool receives the textconv-transformed content rather than the
875-
raw blob. Git does not send binary files to the diff process.
875+
raw blob, matching what the consuming feature itself diffs: patch
876+
output is textconv'd, the summary formats (noted below) are not, and
877+
`git blame` applies textconv only under `--textconv`. Git does not
878+
send binary files to the diff process.
876879

877880
-----------------------
878881
packet: git> command=hunks
@@ -943,8 +946,8 @@ still slide or regroup those changes against matching context for
943946
display, exactly as it compacts its own diffs, so the tool controls
944947
which lines are reported as changed, not the precise hunk boundaries.
945948
Patch output features (word diff, function context, color) work
946-
normally. Summary formats such as `--stat` still compute their counts
947-
with the builtin diff for now; see "Which features consult the diff
949+
normally, as do summary formats like `--stat`. Not every feature
950+
consults the process, though; see "Which features consult the diff
948951
process" below for the full picture and the reasoning behind it.
949952

950953
If no hunk lines precede the flush, followed by "success", Git
@@ -1018,6 +1021,17 @@ of the builtin algorithm:
10181021
hunks without any further negotiation.
10191022
- `git blame`: a commit whose change the tool reports as equivalent is
10201023
skipped, and its lines are attributed to an earlier commit.
1024+
- `--stat`, `--numstat`, and `--shortstat`: the inserted and deleted
1025+
counts come from the tool's hunks, so a file the tool calls
1026+
equivalent contributes no stat line, matching the empty patch that
1027+
`git diff` produces for it. These summary formats do not apply
1028+
textconv (just as the builtin summary path does not), so the tool
1029+
is consulted on the raw blob content even when a `textconv` is also
1030+
configured for patch output; this mirrors how builtin `--stat`
1031+
already counts raw lines rather than the textconv'd view. The
1032+
line-counting `--dirstat=lines` uses these same counts; the default
1033+
`--dirstat`, which weighs byte changes, is computed on its own and
1034+
does not consult the tool.
10211035

10221036
Features that ask a different question do not consult the process, by
10231037
design:
@@ -1043,13 +1057,12 @@ design:
10431057
- `--raw`, `--name-only`, and `--name-status` compare object ids at
10441058
the tree level and never run a line-level diff at all.
10451059

1046-
Some features ask "which lines changed" but still use the builtin
1047-
algorithm for now, and may consult the process in a later change: the
1048-
summary formats (`--stat`, `--numstat`, `--shortstat`); `git log -L`'s
1049-
commit selection and parent range propagation (as distinct from its
1050-
display, which is covered above); and combined diffs (`--cc` and merge
1051-
diffs), whose protocol would have to be extended from a single old/new
1052-
pair to one comparison per merge parent.
1060+
Two cases ask "which lines changed" but still use the builtin
1061+
algorithm, and may consult the process in a later change: `git log
1062+
-L`'s commit selection and parent range propagation (as distinct from
1063+
its display, which is covered above), and combined diffs (`--cc` and
1064+
merge diffs), whose protocol would have to be extended from a single
1065+
old/new pair to one comparison per merge parent.
10531066

10541067
`--no-ext-diff` and `--diff-algorithm` bypass the process entirely,
10551068
for every feature listed above. The whitespace-ignoring options

diff.c

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4261,9 +4261,24 @@ static void builtin_diffstat(const char *name_a, const char *name_b,
42614261
xecfg.ctxlen = o->context;
42624262
xecfg.interhunkctxlen = o->interhunkcontext;
42634263
xecfg.flags = XDL_EMIT_NO_HUNK_HDR;
4264-
if (xdi_diff_outf(&mf1, &mf2, NULL,
4264+
/*
4265+
* Consult the diff process so --stat reflects the
4266+
* tool's view of which lines changed rather than the
4267+
* builtin line diff. As in the builtin path, --stat
4268+
* does not apply textconv, so the tool is fed the same
4269+
* raw mmfiles the stat itself diffs (unlike builtin_diff,
4270+
* which consults the process on textconv'd content).
4271+
* When the tool reports the files as equivalent we skip
4272+
* xdiff entirely, leaving added and deleted at zero so
4273+
* the file is pruned below, just as builtin_diff() emits
4274+
* no patch for an equivalent file.
4275+
*/
4276+
if (diff_process_fill_hunks(o, name_a, &mf1, &mf2, &xpp)
4277+
!= DIFF_PROCESS_EQUIVALENT &&
4278+
xdi_diff_outf(&mf1, &mf2, NULL,
42654279
diffstat_consume, diffstat, &xpp, &xecfg))
42664280
die("unable to generate diffstat for %s", one->path);
4281+
free(xpp.external_hunks);
42674282

42684283
if (DIFF_FILE_VALID(one) && DIFF_FILE_VALID(two)) {
42694284
struct diffstat_file *file =

t/t4080-diff-process.sh

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -248,6 +248,21 @@ test_expect_success 'diff process works alongside textconv' '
248248
test_must_be_empty stderr
249249
'
250250

251+
test_expect_success 'diff process --stat is fed raw, not textconv, content' '
252+
# Reuses textconv.c from the previous test (committed "hello
253+
# world", modified to "goodbye world"). Unlike patch output,
254+
# --stat does not apply textconv, so the tool sees raw lowercase
255+
# content here even with a textconv configured.
256+
test_when_finished "rm -f backend.log" &&
257+
git -c diff.cdiff.textconv="./uppercase-filter" \
258+
-c diff.cdiff.process="$BACKEND --log=backend.log" \
259+
diff --stat -- textconv.c >actual 2>stderr &&
260+
test_grep "pathname=textconv.c" backend.log &&
261+
test_grep "old=hello world" backend.log &&
262+
test_grep "new=goodbye world" backend.log &&
263+
test_must_be_empty stderr
264+
'
265+
251266
#
252267
# Downstream features: word diff, log, equivalent files, exit code.
253268
#
@@ -331,6 +346,119 @@ test_expect_success 'diff process with --exit-code and hunks returns failure' '
331346
diff --exit-code newfile.c
332347
'
333348

349+
test_expect_success 'diff process feeds --numstat counts' '
350+
# fixed-hunk reports only lines 5-6 as changed, so the stat
351+
# counts come from the tool (2/2), not the builtin diff (4/4).
352+
test_when_finished "rm -f backend.log" &&
353+
git -c diff.cdiff.process="$BACKEND --mode=fixed-hunk --log=backend.log" \
354+
diff --numstat boundary.c >actual 2>stderr &&
355+
printf "2\t2\tboundary.c\n" >expect &&
356+
test_cmp expect actual &&
357+
test_grep "command=hunks pathname=boundary.c" backend.log &&
358+
test_must_be_empty stderr
359+
'
360+
361+
test_expect_success 'diff process --numstat sums multi-hunk counts' '
362+
# multi-hunk reports both 2-line regions (5-6 and 9-10), so the
363+
# counts add up across both hunks: 4 inserted, 4 deleted. This
364+
# exercises the two-region hunk path through builtin_diffstat.
365+
git -c diff.cdiff.process="$BACKEND --mode=multi-hunk" \
366+
diff --numstat boundary.c >actual &&
367+
printf "4\t4\tboundary.c\n" >expect &&
368+
test_cmp expect actual
369+
'
370+
371+
test_expect_success 'diff process equivalent files produce no --stat line' '
372+
# A file the tool calls equivalent contributes no stat line,
373+
# matching the empty patch that git diff produces for it.
374+
test_when_finished "rm -f backend.log" &&
375+
git -c diff.cdiff.process="$BACKEND --mode=no-hunks --log=backend.log" \
376+
diff --stat worddiff.c >actual 2>stderr &&
377+
test_must_be_empty actual &&
378+
test_grep "command=hunks pathname=worddiff.c" backend.log &&
379+
test_must_be_empty stderr
380+
'
381+
382+
test_expect_success 'diff process feeds --shortstat counts' '
383+
# fixed-hunk reports lines 5-6 only, so the summary counts come
384+
# from the tool (2 insertions, 2 deletions), not builtin (4/4).
385+
git -c diff.cdiff.process="$BACKEND --mode=fixed-hunk" \
386+
diff --shortstat boundary.c >actual &&
387+
test_grep "2 insertions" actual &&
388+
test_grep "2 deletions" actual
389+
'
390+
391+
test_expect_success 'diff process equivalent file makes --stat --exit-code succeed' '
392+
# The tool reports worddiff.c equivalent, so --exit-code reports
393+
# no change (0); the builtin diff would report a change (1).
394+
git -c diff.cdiff.process="$BACKEND --mode=no-hunks" \
395+
diff --stat --exit-code worddiff.c &&
396+
test_expect_code 1 git diff --no-ext-diff --stat --exit-code worddiff.c
397+
'
398+
399+
test_expect_success 'diff process --numstat with mixed equivalent and changed files' '
400+
test_when_finished "rm -f c.log h.log" &&
401+
# Self-contained fixtures: *.c uses whole-file (changed); *.mh
402+
# uses no-hunks (equivalent).
403+
echo "*.mh diff=hdiff" >>.gitattributes &&
404+
git add .gitattributes &&
405+
printf "int a(void) { return 1; }\n" >mixed.c &&
406+
printf "int b(void) { return 1; }\n" >mixed.mh &&
407+
git add mixed.c mixed.mh &&
408+
git commit -m "add mixed fixtures" &&
409+
printf "int a(void) { return 2; }\n" >mixed.c &&
410+
printf "int b(void) { return 2; }\n" >mixed.mh &&
411+
git -c diff.cdiff.process="$BACKEND --mode=whole-file --log=c.log" \
412+
-c diff.hdiff.process="$BACKEND --mode=no-hunks --log=h.log" \
413+
diff --numstat mixed.c mixed.mh >actual 2>stderr &&
414+
test_grep "mixed.c" actual &&
415+
test_grep ! "mixed.mh" actual &&
416+
test_grep "pathname=mixed.c" c.log &&
417+
test_grep "pathname=mixed.mh" h.log &&
418+
test_must_be_empty stderr
419+
'
420+
421+
test_expect_success POSIXPERM 'diff process keeps mode-only change in --stat' '
422+
test_when_finished "rm -f backend.log" &&
423+
cat >modeonly.c <<-\EOF &&
424+
int m(void) { return 1; }
425+
EOF
426+
git add modeonly.c &&
427+
git commit -m "add modeonly.c" &&
428+
cat >modeonly.c <<-\EOF &&
429+
int m(void) { return 2; }
430+
EOF
431+
git add modeonly.c &&
432+
test_chmod +x modeonly.c &&
433+
git commit -m "edit and chmod modeonly.c" &&
434+
# Content and mode both changed, but no-hunks reports the content
435+
# equivalent. The tool is consulted (counts are zero, not the
436+
# builtin 1/1), yet the mode change keeps the file from being
437+
# pruned.
438+
git -c diff.cdiff.process="$BACKEND --mode=no-hunks --log=backend.log" \
439+
diff --stat HEAD^ HEAD >actual 2>stderr &&
440+
test_grep "modeonly.c" actual &&
441+
test_grep "command=hunks pathname=modeonly.c" backend.log &&
442+
test_grep ! "1 insertion" actual &&
443+
test_must_be_empty stderr
444+
'
445+
446+
test_expect_success 'diff process not consulted for default --dirstat' '
447+
# Default --dirstat counts via its own path and never contacts
448+
# the tool, so the change is still reported even though no-hunks
449+
# would call it equivalent. (--dirstat=lines uses the stat path.)
450+
test_when_finished "rm -f backend.log" &&
451+
mkdir -p dsub &&
452+
printf "a\nb\nc\n" >dsub/d.c &&
453+
git add dsub/d.c &&
454+
git commit -m "add dsub/d.c" &&
455+
printf "a\nB\nc\n" >dsub/d.c &&
456+
git -c diff.cdiff.process="$BACKEND --mode=no-hunks --log=backend.log" \
457+
diff --dirstat=0 dsub/d.c >actual &&
458+
test_grep "dsub" actual &&
459+
test_path_is_missing backend.log
460+
'
461+
334462
#
335463
# Bypass mechanisms: flags and commands that skip the diff process.
336464
#

0 commit comments

Comments
 (0)