[PATCH v4] read-cache: do not trust a size change when conversion is active

[Curtis Allen Smith <curtis.allen.smith@gmail.com>]

"git status" can report a file as modified while "git diff" and
"git add" agree that its contents are unchanged:

	git init t && cd t
	printf '* text eol=lf\n' >.gitattributes
	printf 'one\ntwo\nthree\n' >file.txt
	git add . && git commit -m init

	printf 'one\r\ntwo\r\nthree\r\n' >file.txt

	git status --short      # ->  M file.txt
	git diff                # -> empty
	git add file.txt        # -> nothing to commit

ie_modified() takes a change in the file's size since the index
entry was last refreshed as proof that the file is modified, without
looking at the contents.  A size change proves only that the bytes in
the working tree changed.  When the path is subject to conversion
(the "text" and "eol" attributes, core.autocrlf, a clean filter),
different bytes can convert to the same blob, as the CRLF rewrite
above does.

For such a path, fall through to ce_modified_check_fs(), as is
already done when only the mtime changed.  Paths without conversion
return early as before.

Helped-by: Torsten Bögershausen <tboegi@web.de>
Signed-off-by: Curtis Allen Smith <curtis.allen.smith@gmail.com>
---
Changes since v3:
 - The fix is inlined into the condition in ie_modified(), with no
   helper function.
 - Shorter log message, using Junio's wording; the "real life"
   examples are gone.

Cost, i9-14900K under WSL2, warm cache, fastest of 5 runs of
"git status -uno", stock 6de20f6 against this patch:

	10000 files x 2.6 KB, no conversion       stock      v4
	  clean tree                               5 ms     5 ms
	  10000 files with a size change          26 ms    24 ms

	10000 files x 2.6 KB, "* text=auto eol=lf"
	  clean tree                               5 ms     5 ms
	  10000 genuinely modified                25 ms   137 ms
	  10000 CRLF-rewritten, 1st run           21 ms   143 ms
	  the same, every run after               21 ms     5 ms

	200 files x 1 MB, "* text=auto eol=lf"
	  clean tree                               3 ms     3 ms
	  200 genuinely modified                   3 ms   498 ms
	  200 CRLF-rewritten, 1st run              3 ms   584 ms
	  the same, every run after                3 ms     3 ms

The attribute lookup happens only for a path whose size changed, so a
clean tree pays nothing, and a tree without conversion pays one
lookup per size-changed path (the first block).  A converted path
whose size changed is now read, converted and hashed, which is what
already happens when only its mtime changes.  Stock git keeps
reporting the CRLF-rewritten files as modified on every run.

A follow-up that skips the conversion and the hash when the length
already rules out a match is ready if wanted.  It takes the
"genuinely modified" rows to 57 ms and 25 ms.

 read-cache.c    |  4 +++-
 t/t0020-crlf.sh | 45 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 48 insertions(+), 1 deletion(-)

diff --git a/read-cache.c b/read-cache.c
index c4cf08a3a..70f4787de 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -9,6 +9,7 @@
 
 #include "git-compat-util.h"
 #include "config.h"
+#include "convert.h"
 #include "date.h"
 #include "diff.h"
 #include "diffcore.h"
@@ -480,7 +481,8 @@ int ie_modified(struct index_state *istate,
 	     */
 	    (!S_ISLNK(st->st_mode) || ce->ce_stat_data.sd_size != MAX_PATH) &&
 #endif
-	    (S_ISGITLINK(ce->ce_mode) || ce->ce_stat_data.sd_size != 0))
+	    (S_ISGITLINK(ce->ce_mode) || ce->ce_stat_data.sd_size != 0) &&
+	    !(S_ISREG(st->st_mode) && would_convert_to_git(istate, ce->name)))
 		return changed;
 
 	changed_fs = ce_modified_check_fs(istate, ce, st);
diff --git a/t/t0020-crlf.sh b/t/t0020-crlf.sh
index fd1cae09e..b76c678ce 100755
--- a/t/t0020-crlf.sh
+++ b/t/t0020-crlf.sh
@@ -397,4 +397,49 @@ test_expect_success 'New CRLF file gets LF in repo' '
 	test_cmp alllf alllf2
 '
 
+test_expect_success 'status does not report a CRLF-only rewrite as modified' '
+	git init eol-status &&
+	(
+		cd eol-status &&
+		echo "* text eol=lf" >.gitattributes &&
+		printf "one\ntwo\nthree\n" >file.txt &&
+		git add .gitattributes file.txt &&
+		git commit -m initial &&
+
+		# a generator rewrites the file with CRLF, same content
+		printf "one\r\ntwo\r\nthree\r\n" >file.txt &&
+		git status --porcelain -uno >actual &&
+		test_must_be_empty actual &&
+		git diff --exit-code &&
+
+		# a real change is still reported
+		printf "one\r\ntwo\r\nfour\r\n" >file.txt &&
+		git status --porcelain -uno >actual &&
+		echo " M file.txt" >expect &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'status sees through a CRLF rewrite of a file with a lone CR' '
+	git init eol-status-lone-cr &&
+	(
+		cd eol-status-lone-cr &&
+		echo "* text eol=lf" >.gitattributes &&
+		printf "one\rtwo\nthree\n" >file.txt &&
+		git add .gitattributes file.txt &&
+		git commit -m initial &&
+
+		# "git add" keeps the lone CR and drops the others
+		printf "one\rtwo\r\nthree\r\n" >file.txt &&
+		git status --porcelain -uno >actual &&
+		test_must_be_empty actual &&
+
+		# the converted length matches the blob, the content does not
+		printf "one\rtwo\nthrEE\r\n" >file.txt &&
+		git status --porcelain -uno >actual &&
+		echo " M file.txt" >expect &&
+		test_cmp expect actual
+	)
+'
+
 test_done
-- 
2.53.0