diff: mode-only change should be noticed by "--patch -w --exit-code"

The codepath to notice the content-level changes, taking certain no-op changes like "ignore whitespace" into account, forgot that a mode-only change is still a change. This resulted in $ git diff --patch --exit-code -w to exit with status 0 even when there is such a mode-only change, breaking both "--patch" and "--quiet" output formats. Teach the builtin_diff() codepath that creation and deletion as well as mode changes are all interesting changes. Note that the test specifically checks removal of an empty file, because if there is anything in the preimage (i.e. the removed file is not empty), the removal would still trigger textual patch output and the codepath for that does update .found_changes bit to report that it found an interesting change. We need to make sure that the .found_changes bit is set even without triggering textual patch output. Signed-off-by: Junio C Hamano <gitster@pobox.com>

Junio C Hamano committed Aug 18, 2023 at 16:59 UTC c9a3e724cf75b44e47cfca5ae37cd0d7864c2220
2 files changed +37 -1
diff.c
+3
@@ -3497,18 +3497,21 @@ static void builtin_diff(const char *name_a,
3497 strbuf_addf(&header, "%s%snew file mode %06o%s\n", line_prefix, meta, two->mode, reset);
3498 if (xfrm_msg)
3499 strbuf_addstr(&header, xfrm_msg);
3500 + o->found_changes = 1;
3501 must_show_header = 1;
3502 }
3503 else if (lbl[1][0] == '/') {
3504 strbuf_addf(&header, "%s%sdeleted file mode %06o%s\n", line_prefix, meta, one->mode, reset);
3505 if (xfrm_msg)
3506 strbuf_addstr(&header, xfrm_msg);
3507 + o->found_changes = 1;
3508 must_show_header = 1;
3509 }
3510 else {
3511 if (one->mode != two->mode) {
3512 strbuf_addf(&header, "%s%sold mode %06o%s\n", line_prefix, meta, one->mode, reset);
3513 strbuf_addf(&header, "%s%snew mode %06o%s\n", line_prefix, meta, two->mode, reset);
3514 + o->found_changes = 1;
3515 must_show_header = 1;
3516 }
3517 if (xfrm_msg)
t/t4015-diff-whitespace.sh
+34 -1
@@ -1,7 +1,7 @@
1 #!/bin/sh
2 #
3 # Copyright (c) 2006 Johannes E. Schindelin
4 -#
4 +# Copyright (c) 2023 Google LLC
5
6 test_description='Test special whitespace in diff engine.
7
@@ -11,6 +11,39 @@ TEST_PASSES_SANITIZE_LEAK=true
11 . ./test-lib.sh
12 . "$TEST_DIRECTORY"/lib-diff.sh
13
14 +for opts in --patch --quiet -s
15 +do
16 +
17 + test_expect_success "status with $opts (different)" '
18 + echo foo >x &&
19 + git add x &&
20 + echo bar >x &&
21 + test_expect_code 1 git diff -w $opts --exit-code x
22 + '
23 +
24 + test_expect_success POSIXPERM "status with $opts (mode differs)" '
25 + test_when_finished "git update-index --chmod=-x x" &&
26 + echo foo >x &&
27 + git add x &&
28 + git update-index --chmod=+x x &&
29 + test_expect_code 1 git diff -w $opts --exit-code x
30 + '
31 +
32 + test_expect_success "status with $opts (removing an empty file)" '
33 + : >x &&
34 + git add x &&
35 + rm x &&
36 + test_expect_code 1 git diff -w $opts --exit-code -- x
37 + '
38 +
39 + test_expect_success "status with $opts (different but equivalent)" '
40 + echo foo >x &&
41 + git add x &&
42 + echo " foo" >x &&
43 + git diff -w $opts --exit-code x
44 + '
45 +done
46 +
47 test_expect_success "Ray Lehtiniemi's example" '
48 cat <<-\EOF >x &&
49 do {