Date: Sun, 24 Feb 2008 20:11:55 +0300 From: Stanislav Sedov <stas@FreeBSD.org> To: "M. Warner Losh" <imp@bsdimp.com> Cc: hackers@freebsd.org Subject: Re: find -lname and -ilname implemented Message-ID: <20080224171155.GD51827@dracon.ht-systems.ru> In-Reply-To: <20080223.000308.686168314.imp@bsdimp.com> References: <20080222.225937.-146245356.imp@bsdimp.com> <20080223.000308.686168314.imp@bsdimp.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sat, Feb 23, 2008 at 12:03:08AM -0700 M. Warner Losh mentioned:
>
> Comments?
>
> Warner
>
> Index: find.1
> ===================================================================
> RCS file: /cache/ncvs/src/usr.bin/find/find.1,v
> retrieving revision 1.82
> diff -u -r1.82 find.1
> --- find.1 28 Feb 2007 10:19:25 -0000 1.82
> +++ find.1 23 Feb 2008 06:53:39 -0000
> @@ -293,6 +293,12 @@
> Please refer to the
> .Ic -atime
> primary description for information on supported time units.
> +.It Ic -d
> +Same as
> +.Ic depth .
> +Gnu find implements this as a primary in mistaken emulation of
Should be, probably, spelled as GNU, it's an abbrev.
> +.Fx
> +.Xr find 1 .
> .It Ic -delete
> Delete found files and/or directories.
> Always returns true.
> @@ -365,6 +371,15 @@
> the string
> .Dq Li {}
> is not qualified.
> +.It Ic -execdir Ar utility Oo Ar argument ... Oc Li {} +
> +Same as
> +.Ic -execdir ,
> +except that
> +.Dq Li {}
> +is replaced with as many pathnames as possible for each invocation of
> +.Ar utility .
> +This behaviour is similar to that of
> +.Xr xargs 1 .
> .It Ic -flags Oo Cm - Ns | Ns Cm + Oc Ns Ar flags , Ns Ar notflags
> The flags are specified using symbolic names (see
> .Xr chflags 1 ) .
> @@ -436,6 +451,10 @@
> is numeric and there is no such group name, then
> .Ar gname
> is treated as a group ID.
> +.It Ic -ilname Ar pattern
> +Like
> +.Ic -lname ,
> +but the match is case insensitive.
> .It Ic -iname Ar pattern
> Like
> .Ic -name ,
> @@ -451,10 +470,18 @@
> Like
> .Ic -regex ,
> but the match is case insensitive.
> +.It Ic -iwholename Ar pattern
> +The same thing as
> +.Ic -ipath ,
> +for Gnu find compatibility.
The same.
> .It Ic -links Ar n
> True if the file has
> .Ar n
> links.
> +.It Ic -lname Ar pattern
> +Like
> +.Ic -name ,
> +but the matched file must also be a symbolic link.
> .It Ic -ls
> This primary always evaluates to true.
> The following information for the current file is written to standard output:
> @@ -497,6 +524,10 @@
> .It Ic -mnewer Ar file
> Same as
> .Ic -newer .
> +.It Ic -mount
> +The same thing as
> +.Ic -xdev ,
> +for Gnu find compatibility.
GNU?
> .It Ic -mtime Ar n Ns Op Cm smhdw
> If no units are specified, this primary evaluates to
> true if the difference between the file last modification time and the time
> @@ -747,6 +778,10 @@
> is numeric and there is no such user name, then
> .Ar uname
> is treated as a user ID.
> +.It Ic -wholename Ar pattern
> +The same thing as
> +.Ic -path ,
> +for Gnu find compatibility.
Similar.
> @@ -1353,7 +1357,7 @@
> return new;
> }
>
> -/* c_simple covers c_prune, c_openparen, c_closeparen, c_not, c_or */
> +/* c_simple covers c_prune, c_openparen, c_closeparen, c_not, c_or, c_true, c_false */
>
As the result of modification the line now exceeds 80 characters, which is, obviously, bad.
> Index: option.c
> ===================================================================
> RCS file: /cache/ncvs/src/usr.bin/find/option.c,v
> retrieving revision 1.25
> diff -u -r1.25 option.c
> --- option.c 5 Apr 2006 23:06:11 -0000 1.25
> +++ option.c 23 Feb 2008 06:57:54 -0000
> @@ -58,6 +58,7 @@
> int typecompare(const void *, const void *);
>
> /* NB: the following table must be sorted lexically. */
> +/* Options listed with C++ comments are in gnu find, but not our find */
Please, don't commit C++ comments, that violates style(9). Also, gnu should be
spelled as GNU.
> static OPTION const options[] = {
> { "!", c_simple, f_not, 0 },
> { "(", c_simple, f_openparen, 0 },
> @@ -74,26 +75,38 @@
> { "-cmin", c_Xmin, f_Xmin, F_TIME_C },
> { "-cnewer", c_newer, f_newer, F_TIME_C },
> { "-ctime", c_Xtime, f_Xtime, F_TIME_C },
> + { "-d", c_depth, f_depth, 0 },
> +// -daystart
> { "-delete", c_delete, f_delete, 0 },
> { "-depth", c_depth, f_depth, 0 },
Why to add just a shortcut for already existent -depth option?
It makes the utility much complex, but don't add value at all.
> { "-empty", c_empty, f_empty, 0 },
> { "-exec", c_exec, f_exec, 0 },
> { "-execdir", c_exec, f_exec, F_EXECDIR },
> - { "-false", c_simple, f_not, 0 },
> + { "-false", c_simple, f_false, 0 },
This brakes FreeBSD compatiblity in the favor of GNU. What will
old FreeBSD user think when his scripts will stop working
after next cvsup? I suppose our target not to make FreeBSD
to look like Linux. If you want to add GNU-like false option,
please, add it under the different name.
> { "-flags", c_flags, f_flags, 0 },
> +// -fls
> { "-follow", c_follow, f_always_true, 0 },
> +// -fprint
> +// -fprint0
> +// -fprintf
> { "-fstype", c_fstype, f_fstype, 0 },
> +// -gid
> { "-group", c_group, f_group, 0 },
> +// -ignore_readdir_race
> + { "-ilname", c_name, f_name, F_LINK | F_IGNCASE },
> { "-iname", c_name, f_name, F_IGNCASE },
> { "-inum", c_inum, f_inum, 0 },
> { "-ipath", c_name, f_path, F_IGNCASE },
> { "-iregex", c_regex, f_regex, F_IGNCASE },
> + { "-iwholename",c_name, f_path, F_IGNCASE },
No space between "-iwholename" and c_name.
Also, just a shortcut to -ipath. Also, -iname is much shorter.
> { "-links", c_links, f_links, 0 },
> + { "-lname", c_name, f_name, F_LINK },
> { "-ls", c_ls, f_ls, 0 },
> { "-maxdepth", c_mXXdepth, f_always_true, F_MAXDEPTH },
> { "-mindepth", c_mXXdepth, f_always_true, 0 },
> { "-mmin", c_Xmin, f_Xmin, 0 },
> { "-mnewer", c_newer, f_newer, 0 },
> + { "-mount", c_xdev, f_always_true, 0 },
SHortuct to -xdev. Again, much longer one.
> { "-mtime", c_Xtime, f_Xtime, 0 },
> { "-name", c_name, f_name, 0 },
> { "-newer", c_newer, f_newer, 0 },
> @@ -118,6 +131,8 @@
> { "-newermm", c_newer, f_newer, 0 },
> { "-newermt", c_newer, f_newer, F_TIME2_T },
> { "-nogroup", c_nogroup, f_nogroup, 0 },
> +// -noignore_readdir_race
> +// -noleaf
> { "-not", c_simple, f_not, 0 },
> { "-nouser", c_nouser, f_nouser, 0 },
> { "-o", c_simple, f_or, 0 },
> @@ -128,12 +143,19 @@
> { "-perm", c_perm, f_perm, 0 },
> { "-print", c_print, f_print, 0 },
> { "-print0", c_print, f_print0, 0 },
> +// -printf
> { "-prune", c_simple, f_prune, 0 },
> +// -quit
> { "-regex", c_regex, f_regex, 0 },
> +// -samefile
> { "-size", c_size, f_size, 0 },
> + { "-true", c_simple, f_always_true, 0 },
> { "-type", c_type, f_type, 0 },
> +// -uid n
> { "-user", c_user, f_user, 0 },
> + { "-wholename", c_name, f_path, 0 },
Wouldn't be "-path" better?
> { "-xdev", c_xdev, f_always_true, 0 },
> +// -xtype
> };
>
> /*
Warner, I fully understand your intention to make FreeBSD more compatible with
GNU, but FreeBSD Project historically had much more important target - to make
a clean designed and not bloated system. I'll be happy to see new useful options
added to find, but in case they isn't just shortcuts to existing ones and
don't break existing functionality.
If'll take a direction to become a Linux one day - we'll certainly loose. There'a
lot of examples of this in history - I don't want to make the same mistake. It's quite
common for BSD and GNU world to make different desicions on design of their systems.
That's pretty normal. BSD people tend to prefer a clean and simple design, where
duplication is a bad thing, where GNU people prefer to implement every possible
functionality in their programs. Linux camp - is 'just works' camp, where BSD -
'works perfectly' camp.
I understand, that you want to attract more people from the GNU/Linux camp to FreeBSD.
But think - people go from Linux to FreeBSD for a reason, a reason they want a more
clean and simple disigned system, perfect system, where good design - is the first,
and everything another - the last. If we'll take the GNU way - what will be the reason
for them to go for FreeBSD?
--
Stanislav Sedov
ST4096-RIPE
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20080224171155.GD51827>