Fix backup file detection for deleted files

* src/util.c (create_backup): Document patch's backup file logic.
(create_backup, create_backup_copy): Add a flag to remember the backup
file; use when a patch deletes a file.
(move_file, copy_file): Better error messages.
* tests/Makefile.am (TESTS): Remove remember-backup-files-2.
* tests/remember-backup-files: Add tests from remember-backup-files-2.
* tests/symlinks: Add a symlink backup file test.

8 files changed, 124 insertions, 69 deletions

diff --git a/ChangeLog b/ChangeLog
index d25482b..2a2d82f 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,13 @@
 2009-05-04  Andreas Gruenbacher  <agruen@suse.de>
 
+	* src/util.c (create_backup): Document patch's backup file logic.
+	(create_backup, create_backup_copy): Add a flag to remember the backup
+	file; use when a patch deletes a file.
+	(move_file, copy_file): Better error messages.
+	* tests/Makefile.am (TESTS): Remove remember-backup-files-2.
+	* tests/remember-backup-files: Add tests from remember-backup-files-2.
+	* tests/symlinks: Add a symlink backup file test.
+
 	* tests/test-lib.sh: Flag tests with missing pre-requirements as
 	SKIPped instead of PASSed.  Do not use GNU diff extensions, but
 	still require a diff that understands "-u".
diff --git a/src/patch.c b/src/patch.c
index 1d42d90..7c4660b 100644
--- a/src/patch.c
+++ b/src/patch.c
@@ -465,13 +465,13 @@ main (int argc, char **argv)
 		      if (pch_rename ())
 		        {
 			  if (backup)
-			    create_backup (inname, 0, 0, false);
+			    create_backup (inname, 0, 0, false, true);
 			  else if (unlink (inname))
 			    pfatal ("Can't remove file %s", quotearg (inname));
 			}
 		    }
 		  else if (backup)
-		    create_backup (outname, 0, 0, true);
+		    create_backup (outname, 0, 0, true, false);
 		}
 	    }
       }
diff --git a/src/util.c b/src/util.c
index 33f8d63..c32f4e7 100644
--- a/src/util.c
+++ b/src/util.c
@@ -190,19 +190,39 @@ set_file_attributes (char const *to, enum file_attributes attr,
 
 static void
 create_backup_copy (char const *from, char const *to, struct stat *st,
-		    bool to_dir_known_to_exist)
+		    bool to_dir_known_to_exist, bool remember_backup)
 {
-  copy_file (from, to, 0, 0, st->st_mode, to_dir_known_to_exist);
+  struct stat backup_st;
+  copy_file (from, to, remember_backup ? &backup_st : NULL, 0, st->st_mode,
+	     to_dir_known_to_exist);
+  if (remember_backup)
+    insert_file (&backup_st);
   set_file_attributes (to, FA_TIMES | FA_IDS | FA_MODE, st, 0, NULL);
 }
 
 void
 create_backup (char const *to, struct stat *to_st, int *to_errno,
-	       bool leave_original)
+	       bool leave_original, bool remember_backup)
 {
   struct stat tmp_st;
   int tmp_errno;
 
+  /* When the input to patch modifies the same file more than once, patch only
+     backs up the initial version of each file.
+
+     To figure out which files have already been backed up, patch remembers the
+     files that replace the original files.  Files not known already are backed
+     up; files already known have already been backed up before, and are
+     skipped.
+
+     When a patch deletes a file, this leaves patch without such a "sentinel"
+     file.  In that case, patch remembers the *backup file* instead; when a
+     patch creates a file, patch checks if the *backup file* is already known.
+
+     This strategy is not fully compatible with numbered backups: when a patch
+     deletes and later recreates a file with numbered backups, two numbered
+     backups will be created.  */
+
   if (! to_st || ! to_errno)
     {
       to_st = &tmp_st;
@@ -265,25 +285,38 @@ create_backup (char const *to, struct stat *to_st, int *to_errno,
 
       if (*to_errno)
 	{
+	  struct stat backup_st;
 	  int fd;
 
-	  if (debug & 4)
-	    say ("Creating empty file %s\n", quotearg (bakname));
-
-	  try_makedirs_errno = ENOENT;
-	  unlink (bakname);
-	  while ((fd = creat (bakname, 0666)) < 0)
+	  if (lstat (bakname, &backup_st) == 0
+	      && file_already_seen (&backup_st))
 	    {
-	      if (errno != try_makedirs_errno)
-		pfatal ("Can't create file %s", quotearg (bakname));
-	      makedirs (bakname);
-	      try_makedirs_errno = 0;
+	      if (debug & 4)
+		say ("File %s already seen\n", quotearg (to));
+	    }
+	  else
+	    {
+	      if (debug & 4)
+		say ("Creating empty file %s\n", quotearg (bakname));
+
+	      try_makedirs_errno = ENOENT;
+	      unlink (bakname);
+	      while ((fd = creat (bakname, 0666)) < 0)
+		{
+		  if (errno != try_makedirs_errno)
+		    pfatal ("Can't create file %s", quotearg (bakname));
+		  makedirs (bakname);
+		  try_makedirs_errno = 0;
+		}
+	      if (remember_backup && fstat (fd, &backup_st) == 0)
+		insert_file (&backup_st);
+	      if (close (fd) != 0)
+		pfatal ("Can't close file %s", quotearg (bakname));
 	    }
-	  if (close (fd) != 0)
-	    pfatal ("Can't close file %s", quotearg (bakname));
 	}
       else if (leave_original)
-	create_backup_copy (to, bakname, to_st, try_makedirs_errno == 0);
+	create_backup_copy (to, bakname, to_st, try_makedirs_errno == 0,
+			    remember_backup);
       else
 	{
 	  if (debug & 4)
@@ -299,7 +332,8 @@ create_backup (char const *to, struct stat *to_st, int *to_errno,
 	      else if (errno == EXDEV)
 		{
 		  create_backup_copy (to, bakname, to_st,
-				      try_makedirs_errno == 0);
+				      try_makedirs_errno == 0,
+				      remember_backup);
 		  unlink (to);
 		  break;
 		}
@@ -307,6 +341,8 @@ create_backup (char const *to, struct stat *to_st, int *to_errno,
 		pfatal ("Can't rename file %s to %s",
 			quotearg_n (0, to), quotearg_n (1, bakname));
 	    }
+	  if (remember_backup)
+	    insert_file (to_st);
 	}
       free (bakname);
     }
@@ -331,7 +367,7 @@ move_file (char const *from, int *from_needs_removal,
   int to_errno = -1;
 
   if (backup)
-    create_backup (to, &to_st, &to_errno, false);
+    create_backup (to, &to_st, &to_errno, false, from == NULL);
 
   if (from)
     {
@@ -365,7 +401,9 @@ move_file (char const *from, int *from_needs_removal,
 		pfatal ("Can't create %s %s", "symbolic link", to);
 	    }
 	  free (buffer);
-	  /* FIXME: insert_file (&tost); */
+	  if (lstat (to, &to_st) != 0)
+	    pfatal ("Can't get file attributes of %s %s", "symbolic link", to);
+	  insert_file (&to_st);
 	}
       else
 	{
@@ -494,7 +532,8 @@ copy_file (char const *from, char const *to, struct stat *tost,
 	pfatal ("Can't read %s %s", "symbolic link", from);
       if (symlink (buffer, to) != 0)
 	pfatal ("Can't create %s %s", "symbolic link", to);
-
+      if (tost && lstat (to, tost) != 0)
+	pfatal ("Can't get file attributes of %s %s", "symbolic link", to);
       free (buffer);
     }
   else
@@ -503,8 +542,9 @@ copy_file (char const *from, char const *to, struct stat *tost,
       tofd = create_file (to, O_WRONLY | O_BINARY | to_flags, mode,
 			  to_dir_known_to_exist);
       copy_to_fd (from, tofd);
-      if ((tost && fstat (tofd, tost) != 0)
-	  || close (tofd) != 0)
+      if (tost && fstat (tofd, tost) != 0)
+	pfatal ("Can't get file attributes of %s %s", "file", to);
+      if (close (tofd) != 0)
 	write_fatal ();
     }
 }
diff --git a/src/util.h b/src/util.h
index d8b851d..388a319 100644
--- a/src/util.h
+++ b/src/util.h
@@ -55,7 +55,7 @@ void ignore_signals (void);
 void init_backup_hash_table (void);
 void init_time (void);
 void xalloc_die (void) __attribute__ ((noreturn));
-void create_backup (char const *, struct stat *, int *, bool);
+void create_backup (char const *, struct stat *, int *, bool, bool);
 void move_file (char const *, int *, struct stat const *, char const *, mode_t, bool);
 void read_fatal (void) __attribute__ ((noreturn));
 void remove_prefix (char *, size_t);
diff --git a/tests/Makefile.am b/tests/Makefile.am
index bbf9d6a..80684aa 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -41,14 +41,12 @@ TESTS = \
 	read-only-files \
 	reject-format \
 	remember-backup-files \
-	remember-backup-files-2 \
 	remember-reject-files \
 	symlinks \
 	unmodified-files
 
 XFAIL_TESTS = \
-	dash-o-append \
-	remember-backup-files-2
+	dash-o-append
 
 EXTRA_DIST = \
 	$(TESTS) \
diff --git a/tests/remember-backup-files b/tests/remember-backup-files
index a12b1d6..635a7d4 100644
--- a/tests/remember-backup-files
+++ b/tests/remember-backup-files
@@ -5,6 +5,7 @@
 # notice and this notice are preserved.
 
 # Patch must not overwrite backup files it has created itself.
+# (Backup file tests for symlinks are in tests/symlinks.)
 
 . $srcdir/test-lib.sh
 
@@ -81,3 +82,31 @@ EOF
 check 'cat g.orig' <<EOF
 one
 EOF
+
+# ==============================================================
+
+echo one > f
+
+cat > f.diff <<EOF
+--- f
++++ /dev/null
+@@ -1 +0,0 @@
+-one
+--- /dev/null
++++ f
+@@ -0,0 +1 @@
++two
+EOF
+
+check 'patch --backup -p0 < f.diff' <<EOF
+patching file f
+patching file f
+EOF
+
+check 'cat f.orig' <<EOF
+one
+EOF
+
+check 'cat f' <<EOF
+two
+EOF
diff --git a/tests/remember-backup-files-2 b/tests/remember-backup-files-2
deleted file mode 100644
index 537d970..0000000
--- a/tests/remember-backup-files-2
+++ /dev/null
@@ -1,41 +0,0 @@
-# Copyright (C) 2009 Free Software Foundation, Inc.
-#
-# Copying and distribution of this file, with or without modification,
-# in any medium, are permitted without royalty provided the copyright
-# notice and this notice are preserved.
-
-# Patch must not overwrite backup files it has created itself.
-
-. $srcdir/test-lib.sh
-
-require_cat
-use_local_patch
-use_tmpdir
-
-# ==============================================================
-
-echo one > f
-
-cat > f.diff <<EOF
---- f
-+++ /dev/null
-@@ -1 +0,0 @@
--one
---- /dev/null
-+++ f
-@@ -0,0 +1 @@
-+two
-EOF
-
-check 'patch --backup -p0 < f.diff' <<EOF
-patching file f
-patching file f
-EOF
-
-check 'cat f.orig' <<EOF
-one
-EOF
-
-check 'cat f' <<EOF
-two
-EOF
diff --git a/tests/symlinks b/tests/symlinks
index 1635517..f7dea76 100644
--- a/tests/symlinks
+++ b/tests/symlinks
@@ -176,3 +176,24 @@ check 'echo b > symlink.orig && cat target2' <<EOF
 b
 EOF
 rm -f target2
+
+# --------------------------------------------------------------
+
+# Delete and then recreate the same symlink: make sure the original
+# symlink is backed up
+
+ncheck 'ln -s target2 symlink'
+rm -f symlink.orig
+
+check 'cat delete-symlink.diff create-symlink.diff | patch -p1 --backup || echo "Status: $?"' <<EOF
+patching symbolic link symlink
+patching symbolic link symlink
+EOF
+
+check 'echo a > symlink.orig && cat target2' <<EOF
+a
+EOF
+check 'echo b > symlink.orig && cat target2' <<EOF
+b
+EOF
+rm -f target2