GNU bug report logs - #6402
[PATCH] rm: added --directory (-d) option

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 6402 in the body.
You can then email your comments to 6402 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Eric Blake <eblake <at> redhat.com>
To: pwplusnick2 <at> gmail.com
Cc: bug-coreutils <at> gnu.org
Subject: Re: [PATCH] rm: added --directory (-d) option
Date: Fri, 11 Jun 2010 09:13:19 -0600

[Message part 1 (text/plain, inline)]

On 03/07/2010 12:47 PM, pwplusnick2 <at> gmail.com wrote:
> Hello again,

Hello,

Sorry for the long delay in reading through my inbox.  It looks like
this was written before we moved to the debbugs interface, making it
easier to lose in the mists of time; but now that I am responding, it
will get an id number to help us remember it.

> 
> I have finally completed my --directory (-d) feature, like the FreeBSD one.

Well, first, I would need to understand if there is any difference
between 'rmdir dir' (specified by POSIX) and 'rm -d dir' - if they are
no different, then how do you justify the extension?  But, since there
are other implementations out there that do it, it might be nice to
support, if only so GNU tools can serve as drop-in replacement for
vendor tools.

> The directory option deletes a directory only if the directory in question is
> empty. This is a safer alternative to the recursive option is some cases where
> you don't want to delete unempty directories.
> 
> I have documented the features. Although I am not the best technical writer, I
> hope that what I have done is adequate. One thing that I have neglected to do,
> intentionally, is to write an automated test. I am not skilled enough or
> confident enough in my Perl or shell scripting ability to write such a test.

Thanks very much for submitting a patch, and especially for including
documentation as part of the patch.  If others on this list agree that
this is worth having, there's still one hurdle to jump through - your
patch is too long to be considered trivial, so you would have to assign
copyright to the FSF, or else we would have to reimplement the patch
from scratch based on your description.  I have not looked at your
patch, for that reason.  Let us know if this is still something you are
willing to pursue.

> 
> I would love to hear what I could have done better, or possibly what I have
> done wrong. Any comments on my code are very much appreciated. That is a big
> part of why I am doing this is to become a better programmer.
> 
> Thank you, and with out further ado, here is the patch.
> 

-- 
Eric Blake   eblake <at> redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

[signature.asc (application/pgp-signature, attachment)]

Message #8 received at 6402 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> CS.UCLA.EDU>
To: Eric Blake <eblake <at> redhat.com>
Cc: 6402 <at> debbugs.gnu.org, pwplusnick2 <at> gmail.com
Subject: Re: bug#6402: [PATCH] rm: added --directory (-d) option
Date: Tue, 15 Jun 2010 11:59:23 -0700

On 06/11/2010 08:13 AM, Eric Blake wrote:

> I would need to understand if there is any difference
> between 'rmdir dir' (specified by POSIX) and 'rm -d dir' - if they are
> no different, then how do you justify the extension?

I briefly looked at the FreeBSD source code, and there's essentially no
difference.  The "-d" option causes BSD "rm" to use the rmdir system call
rather than the unlink system call.  There is a minor difference if you
use the "-P" option to shred on the file before removing it, in that
"rm -dP" does not attempt to shred a directory, but that minor difference
doesn't apply to coreutils where "shred" is a separate executable.

I'm dubious about this "rm -d" flag.  It's clearly not
portable, and (despite a search) I couldn't find any record of anybody
seriously using it.  On the contrary, all the uses I found of "rm -d" were
errors; e.g., people incorrectly thought that "rm -dr FOO" would recursively
move the directory tree rooted at FOO.

Since "rm -d" causes confusion and provides no extra utility over "rmdir"
that I can see, I suggest that we instead change "rm -d" to report an error,
much as "rm -X" does for other offbeat values of X.

Message #11 received at 6402 <at> debbugs.gnu.org (full text, mbox):

From: Jim Meyering <jim <at> meyering.net>
To: Paul Eggert <eggert <at> CS.UCLA.EDU>
Cc: 6402 <at> debbugs.gnu.org, Eric Blake <eblake <at> redhat.com>, pwplusnick2 <at> gmail.com
Subject: Re: bug#6402: [PATCH] rm: added --directory (-d) option
Date: Tue, 15 Jun 2010 21:37:53 +0200

Paul Eggert wrote:
> On 06/11/2010 08:13 AM, Eric Blake wrote:
>
>> I would need to understand if there is any difference
>> between 'rmdir dir' (specified by POSIX) and 'rm -d dir' - if they are
>> no different, then how do you justify the extension?
>
> I briefly looked at the FreeBSD source code, and there's essentially no
> difference.  The "-d" option causes BSD "rm" to use the rmdir system call
> rather than the unlink system call.  There is a minor difference if you
> use the "-P" option to shred on the file before removing it, in that
> "rm -dP" does not attempt to shred a directory, but that minor difference
> doesn't apply to coreutils where "shred" is a separate executable.
>
> I'm dubious about this "rm -d" flag.  It's clearly not
> portable, and (despite a search) I couldn't find any record of anybody
> seriously using it.  On the contrary, all the uses I found of "rm -d" were
> errors; e.g., people incorrectly thought that "rm -dr FOO" would recursively
> move the directory tree rooted at FOO.
>
> Since "rm -d" causes confusion and provides no extra utility over "rmdir"
> that I can see, I suggest that we instead change "rm -d" to report an error,
> much as "rm -X" does for other offbeat values of X.

Thanks for doing that research.  I like your idea.
I too would prefer to discourage the use of rm's -d option.

Message #14 received at 6402 <at> debbugs.gnu.org (full text, mbox):

From: William Plusnick <pwplusnick2 <at> gmail.com>
To: 6402 <at> debbugs.gnu.org
Subject: Re: bug#6402: [PATCH] rm: added --directory (-d) option
Date: Wed, 18 Aug 2010 08:05:56 -0500

[Message part 1 (text/plain, inline)]

Sorry to take so long to reply, life got busy all of a sudden. I took your
advice and instead made a trivial patch (I don't think I will have to sign
anything) that instead prints an error message that points them to rmdir and
exits.

Thank you,
William

-- 
"Once GNU is written, everyone will be able to obtain good system software
free, just like air."-- RMS in the GNU Manifesto

[Message part 2 (text/html, inline)]

[DIFF (application/octet-stream, attachment)]

Message #17 received at 6402 <at> debbugs.gnu.org (full text, mbox):

From: Eric Blake <eblake <at> redhat.com>
To: William Plusnick <pwplusnick2 <at> gmail.com>
Cc: 6402 <at> debbugs.gnu.org
Subject: Re: bug#6402: [PATCH] rm: added --directory (-d) option
Date: Wed, 15 Sep 2010 16:23:33 -0600

On 08/18/2010 07:05 AM, William Plusnick wrote:
> Sorry to take so long to reply, life got busy all of a sudden.

Equally sorry for my delay in turn.  Now are we even?  :)

>I took your
> advice and instead made a trivial patch (I don't think I will have to sign
> anything) that instead prints an error message that points them to rmdir and
> exits.

Thanks for the patch.  However, it's harder to review patches that 
aren't sent as plain/text MIME types.  Here's the meat:

> @@ -227,10 +227,10 @@ main (int argc, char **argv)
>        switch (c)
>          {
>          case 'd':
> -          /* Ignore this option, for backward compatibility with
> -             coreutils 5.92.  FIXME: Some time after 2005, change this
> -             to report an error (or perhaps behave like FreeBSD does)
> -             instead of ignoring the option.  */
> +          /* This is not supported by vote of the maintainers. Print
> +           * an error message rather than ignore it.   */
> +          error (EXIT_FAILURE, 0, "the -d flag is not supported in rm(1)"
> +                                  ", please see rmdir(1) instead.");
>            break;

Indeed trivial, but missing translation of the string.  At any rate, 
it's certainly after 2005, so according to our own comment, we indeed 
should do something very similar to what you have proposed!

-- 
Eric Blake   eblake <at> redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

Message #20 received at 6402 <at> debbugs.gnu.org (full text, mbox):

From: Eric Blake <eblake <at> redhat.com>
To: 6402 <at> debbugs.gnu.org
Subject: [PATCH] rm: remove no-op -d option
Date: Thu, 16 Sep 2010 11:54:34 -0600

* src/rm.c (long_opts, main): Resolve a fixme.
* NEWS: Document the change.
Based on a report by William Plusnick.
---

Jim, what do you think of this alternative patch, which avoids the
issue of a new translation string by instead letting getopt parsing
reject -d like any other unknown option?

 NEWS     |    2 ++
 src/rm.c |   10 +---------
 2 files changed, 3 insertions(+), 9 deletions(-)

diff --git a/NEWS b/NEWS
index 3eb28b1..dc398a9 100644
--- a/NEWS
+++ b/NEWS
@@ -48,6 +48,8 @@ GNU coreutils NEWS                                    -*- outline -*-
   [The old behavior was introduced in coreutils-6.0 and had been removed
    for English only using a different method since coreutils-8.1]

+  rm -d now issues an error rather than being silently ignored.
+
   sort -g now uses long doubles for greater range and precision.

   sort -h no longer rejects numbers with leading or trailing ".", and
diff --git a/src/rm.c b/src/rm.c
index 42f0a57..3b78e19 100644
--- a/src/rm.c
+++ b/src/rm.c
@@ -63,7 +63,6 @@ enum interactive_type

 static struct option const long_opts[] =
 {
-  {"directory", no_argument, NULL, 'd'},
   {"force", no_argument, NULL, 'f'},
   {"interactive", optional_argument, NULL, INTERACTIVE_OPTION},

@@ -222,17 +221,10 @@ main (int argc, char **argv)
   /* Try to disable the ability to unlink a directory.  */
   priv_set_remove_linkdir ();

-  while ((c = getopt_long (argc, argv, "dfirvIR", long_opts, NULL)) != -1)
+  while ((c = getopt_long (argc, argv, "firvIR", long_opts, NULL)) != -1)
     {
       switch (c)
         {
-        case 'd':
-          /* Ignore this option, for backward compatibility with
-             coreutils 5.92.  FIXME: Some time after 2005, change this
-             to report an error (or perhaps behave like FreeBSD does)
-             instead of ignoring the option.  */
-          break;
-
         case 'f':
           x.interactive = RMI_NEVER;
           x.ignore_missing_files = true;
-- 
1.7.2.3

Message #23 received at 6402 <at> debbugs.gnu.org (full text, mbox):

From: Jim Meyering <jim <at> meyering.net>
To: Eric Blake <eblake <at> redhat.com>
Cc: 6402 <at> debbugs.gnu.org
Subject: Re: bug#6402: [PATCH] rm: remove no-op -d option
Date: Fri, 17 Sep 2010 11:13:25 +0200

Eric Blake wrote:
> * src/rm.c (long_opts, main): Resolve a fixme.
> * NEWS: Document the change.
> Based on a report by William Plusnick.
> ---
>
> Jim, what do you think of this alternative patch, which avoids the
> issue of a new translation string by instead letting getopt parsing
> reject -d like any other unknown option?

I like it.  Thank you.

...
> --- a/NEWS
...
> +  rm -d now issues an error rather than being silently ignored.

How about a slight change in wording to make clear
that the entire "rm" command was not being ignored.

  rm's -d now evokes an error;  before, it was silently ignored.

Message #26 received at 6402 <at> debbugs.gnu.org (full text, mbox):

From: Eric Blake <eblake <at> redhat.com>
To: Jim Meyering <jim <at> meyering.net>
Cc: 6402 <at> debbugs.gnu.org
Subject: Re: bug#6402: [PATCH] rm: remove no-op -d option
Date: Fri, 17 Sep 2010 08:46:18 -0600

On 09/17/2010 03:13 AM, Jim Meyering wrote:
>> Jim, what do you think of this alternative patch, which avoids the
>> issue of a new translation string by instead letting getopt parsing
>> reject -d like any other unknown option?
>
> I like it.  Thank you.
>
> ...
>> --- a/NEWS
> ...
>> +  rm -d now issues an error rather than being silently ignored.
>
> How about a slight change in wording to make clear
> that the entire "rm" command was not being ignored.
>
>    rm's -d now evokes an error;  before, it was silently ignored.

Pushed with that change.

-- 
Eric Blake   eblake <at> redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

Message #31 received at 6402-done <at> debbugs.gnu.org (full text, mbox):

From: Jim Meyering <jim <at> meyering.net>
To: 6402-done <at> debbugs.gnu.org
Subject: Re: bug#6402: [PATCH] rm: remove no-op -d option
Date: Sun, 17 Apr 2011 11:01:26 +0200

Eric Blake wrote:
> On 09/17/2010 03:13 AM, Jim Meyering wrote:
>>> Jim, what do you think of this alternative patch, which avoids the
>>> issue of a new translation string by instead letting getopt parsing
>>> reject -d like any other unknown option?
>>
>> I like it.  Thank you.
>>
>> ...
>>> --- a/NEWS
>> ...
>>> +  rm -d now issues an error rather than being silently ignored.
>>
>> How about a slight change in wording to make clear
>> that the entire "rm" command was not being ignored.
>>
>>    rm's -d now evokes an error;  before, it was silently ignored.
>
> Pushed with that change.

Closing.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.