Git development
 help / color / mirror / Atom feed
* [PATCH] mv: report missing destination leading directory
@ 2026-07-15 14:32 Lucas Zamboni Orioli via GitGitGadget
  2026-07-15 16:46 ` Ben Knoble
  0 siblings, 1 reply; 2+ messages in thread
From: Lucas Zamboni Orioli via GitGitGadget @ 2026-07-15 14:32 UTC (permalink / raw)
  To: git; +Cc: Lucas Zamboni Orioli, Lucas Zamboni Orioli

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.

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

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] mv: report missing destination leading directory
  2026-07-15 14:32 [PATCH] mv: report missing destination leading directory Lucas Zamboni Orioli via GitGitGadget
@ 2026-07-15 16:46 ` Ben Knoble
  0 siblings, 0 replies; 2+ messages in thread
From: Ben Knoble @ 2026-07-15 16:46 UTC (permalink / raw)
  To: Lucas Zamboni Orioli via GitGitGadget; +Cc: git, Lucas Zamboni Orioli


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

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

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-07-15 16:50 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-15 14:32 [PATCH] mv: report missing destination leading directory Lucas Zamboni Orioli via GitGitGadget
2026-07-15 16:46 ` Ben Knoble

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox