mv: replace empty directories in cross file system move

src/copy.c (copy_internal): Use rmdir() rather than unlink()
when the source is a directory, so that empty directories
are replaced in the destination as per POSIX.
* tests/mv/part-rename.sh: Augment with various combinations.
* NEWS: Mention the bug fix.
Fixes http://bugs.gnu.org/14763

diff --git a/NEWS b/NEWS
index 62181ec8de0f31741a3e70bde2fea4854fb8a44c..3d0fb18b1ea003581993fa9af4a9d87632f9c8b5 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -22,6 +22,10 @@ GNU coreutils NEWS                                    -*- outline -*-
   system such as GNU/Linux where directory ACL umasks override process umasks.
   [bug introduced in coreutils-6.0]
 
+  mv will now replace empty directories in the destination with directories
+  from the source, when copying across file systems.
+  [This bug was present in "the beginning".]
+
   od -wN with N larger than 64K on a system with 32-bit size_t would
   print approximately 2*N bytes of extraneous padding.
   [Bug introduced in coreutils-7.0]

diff --git a/src/copy.c b/src/copy.c
index c1c82730d4f115dad6f53154c6a34eb332958a91..f0cebb1b1ad2a9127053c63d725de8826c3e4bcb 100644 (file)
--- a/src/copy.c
+++ b/src/copy.c
@@ -2172,8 +2172,12 @@ copy_internal (char const *src_name, char const *dst_name,
 
       /* The rename attempt has failed.  Remove any existing destination
          file so that a cross-device 'mv' acts as if it were really using
-         the rename syscall.  */
-      if (unlink (dst_name) != 0 && errno != ENOENT)
+         the rename syscall.  Note both src and dst must both be directories
+         or not, and this is enforced above.  Therefore we check the src_mode
+         and operate on dst_name here as a tighter constraint and also because
+         src_mode is readily available here.  */
+      if ((S_ISDIR (src_mode) ? rmdir (dst_name) : unlink (dst_name)) != 0
+          && errno != ENOENT)
         {
           error (0, errno,
              _("inter-device move failed: %s to %s; unable to remove target"),

diff --git a/tests/mv/part-rename.sh b/tests/mv/part-rename.sh
index aa2265b1bfecbefa127b73479ee20ab7f9e93ea1..b45fc3ac680f6cf21b26f5ea878feaf47f01c596 100755 (executable)
--- a/tests/mv/part-rename.sh
+++ b/tests/mv/part-rename.sh
@@ -1,7 +1,5 @@
 #!/bin/sh
-# Moving a directory specified with a trailing slash from one partition to
-# another, and giving it a different name at the destination would cause mv
-# to get a failed assertion.
+# Test various cases for moving directories across file systems
 
 # Copyright (C) 2000-2013 Free Software Foundation, Inc.
 
@@ -23,9 +21,37 @@ print_ver_ mv
 cleanup_() { rm -rf "$other_partition_tmpdir"; }
 . "$abs_srcdir/tests/other-fs-tmpdir"
 
+
+# Moving a directory specified with a trailing slash from one partition to
+# another, and giving it a different name at the destination would cause mv
+# to get a failed assertion.
 mkdir foo || framework_failure_
+mv foo/ "$other_partition_tmpdir/bar" || fail=1
 
 
-mv foo/ "$other_partition_tmpdir/bar" || fail=1
+# Moving a non directory from source shouldn't replace empty dir in dest
+touch bar || framework_failure_
+mv bar "$other_partition_tmpdir/" && fail=1
+
+
+# Moving a directory from source shouldn't replace non directory in dest
+mkdir bar2
+touch "$other_partition_tmpdir/bar2"
+mv bar2 "$other_partition_tmpdir/" && fail=1
+
+
+# As per POSIX moving directory from source should replace empty dir in dest
+mkdir bar3
+touch bar3/file
+mkdir "$other_partition_tmpdir/bar3"
+mv bar3 "$other_partition_tmpdir/" || fail=1
+test -e "$other_partition_tmpdir/bar3/file" || fail=1
+
+
+# As per POSIX moving directory from source shouldn't update dir in dest
+mkdir bar3
+touch bar3/file2
+mv bar3 "$other_partition_tmpdir/" && fail=1
+test -e "$other_partition_tmpdir/bar3/file2" && fail=1
 
 Exit $fail