Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes: > On Thu, Apr 01 2021, Junio C Hamano wrote: > >> Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes: >> >>> Change an early check for non-blobs in verify_working_tree_path() to >>> let any such objects pass, and instead die shortly thereafter in the >>> fake_working_tree_commit() caller's type check. >>> >>> Now e.g. doing "git blame t" in git.git emits: >>> >>> fatal: unsupported file type t >>> >>> Instead of: >>> >>> fatal: no such path 't' in HEAD >> >> Sorry, but I fail to see why "unsupported file type t" is quite an >> improvement. Is this one of these irrelevant clean-up while at it >> whose benefit is unclear until much later, I have to wonder. > > Because "t" is directory we can stat() and which exists in the index, so > it makes more sense to fall through to the stat() codepath. 't' is not in the index, even though many things with 't/' prefix may. But my understanding of the point of that loop is to catch cases where one side may have 't' as a directory, the other side may have it as a blob, and we have a 't' regular file during a process of resolving a conflicted merge (perhaps the tentative resolution has already been "git add"ed to the index). So "git blame t" would want to start from the "working tree state", which is made into a virtual commit, with two "virtual" parents, one is HEAD that has 't' as a directory but the other MERGE_HEAD may have 't' as a regular file, so "blame" should be able to follow the history of that regular file through the merge in progress. If we change, like the proposed patch does, the loop to exit immediately after we notice 't' exists in HEAD (as a tree), without even looking at the second parent to notice that it is a regular file there, wouldn't that change the behaviour? > I think the "unsupported file type" message is a bit odd, but it's the > existing one, perhaps changing it while we're at it to something like: > > fatal: cannot 'blame' a directory > > Would be better,... Sure. As long as the change does not break the original use case the loop intended to support, that is fine, and the message would be a lot better than the "unsupported file type t".
