Re: [PATCH/RFC v3 07/13] Read resolve-undo data

> Thomas Gummerer <t.gummerer@gmail.com> writes:
> 
> > On 08/09, Junio C Hamano wrote:
> >> Thomas Gummerer <t.gummerer@gmail.com> writes:
> >> 
> >> > Make git read the resolve-undo data from the index.
> >> >
> >> > Since the resolve-undo data is joined with the conflicts in
> >> > the ondisk format of the index file version 5, conflicts and
> >> > resolved data is read at the same time, and the resolve-undo
> >> > data is then converted to the in-memory format.
> >> 
> >> This, and the next one, are both about reading extension data from
> >> the v2 formatted index, no?
> >
> > Yes, exactly.
> >
> >> Again, mild NAK.
> >> 
> >> I think it is a lot more logical for the v5 code to read data stored
> >> in the resolve-undo and cache-tree extensions using the public API
> >> just like other users of these data do, and write out whatever in a
> >> way that is specific to the v5 index format.
> >> 
> >> If the v5 codepath needs some information that is not exposed to
> >> other users of istate->resolve_undo and istate->cache_tree, then the
> >> story is different, but I do not think that is the case.
> >
> > Sorry it's not clear to me what you mean with using the public API here.
> > Do you mean using resolve_undo_write() and resolve_undo_read()?
> 
> The code that reads from istate->resolve_undo is fine to do the v5
> specific conversion, but it does not belong to resolve-undo.c file
> which is about the resolve-undo extension.  Moving it to v5 specific
> file you added for this topic, read-cache-v5.c, and everything looks
> more logical.  When we taught ls-files to show the paths with
> resolve-undo data, we didn't add any function to resolve-undo.c that
> does ls-files's work for it.  Instead, ls-files just uses the public
> API (the data structure you find at the_index.resolve_undo is part
> of the API) to find what it needs to learn, and I think v5 code can
> do the same.
> 
> "then the story is different" comment refers to a possibilty that
> v5 code might need something more than callers outside resolve-undo.c
> can find from its public interface, but I do not think it is the
> case.