Re: Strange merge failure (would be overwritten by merge / cannot merge)

Linus Torvalds <torvalds@linux-foundation.org> writes:

There are two levels of internal APIs involved, and I am getting confused as to which level of callers you are referrring to in the above.

My understanding of the current situation is:

 * unpack_trees() takes a callback from the caller in o->fn().  It
   promises (but fails to keep the promise) that the callback is called
   with entries with matching names, so that it gets to see D/F
   conflicting entries in one go.

 * traverse_trees() takes a callback from the caller in info->fn().  It
   feeds the callback the entries with the same name most of the time, but
   that is not a guarantee, and the bug we are seeing is coming from a
   caller, unpack_trees_callback(), assuming it.

Do you mean we would still keep the promise unpack_trees() makes to its callbacks, e.g. threeway_merge(), or do you mean these callbacks are to be prepared to get DF-split input themselves and expected to coalesce them as necessary?

There are only two callers of traverse_trees() interface. unpack_trees() codepath is the primary one (the other being the merge-tree.c, which is in disuse).

If you mean unpack_trees() by "callers, ... have to do their DF checking", it may be a much isolated fix than what I had in mind in the last message from me---the one that makes traverse_trees() pay attention to the caller supplied "candiate 'earliest' name" and tries to push the "give entries from all the trees with the same name to the callback in one go" promise down to traverse_trees() layer.

If traverse_trees() did not do this name coalescing, on the other hand, I wonder if it is doing only half of the job to be useful. I do not think of a plausible scenario where a caller, who wants to walk multiple trees in parallel, does not want to be fed the entries with the same name from all the input trees in one invocation of the callback it gives.

If all the callers need to do the name coalescing in order to notice the situation that led to this bug, I wonder if it would be nicer to do so in traverse_trees()?

unpack_trees() would of course be helped if traverse_trees() gave that promise, and I suspect merge-tree.c::threeway_callback() would expect that behaviour, too.