[PATCH v2 2/2] mv: check for missing destination directory before renaming

From: Lucas Zamboni Orioli <[email protected]>

Moving a file into a directory that does not exist fails at rename(2)
with ENOENT. 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 the syscall. As a consequence "git mv -n" does not
detect the problem either: the dry run never reaches rename(2) and
reports a move that would not actually succeed.

Detect this during the checking phase. 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 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.

Add tests covering both the error path and the dry-run detection.

Signed-off-by: Lucas Zamboni Orioli <[email protected]>
---
 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);
+		}
+
 		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 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/ &&
-- 
gitgitgadget