Junio C Hamano <gitster@xxxxxxxxx> writes: >> Then of course you need to split the second patch into several logical >> patches again. We can drop _v5 suffix in read-cache-v5.c (I haven't >> done that). When we add partial read/write for v5, we can add more >> func pointers to index_ops and implement them in v2 (probably as no-op >> or assertion) > > The index_ops abstraction is a right way to go, and I like it, but I > think the split illustrated in this patch might turn out to be at > wrong levels (and it is OK, as I understand this is a illustration > of concept patch). > > For example, add_to_index() interface may be a good candidate to > have in index_ops. Because your in-core index may not be holding > everything in a flat array, "find the location in the flat array the > entry would sit, replace the existing one if there is any, otherwise > insert" cannot be a generic way to add a new entry. If you make the > whole thing an abstract API entry point, a sparse implementation of > the in-core index could still implement it without bringing the > untouched and irrelevant parts of the index to core. [...] > I wish that the development of this topic was done more in a > top-down direction, instead of bottom-up, so that it identified the > necessary access patterns to the in-core index early and come up > with a good set of abstract API first, and then only after that is > done, came up with in-core and on-disk format to support the > necessary operations. I like the general idea, too, but I think there is a long way ahead, and we shouldn't hold up v5 on this. Thomas and me -- it was mostly my bad idea -- spent some time going through all the loops that iterate over the index. You can get some taste of it with 'git grep ce_stage', mostly because many of them either skip unmerged entries or specifically look for them. There are subtle differences between the loops on many points: what do they do when they hit an unmerged entry? Or a CE_REMOVED or CE_VALID one? I gave up after treating half of them and horribly breaking the test suite. I suppose eventually we will have to classify these loops by properties like how they treat unmerged entries, and replace them by some clever for_each_cache_entry macro. It would open some interesting possibilities. For example, for v5 it would be far better if conflicted and resolve-undo entries were a property of the normal index entry, instead of something that so happens to be consecutive entries and in a completely different place, respectively. -- Thomas Rast trast@{inf,student}.ethz.ch -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html