bugfindutils - Bugs: bug #20865, -delete interacts with -prune

 
 

bug #20865: -delete interacts with -prune

Submitter:  None
Submitted:  Wed 22 Aug 2007 08:50:36 AM UTC
Votes: 1
 
Category:  find Severity:  3 - Normal
Item Group:  None Status:  Fixed
Privacy:  Public Assigned to:  jay
Originator Name:  Bruno De Fraine Originator Email:  -email is unavailable-
Open/Closed:  Closed Release:  4.2.3
Fixed Release:  4.3.11
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Fri 30 Nov 2007 02:17:43 PM UTC, comment #8: 

What James wrote in the NEWS sounds good to me:

<pre>
 Using both -delete and -prune without explicitly
 using -depth is now an error.  Traditionally,
 -delete has always turned -depth on anyway, so
 this is not a functional change.  However, using
 -depth (implicitly or explicitly) makes -prune a
 no-op.  This change is intended to avoid nasty
 surprises for people who test with "-print" and
 then change it to "-delete" when they are happy.
</pre>

Great work!

Aaron Hawley <ashawley>
Fri 30 Nov 2007 10:25:48 AM UTC, comment #7: 

I just committed the fix to CVS.

James Youngman <jay>
Group administrator
Sat 10 Nov 2007 03:47:08 PM UTC, 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.

James Youngman <jay>
Group administrator
Sat 08 Sep 2007 11:49:54 AM UTC, comment #5: 

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.

James Youngman <jay>
Group administrator
Thu 23 Aug 2007 11:30:47 AM UTC, comment #4: 

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

Anonymous
Thu 23 Aug 2007 09:17:10 AM UTC, 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.

James Youngman <jay>
Group administrator
Wed 22 Aug 2007 04:27:02 PM UTC, comment #2: 

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.

Aaron Hawley <ashawley>
Wed 22 Aug 2007 12:32:59 PM UTC, 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.

Eric Blake <ericb>
Group administrator
Wed 22 Aug 2007 08:50:36 AM UTC, original submission:  

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


Anonymous

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by jay (Posted a comment)
  • -email is unavailable- added by ashawley (Posted a comment)
  • -email is unavailable- added by ericb (Posted a comment)
  •  

    There is 1 vote so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

    Follow 10 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2007-12-02 jay Open/ClosedOpen Closed
        Fixed ReleaseNone 4.3.11
    2007-11-29 jay StatusIn Progress Fixed
    2007-11-28 jay StatusPostponed In Progress
    2007-11-10 jay StatusIn Progress Postponed
    2007-08-23 jay StatusConfirmed In Progress
        Assigned toNone jay
    2007-08-22 ashawley Carbon-Copy- Added ashawley
    2007-08-22 ericb StatusNone Confirmed
        Release4.2.28 4.2.3

    Back to the top

    Powered by Savane 3.13-758e.
    Corresponding source code