Re: [Qemu-devel] [PATCH v5 01/46] qapi: Sort qapi-schema tests

[Eric Blake]

On 09/23/2015 09:09 AM, Markus Armbruster wrote:
> Eric Blake <address@hidden> writes:
> 
>> On 09/21/2015 03:57 PM, Eric Blake wrote:
>>> Recent changes to qapi have provided quite a bit of churn in
>>> the makefile, because we are inconsistent on what order test
>>> names appear in, and on whether to re-wrap the list of tests or
>>> just add arbitrary line lengths.  Writing the list in a sorted
>>> fashion, one test per line, will make future patches easier
>>> to see what tests are being added or removed by a patch.
>>>
>>> Signed-off-by: Eric Blake <address@hidden>
>>> ---
>>>  tests/Makefile | 160 
>>> ++++++++++++++++++++++++++++++++++++++++-----------------
>>>  1 file changed, 114 insertions(+), 46 deletions(-)
>>>
>>
>>> +qapi-schema += alternate-array.json
>>> +qapi-schema += alternate-base.json
>>
>> Hmm, I just realized we require GNU make, and that we already use
>> $(wildcard) when building up other tests.  Would it be worth writing
>> this patch to merely use $(wildcard qapi-tests/*.json)?  Then further
>> additions (and removals) of .json files would automatically be picked up
>> without requiring Makefile tweaking.
> 
> I really dislike picking up source files with $(wildcard), because it
> can also pick up random junk.

I agree that it is dangerous (the automake manual specifically
recommends against wildcarding for this reason, even when done without
relying on GNU $(wildcard) syntax).  It was more a question of "since we
are already doing it, should we do it more?"

Looking closer, the existing uses of $(wildcard) in tests/Makefile are
for including .d files (those are generated on the fly, and while still
prone to accidentally including leftover garbage files, such extra
inclusions tend to have no negative impact to make dependency tracking
for the targets we still care about), and for the SYSEMU_TARGET_LIST
(again something that relies on the just-generated default-configs/*.mak
magic).  Whereas choosing which tests to run does cause problems if
bogus tests are added.

> 
> Something like $(shell git ls-files tests/qapi-schema/*.json) avoids
> random junk, but doesn't work when you build a tarball.

Then it sounds like my approach of keeping a verbose list is still best
after all, and at most I should update the commit message to say _why_ I
specifically chose not to use $(wildcard) here.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature