On Fri, Aug 10, 2018 at 12:30 PM Duy Nguyen <pclouds@xxxxxxxxx> wrote: > On Fri, Aug 10, 2018 at 8:39 PM Elijah Newren <newren@xxxxxxxxx> wrote: ... > > Why do we still need to go through add_index_entry()? I thought that > > the whole point was that you already checked that at the current path, > > the trees being unpacked were all equal and matched both the index and > > the cache_tree. If so, why is there any need for an update at all? > > (Did I read your all_trees_same_as_cache_tree() function wrong, and > > you don't actually know these all match in some important way?) > > Unless fn is oneway_diff, we have to create a new index (in o->result) > based on o->src_index and some other trees. So we have to add entries Oh, right, o->src_index may not equal o->dst_index (because of people like me who call it that way from merge-recursive.c) and even if it does, we still have the temporary o->result in the mean time. I should have remembered that; just didn't. > to o->result and add_index_entry() is the way to do that (granted if > we feel confident we could add ADD_CACHE_JUST_APPEND which makes it > super cheap). This is the outcome of n-way merge, > > all_trees_same_as_cache_tree() only gurantees the input condition (all > trees the same, index also the same) but it can't affect what fn does. > I don't think we can just simply skip and not update anything (like > o->diff_index_cached case) because o->result would be empty in the > end. And we need to create (temporary) o->result before we can swap it > to o->dst_index as the result of a merge operation. > ... > I have a feeling that you're thinking we can swap o->src_index to > o->dst_index at the end? That might explain your confusion about > o->result (or I misread your replies horribly) and the original > index... Yeah, thanks for figuring out my confusion and jogging my memory.