Re: [PATCH v3] diff-lib: Fix check_removed when fsmonitor is on

[Junio C Hamano <gitster@pobox.com>]

Josip Sokcevic <sokcevic@google.com> writes:

> Yes, I agree we should optimize this in a follow up. One thing I'm not
> sure about is if we should try to construct `struct stat` using
> `cache_index`, or we should check for `CE_FSMONITOR_VALID` in a way
> that `stat` would no longer be needed for those code paths.

Good point.  

It seems to be entirely doable, even though the stench from the
abstraction layer violation may be horrible.

ie_match_stat(), which is called by match_stat_with_submodule(),
does pay attention to CE_FSMONITOR_VALID bit, so none of the members
of struct stat matters when the bit is set.  But the bit is not set,
all members that fill_stat_data() and ce_match_stat_basic() care
about do matter.  Other code that follows callers of check_removed()
do care about at least .st_mode member, and I suspect that in the
current code .st_mode is the only member that gets looked at.

So after all, I think your original "fix" was correct, but it took
us some time to figure out why it was, which means we would want to
explain it in the log message for developers who would want to touch
the same area in the future.

Thanks.