Fix Savannah bug #20802, find -delete anomalies

5 files changed, 65 insertions, 5 deletions

diff --git a/ChangeLog b/ChangeLog
index bc33838b..43cb0628 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+2007-12-02  James Youngman  <jay@gnu.org>
+
+	Fix Savannah bug #20802, find -delete anomalies
+	* find/pred.c (pred_delete): Set find's exit status to nonzero if
+	-delete fails.
+	* find/find.1 (-delete): Document this.
+	* doc/find.texi (Delete Files): Document this.
+	* NEWS: Mention the fix.
+
 2007-11-30  James Youngman  <jay@gnu.org>
 
 	Fix Savannah bug #20865 (-prune -delete without an explicit
diff --git a/NEWS b/NEWS
index d40115d1..c18d991d 100644
--- a/NEWS
+++ b/NEWS
@@ -37,6 +37,9 @@ intended to avoid nasty surprises for people who test with
 #20803: POSIX requires that -prune always returns true.  Previously it
 returned false when -depth was in effect and true otherwise.
 
+#20802: If -delete fails, find's exit status will now be non-zero.
+However, find still skips trying to delete ".".
+
 ** Documentation Fixes
 #21635: Some of the documentation files had missing copying
 conditions.   The missing files now have copying headers, and these
diff --git a/doc/find.texi b/doc/find.texi
index 0eca9a5b..6ef60ad2 100644
--- a/doc/find.texi
+++ b/doc/find.texi
@@ -2553,6 +2553,9 @@ turns on the @samp{-depth} option (@pxref{find Expressions}).  This
 can be surprising if you were previously just testing with
 @samp{-print}, so it is usually best to remember to use @samp{-depth}
 explicitly.
+
+If @samp{-delete} fails, @code{find}'s exit status will be nonzero
+(when it eventually exits).
 @end deffn
 
 @node Adding Tests
diff --git a/find/find.1 b/find/find.1
index 1f3a49e3..1aa9da34 100644
--- a/find/find.1
+++ b/find/find.1
@@ -744,16 +744,38 @@ the type of the file that \-type does not check.
 .SS ACTIONS
 .IP "\-delete\fR"
 Delete files; true if removal succeeded.  If the removal failed, an
-error message is issued.  Use of this action automatically turns on
-the `\-depth' option.   Don't forget that the find command line is
+error message is issued.  
+If 
+.B \-delete
+fails, 
+.BR find 's 
+exit status will be nonzero
+(when it eventually exits).  
+Use of 
+.B \-delete 
+automatically turns on the 
+.B \-depth 
+option.   
+
+.BR Warnings :
+Don't forget that the find command line is 
 evaluated as an expression, so putting \-delete first will make 
 .B find
 try to delete everything below the starting points you specified.
 When testing a 
 .B find
-command line that you later intend to use with \-delete, you should
-explicitly specify \-depth in order to avoid later surprises.  Because
-\-delete implies \-depth, you cannot usefully use \-prune and \-delete
+command line that you later intend to use with 
+.BR \-delete , 
+you should explicitly specify 
+.B \-depth 
+in order to avoid later surprises.  Because
+.B \-delete 
+implies 
+.BR \-depth , 
+you cannot usefully use 
+.B \-prune 
+and 
+.B \-delete
 together. 
 
 .IP "\-exec \fIcommand\fR ;"
@@ -1613,6 +1635,18 @@ now matches all files instead of none.
 .P
 Nanosecond-resolution
 timestamps were implemented in findutils-4.3.3.
+.P
+As of findutils-4.3.11, the 
+.B \-delete 
+action sets 
+.BR find 's 
+exit status to a nonzero value when it fails.
+However, 
+.B find
+will not exit immediately.  Previously, 
+.BR find 's 
+exit status was unaffected by the failure of 
+.BR \-delete .
 .TS
 l l l .
 Feature	Added in	Also occurs in 
diff --git a/find/pred.c b/find/pred.c
index 7fe81e9f..bc26b588 100644
--- a/find/pred.c
+++ b/find/pred.c
@@ -410,6 +410,17 @@ pred_delete (const char *pathname, struct stat *stat_buf, struct predicate *pred
 	}
       error (0, errno, _("cannot delete %s"),
 	     safely_quote_err_filename(0, pathname));
+      /* Previously I had believed that having the -delete action
+       * return false provided the user with control over whether an
+       * error message is issued.  While this is true, the policy of
+       * not affecting the exit status is contrary to the POSIX
+       * requirement that diagnostic messages are accompanied by a
+       * nonzero exit status.  While -delete is not a POSIX option and
+       * we can therefore opt not to follow POSIX in this case, that
+       * seems somewhat arbitrary and confusing.  So, as of
+       * findutils-4.3.11, we also set the exit status in this case.
+       */
+      state.exit_status = 1;
       return false;
     }
   else