Re: [PATCH 3/3] diff: detect pathspec magic not supported by --follow

Jeff King <peff@peff.net> writes:

If we consider a middle-ground where we do not keep a separate pathname that each history traversal is following but only keep a single pathname, then the answer is one of "ignore others and pick one" (which is essentially what needs to happen in the current implementation when a path gets renamed between a child commit and its parent while the other parents do not have renames), "ignore others and pick one, but give a warning that the result may be incomplete", or "barf". The latter two would need us to look at all treeishes.

A false-positive-ridden "solution" that would slightly be better than that would be to still keep a global, single, pathspec that has more than one elements (all of which are literal pathspec elements). Every time we discover a rename, we add the newly discovered old name as an additional element, instead of replacing the single element. If we were to go that route, then starting from multiple points, each following a pathname, would be a simple matter of looking at all trees and finding which pathname each history traversal wants to follow and creating a pathspec with all these (literal) pathnames.

But once we fix the underlying problem, and start tracking which path each history traversal trajectory is currently following, then it is not a problem for each starting point to have a pathname that is being followed. Each commit would _know_ which pathname it is interested in, and when a traversal from a commit to its parent sees a rename, the pathname being followed for the parent commit will be different from the child.

When this happens, the parent may have a pathname it is already interested in, set by traversing from another child (i.e. one side of a fork renamed the path one way, while the other side of a fork kept the same name or renamed it to another way, and we traversed both forks down and are now at the fork point). It most likely should be the same name we discovered from the other child, but in weird cases it may be different (e.g. a child thinks its path A came from parent's path X while another child thinks its path A came from parent's path Y), in which case we may have to follow two pathnames in the parent commit, as if the pathname we have been following in the child had combined contents from multiple paths.

I think we are in agreement on this---otherwise I wouldn't have merged the patch as-is down to 'next' ;-)

As I said multiple times in the past, the current "log --follow" is more of a checkbox item than a feature, and in that context, I think your "fix", which may be papering over just one obvious breakage, is what the codepath should be happy with, before it gets reworked more seriously.

Thanks.