"cp -i --update older newer" no longer prompts; same for mv

* src/copy.c (copy_internal): Perform "update" check before the
possible interactive prompt.  Reported by zeno_AT_biyg_DOT_org
in <http://bugzilla.redhat.com/248591>
* tests/mv/update: Add tests for the above.
* NEWS: Mention the bug fix.

4 files changed, 46 insertions, 30 deletions

diff --git a/ChangeLog b/ChangeLog
index 9219159dd..3e4bc423c 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+2007-07-18  Jim Meyering  <jim@meyering.net>
+
+	"cp -i --update older newer" no longer prompts; same for mv
+	* src/copy.c (copy_internal): Perform "update" check before the
+	possible interactive prompt.  Reported by zeno_AT_biyg_DOT_org
+	in <http://bugzilla.redhat.com/248591>
+	* tests/mv/update: Add tests for the above.
+	* NEWS: Mention the bug fix.
+
 2007-07-15  Jim Meyering  <jim@meyering.net>
 
 	ls --color: Don't stat symlinks when neither ORPHAN nor MISSING
diff --git a/NEWS b/NEWS
index 89c20bb67..423cf498b 100644
--- a/NEWS
+++ b/NEWS
@@ -65,6 +65,8 @@ GNU coreutils NEWS                                    -*- outline -*-
   doing nothing.  The behavior of 'cp' is now better documented when
   the destination is a symlink.
 
+  "cp -i --update older newer" no longer prompts; same for mv
+
   cut now diagnoses a range starting with zero (e.g., -f 0-2) as invalid;
   before, it would treat it as if it started with 1 (-f 1-2).
 
diff --git a/src/copy.c b/src/copy.c
index b7bf73b83..0e549d253 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -1210,6 +1210,30 @@ copy_internal (char const *src_name, char const *dst_name,
 	      return false;
 	    }
 
+	  if (!S_ISDIR (src_mode) && x->update)
+	    {
+	      /* When preserving time stamps (but not moving within a file
+		 system), don't worry if the destination time stamp is
+		 less than the source merely because of time stamp
+		 truncation.  */
+	      int options = ((x->preserve_timestamps
+			      && ! (x->move_mode
+				    && dst_sb.st_dev == src_sb.st_dev))
+			     ? UTIMECMP_TRUNCATE_SOURCE
+			     : 0);
+
+	      if (0 <= utimecmp (dst_name, &dst_sb, &src_sb, options))
+		{
+		  /* We're using --update and the destination is not older
+		     than the source, so do not copy or move.  Pretend the
+		     rename succeeded, so the caller (if it's mv) doesn't
+		     end up removing the source file.  */
+		  if (rename_succeeded)
+		    *rename_succeeded = true;
+		  return true;
+		}
+	    }
+
 	  /* When there is an existing destination file, we may end up
 	     returning early, and hence not copying/moving the file.
 	     This may be due to an interactive `negative' reply to the
@@ -1302,30 +1326,6 @@ copy_internal (char const *src_name, char const *dst_name,
 		      return false;
 		    }
 		}
-
-	      if (x->update)
-		{
-		  /* When preserving time stamps (but not moving within a file
-		     system), don't worry if the destination time stamp is
-		     less than the source merely because of time stamp
-		     truncation.  */
-		  int options = ((x->preserve_timestamps
-				  && ! (x->move_mode
-					&& dst_sb.st_dev == src_sb.st_dev))
-				 ? UTIMECMP_TRUNCATE_SOURCE
-				 : 0);
-
-		  if (0 <= utimecmp (dst_name, &dst_sb, &src_sb, options))
-		    {
-		      /* We're using --update and the destination is not older
-			 than the source, so do not copy or move.  Pretend the
-			 rename succeeded, so the caller (if it's mv) doesn't
-			 end up removing the source file.  */
-		      if (rename_succeeded)
-			*rename_succeeded = true;
-		      return true;
-		    }
-		}
 	    }
 
 	  if (x->move_mode)
diff --git a/tests/mv/update b/tests/mv/update
index 0c0602414..6c3d14983 100755
--- a/tests/mv/update
+++ b/tests/mv/update
@@ -1,7 +1,7 @@
 #!/bin/sh
 # make sure --update works as advertised
 
-# Copyright (C) 2001, 2004, 2006 Free Software Foundation, Inc.
+# Copyright (C) 2001, 2004, 2006-2007 Free Software Foundation, Inc.
 
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -46,11 +46,16 @@ fi
 
 fail=0
 
-for cp_or_mv in cp mv; do
-  # This is a no-op.
-  $cp_or_mv --update old new || fail=1
-  case "`cat new`" in new) ;; *) fail=1 ;; esac
-  case "`cat old`" in old) ;; *) fail=1 ;; esac
+for interactive in '' -i; do
+  for cp_or_mv in cp mv; do
+    # This is a no-op, with no prompt.
+    # With coreutils-6.9 and earlier, using --update with -i would
+    # mistakenly elicit a prompt.
+    $cp_or_mv $interactive --update old new < /dev/null > out 2>&1 || fail=1
+    test -s out && fail=1
+    case "`cat new`" in new) ;; *) fail=1 ;; esac
+    case "`cat old`" in old) ;; *) fail=1 ;; esac
+  done
 done
 
 # This will actually perform the rename.