On Thu, Mar 19, 2020 at 10:38:30PM +0100, Christian Weisgerber wrote:
> Make use of "find -exec {} +" (which is POSIX) and "find -delete"
> (which is not) throughout the ports Makefiles.
Thanks, I've been building up a diff for stuff I came across but never
did the full sweep.
> Specifically:
>
> * Replace find|xargs with find -exec {} +
> find|xargs is an outdated construct. The -exec {} + invocation
> automatically avoids a number of corner cases (see xargs -0, -r).
> Also, since the exit status of the left-hand side of a pipe is
> ignored, find|xargs hides some problem.
xargs also executes the command even if find doesn't yield entries,
unless -r is used.
> * Replace -exec {} \; with -exec {} + if applicable.
> Calling a command a few times with many arguments is simply more
> efficient than calling it many times with a single argument.
>
> * Use the -delete operator to remove files and empty directories.
> I'm ambivalent about this since it isn't POSIX, but people seem
> to like it.
All major find implementations have it, its faster and much less error
prone; we've only had it since 2017, though.
> I also combined and tweaked some find(1) invocations while there.
Some invocations omit the `-type' primary entirely or put it *after*
the `-name' primary.
Missing type checks should be easily added to every incovation lacking
them, they're clearer to read and might even speed things up by
preventing direcctory names to be matched against "*.orig" for example.
When both primaries are present, `-type' should occur first for similar
reasons: if you're looking for files with specific names, you don't want
to string compare names first only to discard the file later on because
it is a directory or symbolic link; some invocations do `-name' before
`-type' and I'd argue that swapping their order is both safe to do and
actually a bit faster (for big file trees (with slow I/O)).
> The patch touches 136 files, so it's possible a mistake has slipped
> in, but it successfully passed an amd64 bulk build.
Passing bulks sound good, I skimmed through the diff but couldn't find
issues.
> x11/qt5/Makefile.inc runs find(1) on directories that don't exist
> in all qt5/* ports. Previously the find|xargs construct accidentally
> hid the find errors. I went with -find here, i.e., use make's "-"
> to ignore any non-zero exit status.
Yes, I'd argue hiding errors through the command or showing them but
igoring the exit code within make is practically the same.
OK kn
NB: Feel free to incorporate my feedback and commit together, otherwise
I can simply go over the things I mentioned after your diff landed.
