Changes since v1:
* altered the error message to include both source and destination as
suggested by Ben Knoble
Lucas Zamboni Orioli (2):
mv: name both source and destination when rename fails
mv: check for missing destination directory before renaming
builtin/mv.c | 23 ++++++++++++++++++++++-
t/t7001-mv.sh | 14 ++++++++++++++
2 files changed, 36 insertions(+), 1 deletion(-)
base-commit: 9a0c4701dcd5725c4184599322b52933ff5005ca
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2356%2FZamboniL%2Fmv-detect-non-existing-target-folder-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2356/ZamboniL/mv-detect-non-existing-target-folder-v2
Pull-Request: https://github.com/git/git/pull/2356
Range-diff vs v1:
-: ---------- > 1: 0d67da588b mv: name both source and destination when rename fails
1: 692f44456f ! 2: 1a790e0016 mv: report missing destination leading directory
@@ Metadata
Author: Lucas Zamboni Orioli <lucaszam0@gmail.com>
## Commit message ##
- mv: report missing destination leading directory
+ mv: check for missing destination directory before renaming
- 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:
+ 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.
- fatal: renaming 'src' failed: No such file or directory
+ 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.
- the message misleadingly blames the source, even though it is the
- destination's parent directory that is missing. A user who runs
+ 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.
- 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.
+ Add tests covering both the error path and the dry-run detection.
Signed-off-by: Lucas Zamboni Orioli <lucaszam0@gmail.com>
Lucas Zamboni Orioli via GitGitGadget <gitgitgadget@gmail.com>