On Tue, Sep 12, 2023 at 10:07 AM Junio C Hamano <gitster@xxxxxxxxx> wrote: > 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. I finished testing this - after my original fix and after running all tests, I can confirm that `st_mode` of `struct stat` is indeed not consumed if CE_FSMONITOR_VALID is set. But, it's fragile and likely to cause problems in the future. Your approach of constructing `struct stat` based on `struct cache_entry` is the way to go. I see you created a new set of patches in a separate thread, so I'll start those tests and report back there. Thanks! -- Josip Sokcevic
