Re: [PATCH 1/2] read-cache: do not trust a size change when conversion is active

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 9, 2026, 05:37 UTC
Message-ID: <xmqqfqyfwi20.fsf@gitster.g>
In-Reply-To: <20261008204603.1988-2-curtis.allen.smith@gmail.com>

Curtis Allen Smith <curtis.allen.smith@gmail.com> writes:

Show 6 quoted lines

> "git status" can report a file as modified while "git diff" and
> ...
> next run of the tool flags everything again.
>
> Signed-off-by: Curtis Allen Smith <curtis.allen.smith@gmail.com>
> ---

That's overly verbose.

>  read-cache.c    | 129 ++++++++++++++++++++++++++++++++++++++++++++++--
>  t/t0020-crlf.sh |  45 +++++++++++++++++
>  2 files changed, 171 insertions(+), 3 deletions(-)

And it is curious why we need so much new code, especially after reading an explaination in the proposed log message that makes it sound as if "we let ce_modified_check_fs() to compare converted result already when timestamps differ, and it is just the matter of doing the same when sizes are the same" is what is happening in the patch. Why do we need to add a new function that compares converted data? A new function is not automatically a bad thing. If there is already an existing code path that does the same thing, a new function may be a good way to replace that code path with a more generic code and apply essentially the same logic implemented by that new more generic code to a new code path. But in such a refactoring patch, we usually see a comparable number of removed lines, which is not what we see in the diffstat above.