Re: [PATCH] mv: report missing destination leading directory
Show 33 quoted lines
> Le 15 juil. 2026 à 10:51, Lucas Zamboni Orioli via GitGitGadget <gitgitgadget@gmail.com> a écrit :
>
> From: Lucas Zamboni Orioli <lucaszam0@gmail.com>
>
> When moving a file to a destination whose leading directory does not
> exist, "git mv" fails at the rename(2) syscall with ENOENT. Because
> the error is reported via die_errno() using only the source path:
>
> fatal: renaming 'src' failed: No such file or directory
>
> the message misleadingly blames the source, even though it is the
> destination's parent directory that is missing. A user who runs
>
> git mv a/file b/does-not-exist/file
>
> is told the problem is with 'a/file', which exists, giving no hint
> that 'b/does-not-exist/' needs to be created first.
>
> The checking phase already rejects a missing destination directory
> when the destination ends in a slash, but a destination that names a
> file inside a non-existent directory is not caught and only fails
> later at rename(2). As a result "git mv -n" also fails to detect the
> problem, since the dry run never reaches the syscall and reports a
> move that would not actually succeed.
>
> Detect this during the checking phase instead: for entries that will
> be renamed on disk, stat the destination's leading directory and, if
> it is missing, fail with the existing "destination directory does not
> exist" message. Guard the check with the same condition under which
> rename(2) is invoked so that directory moves, whose child entries are
> expanded to paths under a not-yet-created directory, and sparse or
> out-of-cone destinations, which are not written to the worktree, are
> not flagged incorrectly.
I suppose this still allows a TOCTOU issue where the check succeeds and (with lucky timing) the destination then disappears?
In that case, I think a worthwhile additional change would also be for the error message to diagnose which file is missing (or at least include both source and destination).
Now, without checking I somehow doubt whether rename(2) tells us which entry is missing. Worse, if we check afterwards, we could have a « TOUTOC » :p where the entry reappears to confuse the error diagnosis.
So perhaps
fatal: renaming A -> B failed: no such file or directory
taking some inspiration from the -i modes of cp, mv?
Show 76 quoted lines
> This gives a clear message and lets "git mv -n" report the failure.
>
> Signed-off-by: Lucas Zamboni Orioli <lucaszam0@gmail.com>
> ---
> mv: report missing destination leading directory
>
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2356%2FZamboniL%2Fmv-detect-non-existing-target-folder-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2356/ZamboniL/mv-detect-non-existing-target-folder-v1
> Pull-Request: https://github.com/git/git/pull/2356
>
> 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 e03823370c..a95531f0b2 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);
> + }
> +
> if (ignore_sparse &&
> (dst_mode & (SKIP_WORKTREE_DIR | SPARSE)) &&
> index_entry_exists(the_repository->index, dst, strlen(dst))) {
> diff --git a/t/t7001-mv.sh b/t/t7001-mv.sh
> index 920479e925..8a45997b33 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/ &&
>
> base-commit: 55526a18268bbc1ddaf8a6b7850c33d984eac9e9
> --
> gitgitgadget
> Ben Knoble <ben.knoble@gmail.com>