The style "test X -o Y" is obsolescent in POSIX (citation: https://pubs.opengroup.org/onlinepubs/9699919799/utilities/test.html). The POSIX standard recommends the use instead of test X || test Y. Which is, in effect, what we are doing in the existing code.
Supposing efficiency is an overriding concern we could use something like this: -exec sh -c 'test -d "$1"/.svn || test -d "$1"/.git || test -d "$1"/CVS' fnord {} \; The fnord there of course is assigned to $0. The above would need careful testing for space handling in particular. On Wed, Oct 26, 2022 at 1:16 AM raf <r...@raf.org> wrote: > On Wed, Oct 26, 2022 at 12:35:34AM +0200, Bernhard Voelker < > m...@bernhard-voelker.de> wrote: > > > First of all: thanks for the patch. > > A concrete code change is always a good basis for discussion. :-) > > > > On 10/23/22 01:54, raf wrote: > > > Subject: [PATCH] find: doc: Fix -prune SCM example and really make it > efficient > > > > IMO we shouldn't call this a "fix", because there was nothing wrong with > the previous sample code. > > See below. > > The term "fix" really only applied to the fact that the > directories labelled as sample output where really the > sample input directories. > > But the use of the word "efficient" was arguably > incorrect when three test processes are being executed > for every file as well as every directory. > > I consider these to both be documentation bugs, even if > the second one isn't a functional bug. > > > > * find/find.1 - Fix explanation of -prune SCM example and make it > efficient > > > * doc/find.texi - Make -prune SCM example efficient > > > --- > > > NEWS | 5 +++++ > > > doc/find.texi | 6 +++--- > > > find/find.1 | 19 ++++++++++++++----- > > > 3 files changed, 22 insertions(+), 8 deletions(-) > > > > > > diff --git a/NEWS b/NEWS > > > index dffca5aa..b3f2074c 100644 > > > --- a/NEWS > > > +++ b/NEWS > > > @@ -13,6 +13,11 @@ GNU findutils NEWS - User visible changes. -*- > outline -*- (allout) > > > findutils now builds again on systems with musl-libc. > > > This requires gettext-0.19.8. > > > +** Documentation Changes > > > + > > > + The find.1 manual and the Texinfo manual -prune SCM example has > > > + been corrected (manual) and made much more efficient (both) [#62259] > > > > again, the previous version was not incorrect. > > > > > + > > > * Noteworthy changes in release 4.9.0 (2022-02-22) [stable] > > > diff --git a/doc/find.texi b/doc/find.texi > > > index 1f295837..8c3972d4 100644 > > > --- a/doc/find.texi > > > +++ b/doc/find.texi > > > @@ -5139,9 +5139,9 @@ 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 -o -d @{@}/.git -o -d @{@}/CVS \; \ > > > + -print -prune > > > @end smallexample > > > > The purpose was more to get an idea about how pruning works rather than > getting the most > > efficient way for the example use case. Of course, we could and should > also give a direction > > about an even more efficient way. > > In that example, this works because the 'test' utility - most probably > coming from coreutils > > or a compatible implementation - allows to use the -o operator to do > more checks in one > > process invocation. > > > > What about guiding the user in the documentation by saying that the > '-exec ... -or -exec ...' > > is the basic way in find to run several checks against the current > entry, and to give a hint > > that in this particular case the `test` utility allows to reduce the > number of execv()s by > > using the -o operator of that tool? > > That sounds to me like too much explication for an > example. And it's not related to the stated purpose of > the example which is to demonstrate prune. The purpose > is not to demonstrate -exec or -or. That's why I > tbought it would be OK to remove the triple -exec test. > It's not relevant to the example. Unless it is, and > it's just not obvious that it's another (unstated) > purpose of the example (which is fine, of course). > > If you prefer the triple test processes, that's fine, > but I really think that at least the -type d should be > added to the example, just so that the triple test > processes are only executed when the candidate entry is > a directory. > > > BTW: `make syntax-check` complains about the new syntax: > > > > $ make syntax-check > > ... > > prohibit_test_minus_ao > > doc/find.texi:5143: -exec test -d @{@}/.svn -o -d @{@}/.git -o -d > @{@}/CVS \; \ > > maint.mk: use "test C1 && test C2", not "test C1 -a C2"; use "test C1 > || test C2", not "test C1 -o C2" > > make: *** [maint.mk:1099: sc_prohibit_test_minus_ao] Error 1 > > That looks to me like a false positive in make syntax-check. > It's not possible to replace the -o with || in this context. > It's assuming that the test command is being parsed by a shell, > when it's actually being parsed by find. > > But if we leave the triple exec in place, this won't matter. > > I am surprised that I didn't spot that. I think I checked > it after changing the manual entry but before changing the > texi. Sorry about that. > > > It seems we have to exempt the texi file from that check. > > > > > In this example, @command{test} is used to tell if we are currently > > > diff --git a/find/find.1 b/find/find.1 > > > index 429aa2f0..f5fa4eee 100644 > > > --- a/find/find.1 > > > +++ b/find/find.1 > > > @@ -2494,15 +2494,14 @@ projects' roots: > > > .in +4m > > > .B $ find repo/ \e > > > .in +4m > > > -.B \e( \-exec test \-d \(aq{}/.svn\(aq \e; \e > > > -.B \-or \-exec test \-d \(aq{}/.git\(aq \e; \e > > > -.B \-or \-exec test \-d \(aq{}/CVS\(aq \e; \e > > > -.B \e) \-print \-prune > > > +.B \-type d \e > > > +.B \-exec test \-d \(aq{}/.svn\(aq \-o \-d \(aq{}/.git\(aq \-o \-d > \(aq{}/CVS\(aq \e; \e > > > +.B \-print \-prune > > > .in -4m > > > .in -4m > > > \& > > > .fi > > > -Sample output: > > > +Sample directories: > > > > ugg, yes, the output does not contain the SCM directories. That was > wrong. Good catch! > > > > > .nf > > > \& > > > .in +4m > > > @@ -2513,6 +2512,16 @@ Sample output: > > > .B repo/project4/.git > > > .in > > > \& > > > +Sample output: > > > +.nf > > > +\& > > > +.in +4m > > > +.B repo/project1 > > > +.B repo/gnu/project2 > > > +.B repo/gnu/project3 > > > +.B repo/project4 > > > +.in > > > +\& > > > .fi > > > In this example, > > > .B \-prune > > > > Would you like to propose a v2? > > Sure. But what should it look like? It sounds like the > following might be OK: > > Changing "Sample output" to "Sample directories". > Adding a "Sample output" paragraph. > Adding -type d to the command. > Leave the triple exec test in place. > > Is that what you have in mind? > > > P.S. You've sent more patches already, and they're becoming more > non-trivial. > > This requires that we have a Copyright assignment of you and your > employer in > > place. I think I mentioned this already last time, but this got out of > my focus. > > Would you like to proceed with the FSF copyright paperwork, please? > > It's just that we can only accept trivial patches without an official > Copyright > > assignment to the FSF - this one would still be okay because of its size, > > but it seems you want to contribute more ... which is great. > > Of course, I can and will assist you in that regard - we're always in > need of > > more official contributors. > > Sure. > > > Thanks & have a nice day, > > Berny > > cheers, > raf > > >