From: Ken Booth <ken@booths.org.uk>
Date: Tue, 2 Jul 2013 00:06:32 +0000 (+0100)
Subject: mv: replace empty directories in cross file system move
X-Git-Tag: v8.22~99
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=2bdb74ec1a453f6c6084d042e573de436ec205f3;p=thirdparty%2Fcoreutils.git

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 62181ec8de..3d0fb18b1e 100644
--- 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 c1c82730d4..f0cebb1b1a 100644
--- 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 aa2265b1bf..b45fc3ac68 100755
--- 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