Re: [PATCH] find: doc: Fix -prune SCM example and really make it efficient

[raf]

On Wed, Oct 26, 2022 at 03:20:09PM +0200, Bernhard Voelker 
<mail@bernhard-voelker.de> wrote:

> On 10/26/22 11:33, James Youngman wrote:
> > The style "test X -o Y" is obsolescent in POSIX (citation:
> > https://pubs.opengroup.org/onlinepubs/9699919799/utilities/test.html
> > <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:

Efficiency is only a concern because the example is
described as being efficient, but the reason for that
is its use of -prune. Three sh processes per directory
would be fine, but it's currently doing three sh
processes for every file type, not just directories.
That seems to me to just be too wasteful. That's why I
want to insert "-type d" before the -exec. I'm not that
concerned with combining the three -execs into one. It
just seemed like a good idea at the time.

> > -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.

The quoted environment variables do protect against
shell meta-characters like spaces and quotes in names.

I think this would have been nicer:

  -exec sh -c 'test -d {}/.svn || test -d {}/.git || test -d {}/CVS' \;

BUT: I just tested this with a directory name
containing a space, and {} doesn't automatically
backslash-quote shell meta-characters when it's used as
part of a larger command line argument that might
result in multiple shell tokens. That's a surprise to
me. I'd consider it a bug, but changing it might not be
backwards-compatible (but it might be). Quoting the {}
isn't enough. Shells won't treat it the same as quoted
environment variables.

This also works and is shorter:

  -exec sh -c 'test -d "$0"/.svn || test -d "$0"/.git || test -d "$0"/CVS' {} \;

Would that be OK? Or is it too wierd to use $0 that way?

> Thanks for the reminder - I had forgotten about that.
> Personally I never use `test X -o Y` ... because of the ambiguity mentioned 
> there.
> 
> Still, I think the find manual should first use the triple -exec example as 
> today,
> and then mention that this special case can be tuned by using the above 
> '-exec sh -c ...'.
> 
> I believe documenting both ways is good because the tests for certain child 
> names is
> just a special case which can be done by the test utility - while other 
> real-life
> examples would need to perform other checks.
> 
> Have a nice day,
> Berny

OK. I'll submit another version of the patch that does that.

But which version of the single -exec command would be best:

  -exec sh -c 'test -d "$1"/.svn || test -d "$1"/.git || test -d "$1"/CVS' 
fnord {} \;
  -exec sh -c '[ -d "$1"/.svn ] || [ -d "$1"/.git ] || [ -d "$1"/CVS ]' fnord 
{} \;

  -exec sh -c 'test -d "$0"/.svn || test -d "$0"/.git || test -d "$0"/CVS' {} \;
  -exec sh -c '[ -d "$0"/.svn ] || [ -d "$0"/.git ] || [ -d "$0"/CVS ]' {} \;

I don't like including "fnord", because I feel many
readers won't immediately understand why it's there,
and so it should be accompanied by an explanation of
why it's there, but including such an explanation is
not desirable because it has nothing to do with the
point of the example which is to demonstrate -prune
as an efficiency measure, not to demonstrate sh -c.
But I'll use whichever version you prefer.

cheers,
raf