[Findutils-patches] [PATCH] -delete requires -depth

[James Youngman]

2007-08-23  James Youngman  <address@hidden>

        The -delete action now requires -depth rather than automatically
        turning it on.
        * find/parser.c (parse_delete): Require -depth to be selected, but
        don't turn it on by default.  This fixes Savannah bug #20865.
        (HISTORY): Indicate when we introduced this change.
        * doc/find.texi (Cleaning Up): Comply with this requirement.
        * find/testsuite/find.gnu/delete.exp: Comply.
        * find/testsuite/find.gnu/deletefile.exp: Comply.
        * find/testsuite/find.gnu/deletedir.exp: Comply.
        * doc/find.texi (Deleting Files): Document this requirement.
        * find/find.1 (-delete): Document it.
        * NEWS: Mention this change.

Signed-off-by: James Youngman <address@hidden>
---
 ChangeLog                              |   13 +++++++++++++
 NEWS                                   |   10 ++++++++++
 doc/find.texi                          |   26 ++++++++++++++++----------
 find/find.1                            |   11 +++++++----
 find/parser.c                          |    9 +++++++--
 find/testsuite/find.gnu/delete.exp     |    2 +-
 find/testsuite/find.gnu/deletedir.exp  |    2 +-
 find/testsuite/find.gnu/deletefile.exp |    2 +-
 8 files changed, 56 insertions(+), 19 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 41e47df..6f84fe0 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,18 @@
 2007-08-23  James Youngman  <address@hidden>
 
+       The -delete action now requires -depth rather than automatically
+       turning it on.
+       * find/parser.c (parse_delete): Require -depth to be selected, but
+       don't turn it on by default.  This fixes Savannah bug #20865.
+       (HISTORY): Indicate when we introduced this change.
+       * doc/find.texi (Cleaning Up): Comply with this requirement.
+       * find/testsuite/find.gnu/delete.exp: Comply.
+       * find/testsuite/find.gnu/deletefile.exp: Comply.
+       * find/testsuite/find.gnu/deletedir.exp: Comply.
+       * doc/find.texi (Deleting Files): Document this requirement.
+       * find/find.1 (-delete): Document it.
+       * NEWS: Mention this change.
+
        * po/nl.po: Updated from Translation Project
 
        * find/parser.c (check_path_safety): Assume the path is safe is
diff --git a/NEWS b/NEWS
index bb7371d..1434d64 100644
--- a/NEWS
+++ b/NEWS
@@ -6,8 +6,18 @@ GNU findutils NEWS - User visible changes.     -*- outline -*- 
(allout)
 Findutils version 4.3.9 is released under version 3 of the GNU General
 Public License.
 
+** Functional Changes
+
+Uses of find that use -delete without previously specifying -depth
+will now fail without doing anything.  See Savannah bug #20865.
+
 ** Bug Fixes
 
+#20865: -delete now requires -depth, instead of just automatically
+turning it on.  Previously, changing "-print" to "-delete" in your
+test script would automatically turn on "-depth" and might cause a
+different set of files to be deleted.
+
 #20834: Avoid segmentation violation for -execdir when $PATH is
 unset.  Assume that the PATH is safe in this situation.
 
diff --git a/doc/find.texi b/doc/find.texi
index a9e827c..ab91bcf 100644
--- a/doc/find.texi
+++ b/doc/find.texi
@@ -2538,8 +2538,8 @@ with @samp{y} or @samp{Y}.  Implies @samp{-t}.
 Delete files or directories; true if removal succeeded.  If the
 removal failed, an error message is issued.
 
-The use of the @samp{-delete} action on the command line automatically
-turns on the @samp{-depth} option (@pxref{find Expressions}).
+If you want to use the @samp{-delete} action, you must first select
+the @samp{-depth} option (@pxref{find Expressions}).
 @end deffn
 
 @node Adding Tests
@@ -3680,7 +3680,7 @@ find . -name '.#*' -print0 | xargs -0r rm -f
 The command above works, but the following is safer:
 
 @example
-find . -name '.#*' -depth -delete
+find . -depth -name '.#*' -delete
 @end example
 
 @c Idea from Franc,ois Pinard.
@@ -3714,7 +3714,7 @@ Removing old files from @file{/tmp} is commonly done from 
@code{cron}:
 
 @c Idea from Kaveh Ghazi.
 @example
-find /tmp /var/tmp -not -type d -mtime +3 -delete
+find /tmp /var/tmp -depth -not -type d -mtime +3 -delete
 find /tmp /var/tmp -depth -mindepth 1 -type d -empty -delete
 @end example
 
@@ -4088,7 +4088,7 @@ The most efficient and secure method of solving this 
problem is to use
 the @samp{-delete} action:
 
 @smallexample
-find /var/tmp/stuff -mtime +90 -delete
+find /var/tmp/stuff -depth -mtime +90 -delete
 @end smallexample
 
 This alternative is more efficient than any of the @samp{-exec} or
@@ -4100,7 +4100,13 @@ the entry to be deleted, so the @samp{-delete} action 
has the same
 security advantages as the @samp{-execdir} action has.  
 
 The @samp{-delete} action was introduced by the BSD family of
-operating systems.
+operating systems.   The @samp{-delete} option requires
address@hidden  Before findutils version 4.3.9, @samp{-delete} used to
+silently turn on @samp{-depth}, but this caused unexpected differences
+in behaviour when people tried out a command line (using
address@hidden) and then ran it for real (using @samp{-depth}).   In
+order to prevent unexpected data loss, @code{find} will now refuse to
+run if @samp{-delete} is used without @samp{-depth}.
 
 @subsection Improving things still further
 
@@ -4141,10 +4147,10 @@ However, since the system administrator can have such 
an understanding
 they can take advantage of it like so:
 
 @smallexample
-find /var/tmp/stuff1 -mtime +90 -delete &
-find /var/tmp/stuff2 -mtime +90 -delete &
-find /var/tmp/stuff3 -mtime +90 -delete &
-find /var/tmp/stuff4 -mtime +90 -delete &
+find /var/tmp/stuff1 -depth -mtime +90 -delete &
+find /var/tmp/stuff2 -depth -mtime +90 -delete &
+find /var/tmp/stuff3 -depth -mtime +90 -delete &
+find /var/tmp/stuff4 -depth -mtime +90 -delete &
 wait
 @end smallexample
 
diff --git a/find/find.1 b/find/find.1
index 197a4e6..35308c8 100644
--- a/find/find.1
+++ b/find/find.1
@@ -743,8 +743,10 @@ 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.
+error message is issued.  You must use \-depth if you use \-delete.
+The \-delete action used to automatically turn on \-depth, but that
+causes confusing differences in behaviour when people test their
+command line and when they execute it for real.
 
 .IP "\-exec \fIcommand\fR ;"
 Execute \fIcommand\fR; true if 0 status is returned.  All following
@@ -1573,7 +1575,8 @@ As of findutils-4.3.3,
 now matches all files instead of none.  
 .P
 Nanosecond-resolution
-timestamps were implemented in findutils-4.3.3.
+timestamps were implemented in findutils-4.3.3.  The \-delete action
+requires \-depth as of 4.3.9. 
 .TS
 l l l .
 Feature        Added in        Also occurs in 
@@ -1591,7 +1594,7 @@ Feature   Added in        Also occurs in
 \-H    4.2.5   POSIX
 \-L    4.2.5   POSIX
 \-P    4.2.5   BSD
-\-delete       4.2.3   
+\-delete       4.2.3
 \-quit 4.2.3
 \-d    4.2.3   BSD
 \-wholename    4.2.0
diff --git a/find/parser.c b/find/parser.c
index d46127f..91c2529 100644
--- a/find/parser.c
+++ b/find/parser.c
@@ -741,8 +741,13 @@ parse_delete (const struct parser_table* entry, char 
*argv[], int *arg_ptr)
 
   our_pred = insert_primary (entry);
   our_pred->side_effects = our_pred->no_default_print = true;
-  /* -delete implies -depth */
-  options.do_dir_first = false;
+  
+  /* -delete requires -depth */
+  if (options.do_dir_first)
+    {
+      error (1, 0, _("If you use the -delete action, "
+                    "you must use the -depth option first"));
+    }
   
   /* We do not need stat information because we check for the case
    * (errno==EISDIR) in pred_delete.
diff --git a/find/testsuite/find.gnu/delete.exp 
b/find/testsuite/find.gnu/delete.exp
index f748321..cedfd5d 100644
--- a/find/testsuite/find.gnu/delete.exp
+++ b/find/testsuite/find.gnu/delete.exp
@@ -2,6 +2,6 @@ global FTSFIND
 global FINDFLAGS
 exec rm -rf tmp
 exec mkdir tmp tmp/top tmp/top/one tmp/top/one/foo tmp/top/two
-eval exec $FTSFIND tmp $FINDFLAGS -path tmp/top/two -delete
+eval exec $FTSFIND tmp $FINDFLAGS -depth -path tmp/top/two -delete
 find_start p {tmp -print }
 exec rm -rf tmp
diff --git a/find/testsuite/find.gnu/deletedir.exp 
b/find/testsuite/find.gnu/deletedir.exp
index 0d849d9..32f53a6 100644
--- a/find/testsuite/find.gnu/deletedir.exp
+++ b/find/testsuite/find.gnu/deletedir.exp
@@ -5,5 +5,5 @@ exec mkdir tmp tmp/top
 proc createdir {} {
     exec mkdir tmp/top/dir
 }
-find_start p {tmp -path tmp/top/dir -delete -print} "" "" createdir
+find_start p {tmp -depth -path tmp/top/dir -delete -print} "" "" createdir
 exec rm -rf tmp
diff --git a/find/testsuite/find.gnu/deletefile.exp 
b/find/testsuite/find.gnu/deletefile.exp
index 5473a26..9391488 100644
--- a/find/testsuite/find.gnu/deletefile.exp
+++ b/find/testsuite/find.gnu/deletefile.exp
@@ -5,5 +5,5 @@ exec mkdir tmp tmp/top
 proc touchfile {} {
     exec touch tmp/top/file
 }
-find_start p {tmp -path tmp/top/file -delete -print} "" "" touchfile
+find_start p {tmp -depth -path tmp/top/file -delete -print} "" "" touchfile
 exec rm -rf tmp
-- 
1.5.2.1