Lluís Vilanova <vilan...@ac.upc.edu> writes: > The primitive uses JSON syntax, and include paths are relative to the file > using the directive: > > { 'include': 'path/to/file.json' } > > Signed-off-by: Lluís Vilanova <vilan...@ac.upc.edu> > --- > docs/qapi-code-gen.txt | 11 +++++ > scripts/qapi.py | 66 > +++++++++++++++++++++++----- > tests/Makefile | 5 ++ > tests/qapi-schema/include-after-err.err | 1 > tests/qapi-schema/include-after-err.exit | 1 > tests/qapi-schema/include-after-err.json | 2 + > tests/qapi-schema/include-after-err.out | 0 > tests/qapi-schema/include-cycle-b.json | 1 > tests/qapi-schema/include-cycle-c.json | 1 > tests/qapi-schema/include-cycle.err | 3 + > tests/qapi-schema/include-cycle.exit | 1 > tests/qapi-schema/include-cycle.json | 1 > tests/qapi-schema/include-cycle.out | 0 > tests/qapi-schema/include-format-err.err | 1 > tests/qapi-schema/include-format-err.exit | 1 > tests/qapi-schema/include-format-err.json | 2 + > tests/qapi-schema/include-format-err.out | 0 > tests/qapi-schema/include-nested-err.err | 2 + > tests/qapi-schema/include-nested-err.exit | 1 > tests/qapi-schema/include-nested-err.json | 1 > tests/qapi-schema/include-nested-err.out | 0 > tests/qapi-schema/include-no-file.err | 1 > tests/qapi-schema/include-no-file.exit | 1 > tests/qapi-schema/include-no-file.json | 1 > tests/qapi-schema/include-no-file.out | 0 > tests/qapi-schema/include-non-file.err | 1 > tests/qapi-schema/include-non-file.exit | 1 > tests/qapi-schema/include-non-file.json | 1 > tests/qapi-schema/include-non-file.out | 0 > tests/qapi-schema/include-relpath-sub.json | 2 + > tests/qapi-schema/include-relpath.err | 0 > tests/qapi-schema/include-relpath.exit | 1 > tests/qapi-schema/include-relpath.json | 1 > tests/qapi-schema/include-relpath.out | 3 + > tests/qapi-schema/include-self-cycle.err | 1 > tests/qapi-schema/include-self-cycle.exit | 1 > tests/qapi-schema/include-self-cycle.json | 1 > tests/qapi-schema/include-self-cycle.out | 0 > tests/qapi-schema/include-simple-sub.json | 2 + > tests/qapi-schema/include-simple.err | 0 > tests/qapi-schema/include-simple.exit | 1 > tests/qapi-schema/include-simple.json | 1 > tests/qapi-schema/include-simple.out | 3 + > tests/qapi-schema/include/relpath.json | 1 > 44 files changed, 113 insertions(+), 12 deletions(-) > create mode 100644 tests/qapi-schema/include-after-err.err > create mode 100644 tests/qapi-schema/include-after-err.exit > create mode 100644 tests/qapi-schema/include-after-err.json > create mode 100644 tests/qapi-schema/include-after-err.out > create mode 100644 tests/qapi-schema/include-cycle-b.json > create mode 100644 tests/qapi-schema/include-cycle-c.json > create mode 100644 tests/qapi-schema/include-cycle.err > create mode 100644 tests/qapi-schema/include-cycle.exit > create mode 100644 tests/qapi-schema/include-cycle.json > create mode 100644 tests/qapi-schema/include-cycle.out > create mode 100644 tests/qapi-schema/include-format-err.err > create mode 100644 tests/qapi-schema/include-format-err.exit > create mode 100644 tests/qapi-schema/include-format-err.json > create mode 100644 tests/qapi-schema/include-format-err.out > create mode 100644 tests/qapi-schema/include-nested-err.err > create mode 100644 tests/qapi-schema/include-nested-err.exit > create mode 100644 tests/qapi-schema/include-nested-err.json > create mode 100644 tests/qapi-schema/include-nested-err.out > create mode 100644 tests/qapi-schema/include-no-file.err > create mode 100644 tests/qapi-schema/include-no-file.exit > create mode 100644 tests/qapi-schema/include-no-file.json > create mode 100644 tests/qapi-schema/include-no-file.out > create mode 100644 tests/qapi-schema/include-non-file.err > create mode 100644 tests/qapi-schema/include-non-file.exit > create mode 100644 tests/qapi-schema/include-non-file.json > create mode 100644 tests/qapi-schema/include-non-file.out > create mode 100644 tests/qapi-schema/include-relpath-sub.json > create mode 100644 tests/qapi-schema/include-relpath.err > create mode 100644 tests/qapi-schema/include-relpath.exit > create mode 100644 tests/qapi-schema/include-relpath.json > create mode 100644 tests/qapi-schema/include-relpath.out > create mode 100644 tests/qapi-schema/include-self-cycle.err > create mode 100644 tests/qapi-schema/include-self-cycle.exit > create mode 100644 tests/qapi-schema/include-self-cycle.json > create mode 100644 tests/qapi-schema/include-self-cycle.out > create mode 100644 tests/qapi-schema/include-simple-sub.json > create mode 100644 tests/qapi-schema/include-simple.err > create mode 100644 tests/qapi-schema/include-simple.exit > create mode 100644 tests/qapi-schema/include-simple.json > create mode 100644 tests/qapi-schema/include-simple.out > create mode 100644 tests/qapi-schema/include/relpath.json > > diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt > index 63b03cf..051d109 100644 > --- a/docs/qapi-code-gen.txt > +++ b/docs/qapi-code-gen.txt > @@ -40,6 +40,17 @@ 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. > > + > +=== Includes === > + > +The QAPI schema definitions can be modularized using the 'include' directive: > + > + { 'include': 'path/to/file.json'} > + > +The directive is evaluated recursively, and include paths are relative to the > +file using the directive. > + > + > === Complex types === > > A complex type is a dictionary containing a single key whose value is a > diff --git a/scripts/qapi.py b/scripts/qapi.py > index 3a38e27..a013742 100644 > --- a/scripts/qapi.py > +++ b/scripts/qapi.py > @@ -11,6 +11,8 @@ > # This work is licensed under the terms of the GNU GPL, version 2. > # See the COPYING file in the top-level directory. > > +import os > +import re > from ordereddict import OrderedDict > import os > import sys > @@ -36,9 +38,17 @@ builtin_type_qtypes = { > 'uint64': 'QTYPE_QINT', > } > > +def error_path(parent): > + res = "" > + while parent: > + res = ("In file included from %s:%d:\n" % (parent['file'], > + parent['line'])) + res > + parent = parent['parent'] > + return res > + > class QAPISchemaError(Exception): > def __init__(self, schema, msg): > - self.fp = schema.fp > + self.input_file = schema.input_file > self.msg = msg > self.col = 1 > self.line = schema.line > @@ -47,23 +57,32 @@ class QAPISchemaError(Exception): > self.col = (self.col + 7) % 8 + 1 > else: > self.col += 1 > + self.info = schema.parent_info > > def __str__(self): > - return "%s:%s:%s: %s" % (self.fp.name, self.line, self.col, self.msg) > + return error_path(self.info) + \ > + "%s:%d:%d: %s" % (self.input_file, self.line, self.col, self.msg) > > class QAPIExprError(Exception): > def __init__(self, expr_info, msg): > - self.fp = expr_info['fp'] > - self.line = expr_info['line'] > + self.info = expr_info > self.msg = msg > > def __str__(self): > - return "%s:%s: %s" % (self.fp.name, self.line, self.msg) > + return error_path(self.info['parent']) + \ > + "%s:%d: %s" % (self.info['file'], self.info['line'], self.msg) > > class QAPISchema: > > - def __init__(self, fp): > + def __init__(self, fp, input_rel=None, include_hist=[], > parent_info=None): > self.fp = fp > + input_path = os.path.abspath(fp.name)
This is a file name, not a path. I'd call it something like input_fname. > + if input_rel is None: > + input_rel = fp.name input_relname? > + self.input_dir = os.path.dirname(input_path) > + self.input_file = input_rel > + self.include_hist = include_hist + [(input_rel, input_path)] > + self.parent_info = parent_info > self.src = fp.read() > if self.src == '' or self.src[-1] != '\n': > self.src += '\n' > @@ -74,10 +93,35 @@ class QAPISchema: > self.accept() > > while self.tok != None: > - expr_info = {'fp': fp, 'line': self.line} > - expr_elem = {'expr': self.get_expr(False), > - 'info': expr_info} > - self.exprs.append(expr_elem) > + expr_info = {'file': input_rel, 'line': self.line, 'parent': > self.parent_info} > + expr = self.get_expr(False) > + if isinstance(expr, dict) and "include" in expr: > + if len(expr) != 1: > + raise QAPIExprError(expr_info, "Invalid 'include' > directive") > + include = expr["include"] > + if not isinstance(include, str): > + raise QAPIExprError(expr_info, > + 'Expected a file name (string), got: > %s' > + % include) > + include_path = include > + if not os.path.isabs(include_path): > + include_path = os.path.join(self.input_dir, include_path) The conditional isn't needed; os.path.join() does the right thing when an include_path is absolute. > + if not os.path.isfile(include_path): > + raise QAPIExprError( > + expr_info, > + 'Non-existing include file "%s"' % > + include) Unusual indentation. I'd leave detecting "input file doesn't exist" to open, because open needs to diagnose errors anyway: the input file could go away between the above check and open (TOCTTOU), and there are errors other than ENOENT, e.g. EACCES. > + if any(include_path == elem[1] > + for elem in self.include_hist): > + raise QAPIExprError(expr_info, "Inclusion loop for %s" > + % include) > + exprs_include = QAPISchema(open(include_path, "r"), include, > + self.include_hist, expr_info) > + self.exprs.extend(exprs_include.exprs) > + else: > + expr_elem = {'expr': expr, > + 'info': expr_info} > + self.exprs.append(expr_elem) > > def accept(self): > while True: ENOENT is reported nicely: t.json:1: Non-existing include file "nonexistant" Other errors aren't: File "scripts/qapi-types.py", line 384, in <module> exprs = parse_schema(input_file) File "/work/armbru/qemu/scripts/qapi.py", line 313, in parse_schema schema = QAPISchema(open(input_path, "r")) File "/work/armbru/qemu/scripts/qapi.py", line 118, in __init__ exprs_include = QAPISchema(open(include_path, "r"), include, IOError: [Errno 13] Permission denied: '/work/armbru/qemu/nonexistant' > @@ -267,7 +311,7 @@ def check_exprs(schema): > def parse_schema(input_path): > try: > schema = QAPISchema(open(input_path, "r")) > - except QAPISchemaError, e: > + except (QAPISchemaError, QAPIExprError), e: > print >>sys.stderr, e > exit(1) > Another case of cruel error reporting. Not your fault, not your problem to fix now. > diff --git a/tests/Makefile b/tests/Makefile > index 6803e7b..4ee7dc5 100644 > --- a/tests/Makefile > +++ b/tests/Makefile > @@ -164,7 +164,10 @@ check-qapi-schema-y := $(addprefix tests/qapi-schema/, \ > duplicate-key.json union-invalid-base.json flat-union-no-base.json \ > flat-union-invalid-discriminator.json \ > flat-union-invalid-branch-key.json flat-union-reverse-define.json \ > - flat-union-string-discriminator.json) > + flat-union-string-discriminator.json \ > + include-simple.json include-relpath.json include-format-err.json \ > + include-non-file.json include-no-file.json include-after-err.json \ > + include-nested-err.json include-self-cycle.json include-cycle.json) > > GENERATED_HEADERS += tests/test-qapi-types.h tests/test-qapi-visit.h > tests/test-qmp-commands.h > > diff --git a/tests/qapi-schema/include-after-err.err > b/tests/qapi-schema/include-after-err.err > new file mode 100644 > index 0000000..eaf632a > --- /dev/null > +++ b/tests/qapi-schema/include-after-err.err > @@ -0,0 +1 @@ > +tests/qapi-schema/include-after-err.json:2:13: Expected ":" > diff --git a/tests/qapi-schema/include-after-err.exit > b/tests/qapi-schema/include-after-err.exit > new file mode 100644 > index 0000000..d00491f > --- /dev/null > +++ b/tests/qapi-schema/include-after-err.exit > @@ -0,0 +1 @@ > +1 > diff --git a/tests/qapi-schema/include-after-err.json > b/tests/qapi-schema/include-after-err.json > new file mode 100644 > index 0000000..afb6cb6 > --- /dev/null > +++ b/tests/qapi-schema/include-after-err.json > @@ -0,0 +1,2 @@ > +{ 'include': 'include-simple-sub.json' } > +{ 'command' 'missing-colon' } "include after error"? Isn't this "error after include"? > diff --git a/tests/qapi-schema/include-after-err.out > b/tests/qapi-schema/include-after-err.out > new file mode 100644 > index 0000000..e69de29 > diff --git a/tests/qapi-schema/include-cycle-b.json > b/tests/qapi-schema/include-cycle-b.json > new file mode 100644 > index 0000000..4fa985d > --- /dev/null > +++ b/tests/qapi-schema/include-cycle-b.json > @@ -0,0 +1 @@ > +{ 'include': 'include-cycle-c.json' } > diff --git a/tests/qapi-schema/include-cycle-c.json > b/tests/qapi-schema/include-cycle-c.json > new file mode 100644 > index 0000000..d12b592 > --- /dev/null > +++ b/tests/qapi-schema/include-cycle-c.json > @@ -0,0 +1 @@ > +{ 'include': 'include-cycle.json' } > diff --git a/tests/qapi-schema/include-cycle.err > b/tests/qapi-schema/include-cycle.err > new file mode 100644 > index 0000000..602cf62 > --- /dev/null > +++ b/tests/qapi-schema/include-cycle.err > @@ -0,0 +1,3 @@ > +In file included from tests/qapi-schema/include-cycle.json:1: > +In file included from include-cycle-b.json:1: > +include-cycle-c.json:1: Inclusion loop for include-cycle.json > diff --git a/tests/qapi-schema/include-cycle.exit > b/tests/qapi-schema/include-cycle.exit > new file mode 100644 > index 0000000..d00491f > --- /dev/null > +++ b/tests/qapi-schema/include-cycle.exit > @@ -0,0 +1 @@ > +1 > diff --git a/tests/qapi-schema/include-cycle.json > b/tests/qapi-schema/include-cycle.json > new file mode 100644 > index 0000000..6fcf1eb > --- /dev/null > +++ b/tests/qapi-schema/include-cycle.json > @@ -0,0 +1 @@ > +{ 'include': 'include-cycle-b.json' } > diff --git a/tests/qapi-schema/include-cycle.out > b/tests/qapi-schema/include-cycle.out > new file mode 100644 > index 0000000..e69de29 > diff --git a/tests/qapi-schema/include-format-err.err > b/tests/qapi-schema/include-format-err.err > new file mode 100644 > index 0000000..721ff4e > --- /dev/null > +++ b/tests/qapi-schema/include-format-err.err > @@ -0,0 +1 @@ > +tests/qapi-schema/include-format-err.json:1: Invalid 'include' directive > diff --git a/tests/qapi-schema/include-format-err.exit > b/tests/qapi-schema/include-format-err.exit > new file mode 100644 > index 0000000..d00491f > --- /dev/null > +++ b/tests/qapi-schema/include-format-err.exit > @@ -0,0 +1 @@ > +1 > diff --git a/tests/qapi-schema/include-format-err.json > b/tests/qapi-schema/include-format-err.json > new file mode 100644 > index 0000000..44980f0 > --- /dev/null > +++ b/tests/qapi-schema/include-format-err.json > @@ -0,0 +1,2 @@ > +{ 'include': 'include-simple-sub.json', > + 'foo': 'bar' } > diff --git a/tests/qapi-schema/include-format-err.out > b/tests/qapi-schema/include-format-err.out > new file mode 100644 > index 0000000..e69de29 > diff --git a/tests/qapi-schema/include-nested-err.err > b/tests/qapi-schema/include-nested-err.err > new file mode 100644 > index 0000000..1dacbda > --- /dev/null > +++ b/tests/qapi-schema/include-nested-err.err > @@ -0,0 +1,2 @@ > +In file included from tests/qapi-schema/include-nested-err.json:1: > +missing-colon.json:1:10: Expected ":" > diff --git a/tests/qapi-schema/include-nested-err.exit > b/tests/qapi-schema/include-nested-err.exit > new file mode 100644 > index 0000000..d00491f > --- /dev/null > +++ b/tests/qapi-schema/include-nested-err.exit > @@ -0,0 +1 @@ > +1 > diff --git a/tests/qapi-schema/include-nested-err.json > b/tests/qapi-schema/include-nested-err.json > new file mode 100644 > index 0000000..5631e56 > --- /dev/null > +++ b/tests/qapi-schema/include-nested-err.json > @@ -0,0 +1 @@ > +{ 'include': 'missing-colon.json' } > diff --git a/tests/qapi-schema/include-nested-err.out > b/tests/qapi-schema/include-nested-err.out > new file mode 100644 > index 0000000..e69de29 > diff --git a/tests/qapi-schema/include-no-file.err > b/tests/qapi-schema/include-no-file.err > new file mode 100644 > index 0000000..526ab9d > --- /dev/null > +++ b/tests/qapi-schema/include-no-file.err > @@ -0,0 +1 @@ > +tests/qapi-schema/include-no-file.json:1: Non-existing include file > "include-no-file-sub.json" > diff --git a/tests/qapi-schema/include-no-file.exit > b/tests/qapi-schema/include-no-file.exit > new file mode 100644 > index 0000000..d00491f > --- /dev/null > +++ b/tests/qapi-schema/include-no-file.exit > @@ -0,0 +1 @@ > +1 > diff --git a/tests/qapi-schema/include-no-file.json > b/tests/qapi-schema/include-no-file.json > new file mode 100644 > index 0000000..9249ebd > --- /dev/null > +++ b/tests/qapi-schema/include-no-file.json > @@ -0,0 +1 @@ > +{ 'include': 'include-no-file-sub.json' } > diff --git a/tests/qapi-schema/include-no-file.out > b/tests/qapi-schema/include-no-file.out > new file mode 100644 > index 0000000..e69de29 > diff --git a/tests/qapi-schema/include-non-file.err > b/tests/qapi-schema/include-non-file.err > new file mode 100644 > index 0000000..9658c78 > --- /dev/null > +++ b/tests/qapi-schema/include-non-file.err > @@ -0,0 +1 @@ > +tests/qapi-schema/include-non-file.json:1: Expected a file name (string), > got: ['foo', 'bar'] > diff --git a/tests/qapi-schema/include-non-file.exit > b/tests/qapi-schema/include-non-file.exit > new file mode 100644 > index 0000000..d00491f > --- /dev/null > +++ b/tests/qapi-schema/include-non-file.exit > @@ -0,0 +1 @@ > +1 > diff --git a/tests/qapi-schema/include-non-file.json > b/tests/qapi-schema/include-non-file.json > new file mode 100644 > index 0000000..cd43c3f > --- /dev/null > +++ b/tests/qapi-schema/include-non-file.json > @@ -0,0 +1 @@ > +{ 'include': [ 'foo', 'bar' ] } > diff --git a/tests/qapi-schema/include-non-file.out > b/tests/qapi-schema/include-non-file.out > new file mode 100644 > index 0000000..e69de29 > diff --git a/tests/qapi-schema/include-relpath-sub.json > b/tests/qapi-schema/include-relpath-sub.json > new file mode 100644 > index 0000000..4bd4af4 > --- /dev/null > +++ b/tests/qapi-schema/include-relpath-sub.json > @@ -0,0 +1,2 @@ > +{ 'enum': 'Status', > + 'data': [ 'good', 'bad', 'ugly' ] } > diff --git a/tests/qapi-schema/include-relpath.err > b/tests/qapi-schema/include-relpath.err > new file mode 100644 > index 0000000..e69de29 > diff --git a/tests/qapi-schema/include-relpath.exit > b/tests/qapi-schema/include-relpath.exit > new file mode 100644 > index 0000000..573541a > --- /dev/null > +++ b/tests/qapi-schema/include-relpath.exit > @@ -0,0 +1 @@ > +0 > diff --git a/tests/qapi-schema/include-relpath.json > b/tests/qapi-schema/include-relpath.json > new file mode 100644 > index 0000000..05018f3 > --- /dev/null > +++ b/tests/qapi-schema/include-relpath.json > @@ -0,0 +1 @@ > +{ 'include': 'include/relpath.json' } > diff --git a/tests/qapi-schema/include-relpath.out > b/tests/qapi-schema/include-relpath.out > new file mode 100644 > index 0000000..4ce3dcf > --- /dev/null > +++ b/tests/qapi-schema/include-relpath.out > @@ -0,0 +1,3 @@ > +[OrderedDict([('enum', 'Status'), ('data', ['good', 'bad', 'ugly'])])] > +[{'enum_name': 'Status', 'enum_values': ['good', 'bad', 'ugly']}] > +[] > diff --git a/tests/qapi-schema/include-self-cycle.err > b/tests/qapi-schema/include-self-cycle.err > new file mode 100644 > index 0000000..981742a > --- /dev/null > +++ b/tests/qapi-schema/include-self-cycle.err > @@ -0,0 +1 @@ > +tests/qapi-schema/include-self-cycle.json:1: Inclusion loop for > include-self-cycle.json > diff --git a/tests/qapi-schema/include-self-cycle.exit > b/tests/qapi-schema/include-self-cycle.exit > new file mode 100644 > index 0000000..d00491f > --- /dev/null > +++ b/tests/qapi-schema/include-self-cycle.exit > @@ -0,0 +1 @@ > +1 > diff --git a/tests/qapi-schema/include-self-cycle.json > b/tests/qapi-schema/include-self-cycle.json > new file mode 100644 > index 0000000..55fb1b5 > --- /dev/null > +++ b/tests/qapi-schema/include-self-cycle.json > @@ -0,0 +1 @@ > +{ 'include': 'include-self-cycle.json' } direct-cycle? > diff --git a/tests/qapi-schema/include-self-cycle.out > b/tests/qapi-schema/include-self-cycle.out > new file mode 100644 > index 0000000..e69de29 > diff --git a/tests/qapi-schema/include-simple-sub.json > b/tests/qapi-schema/include-simple-sub.json > new file mode 100644 > index 0000000..4bd4af4 > --- /dev/null > +++ b/tests/qapi-schema/include-simple-sub.json > @@ -0,0 +1,2 @@ > +{ 'enum': 'Status', > + 'data': [ 'good', 'bad', 'ugly' ] } > diff --git a/tests/qapi-schema/include-simple.err > b/tests/qapi-schema/include-simple.err > new file mode 100644 > index 0000000..e69de29 > diff --git a/tests/qapi-schema/include-simple.exit > b/tests/qapi-schema/include-simple.exit > new file mode 100644 > index 0000000..573541a > --- /dev/null > +++ b/tests/qapi-schema/include-simple.exit > @@ -0,0 +1 @@ > +0 > diff --git a/tests/qapi-schema/include-simple.json > b/tests/qapi-schema/include-simple.json > new file mode 100644 > index 0000000..1dd391a > --- /dev/null > +++ b/tests/qapi-schema/include-simple.json > @@ -0,0 +1 @@ > +{ 'include': 'include-simple-sub.json' } > diff --git a/tests/qapi-schema/include-simple.out > b/tests/qapi-schema/include-simple.out > new file mode 100644 > index 0000000..4ce3dcf > --- /dev/null > +++ b/tests/qapi-schema/include-simple.out > @@ -0,0 +1,3 @@ > +[OrderedDict([('enum', 'Status'), ('data', ['good', 'bad', 'ugly'])])] > +[{'enum_name': 'Status', 'enum_values': ['good', 'bad', 'ugly']}] > +[] > diff --git a/tests/qapi-schema/include/relpath.json > b/tests/qapi-schema/include/relpath.json > new file mode 100644 > index 0000000..45dee24 > --- /dev/null > +++ b/tests/qapi-schema/include/relpath.json > @@ -0,0 +1 @@ > +{ 'include': '../include-relpath-sub.json' } No serious problems :)