git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v3 2/5] diff: mode-only change should be noticed by "--patch -w --exit-code"

From
Junio C Hamano <gitster@pobox.com>
Date
Aug 18, 2023, 23:59 UTC
Message-ID
<20230818235932.3253552-3-gitster@pobox.com>
In-Reply-To
<20230818235932.3253552-1-gitster@pobox.com>

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>
---
 diff.c                     |  3 +++
 t/t4015-diff-whitespace.sh | 35 ++++++++++++++++++++++++++++++++++-
 2 files changed, 37 insertions(+), 1 deletion(-)
diff --git a/diff.c b/diff.c
index 0ce678fc06..998d7ae20c 100644
--- a/diff.c
+++ b/diff.c
@@ -3497,18 +3497,21 @@ static void builtin_diff(const char *name_a,
 		strbuf_addf(&header, "%s%snew file mode %06o%s\n", line_prefix, meta, two->mode, reset);
 		if (xfrm_msg)
 			strbuf_addstr(&header, xfrm_msg);
+		o->found_changes = 1;
 		must_show_header = 1;
 	}
 	else if (lbl[1][0] == '/') {
 		strbuf_addf(&header, "%s%sdeleted file mode %06o%s\n", line_prefix, meta, one->mode, reset);
 		if (xfrm_msg)
 			strbuf_addstr(&header, xfrm_msg);
+		o->found_changes = 1;
 		must_show_header = 1;
 	}
 	else {
 		if (one->mode != two->mode) {
 			strbuf_addf(&header, "%s%sold mode %06o%s\n", line_prefix, meta, one->mode, reset);
 			strbuf_addf(&header, "%s%snew mode %06o%s\n", line_prefix, meta, two->mode, reset);
+			o->found_changes = 1;
 			must_show_header = 1;
 		}
 		if (xfrm_msg)
diff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh
index f3e20dd5bb..02731dccb9 100755
--- a/t/t4015-diff-whitespace.sh
+++ b/t/t4015-diff-whitespace.sh
@@ -1,7 +1,7 @@
 #!/bin/sh
 #
 # Copyright (c) 2006 Johannes E. Schindelin
-#
+# Copyright (c) 2023 Google LLC
 
 test_description='Test special whitespace in diff engine.
 
@@ -11,6 +11,39 @@ TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 . "$TEST_DIRECTORY"/lib-diff.sh
 
+for opts in --patch --quiet -s
+do
+
+	test_expect_success "status with $opts (different)" '
+		echo foo >x &&
+		git add x &&
+		echo bar >x &&
+		test_expect_code 1 git diff -w $opts --exit-code x
+	'
+
+	test_expect_success POSIXPERM "status with $opts (mode differs)" '
+		test_when_finished "git update-index --chmod=-x x" &&
+		echo foo >x &&
+		git add x &&
+		git update-index --chmod=+x x &&
+		test_expect_code 1 git diff -w $opts --exit-code x
+	'
+
+	test_expect_success "status with $opts (removing an empty file)" '
+		: >x &&
+		git add x &&
+		rm x &&
+		test_expect_code 1 git diff -w $opts --exit-code -- x
+	'
+
+	test_expect_success "status with $opts (different but equivalent)" '
+		echo foo >x &&
+		git add x &&
+		echo " foo" >x &&
+		git diff -w $opts --exit-code x
+	'
+done
+
 test_expect_success "Ray Lehtiniemi's example" '
 	cat <<-\EOF >x &&
 	do {
-- 
2.42.0-rc2-7-gf9972720e9
Previous: Junio C HamanoNext: Junio C Hamano
Message 23 of 36 in “git bug report”
  1. Paul WatsonAug 4, 2023
  2. rsbecker@nexbridge.comAug 4, 2023
  3. Paul WatsonAug 4, 2023
  4. rsbecker@nexbridge.comAug 4, 2023
  5. Paul WatsonAug 7, 2023
  6. Paul WatsonAug 8, 2023
  7. rsbecker@nexbridge.comAug 8, 2023
  8. Junio C HamanoAug 8, 2023
  9. diff: tighten interaction between -w and --exit-codeJunio C Hamano, Aug 16, 2023
  10. Jeff KingAug 17, 2023
  11. Junio C HamanoAug 17, 2023
  12. Jeff KingAug 17, 2023
  13. Junio C HamanoAug 17, 2023
  14. 0/5 fix interactions with "-w" and "--exit-code"Junio C Hamano, Aug 17, 2023
  15. 1/5 diff: --dirstat leakfixJunio C Hamano, Aug 17, 2023
  16. 2/5 diff: move the fallback "--exit-code" code downJunio C Hamano, Aug 17, 2023
  17. 3/5 diff: mode-only change should be noticed by "--patch -w --exit-code"Junio C Hamano, Aug 17, 2023
  18. Junio C HamanoAug 18, 2023
  19. 4/5 diff: teach "--stat -w --exit-code" to notice differencesJunio C Hamano, Aug 17, 2023
  20. 5/5 diff: teach "--name-status" and friends to honor "--exit-code -w"Junio C Hamano, Aug 17, 2023
  21. Junio C HamanoAug 17, 2023
  22. 0/5 fix interactions with "-w" and "--exit-code"Junio C Hamano, Aug 18, 2023
  23. 2/5 diff: mode-only change should be noticed by "--patch -w --exit-code"Junio C Hamano, Aug 18, 2023
  24. 3/5 diff: teach "--stat -w --exit-code" to notice differencesJunio C Hamano, Aug 18, 2023
  25. Jeff KingAug 21, 2023
  26. 4/5 t4040: remove test that succeeded for a wrong reasonJunio C Hamano, Aug 18, 2023
  27. 1/5 diff: move the fallback "--exit-code" code downJunio C Hamano, Aug 18, 2023
  28. Jeff KingAug 21, 2023
  29. 5/5 diff: the -w option breaks --exit-code for --raw and other output modesJunio C Hamano, Aug 18, 2023
  30. Jeff KingAug 21, 2023
  31. Jeff KingAug 21, 2023
  32. Jeff KingAug 21, 2023
  33. Junio C HamanoAug 21, 2023
  34. Junio C HamanoAug 21, 2023
  35. Jeff KingAug 22, 2023
  36. Jeff KingAug 21, 2023

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.