Junio C Hamano <[email protected]> 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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
