Git development
 help / color / mirror / Atom feed
* [PATCH 0/2] status: agree with diff and add when conversion is active
@ 2026-10-08 20:45 Curtis Allen Smith
  2026-10-08 20:45 ` [PATCH 1/2] read-cache: do not trust a size change " Curtis Allen Smith
  2026-10-08 20:45 ` [PATCH 2/2] core: add core.convertAwareStatus to opt out of the content check Curtis Allen Smith
  0 siblings, 2 replies; 4+ messages in thread
From: Curtis Allen Smith @ 2026-10-08 20:45 UTC (permalink / raw)
  To: git; +Cc: Curtis Allen Smith, Torsten Bögershausen

"git status", "git diff" and "git add" can disagree about whether a
file has been modified.  Under

	* text eol=lf

a tool that rewrites an otherwise unchanged file with CRLF endings
makes "git status" report it as modified, while "git diff" shows
nothing and "git add" stages nothing: the two commands that actually
run the clean filter both conclude that the contents did not change.
That contradiction, rather than the line endings as such, is what
this series is about.

The cause is the size comparison in ie_match_stat().  ie_modified()
takes a difference between the file's size and the size recorded in
the index as proof of a content change and returns without reading
the file.  That was sound in 2005, when the working tree file and the
blob were the same bytes.  Conversion made it unsound -- the whole
point of a clean filter is that the two representations differ in
their bytes and agree on their content -- and the shortcut was never
revisited.  The mtime branch of the very same function already reads
the file and applies the conversion before deciding, so Git pays for
the conversion-aware check in one branch and refuses to in the other.

Patch 1 makes the size branch behave like the mtime branch whenever
the path is subject to conversion.  Paths with no conversion take the
existing early return untouched.  It also stops ce_compare_data()
hashing a file whose converted length already differs from the size
of its blob, since equal contents must have equal length; that is
where most of the cost of the new check would otherwise go, and it
helps the pre-existing mtime path as well.  When the end-of-line
conversion is the only one that applies, that length comes from a
scan for CR, and the file is not converted either.

Patch 2 adds core.convertAwareStatus for people who would rather keep
the old shortcut, either everywhere (false) or only for paths with an
expensive clean filter such as Git LFS (no-filter).  I defaulted it to
on, including filters: the measurements below say reading is not what
costs, and the 2005 performance argument should not be re-applied in
2026 without evidence.  Being ordinary configuration it also works per
command, as "git -c core.convertAwareStatus=no-filter status".

Note that there is no way to opt out of the size comparison today --
core.checkStat=minimal drops ctime, uid/gid and inode but still
compares the size -- which is why patch 2 adds a variable instead of
extending an existing one.

Numbers
-------

Linux (WSL2, ext4) on an i9-14900K, warm page cache, fastest of 5
runs, "status -uno" to separate the refresh from untracked scanning.
One binary for both columns with core.convertAwareStatus flipped;
"false" is the pre-series code path.

	10000 files x 2.6 KB, "* text=auto eol=lf"
	                                   false      true
	  clean tree                         4 ms      4 ms
	  10000 genuinely modified          19 ms     49 ms
	  10000 CRLF-rewritten, 1st run     20 ms    148 ms
	  the same, steady state            20 ms      6 ms

	200 files x 1 MB, same attributes
	                                   false      true
	  clean tree                         2 ms      2 ms
	  200 genuinely modified             2 ms     26 ms
	  200 CRLF-rewritten, 1st run        2 ms    611 ms
	  the same, steady state             2 ms      3 ms

	10000 files x 2.6 KB, no conversion configured
	  clean tree                         4 ms      4 ms
	  10000 genuinely modified          19 ms     20 ms

A clean tree and a repository without conversion are unaffected.  What
is paid for is stat-dirty converted paths.  A file that was really
edited is read and scanned for CR, but neither converted nor hashed,
because its length already rules out a match.  Without that the
"genuinely modified" rows read 142 ms and 517 ms rather than 49 ms and
26 ms.  Of the 214 MB in the second corpus, reading costs 6 ms from
page cache, the conversion about 180 ms, and SHA-1 about 280 ms.

The last row of the first block is the case the series exists for: the
patched build settles at 6 ms where the unpatched one pays 20 ms on
every invocation and still reports the files as modified, because it
never refreshes their recorded sizes.  The 1st-run rows are the
one-time cost of discovering that.  Their lengths match, so they are
converted and hashed in full.

This was reported against Git for Windows [1], where Torsten suggested
bringing it to the list.  It is not Windows-specific; anything with a
clean filter runs into it, and Git LFS users on Linux see the same
contradiction.

Built with gcc 15.2 on top of 6de20f6; each commit builds and passes
on its own.  t0020 (with the new tests), t0021, t0026, t0027, t1300,
t2106, t2200, t3700, t7508 and t0008 pass.

[1] https://github.com/git-for-windows/git/issues/6410

Curtis Allen Smith (2):
  read-cache: do not trust a size change when conversion is active
  core: add core.convertAwareStatus to opt out of the content check

 Documentation/config/core.adoc |  22 +++++
 environment.c                  |  14 ++++
 environment.h                  |   7 ++
 read-cache.c                   | 141 ++++++++++++++++++++++++++++++++-
 t/t0020-crlf.sh                |  92 +++++++++++++++++++++
 5 files changed, 273 insertions(+), 3 deletions(-)

-- 
2.53.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-10-09  5:37 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-08 20:45 [PATCH 0/2] status: agree with diff and add when conversion is active Curtis Allen Smith
2026-10-08 20:45 ` [PATCH 1/2] read-cache: do not trust a size change " Curtis Allen Smith
2026-10-09  5:37   ` Junio C Hamano
2026-10-08 20:45 ` [PATCH 2/2] core: add core.convertAwareStatus to opt out of the content check Curtis Allen Smith

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox