Re: Help understanding git checkout behavior

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 11, 2012, 22:45 UTC
Message-ID: <7vfwa15uxy.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20120611214705.GC32061@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> I think the problem is in verify_clean_subdirectory, which checks that
> we do not have untracked files in the subdirectory, nor modifications
> between the index and working tree. But I do not see it checking whether
> we have modifications from the HEAD.

I actually think "comparison with HEAD" is a red herring. As long as the version in the index matches the version in the other branch we are checking out, it does not matter if the version in the index is different from the version in HEAD (case 4). Also if the version in the index is new and neither HEAD nor the other branch has it, we should keep it (case 2). And we should error out if these are not possible.

I did a bit more digging on this; even though I am not going to continue it further today, here is a snapshot of my current thinking.

After replacing "something" with "something/f1" in the index, attempting to "read-tree -m -u master b" (which is what checkout is about) decides that the regular file "something" should not exist, because it is in "master" but not in "b", so it calls

	deleted_entry(ce = master's version, old = NULL)

where "old" is the version of "something" in the index, i.e. "does not exist". This in turn calls verify_absent(ce) to make sure that it is OK to remove regular file "something". verify_absent() checks the path "something" with lstat(), and it would be happy if there weren't "something", as there won't be anything necessary to do in that case.

But it finds a directory. It calls check_ok_to_remove() on it. This is where things go wrong. This function is to see if it is ok to nuke the entire directory "something", for the more common case where we are about to create a different "something" there. It is an appropriate check if we were trying to create a version of regular file "something" from the branch we are trying to check out, but it is a wrong thing to do when we are not interested in touching "something" in any way. We do not have regular file "something" now, and after checking out the branch, we do not want to have regular file "something" there, either ---so all we have to do is to do nothing!

Instead, it says "Ah, something/f1 is clean", sets o->cache_bottom to skip it, and the machinery loses sight of that path.

Probably we need to vet the caller of verify_absent() to see why each caller wants to call it. Some may be about to create a new thing there, and really want to make sure there is nothing there after they are done. But this caller, when it _knows_ the path is already removed (it can tell in check_ok_to_remove() after lstat() says there is a directory "something" there---at that point we know the regular file "something" cannot be there), should just be happy and let the later callers to look at the remaining cache entries without marking everything under something/* has been processed.

Or something like that.