Ben Peart <benpeart@xxxxxxxxxxxxx> writes: > Update replace_index_entry() to clear the CE_HASHED flag from the new cache > entry so that it can add it to the name hash in set_index_entry() OK. > diff --git a/read-cache.c b/read-cache.c > index 977921d90c..bdfa552861 100644 > --- a/read-cache.c > +++ b/read-cache.c > @@ -62,6 +62,7 @@ static void replace_index_entry(struct index_state *istate, int nr, struct cache > replace_index_entry_in_base(istate, old, ce); > remove_name_hash(istate, old); > free(old); > + ce->ce_flags &= ~CE_HASHED; > set_index_entry(istate, nr, ce); > ce->ce_flags |= CE_UPDATE_IN_BASE; > mark_fsmonitor_invalid(istate, ce); As we are removing "old" that is not "ce", an earlier call to remove_name_hash() that clears the CE_HASHED bit from the cache entry does not help us at all. We need to clear the bit from "ce" ourselves before calling set_index_entry() on it, otherwise the call would become a no-op wrt the name hash. Makes sense. Makes me wonder why "ce" which is a replacement for what is in the index already has the hashed bit, though. Is that the failure to use copy_cache_entry() in the caller the other part of this patch fixes? To me it looks like copy_cache_entry() is designed for copying an entry's data to another one that has a different name, but in the refresh codepath, we _know_ we are replacing an old entry with an entry with the same name, so it somehow feels a bit strange to use copy_cache_entry(), instead of doing memcpy() (and possibly dropping the HASHED bit from the new copy--but wouldn't that become unnecessary with the fix to replace_index_entry() we saw above?) Is this fix something we can demonstrate in a new test, by the way? Thanks.
