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

From: Jan Beulich <jbeulich@suse.com>
To: "Roger Pau Monné" <roger.pau@citrix.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: Thu, 11 Mar 2021 17:00:12 +0100	[thread overview]
Message-ID: <bb055af8-f69b-f87a-fc35-4d8b8d3ce1a9@suse.com> (raw)
In-Reply-To: <YEo3VzlTRTaBUkqw@Air-de-Roger>

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?

Hmm, I avoided it because I was under the impression that there
are (compatibility) issues with it, and Ian suggesting xargs
seemed to support that. I'd be more than happy to avoid xargs,
of which I've never been a friend.

>> --- a/tools/include/Makefile
>> +++ b/tools/include/Makefile
>> @@ -19,7 +19,7 @@ xen-foreign:
>>  	$(MAKE) -C xen-foreign
>>  
>>  xen-dir:
>> -	@rm -rf xen acpi
>> +	find xen/ acpi/ -type l 2>/dev/null | xargs rm -f --
> 
> Do we care about leaving an empty xen/libelf directory behind?

Why would we? It'll get created immediately afterwards if it's
not there, and it'll initially be empty (not for long of course).

Jan