Re: [PATCH] find: doc: Fix -prune SCM example directories and make it more efficient
On Wed, Nov 23, 2022 at 07:07:14PM +1100, raf wrote:
> On Tue, Nov 22, 2022 at 11:12:00PM +0100, Bernhard Voelker
> wrote:
>
> > Hi raf,
>
> Hi Berny,
>
> > thanks for the patch.
> > Besides busy day job times here, I was actually waiting for the FSF legal
> > to add your entry into the 'copyright.list' file on the GNU fencepost
> > server.
> > It's not yet in place though, so no need to hurry.
>
> No worries. That'll take some time. I've emailed the
> copyright assignment request form to Craig Topham, but
> the paperwork has yet to arrive here for signing.
>
> > On 11/18/22 23:48, raf wrote:
> > > * find/find.1 - Fix -prune SCM example directories and make it more
> > > efficient
> > > * doc/find.texi - Make -prune SCM example more efficient (two ways)
> > > * NEWS - Mention the above
> > > ---
> > > NEWS | 3 +++
> > > doc/find.texi | 25 +
> > > find/find.1 | 13 -
> > > 3 files changed, 36 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/NEWS b/NEWS
> > > index 1fff34f8..7c6fcfb3 100644
> > > --- a/NEWS
> > > +++ b/NEWS
> > > @@ -18,6 +18,9 @@ GNU findutils NEWS - User visible changes. -*-
> > > outline -*- (allout)
> > > When generating the Texinfo manual, `makeinfo` is invoked with the
> > > --no-split
> > > option for all output formats now; this avoids files like
> > > find.info-[12].
> > > + The find.1 manual's -prune SCM example directories/output have been
> > > fixed, and
> > > + the example itself has been made more efficient (find.1 and find.texi)
> > > [#62259]
> > > + Also fixed a typo in find.texi.
> > > * Noteworthy changes in release 4.9.0 (2022-02-22) [stable]
> > > diff --git a/doc/find.texi b/doc/find.texi
> > > index 379fe646..1a36e243 100644
> > > --- a/doc/find.texi
> > > +++ b/doc/find.texi
> > > @@ -5138,13 +5138,15 @@ already found.
> > > @smallexample
> > > find repo/ \
> > > --exec test -d @{@}/.svn \; -or \
> > > --exec test -d @{@}/.git \; -or \
> > > --exec test -d @{@}/CVS \; -print -prune
> > > +-type d \
> > > +\( -exec test -d @{@}/.svn \; \
> > > +-or -exec test -d @{@}/.git \; \
> > > +-or -exec test -d @{@}/CVS \; \
> > > +\) -print -prune
> > > @end smallexample
> >
> > Indeed, the bug omitting the \( ... \) has been introduced in 2008:
> >
> > $ git diff v4.5.6b-28-gaca33f85^..v4.5.6b-28-gaca33f85 -- doc/find.texi |
> > tail -n12
> > @@ -4683,7 +4683,10 @@ searching subdirectories inside projects whose SCM
> > directory we
> >already found.
> >
> >@smallexample
> > -find repo/ -exec test -d @{@}/.svn -o -d @{@}/.git -o -d @{@}/CVS \;
> > -print -prune
> > +find repo/ \
> > +-exec test -d @{@}/.svn \; -or \
> > +-exec test -d @{@}/.git \; -or \
> > +-exec test -d @{@}/CVS \; -print -prune
> >@end smallexample
> >
> >In this example, @command{test} is used to tell if we are currently
> >
> > For find.1, this was already fixed in commit v4.6.0-55-g47d8fd38, but missed
> > to check the analog place in the texi file.
> >
> > Your patch above aims at optimizing things by letting 'find -type d'
> > pre-filter
> > directories in order to avoid -exec invocations on regular and other
> > non-directory
> > files.
> > The '-type d' test is different from 'test -d ...' because the latter
> > transparently
> > follows symbolic links while the former strictly checks on the type of the
> > entry.
> > Therefore, the patch changes the result in the case the repo workspace is a
> > symlink:
> > the new version would skip it. Well, adding -L would help.
>
> Ah, I hadn't thought of that. I can add -L.
>
> > > In this example, @command{test} is used to tell if we are currently
> > > -examining a directory which appears to the a project's root directory
> > > +examining a directory which appears to be a project's root directory
> >
> > good catch!
> >
> > > (because it has an SCM subdirectory). When we find a project root,
> > > there is no need to search inside it, and @code{-prune} makes sure
> > > that we descend no further.
> > > @@ -5153,6 +5155,21 @@ For large, complex trees like the Linux kernel,
> > > this will prevent
> > > searching a large portion of the structure, saving a good deal of
> > > time.
> > > +The @samp{-type d} clause causes the three @samp{test} shell
> > > +processes to only be executed for directories. This can be made even
> > > +more efficient by combining the three @samp{test} shell processes
> > > +into a single process:
> > > +
> > > +@smallexample
> > > +find repo/ \
> > > +-type d \
> > > +-exec sh -c 'test -d "$1"/.svn || test -d "$1"/.git || test -d "$1"/CVS'
> > > . {} \; \
> > > +-print -prune
> > > +@end smallexample
> > > +
> > > +Note that the @samp{.} argument is just a placeholder for the unused
> > > +@samp{$0} environment variable in the @samp{sh -c} command. The
> > > +@samp{@{@}} argument is the @samp{$1} environment variable.
> > ___^^^
Re: [PATCH] find: doc: Fix -prune SCM example directories and make it more efficient
On Tue, Nov 22, 2022 at 11:12:00PM +0100, Bernhard Voelker
wrote:
> Hi raf,
Hi Berny,
> thanks for the patch.
> Besides busy day job times here, I was actually waiting for the FSF legal
> to add your entry into the 'copyright.list' file on the GNU fencepost server.
> It's not yet in place though, so no need to hurry.
No worries. That'll take some time. I've emailed the
copyright assignment request form to Craig Topham, but
the paperwork has yet to arrive here for signing.
> On 11/18/22 23:48, raf wrote:
> > * find/find.1 - Fix -prune SCM example directories and make it more
> > efficient
> > * doc/find.texi - Make -prune SCM example more efficient (two ways)
> > * NEWS - Mention the above
> > ---
> > NEWS | 3 +++
> > doc/find.texi | 25 +
> > find/find.1 | 13 -
> > 3 files changed, 36 insertions(+), 5 deletions(-)
> >
> > diff --git a/NEWS b/NEWS
> > index 1fff34f8..7c6fcfb3 100644
> > --- a/NEWS
> > +++ b/NEWS
> > @@ -18,6 +18,9 @@ GNU findutils NEWS - User visible changes. -*-
> > outline -*- (allout)
> > When generating the Texinfo manual, `makeinfo` is invoked with the
> > --no-split
> > option for all output formats now; this avoids files like
> > find.info-[12].
> > + The find.1 manual's -prune SCM example directories/output have been
> > fixed, and
> > + the example itself has been made more efficient (find.1 and find.texi)
> > [#62259]
> > + Also fixed a typo in find.texi.
> > * Noteworthy changes in release 4.9.0 (2022-02-22) [stable]
> > diff --git a/doc/find.texi b/doc/find.texi
> > index 379fe646..1a36e243 100644
> > --- a/doc/find.texi
> > +++ b/doc/find.texi
> > @@ -5138,13 +5138,15 @@ already found.
> > @smallexample
> > find repo/ \
> > --exec test -d @{@}/.svn \; -or \
> > --exec test -d @{@}/.git \; -or \
> > --exec test -d @{@}/CVS \; -print -prune
> > +-type d \
> > +\( -exec test -d @{@}/.svn \; \
> > +-or -exec test -d @{@}/.git \; \
> > +-or -exec test -d @{@}/CVS \; \
> > +\) -print -prune
> > @end smallexample
>
> Indeed, the bug omitting the \( ... \) has been introduced in 2008:
>
> $ git diff v4.5.6b-28-gaca33f85^..v4.5.6b-28-gaca33f85 -- doc/find.texi |
> tail -n12
> @@ -4683,7 +4683,10 @@ searching subdirectories inside projects whose SCM
> directory we
>already found.
>
>@smallexample
> -find repo/ -exec test -d @{@}/.svn -o -d @{@}/.git -o -d @{@}/CVS \;
> -print -prune
> +find repo/ \
> +-exec test -d @{@}/.svn \; -or \
> +-exec test -d @{@}/.git \; -or \
> +-exec test -d @{@}/CVS \; -print -prune
>@end smallexample
>
>In this example, @command{test} is used to tell if we are currently
>
> For find.1, this was already fixed in commit v4.6.0-55-g47d8fd38, but missed
> to check the analog place in the texi file.
>
> Your patch above aims at optimizing things by letting 'find -type d'
> pre-filter
> directories in order to avoid -exec invocations on regular and other
> non-directory
> files.
> The '-type d' test is different from 'test -d ...' because the latter
> transparently
> follows symbolic links while the former strictly checks on the type of the
> entry.
> Therefore, the patch changes the result in the case the repo workspace is a
> symlink:
> the new version would skip it. Well, adding -L would help.
Ah, I hadn't thought of that. I can add -L.
> > In this example, @command{test} is used to tell if we are currently
> > -examining a directory which appears to the a project's root directory
> > +examining a directory which appears to be a project's root directory
>
> good catch!
>
> > (because it has an SCM subdirectory). When we find a project root,
> > there is no need to search inside it, and @code{-prune} makes sure
> > that we descend no further.
> > @@ -5153,6 +5155,21 @@ For large, complex trees like the Linux kernel, this
> > will prevent
> > searching a large portion of the structure, saving a good deal of
> > time.
> > +The @samp{-type d} clause causes the three @samp{test} shell
> > +processes to only be executed for directories. This can be made even
> > +more efficient by combining the three @samp{test} shell processes
> > +into a single process:
> > +
> > +@smallexample
> > +find repo/ \
> > +-type d \
> > +-exec sh -c 'test -d "$1"/.svn || test -d "$1"/.git || test -d "$1"/CVS' .
> > {} \; \
> > +-print -prune
> > +@end smallexample
> > +
> > +Note that the @samp{.} argument is just a placeholder for the unused
> > +@samp{$0} environment variable in the @samp{sh -c} command. The
> > +@samp{@{@}} argument is the @samp{$1} environment variable.
> ___^^
>
> s/environment/shell/, and maybe s/variable/parameter/
Are you sure? The term "environment variable" is a
fairly standard term. The find documentation (manual
and info) already uses it at least 16 times. There are
no appearances there of the term "shell parameter".
Should I change it anyway?
>
Re: [PATCH] find: doc: Fix -prune SCM example directories and make it more efficient
Hi raf,
thanks for the patch.
Besides busy day job times here, I was actually waiting for the FSF legal
to add your entry into the 'copyright.list' file on the GNU fencepost server.
It's not yet in place though, so no need to hurry.
On 11/18/22 23:48, raf wrote:
* find/find.1 - Fix -prune SCM example directories and make it more efficient
* doc/find.texi - Make -prune SCM example more efficient (two ways)
* NEWS - Mention the above
---
NEWS | 3 +++
doc/find.texi | 25 +
find/find.1 | 13 -
3 files changed, 36 insertions(+), 5 deletions(-)
diff --git a/NEWS b/NEWS
index 1fff34f8..7c6fcfb3 100644
--- a/NEWS
+++ b/NEWS
@@ -18,6 +18,9 @@ GNU findutils NEWS - User visible changes. -*- outline
-*- (allout)
When generating the Texinfo manual, `makeinfo` is invoked with the
--no-split
option for all output formats now; this avoids files like find.info-[12].
+ The find.1 manual's -prune SCM example directories/output have been fixed, and
+ the example itself has been made more efficient (find.1 and find.texi)
[#62259]
+ Also fixed a typo in find.texi.
* Noteworthy changes in release 4.9.0 (2022-02-22) [stable]
diff --git a/doc/find.texi b/doc/find.texi
index 379fe646..1a36e243 100644
--- a/doc/find.texi
+++ b/doc/find.texi
@@ -5138,13 +5138,15 @@ already found.
@smallexample
find repo/ \
--exec test -d @{@}/.svn \; -or \
--exec test -d @{@}/.git \; -or \
--exec test -d @{@}/CVS \; -print -prune
+-type d \
+\( -exec test -d @{@}/.svn \; \
+-or -exec test -d @{@}/.git \; \
+-or -exec test -d @{@}/CVS \; \
+\) -print -prune
@end smallexample
Indeed, the bug omitting the \( ... \) has been introduced in 2008:
$ git diff v4.5.6b-28-gaca33f85^..v4.5.6b-28-gaca33f85 -- doc/find.texi |
tail -n12
@@ -4683,7 +4683,10 @@ searching subdirectories inside projects whose SCM
directory we
already found.
@smallexample
-find repo/ -exec test -d @{@}/.svn -o -d @{@}/.git -o -d @{@}/CVS \; -print
-prune
+find repo/ \
+-exec test -d @{@}/.svn \; -or \
+-exec test -d @{@}/.git \; -or \
+-exec test -d @{@}/CVS \; -print -prune
@end smallexample
In this example, @command{test} is used to tell if we are currently
For find.1, this was already fixed in commit v4.6.0-55-g47d8fd38, but missed
to check the analog place in the texi file.
Your patch above aims at optimizing things by letting 'find -type d' pre-filter
directories in order to avoid -exec invocations on regular and other
non-directory
files.
The '-type d' test is different from 'test -d ...' because the latter
transparently
follows symbolic links while the former strictly checks on the type of the
entry.
Therefore, the patch changes the result in the case the repo workspace is a
symlink:
the new version would skip it. Well, adding -L would help.
In this example, @command{test} is used to tell if we are currently
-examining a directory which appears to the a project's root directory
+examining a directory which appears to be a project's root directory
good catch!
(because it has an SCM subdirectory). When we find a project root,
there is no need to search inside it, and @code{-prune} makes sure
that we descend no further.
@@ -5153,6 +5155,21 @@ For large, complex trees like the Linux kernel, this
will prevent
searching a large portion of the structure, saving a good deal of
time.
+The @samp{-type d} clause causes the three @samp{test} shell
+processes to only be executed for directories. This can be made even
+more efficient by combining the three @samp{test} shell processes
+into a single process:
+
+@smallexample
+find repo/ \
+-type d \
+-exec sh -c 'test -d "$1"/.svn || test -d "$1"/.git || test -d "$1"/CVS' . {}
\; \
+-print -prune
+@end smallexample
+
+Note that the @samp{.} argument is just a placeholder for the unused
+@samp{$0} environment variable in the @samp{sh -c} command. The
+@samp{@{@}} argument is the @samp{$1} environment variable.
___^^
s/environment/shell/, and maybe s/variable/parameter/
@node Security Considerations
@chapter Security Considerations
I'm still unsure about efficiency here. Summary:
1) Originally it was:
$ find repo/ -exec test -d '{}/.svn' -o -d '{}/.git' -o -d '{}/CVS' \; -print
-prune
-> 1 process, doing the OR-ing itself. Nice and neat, and efficient, but maybe
a bit
confusing for the novice reader: it may not be obvious that test(1) is doing
the OR-ing,
because -o is also a find(1) operator.
Besides readability, the test -o operator is discouraged.
$ env '[' --help
...
NOTE: Binary -a and -o are inherently ambiguous. Use 'test EXPR1 && test
EXPR2' or 'test EXPR1 || test EXPR2' instead.
...
2) Now there's the 3x -exec, with fixed -or grouping (with or without '-type
d'):
$ find repo/ -type d \
'(' -exec test -d '{}/.svn' \; -or \
-exec test -d '{}/.git' \; -or \
-
