>From 8ee71b24d74d7cfe81f151de430d38935cf04675 Mon Sep 17 00:00:00 2001
From: Assaf Gordon <address@hidden>
Date: Mon, 29 Jul 2019 00:23:20 -0600
Subject: [PATCH] mv: improve error messages when target directory is at fault

Suggested by Alex Mantel <address@hidden> in
https://bugs.gnu.org/36831 .

    $ mkdir A B B/A
    $ touch A/bar B/A/foo

Before:

    $ mv A B
    mv: cannot move 'A' to 'B/A': Directory not empty

After:

    $ mv A B
    mv: cannot move 'A' to 'B/A': Target directory not empty

The following errors are handled:
EDQUOT, EEXIST, ENOTEMPTY, EISDIR, ENOSPC, ETXTBSY.

* src/copy.c (copy_internal): Print custom messages for errors
that explicitly fault the target directory.
(strerror_target): New function, return custom and translatable error
messages.
* tests/mv/dir2dir.sh: Adjust expected error message.
* NEWS: Mention change.
---
 NEWS                |  6 +++++
 src/copy.c          | 56 ++++++++++++++++++++++++++++++++++++++++++---
 tests/mv/dir2dir.sh |  6 ++---
 3 files changed, 62 insertions(+), 6 deletions(-)

diff --git a/NEWS b/NEWS
index fd0543351..4ec4d0df0 100644
--- a/NEWS
+++ b/NEWS
@@ -44,6 +44,12 @@ GNU coreutils NEWS                                    -*- outline -*-
   stat(1) also supports a new --cached= option to control cache
   coherency of file system attributes, useful on network file systems.
 
+** Improvements
+
+  rm now prints clearer error messages when a failure relates to the
+  target directory (e.g., "Target directory is not empty" instead of
+  "Directory not empty").
+
 
 * Noteworthy changes in release 8.31 (2019-03-10) [stable]
 
diff --git a/src/copy.c b/src/copy.c
index 65cf65895..9cf02ad9c 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -1867,6 +1867,38 @@ source_is_dst_backup (char const *srcbase, struct stat const *src_st,
   return dst_back_status == 0 && SAME_INODE (*src_st, dst_back_sb);
 }
 
+static char*
+strerror_target (int e)
+{
+  /* TRANSLATORS: These strings should mimick libc's standard
+     error messages (from strerror(3)), but explicitly mention
+     the fault is with the target directory. */
+  switch (errno)
+    {
+    case EDQUOT:
+      return _("Disk quota exceeded on target device");
+    case EEXIST:
+    case ENOTEMPTY:
+      return _("Target directory not empty");
+    case EISDIR:
+      return _("Tried to overwrite a directory with a file");
+    case ENOSPC:
+      return _("No space left on target device");
+    case ETXTBSY:
+      /* NOTE: The error is "Text file busy" - but "text" in that context
+         refers to "text segment" of an executable file (as opposed to
+         "data segment" and "BSS segment").
+
+         This error message is meant for users, and 'text file' can be easily
+         confused with an actual text file (i.e., one containing only ASCII
+         characters. Thus, say 'executable' instead of 'text'.*/
+      return _("Target executable file is busy");
+    default:
+      assert (0);
+    }
+}
+
+
 /* Copy the file SRC_NAME to the file DST_NAME.  The files may be of
    any type.  NEW_DST should be true if the file DST_NAME cannot
    exist because its parent directory was just created; NEW_DST should
@@ -2477,9 +2509,27 @@ copy_internal (char const *src_name, char const *dst_name,
              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));
+
+          switch (errno)
+            {
+            case EDQUOT:
+            case EEXIST:
+            case ENOTEMPTY:
+            case EISDIR:
+            case ENOSPC:
+            case ETXTBSY:
+              error (0, 0,
+                     _("cannot move %s to %s: %s"),
+                     quoteaf_n (0, src_name), quoteaf_n (1, dst_name),
+                     strerror_target (rename_errno));
+              break;
+
+            default:
+              error (0, rename_errno,
+                     _("cannot move %s to %s"),
+                     quoteaf_n (0, src_name), quoteaf_n (1, 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 6455386b9..62ed8ce46 100755
--- a/tests/mv/dir2dir.sh
+++ b/tests/mv/dir2dir.sh
@@ -30,11 +30,11 @@ touch a/t/f || framework_failure_
 mv b/t a 2> out && fail=1
 
 # Accept any of these: EEXIST, ENOTEMPTY, EBUSY.
-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
+sed             's/: File exists/: Target directory not empty/'<out>o1;mv o1 out
+sed 's/: Device or resource busy/: Target 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 move 'b/t' to 'a/t': Target directory not empty
 EOF
 
 compare exp out || fail=1
-- 
2.20.1