Re: [Qemu-devel] [PATCH v4 2/3] qapi: Add a primitive to include other files from a QAPI schema file

[Lluís Vilanova]

Markus Armbruster writes:

> Lluís Vilanova <address@hidden> writes:
>> Markus Armbruster writes:
>> 
>>> Lluís Vilanova <address@hidden> writes:
>>>> Adds the "include(...)" primitive to the syntax of QAPI schema files.
>>>> 
>>>> Signed-off-by: Lluís Vilanova <address@hidden>
>>>> ---
>>>> docs/qapi-code-gen.txt |    8 ++++++++
>>>> scripts/qapi.py        |   36 ++++++++++++++++++++++++++++++++++--
>>>> 2 files changed, 42 insertions(+), 2 deletions(-)
>>>> 
>>>> diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
>>>> index 2e9f036..e007807 100644
>>>> --- a/docs/qapi-code-gen.txt
>>>> +++ b/docs/qapi-code-gen.txt
>>>> @@ -40,6 +40,14 @@ enumeration types and union types.
>>>> Generally speaking, types definitions should always use CamelCase
>>>> for the type
>>>> names. Command names should be all lower case with words separated
>>>> by a hyphen.
>>>> 
>>>> +The QAPI schema definitions can be modularized using the 'include'
>>>> directive:
>>>> +
>>>> + include("sub-system/qapi.json")
>> 
>>> And now it isn't JSON anymore.
>> 
>>> To keep it JSON, use syntax like
>> 
>>> { "include": "sub-system/qapi.json" }
>> 
>>> If you absolutely must make it non-JSON, you better rename the .json
>>> files.
>> 
>>> Hmm, we already are non-JSON, because we use ' instead of " for no sane
>>> reason.
>> 
>>> Our JSON parser accepts ' as an extension, to save us quoting in C
>>> strings.  That reason doesn't apply to .json files.
>> 
>> Is it a problem if they are not pure JSON? In the end, they are parsed by
>> qapi.py (which already knows about file syntax), and having a separate syntax
>> for includes makes it somewhat easier to spot when that happens.

> I don't particularly care whether schema syntax is pure JSON, some
> bastardized variation of JSON, or something else entirely.  But as long
> as we advertize schema files it as .json, they better contain JSON.  If
> they contain something else, they should be called something else.

Both are simple enough to implement. Is there any consensus on what's preferred?


>>>> +
>>>> +All paths are interpreted as relative to the initial input file
>>>> passed to the
>>>> +QAPI parsing scripts.
>> 
>>> Really?
>> 
>>> Consider foo.json includes lib/a.json, which wants to include
>>> lib/b.json.
>> 
>>> foo.json:       include("lib/a.json")
>>> lib/a.json:     include("lib/b.json")   # relative to foo.json's directory
>> 
>>> Now throw in bar/bar.json including lib/a.json:
>> 
>>> bar/bar.json:   include("../lib/a.json")
>>> lib/a.json:     include("lib/b.json")   # relative to bar/ -> ENOENT
>> 
>>> Make it relative to the file with the include directive.
>> 
>> Right, sorry. The documentation was not updated after removing the explicit
>> include directory argument. What I'm not sure though is what would be a 
>> better
>> option, having current-file-relative includes, or resuscitating the old 
>> explicit
>> include argument.

> Include relative to the including file is simple.  If we needed
> not-so-simple semantics, I suspect we'd be better off piping through
> cpp.

I tried, but it chokes on the comment syntax.


>>>> +
>>>> +
>>>> === Complex types ===
>>>> 
>>>> A complex type is a dictionary containing a single key whose value is a
>>> [...]
>> 
>>> Are you aware of Wenchao Xia's "[PATCH V8 00/10] qapi script: support
>>> enum as discriminator and better enum name"?  I'm afraid there's a
>>> (semantic?) conflict.  With include files, "[PATCH V8 03/10] qapi
>>> script: remember line number in schema parsing" needs to remember the
>>> source file, too.
>> 
>>> Wenchao's series is likely go in first.  Perhaps you want to base on it
>>> now.
>> 
>> I was not aware of that. Since I'm managing multiple series on a single 
>> branch
>> with stgit (and extracting that is somewhat tedious), I will redo this series
>> once Xia's is merged upstream. Is the merge going to happen anytime soon?

> I reviewed v7, and it looked almost ready to me.  If v8 is ready, it'll
> hopefully get merged fairly quickly (days, not weeks).

Excellent.


Lluis

-- 
 "And it's much the same thing with knowledge, for whenever you learn
 something new, the whole world becomes that much richer."
 -- The Princess of Pure Reason, as told by Norton Juster in The Phantom
 Tollbooth