All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "Torsten Bögershausen" <tboegi@web.de>
Cc: Sebastian Thiel via GitGitGadget <gitgitgadget@gmail.com>,
	git@vger.kernel.org, Sebastian Thiel <sebastian.thiel@icloud.com>
Subject: Re: [PATCH] fix `git mv existing-dir non-existing-dir`*
Date: Tue, 08 Aug 2023 14:53:05 -0700	[thread overview]
Message-ID: <xmqqmsz16w1q.fsf@gitster.g> (raw)
In-Reply-To: <20230808184054.cjhiboifschkwuoz@tb-raspi4> ("Torsten Bögershausen"'s message of "Tue, 8 Aug 2023 20:40:54 +0200")

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

> 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.

  reply	other threads:[~2023-08-08 21:53 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-08 14:53 [PATCH] fix `git mv existing-dir non-existing-dir`* Sebastian Thiel via GitGitGadget
2023-08-08 17:36 ` Junio C Hamano
2023-08-08 18:40   ` Torsten Bögershausen
2023-08-08 21:53     ` Junio C Hamano [this message]
2023-08-12  1:14       ` [PATCH] mv: fix error for moving directory to another Junio C Hamano
2023-08-08 19:00   ` [PATCH] fix `git mv existing-dir non-existing-dir`* Junio C Hamano
2023-08-09  7:47 ` [PATCH v2] fix `git mv existing-dir non-existing-dir` in some environments Sebastian Thiel via GitGitGadget

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqmsz16w1q.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=sebastian.thiel@icloud.com \
    --cc=tboegi@web.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.