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

Thomas Gummerer [off-list ref] writes:

On 08/09, Junio C Hamano wrote:

quoted

Thomas Gummerer [off-list ref] writes:

quoted

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.

quoted

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.

On 08/09, Junio C Hamano wrote:

Thomas Gummerer [off-list ref] writes:

quoted

On 08/09, Junio C Hamano wrote:

quoted

Thomas Gummerer [off-list ref] writes:

quoted

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.

quoted

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.

Ok, thanks for the clarification, will change it for the re-roll.