Re: [Qemu-devel] [PATCH v3 03/49] qapi: add 'if' to top-level expressions

[Markus Armbruster]

Marc-André Lureau <address@hidden> writes:

> On Tue, Jun 19, 2018 at 9:57 AM, Markus Armbruster <address@hidden> wrote:
>> Marc-André Lureau <address@hidden> writes:
>>
>>> Accept 'if' key in top-level elements, accepted as string or list of
>>> string type. The following patches will modify the test visitor to
>>> check the value is correctly saved, and generate #if/#endif code (as a
>>> single #if/endif line or a series for a list).
>>>
>>> Example of 'if' key:
>>> { 'struct': 'TestIfStruct', 'data': { 'foo': 'int' },
>>>   'if': 'defined(TEST_IF_STRUCT)' }
>>>
>>> The generated code is for now *unconditional*. Later patches generate
>>> the conditionals.
>>>
>>> A following patch for qapi-code-gen.txt will provide more complete
>>> documentation for 'if' usage.
>>>
>>> Signed-off-by: Marc-André Lureau <address@hidden>
>>> Reviewed-by: Markus Armbruster <address@hidden>
>> [...]
>>> diff --git a/tests/Makefile.include b/tests/Makefile.include
>>> index 0b277036df..f77ad2ba3b 100644
>>> --- a/tests/Makefile.include
>>> +++ b/tests/Makefile.include
>>> @@ -436,6 +436,10 @@ qapi-schema += args-unknown.json
>>>  qapi-schema += bad-base.json
>>>  qapi-schema += bad-data.json
>>>  qapi-schema += bad-ident.json
>>> +qapi-schema += bad-if.json
>>> +qapi-schema += bad-if-empty.json
>>> +qapi-schema += bad-if-empty-list.json
>>> +qapi-schema += bad-if-list.json
>>>  qapi-schema += bad-type-bool.json
>>>  qapi-schema += bad-type-dict.json
>>>  qapi-schema += bad-type-int.json
>>> @@ -933,6 +937,8 @@ $(patsubst %, check-%, $(check-qapi-schema-y)): 
>>> check-%.json: $(SRC_PATH)/%.json
>>>               "TEST","$*.out")
>>>       @# Sanitize error messages (make them independent of build directory)
>>>       @perl -p -e 's|\Q$(SRC_PATH)\E/||g' $*.test.err | diff -u 
>>> $(SRC_PATH)/$*.err -
>>> +     @if test "$(QAPI_REGENERATE)" == 1 ; then cp $*.test.out 
>>> $(SRC_PATH)/$*.out ; fi
>>> +     @if test "$(QAPI_REGENERATE)" == 1 ; then cp $*.test.err 
>>> $(SRC_PATH)/$*.err ; fi
>>>       @diff -u $(SRC_PATH)/$*.out $*.test.out
>>>       @diff -u $(SRC_PATH)/$*.exit $*.test.exit
>>>
>>
>> Huh?
>
> Oops, leftover. Do you have an alternative? I could propose it as a
> seperate patch if it's generally useful.

I guess what you're trying to do here is updating expected test output.
So far, everybody does that by hand.

A make target to do that (check-accept?) would be nice.  Trivial to do
if we had a single test harness, but of course we have many.

My R-by stands with this hunk dropped.