Re: [PATCH 7/7] Make unpack-tree update removed files before any updated files

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 23, 2008, 05:49 UTC
Message-ID: <7vbq56f0qm.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <alpine.LFD.1.00.0803221038320.3020@woody.linux-foundation.org>

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

Show 6 quoted lines

> Ok, this one looks - and is, really - trivial, but it's actually the only 
> one in the whole series that I'm even remotely nervous about. First off, 
> it actually does what it does regardless of that "core.ignorecase" 
> variable, but that wouldn't worry me if it wasn't for the fact that I 
> don't remember/understand what the heck that "last_symlink" logic was 
> there for.

last_symlink is just a cached information used by underlying has_symlink_leading_path() function for optimization.

The motivation behind has_symlink_leading_path() is reasonably well described in

 - f859c84: Add has_symlink_leading_path() function., 2007-05-11,

 - 64cab59: apply: do not get confused by symlinks in the middle,
   2007-05-11, and

 - 16a4c61: read-tree -m -u: avoid getting confused by intermediate
   symlinks., 2007-05-10

The short version is that:

 - sometimes we want to make sure a path a/b/c/d exists (or does not
   exist) in the work tree;

 - however, !lstat("a/b/c/d") is not quite it.  if a/b is a symlink in the
   work tree that points at somewhere that happens to have c/d underneath,
   !lstat() says "yeah, there is", but that one is _different_ from what
   checking out a cache entry a/b/c/d would produce (because in that case
   we will remove a/b symlink, create a/b/ directory and deposit blob b
   there).

 - So we often need to see if a given path has symlink component in the
   leading part in the work tree (e.g. given "a/b/c/d", we would need to
   check if any of "a", "a/b", "a/b/c" is a symlink).

The function has_symlink_leading_path() answers that question, and its second argument is a buffer to cache "the last work tree path found to be a symlink", so if you call it with "a/b/c/d" and then with "a/b/c/e" in the above example situation, the second call can re-use the information the first call found out, which is "a/b is a symlink".

I do not think your patch breaks the passing around of last_symlink cached information. Although the three commits I quoted above are all backed by real-world breakage cases that they did fix, the issues they deal with are indeed tricky cases. Although your patch (the change in 7/7) should not make any difference to the issues, thinking about them is already making me feel nervous.