GNU bug report logs - #64785
Package: coreutils Version: 8.32-4+b1 program=mv

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 64785 in the body.
You can then email your comments to 64785 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Nir Oren <niro <at> edev.co>
To: bug-coreutils <at> gnu.org
Subject: Package: coreutils Version: 8.32-4+b1 program=mv
Date: Fri, 21 Jul 2023 20:17:00 +0300

[Message part 1 (text/plain, inline)]

*mv: error message "Directory not empty" is confusing *

description: when you try to move a directory to a location already
containing a directory with the same name it would just write "mv: cannot
move 'A' to 'B': Directory not empty"

first, this is technically a wrong error message because there is no
requirement that the destination would be empty.
the destination might as well be populated with some content, we just
require that it would not contain a directory with the same name.

second, the error message is confusing because it doesn't state that the
problem is with the destination. One can think that the problem is actually
with the source, and that the source directory has some kind of attribute
that would require it to be empty prior to moving.

I would suggest to change the error message to: "*a directory with the same
name already exists at destination*"

reproduction:
$:rm -rf /tmp/node_modules
$:mkdir -p proj1/node_modules
$:touch proj1/node_modules/foo
$:mkdir -p proj2/node_modules
$:touch proj2/node_modules/bar
$:mv proj1/node_modules /tmp
$:mv proj2/node_modules /tmp
mv: cannot move 'proj2/node_modules' to '/tmp/node_modules': Directory not
empty

[Message part 2 (text/html, inline)]

Message #8 received at 64785 <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: Nir Oren <niro <at> edev.co>, 64785 <at> debbugs.gnu.org
Subject: Re: bug#64785: Package: coreutils Version: 8.32-4+b1 program=mv
Date: Sat, 22 Jul 2023 11:19:33 +0100

On 21/07/2023 18:17, Nir Oren wrote:
> *mv: error message "Directory not empty" is confusing *
> 
> description: when you try to move a directory to a location already
> containing a directory with the same name it would just write "mv: cannot
> move 'A' to 'B': Directory not empty"
> 
> first, this is technically a wrong error message because there is no
> requirement that the destination would be empty.
> the destination might as well be populated with some content, we just
> require that it would not contain a directory with the same name.

Well that's not exactly the case.
mv will replace empty dirs in the destination,
so it is significant if the dir in the destination is empty or not.

> second, the error message is confusing because it doesn't state that the
> problem is with the destination. One can think that the problem is actually
> with the source, and that the source directory has some kind of attribute
> that would require it to be empty prior to moving.

I think it's unlikely that users would be confused as
to whether the source needs to be empty.

> I would suggest to change the error message to: "*a directory with the same
> name already exists at destination*"

More accurately the error would be:
"a populated directory with the same name already exists"

Now would could argue that the above info is implicit in:
"directory not empty"
(which BTW is just the standard error from ENOTEMPTY).

Given the subtleties in this area,
I'd be reluctant to adjust diagnostics here.

thanks,
Pádraig

Message #11 received at 64785 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Pádraig Brady <P <at> draigBrady.com>, Nir Oren <niro <at> edev.co>
Cc: 64785 <at> debbugs.gnu.org
Subject: Re: bug#64785: Package: coreutils Version: 8.32-4+b1 program=mv
Date: Sat, 22 Jul 2023 10:37:36 -0700

On 2023-07-22 03:19, Pádraig Brady wrote:
> Given the subtleties in this area,
> I'd be reluctant to adjust diagnostics here.

I looked into this a bit more. Given "mv dir e" where e/dir is an 
existing nonempty directory, 7th Edition Unix fails this way:

  mv: e/dir exists

Solaris 10 mv fails this way:

  mv: cannot rename dir to e/dir: File exists

Coreutils 5.93 fails this way:

  mv: cannot overwrite directory `e/dir'

Current coreutils fails this way:

  mv: cannot move 'dir' to 'e/dir': Directory not empty

macOS fails this way:

  mv: rename dir to e/dir: Directory not empty

If you ask me, none of these are all that good. Current coreutils and 
macOS give a confusing "Directory not empty" message (which directory?). 
Solaris 10 has a similar confusion. 7th Edition and coreutils 5.93 don't 
print strerror (errno).

All things considered, how about if we go back to something like 
coreutils 5.93, except output strerror (errno) too? That is, something 
like this:

  mv: cannot overwrite 'e/dir': Directory not empty

This focuses the user on the problem, avoiding confusion from the 
irrelevant source file name. We'd use this format if renameat fails with 
an errno that means the problem must be with the destination, and stick 
with the current format otherwise. Affected errno values would be 
EDQUOT, EISDIR, ENOSPC, EEXIST, ENOTEMPTY.

Message #14 received at 64785 <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: Paul Eggert <eggert <at> cs.ucla.edu>, Nir Oren <niro <at> edev.co>
Cc: 64785 <at> debbugs.gnu.org
Subject: Re: bug#64785: Package: coreutils Version: 8.32-4+b1 program=mv
Date: Sat, 22 Jul 2023 19:25:26 +0100

On 22/07/2023 18:37, Paul Eggert wrote:
> On 2023-07-22 03:19, Pádraig Brady wrote:
>> Given the subtleties in this area,
>> I'd be reluctant to adjust diagnostics here.
> 
> I looked into this a bit more. Given "mv dir e" where e/dir is an
> existing nonempty directory, 7th Edition Unix fails this way:
> 
>     mv: e/dir exists
> 
> Solaris 10 mv fails this way:
> 
>     mv: cannot rename dir to e/dir: File exists
> 
> Coreutils 5.93 fails this way:
> 
>     mv: cannot overwrite directory `e/dir'
> 
> Current coreutils fails this way:
> 
>     mv: cannot move 'dir' to 'e/dir': Directory not empty
> 
> macOS fails this way:
> 
>     mv: rename dir to e/dir: Directory not empty
> 
> If you ask me, none of these are all that good. Current coreutils and
> macOS give a confusing "Directory not empty" message (which directory?).
> Solaris 10 has a similar confusion. 7th Edition and coreutils 5.93 don't
> print strerror (errno).
> 
> All things considered, how about if we go back to something like
> coreutils 5.93, except output strerror (errno) too? That is, something
> like this:
> 
>     mv: cannot overwrite 'e/dir': Directory not empty
> 
> This focuses the user on the problem, avoiding confusion from the
> irrelevant source file name. We'd use this format if renameat fails with
> an errno that means the problem must be with the destination, and stick
> with the current format otherwise. Affected errno values would be
> EDQUOT, EISDIR, ENOSPC, EEXIST, ENOTEMPTY.

Sounds good.

Thanks for looking into all the details.

Padraig

Message #19 received at 64785-done <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Nir Oren <niro <at> edev.co>
Cc: 64785-done <at> debbugs.gnu.org,
 Pádraig Brady <P <at> draigbrady.com>
Subject: Re: bug#64785: Package: coreutils Version: 8.32-4+b1 program=mv
Date: Sat, 22 Jul 2023 13:47:47 -0700

[Message part 1 (text/plain, inline)]

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.

[0001-mv-better-diagnostic-for-mv-dir-x-failure.patch (text/x-patch, attachment)]

Message #22 received at 64785 <at> debbugs.gnu.org (full text, mbox):

From: Nir Oren <niro <at> edev.co>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: Pádraig Brady <P <at> draigbrady.com>, 64785 <at> debbugs.gnu.org
Subject: Re: bug#64785: Package: coreutils Version: 8.32-4+b1 program=mv
Date: Sat, 22 Jul 2023 22:41:00 +0300

[Message part 1 (text/plain, inline)]

I agree with Paul suggestion for an error message.
In any case, Coreutils 5.93 message was better than the current one

On Sat, Jul 22, 2023 at 8:37 PM Paul Eggert <eggert <at> cs.ucla.edu> wrote:

> On 2023-07-22 03:19, Pádraig Brady wrote:
> > Given the subtleties in this area,
> > I'd be reluctant to adjust diagnostics here.
>
> I looked into this a bit more. Given "mv dir e" where e/dir is an
> existing nonempty directory, 7th Edition Unix fails this way:
>
>    mv: e/dir exists
>
> Solaris 10 mv fails this way:
>
>    mv: cannot rename dir to e/dir: File exists
>
> Coreutils 5.93 fails this way:
>
>    mv: cannot overwrite directory `e/dir'
>
> Current coreutils fails this way:
>
>    mv: cannot move 'dir' to 'e/dir': Directory not empty
>
> macOS fails this way:
>
>    mv: rename dir to e/dir: Directory not empty
>
> If you ask me, none of these are all that good. Current coreutils and
> macOS give a confusing "Directory not empty" message (which directory?).
> Solaris 10 has a similar confusion. 7th Edition and coreutils 5.93 don't
> print strerror (errno).
>
> All things considered, how about if we go back to something like
> coreutils 5.93, except output strerror (errno) too? That is, something
> like this:
>
>    mv: cannot overwrite 'e/dir': Directory not empty
>
> This focuses the user on the problem, avoiding confusion from the
> irrelevant source file name. We'd use this format if renameat fails with
> an errno that means the problem must be with the destination, and stick
> with the current format otherwise. Affected errno values would be
> EDQUOT, EISDIR, ENOSPC, EEXIST, ENOTEMPTY.
>

[Message part 2 (text/html, inline)]

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.