Re: [PATCH] rm(1): add -v option for verbosity
On Sun, Jun 25, 2017 at 11:16:00PM +0200, Ingo Schwarze wrote: > Hi, > > Paul de Weerd wrote on Sun, Jun 25, 2017 at 10:46:15PM +0200: > > On Sun, Jun 25, 2017 at 03:12:26PM +0200, Ingo Schwarze wrote: > > > | If you are really unsure, study the output of > > | $ find * > > | first, before typing > > | $ rm -rf * > > | No non-standard option is needed at all for this. > > > Very bad example. You study the output of find * and another process > > writes a new file in the mean time. With rm -v, you would've known. > > I don't buy the talk about race conditions at all. > > If other processes may be writing important files there, then for > god's sake don't rm -rf it. Besides, the rm -v output arrives too > late, when the data is already gone. > > If the directory is full of transient files and removing them is OK, > than it obviously doesn't matter what exactly got removed. > > Seriously, files not important enough to be kept, but so important > that you need an audit trail concerning their removal? That sounds > like a very unusual use case at best. > > > Landry's argument about the progress-meter has some merit, but it > is clearly more important for copying than for removing, and it is > clearly more important for copying over the network than for plain > cp(1), so i'm not sure it is sufficient justification. scp -v? $ mkdir ../abc $ scp -v ./*pdf ../abc/ Executing: cp -- ./iso-prog_lang_C-WG14-N869.pdf ../abc/ Executing: cp -- ./iso-prog_lang_C.WG14-N897.pdf ../abc/ Executing: cp -- ./n1124.pdf ../abc/ Executing: cp -- ./n1256.pdf ../abc/ Executing: cp -- ./n1570.pdf ../abc/ ... $ scp -v n1570.pdf ../abc/single_file.pdf Executing: cp -- n1570.pdf ../abc/single_file.pdf -pk > Anyway, the whole thread looks awfully bikeshedish, so i guess > i should set a better example and shut up... > > Yours, > Ingo > > > P.S. > On the humorous side: > > Landry mentioned about rsync: > > > Technically it could be possible to replicate mv with rsync > > --remove-source-files ... :) > > Talk about the kitchen sink... > > And i heard rumours that some rsync(1) command line options > are so long that they don't fit into the command line length > limit of some shells. ;-) >
Re: [PATCH] rm(1): add -v option for verbosity
Hi, Paul de Weerd wrote on Sun, Jun 25, 2017 at 10:46:15PM +0200: > On Sun, Jun 25, 2017 at 03:12:26PM +0200, Ingo Schwarze wrote: > | If you are really unsure, study the output of > | $ find * > | first, before typing > | $ rm -rf * > | No non-standard option is needed at all for this. > Very bad example. You study the output of find * and another process > writes a new file in the mean time. With rm -v, you would've known. I don't buy the talk about race conditions at all. If other processes may be writing important files there, then for god's sake don't rm -rf it. Besides, the rm -v output arrives too late, when the data is already gone. If the directory is full of transient files and removing them is OK, than it obviously doesn't matter what exactly got removed. Seriously, files not important enough to be kept, but so important that you need an audit trail concerning their removal? That sounds like a very unusual use case at best. Landry's argument about the progress-meter has some merit, but it is clearly more important for copying than for removing, and it is clearly more important for copying over the network than for plain cp(1), so i'm not sure it is sufficient justification. Anyway, the whole thread looks awfully bikeshedish, so i guess i should set a better example and shut up... Yours, Ingo P.S. On the humorous side: Landry mentioned about rsync: > Technically it could be possible to replicate mv with rsync > --remove-source-files ... :) Talk about the kitchen sink... And i heard rumours that some rsync(1) command line options are so long that they don't fit into the command line length limit of some shells. ;-)
Re: [PATCH] rm(1): add -v option for verbosity
On Sun, Jun 25, 2017 at 04:09:11PM +0200, Job Snijders wrote: > --- bin/rm/rm.1 > +++ bin/rm/rm.1 > @@ -95,6 +95,8 @@ that directory is skipped. > .It Fl r > Equivalent to > .Fl R . > +.It Fl v > +Explain what is being done. On second thought, "Display what files were removed." would be a more succinct description of what the "-v" option in 'rm' does.
Re: [PATCH] rm(1): add -v option for verbosity
Hi Ingo, On Sun, Jun 25, 2017 at 03:12:26PM +0200, Ingo Schwarze wrote: | Hi, | | Job Snijders wrote on Sun, Jun 25, 2017 at 02:06:16PM +0200: | | > This patch adds a '-v' option to rm(1) for more verbose output. | | Do not add new options to standard utilities, unless you can show | that they are unusually useful in practice *and* practically | every other system out there has them, with a compatible user | interface across practically all systems. | | Adding -v to rm(1) probably wouldn't make the very high bar against | adding non-standard options even if almost everybody else had it | (which i didn't check, to not waste time) because it's completely | useless. | | If you are really unsure, study the output of | | $ find * | | first, before typing | | $ rm -rf * | | No non-standard option is needed at all for this. Very bad example. You study the output of find * and another process writes a new file in the mean time. With rm -v, you would've known. The solution here (currently) is to find * -delete -print. That solves it for rm (well, it moves the problem elsewhere (and is more to type)), but doesn't help the mv or cp cases. And I'd say that for completeness' sake, the option should be added to all three of these (or none, as is currently the case). | It's also strongly in violation of the UNIX philosophy (each utility, | do one thing, and do it well). rm(1) removes files, and does so | well. rm(1) does *not* trespass on ls(1) and find(1) territory to | list files. Well, find(1) trespasses the rm(1) territory with its -delete option, the only real solution to doing what you suggested. Reporting on what you did is still doing your job. Just a bit more verbose (which, in certain situations, I would consider a BETTER job). Comparing rm(1) to ls(1) is weird: ls doesn't delete files. In this case, rm(1) isn't "listing" the files as ls(1) does, it *reports* on files that have been deleted. A small, but significant difference. | Besides, they way your proposed rm(1) extension lists | files is not doing it well at all. The output is awful. Agreed. Just listing the filename(s) would suffice. Paul 'WEiRD' de Weerd -- >[<++>-]<+++.>+++[<-->-]<.>+++[<+ +++>-]<.>++[<>-]<+.--.[-] http://www.weirdnet.nl/
Re: [PATCH] rm(1): add -v option for verbosity
Hi Ingo,
Thanks for taking the time to review this.
On Sun, Jun 25, 2017 at 03:12:26PM +0200, Ingo Schwarze wrote:
> Job Snijders wrote on Sun, Jun 25, 2017 at 02:06:16PM +0200:
>
> > This patch adds a '-v' option to rm(1) for more verbose output.
>
> Do not add new options to standard utilities, unless you can show that
> they are unusually useful in practice *and* practically every other
> system out there has them, with a compatible user interface across
> practically all systems.
I checked a number of systems: busybox, HP-UX, and SunOS don't have -v.
FreeBSD, NetBSD, DragonFlyBSD, Slackware, Minix, Debian, CentOS, Suse,
and Darwin do have a -v option to report which files were deleted.
> Adding -v to rm(1) probably wouldn't make the very high bar against
> adding non-standard options even if almost everybody else had it
> (which i didn't check, to not waste time) because it's completely
> useless.
I appreciate the bar is high, and I doubted whether I should send the
patch at all. However, without some proposed code on the table we would
not be able to have this conversation.
> If you are really unsure, study the output of
>
> $ find *
>
> first, before typing
>
> $ rm -rf *
>
> No non-standard option is needed at all for this.
The 'find *' will show you what existed at the moment of executing the
'find' command, where on the other hand 'rm -rfv *' gives a report of
which files actually were succesfully deleted through the execution of
the 'rm' command.
> It's also strongly in violation of the UNIX philosophy (each utility,
> do one thing, and do it well). rm(1) removes files, and does so well.
> rm(1) does *not* trespass on ls(1) and find(1) territory to list
> files.
I'm not sure I agree with you on this point. To me ls(1) and rm(1) have
different uses, rm(1) being able to tell which files it deleted, is not
at all the same as requesting a listing of files.
> Besides, they way your proposed rm(1) extension lists files is not
> doing it well at all. The output is awful.
Yes, the output could be simplified, below a version of the patch to
align with how freebsd and netbsd do it:
$ touch f; mkdir d; touch d/f
$ rm -rfv *
d/f
d
f
Sticking to the standards is a strong argument, and I understand that
changes to core utilities like rm must be thoroughly vetted. Please
don't take offense in me attempting to propose a change.
Kind regards,
Job
diff --git bin/rm/rm.1 bin/rm/rm.1
index 5c8aefaab7d..edc68b60001 100644
--- bin/rm/rm.1
+++ bin/rm/rm.1
@@ -95,6 +95,8 @@ that directory is skipped.
.It Fl r
Equivalent to
.Fl R .
+.It Fl v
+Explain what is being done.
.El
.Pp
The
@@ -148,7 +150,7 @@ utility is compliant with the
specification.
.Pp
The flags
-.Op Fl dP
+.Op Fl dPv
are extensions to that specification.
.Sh HISTORY
An
diff --git bin/rm/rm.c bin/rm/rm.c
index 3242ef5f410..fc0904d0325 100644
--- bin/rm/rm.c
+++ bin/rm/rm.c
@@ -50,7 +50,7 @@
extern char *__progname;
-int dflag, eval, fflag, iflag, Pflag, stdin_ok;
+int dflag, eval, fflag, iflag, Pflag, vflag, stdin_ok;
intcheck(char *, char *, struct stat *);
void checkdot(char **);
@@ -73,7 +73,7 @@ main(int argc, char *argv[])
int ch, rflag;
Pflag = rflag = 0;
- while ((ch = getopt(argc, argv, "dfiPRr")) != -1)
+ while ((ch = getopt(argc, argv, "dfiPRrv")) != -1)
switch(ch) {
case 'd':
dflag = 1;
@@ -93,6 +93,9 @@ main(int argc, char *argv[])
case 'r': /* Compatibility. */
rflag = 1;
break;
+ case 'v':
+ vflag = 1;
+ break;
default:
usage();
}
@@ -201,8 +204,11 @@ rm_tree(char **argv)
case FTS_DP:
case FTS_DNR:
if (!rmdir(p->fts_accpath) ||
- (fflag && errno == ENOENT))
+ (fflag && errno == ENOENT)) {
+ if (vflag)
+ (void)fprintf(stdout, "%s\n",
p->fts_path);
continue;
+ }
break;
case FTS_F:
@@ -213,8 +219,11 @@ rm_tree(char **argv)
/* FALLTHROUGH */
default:
if (!unlink(p->fts_accpath) ||
- (fflag && errno == ENOENT))
+ (fflag && errno == ENOENT)) {
+ if (vflag)
+ (void)fprintf(stdout, "%s\n",
p->fts_path);
continue;
+ }
}
warn("%s", p->fts_path);
eval = 1;
@@ -262,7 +271,8 @@ rm_file(char **argv)
if (rval && (!fflag
Re: [PATCH] rm(1): add -v option for verbosity
Hi, Job Snijders wrote on Sun, Jun 25, 2017 at 02:06:16PM +0200: > This patch adds a '-v' option to rm(1) for more verbose output. Do not add new options to standard utilities, unless you can show that they are unusually useful in practice *and* practically every other system out there has them, with a compatible user interface across practically all systems. Adding -v to rm(1) probably wouldn't make the very high bar against adding non-standard options even if almost everybody else had it (which i didn't check, to not waste time) because it's completely useless. If you are really unsure, study the output of $ find * first, before typing $ rm -rf * No non-standard option is needed at all for this. It's also strongly in violation of the UNIX philosophy (each utility, do one thing, and do it well). rm(1) removes files, and does so well. rm(1) does *not* trespass on ls(1) and find(1) territory to list files. Besides, they way your proposed rm(1) extension lists files is not doing it well at all. The output is awful. Yours, Ingo > $ mkdir a; touch a/b; touch c > $ rm -rfv * > removed 'a/b' > removed directory 'a' > removed 'c'
