Thanks to both of you. I installed the attached into Savannah master
coreutils. It implements the suggestion, except that later I noticed
that EMLINK and ETXTBSY can join the throng.
At some point it might make sense to scan for other direct or indirect
calls to renameat, renameat2, and linkat, where the diagnostic could be
improved if it's known to refer to the destination.From 3cb862ce5f10db392cc8e6907dd9d888acfa2a30 Mon Sep 17 00:00:00 2001
From: Paul Eggert <egg...@cs.ucla.edu>
Date: Sat, 22 Jul 2023 13:39:17 -0700
Subject: [PATCH] mv: better diagnostic for 'mv dir x' failure
Problem reported by Nir Oren <https://bugs.gnu.org/64785>.
* src/copy.c (copy_internal): Use a more-specific diagnostic when
a rename fails due to a problem that must be due to the
destination, avoiding user confusion in cases like 'mv dir x'
where x is a nonempty directory.
* tests/mv/dir2dir.sh: Adjust to match.
---
NEWS | 7 +++++++
src/copy.c | 20 +++++++++++++++++---
tests/mv/dir2dir.sh | 2 +-
3 files changed, 25 insertions(+), 4 deletions(-)
diff --git a/NEWS b/NEWS
index 7d2ca7f6b..bd0c59eb5 100644
--- a/NEWS
+++ b/NEWS
@@ -56,6 +56,13 @@ GNU coreutils NEWS -*- outline -*-
short option is reserved to better support emulation of the standalone
checksum utilities with cksum.
+ 'mv dir x' now complains differently if x/dir is a nonempty directory.
+ Previously it said "mv: cannot move 'dir' to 'x/dir': Directory not empty",
+ where it was unclear whether 'dir' or 'x/dir' was the problem.
+ Now it says "mv: cannot overwrite 'x/dir': Directory not empty".
+ Similarly for other renames where the destination must be the problem.
+ [problem introduced in coreutils-6.0]
+
** Improvements
cp, mv, and install now avoid copy_file_range on linux kernels before 5.3
diff --git a/src/copy.c b/src/copy.c
index ddc9eab30..90eebf6bc 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -2840,9 +2840,23 @@ skip:
If the permissions on the directory containing the source or
destination file are made too restrictive, the rename will
fail. Etc. */
- error (0, rename_errno,
- _("cannot move %s to %s"),
- quoteaf_n (0, src_name), quoteaf_n (1, dst_name));
+ char const *quoted_dst_name = quoteaf_n (1, dst_name);
+ switch (rename_errno)
+ {
+ case EDQUOT: case EEXIST: case EISDIR: case EMLINK:
+ case ENOSPC: case ENOTEMPTY: case ETXTBSY:
+ /* The destination must be the problem. Don't mention
+ the source as that is more likely to confuse the user
+ than be helpful. */
+ error (0, rename_errno, _("cannot overwrite %s"),
+ quoted_dst_name);
+ break;
+
+ default:
+ error (0, rename_errno, _("cannot move %s to %s"),
+ quoteaf_n (0, src_name), quoted_dst_name);
+ break;
+ }
forget_created (src_sb.st_ino, src_sb.st_dev);
return false;
}
diff --git a/tests/mv/dir2dir.sh b/tests/mv/dir2dir.sh
index ea99eca56..da6a518e5 100755
--- a/tests/mv/dir2dir.sh
+++ b/tests/mv/dir2dir.sh
@@ -34,7 +34,7 @@ sed 's/: File exists/: Directory not empty/'<out>o1;mv o1 out
sed 's/: Device or resource busy/: Directory not empty/'<out>o1;mv o1 out
cat <<\EOF > exp || framework_failure_
-mv: cannot move 'b/t' to 'a/t': Directory not empty
+mv: cannot overwrite 'a/t': Directory not empty
EOF
compare exp out || fail=1
--
2.39.2
