Hello,

On Sun, Jul 28, 2019 at 08:58:59PM +0200, Alex Mantel wrote:
[...] 
> Ah, the target directory does exist! Hmm... But i'd like the message to be
> like:
> 
>    $ mv thing/ ../things
>    mv: cannot move 'thing' to '../things/things': Targetdirectory not empty
> 
>                                                   ^ this little thing here,
>                                                     it explains everyting.
> 
> Change text from 'Directory not empty' to 'Targetdirectory not empty'.

Thanks for the report.

To clarify, the scenario is:

    $ mkdir A B B/A
    $ touch A/bar B/A/foo
    $ mv A B
    mv: cannot move 'A' to 'B/A': Directory not empty

And the reason (as you've found out) is that the target directory 'B/A'
is not empty (has the 'foo' file in it).
Had this been allowed, moving 'A' to 'B/A' would result in the 'foo'
file disappearing.

---

How is a user expecting to know this error is about that target
directory?

There is a bit of a trade-off here between user-friendliness (especially
for non-technical user) and more technical knowledge.
If we go one step 'lower' to the programming interface, almost all
sources mention this is about the 'target' directory not being empty:

POSIX's says:
https://pubs.opengroup.org/onlinepubs/009695399/functions/rename.html
    [EEXIST] or [ENOTEMPTY]
        The link named by new is a directory that is not an empty directory.

Linux's rename(2) manual page says:
    ENOTEMPTY or EEXIST
        newpath is a nonempty directory, that is, contains entries
        other than "." and "..".

FreeBSD's rename(2) manual page says:
    [ENOTEMPTY]        The to argument is a directory and is not empty.

AIX rename(2) manual page says:
     ENOTEMPTY
       The ToPath parameter specifies an existing directory that is
       not empty.


So there is some merit in claiming this helpful piece of information is
lost when the error message is reported to the user.

---

In GNU coreutils this error message originates from 'copy.c' line 2480:
https://git.savannah.gnu.org/cgit/coreutils.git/tree/src/copy.c#n2480

    error (0, rename_errno,
              _("cannot move %s to %s"),
              quoteaf_n (0, src_name), quoteaf_n (1, dst_name));

And herein lies the (technical) problem: The actual message "Directory
not empty" is not in the source code - it is a system error message
that corresponds to the value of 'rename_errno' variable
(ENOTEMPTY/EEXIST). It originates from GLibc (or another libc).

So there is no trivial way to change the error message in coreutils.

Attached a patch to add special handling for this error.

---

What do others think? If this is a desired improvement, I'll finish the
patch with news/tests/etc.


regards,
 - assaf
>From 430b30104234db719bf15e6fc681a62312c7124f Mon Sep 17 00:00:00 2001
From: Assaf Gordon <assafgor...@gmail.com>
Date: Mon, 29 Jul 2019 00:23:20 -0600
Subject: [PATCH] mv: improve ENOTEMPTY/EEXIST error message

Suggested by Alex Mantel <alexmante...@gmail.com> 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

* src/copy.c (copy_internal): Add special handling for ENOTEMPTY/EEXIST.
TODO: NEWS, tests.
---
 src/copy.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/src/copy.c b/src/copy.c
index 65cf65895..a5af570bf 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -2450,6 +2450,14 @@ copy_internal (char const *src_name, char const 
*dst_name,
           return true;
         }
 
+      if (rename_errno == ENOTEMPTY || rename_errno == EEXIST)
+        {
+          error (0, 0, _("cannot move %s to %s: Target directory not empty"),
+                 quoteaf_n (0, src_name), quoteaf_n (1, dst_name));
+          forget_created (src_sb.st_ino, src_sb.st_dev);
+          return false;
+        }
+
       /* WARNING: there probably exist systems for which an inter-device
          rename fails with a value of errno not handled here.
          If/as those are reported, add them to the condition below.
-- 
2.11.0

Reply via email to