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

From: "Roger Pau Monné" <roger.pau@citrix.com>
To: Jan Beulich <jbeulich@suse.com>
Cc: "xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>,
	Ian Jackson <iwj@xenproject.org>,
	Andrew Cooper <andrew.cooper3@citrix.com>, Wei Liu <wl@xen.org>
Subject: Re: [PATCH v2 1/2][4.15] tools/x86: don't rebuild cpuid-autogen.h every time
Date: Fri, 12 Mar 2021 10:17:31 +0100	[thread overview]
Message-ID: <YEsxq0FIHK6Z9pzV@Air-de-Roger> (raw)
In-Reply-To: <c88981fc-b83b-b3c0-f003-59af72e6065b@suse.com>

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@suse.com>
> >> ---
> >> 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.