Re: [PATCH] do not overwrite untracked symlinks

On Mon, Feb 21, 2011 at 10:54:38PM -0800, Junio C Hamano wrote:

Yes, I agree with that reading.  I'd squash in this on top, first making
sure that "do not overwrite untracked symlinks" (which is the title of
this patch) won't be broken, and then the original test that wanted to
make sure that checking out a/b/c/d when a/b is pointing to an unrelated
part of the tree does not nuke whatever is pointed at that symbolic link.

Ok, perfect.

My unhappiness with the undescriptive "check_leading_path()" still
remains, though...

Ok. If I can't figure anything out this weekend, I'll rename it.
Promise.

It's just that I'm even more unhappy about the actual code rather
than the function name.  And each time I look at it I end up trying
to fix it, only to find out that it affects so many parts of the
code that don't make any sense. 

For example, we already have a function check_path() in entry.c,
which does something very similar and is called by
checkout_entry(), which in turn does something similar to
verify_absent(). In fact, I suspect that both functions are called
twice by the same code path around unpack_trees, for the same paths
and for the same reason.

I strongly feel that we should separate the merge process into two
steps.

 - First, do everything in the index, ignoring the work tree.

 - Second, checkout the index to the work tree while making sure no
   changes or untracked files get overwritten.

I know I have talked about this before, and I don't know if I will
ever find the time to implement such a major change. But I can't
get myself to feel good about fixing the function name, but not the
function.

Clemens