Re: [Qemu-devel] [PATCH] Makefile: simplify qapi-py definition with wildcard

[Eric Blake]

On 5/21/19 7:53 PM, Wei Yang wrote:
> On Tue, May 21, 2019 at 05:28:27PM +0200, Markus Armbruster wrote:
>> Eric Blake <address@hidden> writes:
>>
>>> On 5/21/19 3:12 AM, Wei Yang wrote:
>>>> All the python script in scripts/qapi is used to generate qapi code. Use
>>>> wildcard to simplify it.
>>>>
>>>> Signed-off-by: Wei Yang <address@hidden>
>>>> ---
>>>>  Makefile | 8 +-------
>>>>  1 file changed, 1 insertion(+), 7 deletions(-)
>>>
>>> I'm not a fan of $(wildcard). It makes your tarball creation
>>> non-deterministic - if there is a leftover file from development that is
>>> no longer part of the build, wildcard will still pick it up.  Explicit
>>
>> Actually, in this case can "only" adds spurious prerequisites.
> 
> Hmm... which spurious prerequisites will be added? I may miss something here.

If I touch the file scripts/qapi/foo.py, then that becomes a
prerequisite, and will rebuild my files even though it doesn't need to.
And if the definition of $(qapi-py) is ever applied to determining which
files to include in a tarball, then my tarball will include foo.py, even
though it is not essential to the tarball, while the next person, who
does not have foo.py lying around in their directory, produces a
different tarball.  Non-deterministic results are bad, hence our rule of
thumb to avoid $(wildcard) when listing files in a Makefile.

> 
>>
>>> lists are better.  I'm inclined to NACK this, but Markus has final say
>>> since he maintains the qapi generator.
>>
>> I consider use of $(wildcard) for the purpose of collecting sources a
>> lazy mistake.

Which is another polite way of saying that this patch should not be applied.

If you WANT to factor things for less typing, you could use something like:

var=commands.py events.py introspect.py types.py visit.py common.py doc.py
$(addprefix $(SRC_PATH)/scripts/qapi/,$(var))

for some suitably-named var.  Or even factor out the .py suffix as well.
That doesn't use the problematic $(wildcard), but still lets you get
away with easier maintenance of adding a new file to the explicit list
should we add a file in the future.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

signature.asc
Description: OpenPGP digital signature