Jeff King <peff@xxxxxxxx> writes: > The --follow code doesn't handle most forms of pathspec magic. We check > that no unexpected ones have made it to try_to_follow_renames() with a > runtime GUARD_PATHSPEC() check, which gives behavior like this: > > $ git log --follow ':(icase)makefile' >/dev/null > BUG: tree-diff.c:596: unsupported magic 10 > Aborted Stepping back a bit, isn't the real reason why follow mode is incompatible with the pathspec magic because it does not want to deal with anything but a single literal pathname? In other words, in "git log --follow ':(icase)makefile'", if the given pathspec matches a single path in the starting commit, we should be able to start from the commit inspecting that uniquely found path, compare the commit with its parent(s) at the path, and when the parent does not have the path, detect rename and continue following that the name before the rename as the single path starting from that parent. Perhaps the commit we start at, HEAD, has "Makefile" and not "makefile" (if we have two, that is an error), so we start traversal, following the pathname "Makefile". At some point, we may discover that the parent did not have "Makefile", but has "GNUMakefile" with a similar contents, at which point we swap the path we follow from "Makefile" to "GNUMakefile". At each step, tree-diff does not need to and does not want to do anything fancy with pathspec---the operation is always "this path in the parent, does it exist in the parent? If not, did it come from a different path?", and use of "literal pathspec" mode makes sense there. So why does try_to_follow_renames() even *need* to say anything other than "We are in follow mode; what we have is a single path; use it as a literal pathspec that matches the path literally without any funky magic"? GUARD_PATHSPEC() is "please inspect the pathspec and see what kind of special pattern matching it requests---we cannot allow it to ask for certain things but do allow it to ask for other things", that sounds terribly misguided here. The string is literal, so we do not even want to look at it to see what it asks---it is taken as nothing but a literal string and we do not let it ask for anything special. For example, if we started at HEAD to follow Makefile and in an ancestor commit we find that the Makefile came from a funny path ":(icase)foo", instead of allowing the pathspec match code to interprete it as a magic pathspec that matches any case permutations of 'f/o/o', we want to force the pathspec match code to match nothing but the funny path "colon parens icase close parens foo" by using literal pathspec magic. So perhaps having GUARD_PATHSPEC() there is the mistake, no? For the above idea to work, I think we need to resolve the pathspec early; "log --follow" should find its starting point, apply the given pathspec to its treeish to grab the literal pathname, and use that single literal pathname as the literal pathspec and continue, which I do not think it does right now. But with it done upfront, the pathspec limiting done during the history traversal, including try_to_follow_renames() bit, should be able to limit itself to "is the path literally the string given as the pathspec"? I dunno.
