Re: [PATCH v2 2/2] mv: check for missing destination directory before renaming

Junio C Hamano <[email protected]> writes:

> Lucas Zamboni Orioli <[email protected]> writes:
>
>>> lstat() can succeed and 'dir_st' may indicate something other than a
>>> directory (for example, a symbolic link or a regular file).
>>> Alternatively, it can fail with ENOTDIR when, for example, 'dst_dir'
>>> is 'a/b/c' and 'a/b' is a file rather than a directory.
>>>
>>> Both cases will cause 'git mv' into a path assumed to be a directory
>>> to fail.  Shouldn't we handle these conditions as well?
>>
>> Yes, agreed, both should be handled. For v3 I switched from lstat()
>> to stat() so that the check follows symlinks the same way rename()
>> does, and I handle the non-directory cases:
>
> Generally, a symbolic link in a Git-managed working tree should not
> be followed.  Following a symbolic link would mean that 'git mv x y'
> could move 'x' outside the working tree if 'y' is a tracked symbolic
> link pointing to a directory outside the working tree.  'git apply',
> for example, avoids being fooled by a symbolic link for the same
> reason.
>
> I doubt that using stat() instead of lstat() is the right approach.
> Doing so essentially amounts to ignoring the presence of symbolic
> links.

I actually think "outside the working tree" is an irrelevant red
herring.  What is relevant is the fact that Git tracks symbolic
links.

If you have x (file) and y (another file), you would want to
complain when the user says:

    $ git mv x y  

because the location y is "taken" and the command line tells us only
about what it wants to do to x, without saying anything about what
you want to do to that existing y.  If y were a symbolic link
instead, you should behave exactly the same way.

It actually takes even more care, and I do not know if the
implementation of git-mv is done carefully enough, but think about
what should happen to:

    $ git mv x a/b/c  

when 'a' is a tracked symbolic link, and it points at, say, '.'.
Should it behave exactly the same as:

    $ git mv x b/c  

or should it simply error out?  I think the latter, "I see a symlink
in the middle, so I refuse to follow," is the right behavior.

Think carefully about cases where 'a' is a directory and 'a/b' is a
symlink, or where 'a' and 'a/b' are directories and 'a/b/c' is a
symlink, and so on.  We do not want to craft an arbitrary rule that
says we allow or refuse to operate depending on the link target.