On Mon, Dec 10, 2018 at 8:09 AM Duy Nguyen <pclouds@xxxxxxxxx> wrote: > > On Sun, Dec 9, 2018 at 9:05 PM Thomas Gummerer <t.gummerer@xxxxxxxxx> wrote: > > > > When marking cache entries for removal, and later removing them all at > > once using 'remove_marked_cache_entries()', cache entries currently > > have to be invalidated manually in the cache tree and in the untracked > > cache. > > > > Add an invalidate flag to the function. With the flag set, the > > function will take care of invalidating the path in the cache tree and > > in the untracked cache. > > > > This will be useful in a subsequent commit. > > > > Signed-off-by: Thomas Gummerer <t.gummerer@xxxxxxxxx> > > --- > > > > For the two current callsites, unpack-trees seems to do this > > invalidation itself internally. > > I'm still a bit scared of this invalidation business in unpack-trees. > The thing is, we handle two separate index_state there, src_index and > result and invalidation has to be done on the right one (because index > extensions are on src_index until the very end of unpack-trees; > invalidating on 'result' would be no-op and wrong). > remove_marked_cache_entries() seems to be called on 'result' while > invalidate_ce_path() is on src_index, hm.... Is Thomas avoiding problems here simply because merge is the only caller of unpack_trees with src_index != dst_index? Or does src_index == dst_index for checkout not actually help? If that does help with the checkout case, then allow me to find a different way to muddy the waters... I think I might want to make use of this function in the merge machinery at some point, so I either need to figure out how to convince you to verify if all this cache tree invalidation stuff is sane, or somehow figure out all the cache_tree stuff stuff myself so I can figure out what is right here. :-) > > I don't quite understand why we don't > > need it in split-index mode though. I assume it's because the cache > > tree in the main index would already have been invalidated? I didn't > > have much time to dig, but couldn't produce any failures with it > > either, so I assume not invalidating paths is the right thing to do > > here. > > Yeah I think it's because cache-tree and untracked cache are already > properly invalidated. This merge base thingy is done when we load the > index files up, not when we write them down. The "front" index may > record that a few paths in the base index are no longer valid and need > to be deleted. But untracked cache and cache-tree both should have > recorded that same info when these paths are marked for delete at > index write time.
