Re: [PATCH v2 2/2] mv: check for missing destination directory before renaming
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jul 23, 2026, 17:42 UTC
- Message-ID
- <xmqq8q71k3fy.fsf@gitster.g>
- In-Reply-To
- <1a790e001610d3324ec45d86ac67ca5720678cb8.1784812390.git.gitgitgadget@gmail.com>
"Lucas Zamboni Orioli via GitGitGadget" <gitgitgadget@gmail.com> writes:
> This is a best-effort diagnostic rather than a guarantee: the > destination directory can still disappear between the check and the > rename(2). It fixes the common case and, unlike the syscall path, > lets "git mv -n" report the failure.
If "can still disappear" is because we are not taking into account a move that we are scheduled to make, then that is not very nice, but as long as it is *not* our making (in other words, somebody else may actively interferring with the mv we are trying to perform), I think this is OK. It is the best we can do.
Show 36 quoted lines
> Add tests covering both the error path and the dry-run detection.
>
> Signed-off-by: Lucas Zamboni Orioli <lucaszam0@gmail.com>
> ---
> builtin/mv.c | 21 +++++++++++++++++++++
> t/t7001-mv.sh | 14 ++++++++++++++
> 2 files changed, 35 insertions(+)
>
> diff --git a/builtin/mv.c b/builtin/mv.c
> index 35e504484a..eb59fe0f31 100644
> --- a/builtin/mv.c
> +++ b/builtin/mv.c
> @@ -444,6 +444,27 @@ dir_check:
> goto act_on_entry;
> }
>
> + /*
> + * If we are going to move SRC to DST on disk, DST's leading
> + * directories must already exist.
> + */
> + if (!(modes[i] & (INDEX | SPARSE | SKIP_WORKTREE_DIR)) &&
> + !(dst_mode & (SKIP_WORKTREE_DIR | SPARSE))) {
> + char *dst_dir = xstrdup(dst);
> + char *slash = strrchr(dst_dir, '/');
> +
> + if (slash) {
> + struct stat dir_st;
> + *slash = '\0';
> + if (lstat(dst_dir, &dir_st) < 0 && errno == ENOENT) {
> + free(dst_dir);
> + bad = _("destination directory does not exist");
> + goto act_on_entry;
> + }
> + }
> + free(dst_dir);
> + }Horrible. Please fix this overly deep indentation.
Show 25 quoted lines
> diff --git a/t/t7001-mv.sh b/t/t7001-mv.sh > index 7cf4aa5ba1..2d8a98d8b0 100755 > --- a/t/t7001-mv.sh > +++ b/t/t7001-mv.sh > @@ -114,6 +114,20 @@ test_expect_success 'clean up' ' > git reset --hard > ' > > +test_expect_success 'moving to non-existent destination parent directory' ' > + git reset --hard && > + mkdir -p from && > + echo content >from/file && > + git add from/file && > + test_must_fail git mv from/file no-such-dir/file 2>actual && > + test_grep "destination directory does not exist" actual > +' > + > +test_expect_success 'mv --dry-run detects non-existent destination parent directory' ' > + test_must_fail git mv -n from/file no-such-dir/file 2>actual && > + test_grep "destination directory does not exist" actual > +' > + > test_expect_success 'moving to existing untracked target with trailing slash' ' > mkdir path1 && > git mv path0/ path1/ &&