Re: [PATCH v2 1/2][4.15] tools/x86: don't rebuild cpuid-autogen.h every time

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Fri, Mar 12, 2021 at 09:45:35AM +0100, Jan Beulich wrote:
> On 11.03.2021 16:29, Roger Pau Monné wrote:
> > On Thu, Mar 11, 2021 at 03:40:05PM +0100, Jan Beulich wrote:
> >> The first thing the "xen-dir" rule does is delete the entire xen/
> >> subtree. Obviously this includes deleting xen/lib/x86/*autogen.h. As a
> >> result there's no original version for $(move-if-changed ...) to compare
> >> against, and hence the file and all its consumers would get rebuilt
> >> every time. Instead only find and delete all the symlinks.
> >>
> >> Fixes: eddf9559c977 ("libx86: generate cpuid-autogen.h in the libx86 
> >> include dir")
> >> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> >> ---
> >> v2: Different approach.
> >> ---
> >> Ian did suggest to pass -0r to xargs (and -print0 to find), but I
> >> couldn't convince myself that these are standard compliant options. We
> >> don't use any special characters in file names, so -print0 / -0
> >> shouldn't be necessary at all. The stray rm invocation when there is no
> >> output from find can be taken care of by passing -f to it.
> > 
> > Why not use `-exec rm -f {} +` instead? That seems to be part of
> > POSIX and is likely nicer than piping to xargs?
> 
> I've looked around some more and also remembered an aspect because
> of the looking around.
> 
> 1) xargs has special provisions to not overrun argument space (leaving
> a gap of 2048 bytes). Nothing like this is said in the spec for find,
> and in fact I recall having had issues when the accumulated arguments
> needed splitting. This was on an old distro, but I wanted to mention
> it.

Oh, the POSIX spec does note this:

"The size of any set of two or more pathnames shall be limited such
that execution of the utility does not cause the system's {ARG_MAX}
limit to be exceeded."

I would argue that not doing this is a bug.

> 2) I've found (old) indications that the {} may be troublesome to
> some distros (Solaris was mentioned) and might need quoting. (This
> would of course be easy to deal with.)

Hm, OK. I won't insist then. I'm not sure we should still consider
Solaris as supported anyway. It's likely to explode in a lot of ways
when trying to build.

Do you really need the stderr redirection to /dev/null of find output
when using xargs though? As that will just drop error messages, making
failure diagnostic harder.

Thanks, Roger.