Re: [PATCH] fix `git mv existing-dir non-existing-dir`*

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 8, 2023, 21:53 UTC
Message-ID: <xmqqmsz16w1q.fsf@gitster.g>
In-Reply-To: <20230808184054.cjhiboifschkwuoz@tb-raspi4>

Torsten Bögershausen <tboegi@web.de> writes:

Show 16 quoted lines

> On Tue, Aug 08, 2023 at 10:36:54AM -0700, Junio C Hamano wrote:
>> "Sebastian Thiel via GitGitGadget" <gitgitgadget@gmail.com> writes:
>>
>> > From: Sebastian Thiel <sebastian.thiel@icloud.com>
>> >
>
> The patch makes sense to me, Junio's comments included.
>
>> Shouldn't it do something similar to
>>
>>     $ mv D1 D2
>
> Couldn't resist to test it ;-)
>
> The result would be
>  renamed: D1/file1 -> D2/D1/file1

Sure. The lstat() in question is about the case where a different D2/D1 already exists, either as a file (which will definitely break as we do not and should not do unlink-and-then-mkdir) or as a directory (which may be OK in some cases to get a union of the contents in the original D1 and D2/D1, but in general not a good idea).

And in the latter case, i.e. when D2/D1 exists as a directory, we should not say "cannot move directory over file". So, the check that does not care what the dest_dir's type is fine. but the error message is wrong.

    "cannot move directory over file, source=D1, destination=D1/D2"

is the message we would get in such a case. We probably just should say

    "destination already exists, source=D1, destination=D1/D2"

or something like that.