-delete interacts with -prune

URL:
  <http://savann
ah.gnu.org/bugs/?20865>

                 Summary: -delete interacts with -prune
                 Project: findutils
            Submitted by: None
            Submitted on: Wednesday 08/22/2007 at 08:50 UTC
                Category: find
                Severity: 3 - Normal
              Item Group: None
                  Status: None
                 Privacy: Public
             Assigned to: None
         Originator Name: Bruno De Fraine
        Originator Email: brunodefraine.net
             Open/Closed: Open
         Discussion Lock: Any
                 Release: 4.2.28
           Fixed Release: None

    _______________________________________________________

Details:

There is a potentially dangerous interaction between -delete
and -prune:
-delete turns on -depth, which (in turn) silently disables
the effects of
-prune. This caused seemingly inexplicable data loss for me
on a number of
occassions before I understood the problem.

This interaction is specifically malicious because it only
occurs when you
proceed to deleting. For example, you can use -prune to list
only the 'real'
files in a Subversion working copy:
$ find -not "(" -name .svn -type d -prune
")" -type f
./a.txt

If you rerun the command-line with -delete added to the end,
intending to
proceed with the deletion of those files, you actually
silently delete more,
as only a listing with -depth reveals:
$ find -depth -not "(" -name .svn -type d -prune
")" -type f
./.svn/text-base/a.txt.svn-base
./.svn/entries
./.svn/all-wcprops
./.svn/format
./a.txt

Given that this is documented behavior of two (perhaps
ill-considered)
features, is there room for any of the following changes to
mitigate this
problem?

* -delete does not turn on -depth implicitly
* issue a warning/error when using -prune in case of -depth
* issue an ad hoc warning/error when detecting the
combination of -prune and
-delete
 




    _______________________________________________________

Reply to this item at:

  <http://savann
ah.gnu.org/bugs/?20865>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.gnu.org/

Update of bug #20865 (project findutils):

                  Status:                    None =>
Confirmed              
                 Release:                  4.2.28 =>
4.2.3                  

    _______________________________________________________

Follow-up Comment #1:

This has been present since -delete was introduced in 4.2.3.
 I agree that
the silent enabling of -depth defeats the purpose of -prune,
but on the other
hand, the use of -delete without -depth is unable to delete
directory trees.

For backwards compatibility, it is easier to keep the
implicit -depth
triggered by -delete.  Both -depth and -prune are specified
by POSIX, so we
can't do anything about those in isolation.  However, since
-delete is not
specified by POSIX, we are okay to make the -delete/-prune
combination issue
an error and refuse to do anything if -depth is not
explicitly requested (I
don't think a warning is good enough, because the changed
traversal order
would still be attempted after printing the warning).  The
error should
suggest the use of -wholename (aka -path) as a way to
achieve what -prune was
doing.

    _______________________________________________________

Reply to this item at:

  <http://savann
ah.gnu.org/bugs/?20865>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.gnu.org/

Follow-up Comment #2, bug #20865 (project findutils):

I was bit by this, too.

Besides using -wholename, one can also just use -type f
-exec rm -f {} ;

It's hard to break a habit -- like using -delete -- after
you start.  It's
more convenient writing -delete than the -exec statment, and
I also feel more
confident having `find' handle my deletes, because I assume
it's "doing it
right".

In 6 months, I predict I'll forget all this and fall into
the -prune/-delete
trap.  So, even I would benefit from having an error message
issued in such a
scenario.   One finds themself writing extensive find
expressions and then
typing -delete at the end without considering the
side-effects of -delete.

+1 to fix.

Thanks to Bruno for bringing my and others attention to
this.

    _______________________________________________________

Reply to this item at:

  <http://savann
ah.gnu.org/bugs/?20865>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.gnu.org/

Update of bug #20865 (project findutils):

                  Status:               Confirmed => In
Progress            
             Assigned to:                    None => jay 
                  

    _______________________________________________________

Follow-up Comment #3:

I agree that the right way to tackle this is almost
certainly to fail with an
explanatory error message if -delete is specified but -depth
is not.

Using -path ... -exec rm -f {} + is less secure, as
explained in the manual.

    _______________________________________________________

Reply to this item at:

  <http://savann
ah.gnu.org/bugs/?20865>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.gnu.org/

Follow-up Comment #4, bug #20865 (project findutils):

James: I have not looked into your patch, but I don't really
understand whose
proposal you are following.

To clarify:

- The fix that I would prefer the most, would be to make
-delete work with
either type of traversal (depth or normal). Of course, if
you want to make
-delete delete directories, you have to assure that these
directories are
empty, or, if you want to empty them as a part of the
traversal, you have to
select a -depth traversal explicitly.
Advantage: if you are only going to delete files, you can
still use -delete
and a normal traversal and thus -prune; with all the other
solutions, if you
require pruning, you have to resort some -print0 | xargs -0
rm, which is
suboptimal. (Alternatively, Eric Blake suggest to use a
-wholename test as a
workaround for a -prune, but isn't it less efficient?)

- The fix that Eric Blake seems to prefer the most, would be
to issue an
error when find detects that its condition contains both
-delete and -prune
but no explicit -depth, since in that case, the user
probably does not expect
for the -prune to get ignored, or he should at least be
notified of the
potential danger.
Advantage: least likely to be backwards incompatible

- You seem to propose to make -delete require an explicit
-depth in any case
(not just a -prune). This is backwards incompatible, and you
still have to
delete with "rm" in case you need pruning?

Regards,
Bruno De Fraine

    _______________________________________________________

Reply to this item at:

  <http://savann
ah.gnu.org/bugs/?20865>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.gnu.org/

Follow-up Comment #5, bug #20865 (project findutils):

Following some discussion on the mailing list, I'm inclined
to think that
Bruno's point is rational.  POSIX doesn't define -delete, so
in theory we
could change it.    However, other implementations (notably
*BSD) have -delete
imply -depth.   I am inclined to avoid a gratuitous
incompatibility.

Options:
0. Make the documentation more explicit
1. Issue a warning message if -delete is specified without
-depth
2. Add a -nodepth option (but this is a poor name I
suppose)
3. Add a new action (say, -remove) which does not imply
-depth

Looks like (0) is obvious, so I'll go ahead regardless with
that. 

Doing (1) seems reasonable, but there was widespread
complaining about the
addition of the "options should appear before
actions" warning message...  

Option (2) is a more general solution but I haven't
considered it much yet.  


Option (3) is superficially attractive but it's arbitrary,
solves just this
specific problem, and it will be hard for everybody to
remember which action
implies -depth and which does not.


    _______________________________________________________

Reply to this item at:

  <http://savann
ah.gnu.org/bugs/?20865>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.gnu.org/

Update of bug #20865 (project findutils):

                  Status:             In Progress =>
Postponed              

    _______________________________________________________

Follow-up Comment #6:

Although I said this was in progress, it isn't right now. 
I'm gonig to try
to make a 4.3.x release soon (long overdue!), and work on
this problem
afterward.

    _______________________________________________________

Reply to this item at:

  <http://savann
ah.gnu.org/bugs/?20865>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.gnu.org/

Update of bug #20865 (project findutils):

                  Status:               Postponed => In
Progress            


    _______________________________________________________

Reply to this item at:

  <http://savann
ah.gnu.org/bugs/?20865>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.gnu.org/

Update of bug #20865 (project findutils):

                  Status:             In Progress =>
Fixed                  


    _______________________________________________________

Reply to this item at:

  <http://savann
ah.gnu.org/bugs/?20865>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.gnu.org/

Follow-up Comment #7, bug #20865 (project findutils):

I just committed the fix to CVS.

    _______________________________________________________

Reply to this item at:

  <http://savann
ah.gnu.org/bugs/?20865>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.gnu.org/