| Message ID | 20140413190809.5822.6755.stgit@fimbulvetr.bsc.es |
|---|---|
| State | New |
| Headers | show |
On 04/13/2014 01:08 PM, Lluís Vilanova wrote: > Use an explicit input file on the command-line instead of reading from standard input > > Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu> > --- Reviewed-by: Eric Blake <eblake@redhat.com>
Lluís Vilanova <vilanova@ac.upc.edu> writes: > Use an explicit input file on the command-line instead of reading from standard input Please limit commit message line length to 70 characters. Worth mentioning that this commit improves error messages! <stdin>:123: Borked becomes qapi-schema.json:123: Borked which enables Emacs to jump to the error. > > Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu> > --- > Makefile | 12 ++++++------ > docs/qapi-code-gen.txt | 4 ++-- > scripts/qapi-commands.py | 9 ++++++--- > scripts/qapi-types.py | 9 ++++++--- > scripts/qapi-visit.py | 9 ++++++--- > scripts/qapi.py | 5 +++-- > tests/Makefile | 11 ++++++----- > tests/qapi-schema/duplicate-key.err | 2 +- > .../qapi-schema/flat-union-invalid-branch-key.err | 2 +- > .../flat-union-invalid-discriminator.err | 2 +- > tests/qapi-schema/flat-union-no-base.err | 2 +- > .../flat-union-string-discriminator.err | 2 +- > tests/qapi-schema/funny-char.err | 2 +- > tests/qapi-schema/missing-colon.err | 2 +- > tests/qapi-schema/missing-comma-list.err | 2 +- > tests/qapi-schema/missing-comma-object.err | 2 +- > tests/qapi-schema/non-objects.err | 2 +- > tests/qapi-schema/quoted-structural-chars.err | 2 +- > tests/qapi-schema/test-qapi.py | 3 ++- > tests/qapi-schema/trailing-comma-list.err | 2 +- > tests/qapi-schema/trailing-comma-object.err | 2 +- > tests/qapi-schema/unclosed-list.err | 2 +- > tests/qapi-schema/unclosed-object.err | 2 +- > tests/qapi-schema/unclosed-string.err | 2 +- > tests/qapi-schema/union-invalid-base.err | 2 +- > 25 files changed, 54 insertions(+), 42 deletions(-) > > diff --git a/Makefile b/Makefile > index 84345ee..ac6a047 100644 > --- a/Makefile > +++ b/Makefile > @@ -238,33 +238,33 @@ qapi-py = $(SRC_PATH)/scripts/qapi.py $(SRC_PATH)/scripts/ordereddict.py > qga/qapi-generated/qga-qapi-types.c qga/qapi-generated/qga-qapi-types.h :\ > $(SRC_PATH)/qga/qapi-schema.json $(SRC_PATH)/scripts/qapi-types.py $(qapi-py) > $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py \ > - $(gen-out-type) -o qga/qapi-generated -p "qga-" < $<, \ > + $(gen-out-type) -o qga/qapi-generated -p "qga-" -i $<, \ > " GEN $@") > qga/qapi-generated/qga-qapi-visit.c qga/qapi-generated/qga-qapi-visit.h :\ > $(SRC_PATH)/qga/qapi-schema.json $(SRC_PATH)/scripts/qapi-visit.py $(qapi-py) > $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py \ > - $(gen-out-type) -o qga/qapi-generated -p "qga-" < $<, \ > + $(gen-out-type) -o qga/qapi-generated -p "qga-" -i $<, \ > " GEN $@") > qga/qapi-generated/qga-qmp-commands.h qga/qapi-generated/qga-qmp-marshal.c :\ > $(SRC_PATH)/qga/qapi-schema.json $(SRC_PATH)/scripts/qapi-commands.py $(qapi-py) > $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py \ > - $(gen-out-type) -o qga/qapi-generated -p "qga-" < $<, \ > + $(gen-out-type) -o qga/qapi-generated -p "qga-" -i $<, \ > " GEN $@") > > qapi-types.c qapi-types.h :\ > $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-types.py $(qapi-py) > $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py \ > - $(gen-out-type) -o "." -b < $<, \ > + $(gen-out-type) -o "." -b -i $<, \ > " GEN $@") > qapi-visit.c qapi-visit.h :\ > $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-visit.py $(qapi-py) > $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py \ > - $(gen-out-type) -o "." -b < $<, \ > + $(gen-out-type) -o "." -b -i $<, \ > " GEN $@") > qmp-commands.h qmp-marshal.c :\ > $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-commands.py $(qapi-py) > $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py \ > - $(gen-out-type) -o "." -m < $<, \ > + $(gen-out-type) -o "." -m -i $<, \ > " GEN $@") > > QGALIB_GEN=$(addprefix qga/qapi-generated/, qga-qapi-types.h qga-qapi-visit.h qga-qmp-commands.h) > diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt > index d78921f..63b03cf 100644 > --- a/docs/qapi-code-gen.txt > +++ b/docs/qapi-code-gen.txt > @@ -221,7 +221,7 @@ created code. > Example: > > mdroth@illuin:~/w/qemu2.git$ python scripts/qapi-types.py \ > - --output-dir="qapi-generated" --prefix="example-" < example-schema.json > + --output-dir="qapi-generated" --prefix="example-" --input-file=example-schema.json > mdroth@illuin:~/w/qemu2.git$ cat qapi-generated/example-qapi-types.c > /* AUTOMATICALLY GENERATED, DO NOT MODIFY */ > > @@ -291,7 +291,7 @@ $(prefix)qapi-visit.h: declarations for previously mentioned visitor > Example: > > mdroth@illuin:~/w/qemu2.git$ python scripts/qapi-visit.py \ > - --output-dir="qapi-generated" --prefix="example-" < example-schema.json > + --output-dir="qapi-generated" --prefix="example-" --input-file=example-schema.json > mdroth@illuin:~/w/qemu2.git$ cat qapi-generated/example-qapi-visit.c > /* THIS FILE IS AUTOMATICALLY GENERATED, DO NOT MODIFY */ > > diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py > index 9734ab0..8d9096f 100644 > --- a/scripts/qapi-commands.py > +++ b/scripts/qapi-commands.py > @@ -369,9 +369,10 @@ def gen_command_def_prologue(prefix="", proxy=False): > > > try: > - opts, args = getopt.gnu_getopt(sys.argv[1:], "chp:o:m", > + opts, args = getopt.gnu_getopt(sys.argv[1:], "chp:i:o:m", > ["source", "header", "prefix=", > - "output-dir=", "type=", "middle"]) > + "input-file=", "output-dir=", > + "type=", "middle"]) > except getopt.GetoptError, err: > print str(err) > sys.exit(1) > @@ -389,6 +390,8 @@ do_h = False > for o, a in opts: > if o in ("-p", "--prefix"): > prefix = a > + elif o in ("-i", "--input-file"): > + input_file = a Here, you don't initialize input_file. Missing -i is "diagnosed" as "NameError: name 'input_file' is not defined". Since this fits right in with the existing, disgusting command line error reporting, I'm not asking you to do better. > elif o in ("-o", "--output-dir"): > output_dir = a + "/" > elif o in ("-t", "--type"): Not your fault: -t is missing in getopt.gnu_getopt()'s second argument above. > @@ -420,7 +423,7 @@ except os.error, e: > if e.errno != errno.EEXIST: > raise > > -exprs = parse_schema(sys.stdin) > +exprs = parse_schema(input_file) > commands = filter(lambda expr: expr.has_key('command'), exprs) > commands = filter(lambda expr: not expr.has_key('gen'), commands) > > diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py > index 10864ef..b463232 100644 > --- a/scripts/qapi-types.py > +++ b/scripts/qapi-types.py > @@ -279,14 +279,15 @@ void qapi_free_%(type)s(%(c_type)s obj) > > > try: > - opts, args = getopt.gnu_getopt(sys.argv[1:], "chbp:o:", > + opts, args = getopt.gnu_getopt(sys.argv[1:], "chbp:i:o:", > ["source", "header", "builtins", > - "prefix=", "output-dir="]) > + "prefix=", "input-file=", "output-dir="]) > except getopt.GetoptError, err: > print str(err) > sys.exit(1) > > output_dir = "" > +input_file = "" > prefix = "" > c_file = 'qapi-types.c' > h_file = 'qapi-types.h' Here, you do initialize input_file. Missing -i is "diagnosed" as "IOError: [Errno 2] No such file or directory: ''". Again, I'm not asking you to do better. The inconsistency annoys me, though. If it still annoys me next time I touch this piece of crap, I'll clean it up. > @@ -298,6 +299,8 @@ do_builtins = False > for o, a in opts: > if o in ("-p", "--prefix"): > prefix = a > + elif o in ("-i", "--input-file"): > + input_file = a > elif o in ("-o", "--output-dir"): > output_dir = a + "/" > elif o in ("-c", "--source"): > @@ -378,7 +381,7 @@ fdecl.write(mcgen(''' > ''', > guard=guardname(h_file))) > > -exprs = parse_schema(sys.stdin) > +exprs = parse_schema(input_file) > exprs = filter(lambda expr: not expr.has_key('gen'), exprs) > > fdecl.write(guardstart("QAPI_TYPES_BUILTIN_STRUCT_DECL")) > diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py > index 45ce3a9..c6579be 100644 > --- a/scripts/qapi-visit.py > +++ b/scripts/qapi-visit.py > @@ -397,13 +397,14 @@ void visit_type_%(name)s(Visitor *m, %(name)s * obj, const char *name, Error **e > name=name) > > try: > - opts, args = getopt.gnu_getopt(sys.argv[1:], "chbp:o:", > + opts, args = getopt.gnu_getopt(sys.argv[1:], "chbp:i:o:", > ["source", "header", "builtins", "prefix=", > - "output-dir="]) > + "input-file=", "output-dir="]) > except getopt.GetoptError, err: > print str(err) > sys.exit(1) > > +input_file = "" > output_dir = "" > prefix = "" > c_file = 'qapi-visit.c' > @@ -416,6 +417,8 @@ do_builtins = False > for o, a in opts: > if o in ("-p", "--prefix"): > prefix = a > + elif o in ("-i", "--input-file"): > + input_file = a > elif o in ("-o", "--output-dir"): > output_dir = a + "/" > elif o in ("-c", "--source"): > @@ -494,7 +497,7 @@ fdecl.write(mcgen(''' > ''', > prefix=prefix, guard=guardname(h_file))) > > -exprs = parse_schema(sys.stdin) > +exprs = parse_schema(input_file) > > # to avoid header dependency hell, we always generate declarations > # for built-in types in our header files and simply guard them > diff --git a/scripts/qapi.py b/scripts/qapi.py > index b474c39..3a38e27 100644 > --- a/scripts/qapi.py > +++ b/scripts/qapi.py > @@ -12,6 +12,7 @@ > # See the COPYING file in the top-level directory. > > from ordereddict import OrderedDict > +import os > import sys > > builtin_types = [ > @@ -263,9 +264,9 @@ def check_exprs(schema): > if expr.has_key('union'): > check_union(expr, expr_elem['info']) > > -def parse_schema(fp): > +def parse_schema(input_path): The parameter is not a path. I'd call it fname, or maybe input_file. > try: > - schema = QAPISchema(fp) > + schema = QAPISchema(open(input_path, "r")) > except QAPISchemaError, e: > print >>sys.stderr, e > exit(1) > diff --git a/tests/Makefile b/tests/Makefile > index 42ed652..6803e7b 100644 > --- a/tests/Makefile > +++ b/tests/Makefile > @@ -217,17 +217,17 @@ tests/test-vmstate$(EXESUF): tests/test-vmstate.o \ > tests/test-qapi-types.c tests/test-qapi-types.h :\ > $(SRC_PATH)/tests/qapi-schema/qapi-schema-test.json $(SRC_PATH)/scripts/qapi-types.py > $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py \ > - $(gen-out-type) -o tests -p "test-" < $<, \ > + $(gen-out-type) -o tests -p "test-" -i $<, \ > " GEN $@") > tests/test-qapi-visit.c tests/test-qapi-visit.h :\ > $(SRC_PATH)/tests/qapi-schema/qapi-schema-test.json $(SRC_PATH)/scripts/qapi-visit.py > $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py \ > - $(gen-out-type) -o tests -p "test-" < $<, \ > + $(gen-out-type) -o tests -p "test-" -i $<, \ > " GEN $@") > tests/test-qmp-commands.h tests/test-qmp-marshal.c :\ > $(SRC_PATH)/tests/qapi-schema/qapi-schema-test.json $(SRC_PATH)/scripts/qapi-commands.py > $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py \ > - $(gen-out-type) -o tests -p "test-" < $<, \ > + $(gen-out-type) -o tests -p "test-" -i $<, \ > " GEN $@") > > tests/test-string-output-visitor$(EXESUF): tests/test-string-output-visitor.o $(test-qapi-obj-y) libqemuutil.a libqemustub.a > @@ -370,13 +370,14 @@ check-tests/test-qapi.py: tests/test-qapi.py > $(patsubst %, check-%, $(check-qapi-schema-y)): check-%.json: $(SRC_PATH)/%.json > $(call quiet-command, PYTHONPATH=$(SRC_PATH)/scripts \ > $(PYTHON) $(SRC_PATH)/tests/qapi-schema/test-qapi.py \ > - <$^ \ > + $^ \ > >$*.test.out \ > 2>$*.test.err; \ > echo $$? >$*.test.exit, \ > " TEST $*.out") > @diff -q $(SRC_PATH)/$*.out $*.test.out > - @diff -q $(SRC_PATH)/$*.err $*.test.err > + @# Sanitize error messages (make them independent of build directory) > + @sed 's|$(SRC_PATH)/||g' $*.test.err | diff -q $(SRC_PATH)/$*.err - > @diff -q $(SRC_PATH)/$*.exit $*.test.exit > Breaks when $(SRC_PATH) interpreted as regexp matches anything but a prefix of the source file part. In particular, it breaks when SRC_PATH is ".." (which is common) and the error message contains "/". Here's a safer version: @perl -p -e 's|^\Q$(SRC_PATH)\E/||' $*.test.err | diff -q $(SRC_PATH)/$*.err - > # Consolidated targets > diff --git a/tests/qapi-schema/duplicate-key.err b/tests/qapi-schema/duplicate-key.err > index 0801c6a..768b276 100644 > --- a/tests/qapi-schema/duplicate-key.err > +++ b/tests/qapi-schema/duplicate-key.err > @@ -1 +1 @@ > -<stdin>:2:10: Duplicate key "key" > +tests/qapi-schema/duplicate-key.json:2:10: Duplicate key "key" > diff --git a/tests/qapi-schema/flat-union-invalid-branch-key.err b/tests/qapi-schema/flat-union-invalid-branch-key.err > index 1125caf..ccf72d2 100644 > --- a/tests/qapi-schema/flat-union-invalid-branch-key.err > +++ b/tests/qapi-schema/flat-union-invalid-branch-key.err > @@ -1 +1 @@ > -<stdin>:13: Discriminator value 'value_wrong' is not found in enum 'TestEnum' > +tests/qapi-schema/flat-union-invalid-branch-key.json:13: Discriminator value 'value_wrong' is not found in enum 'TestEnum' > diff --git a/tests/qapi-schema/flat-union-invalid-discriminator.err b/tests/qapi-schema/flat-union-invalid-discriminator.err > index cad9dbf..790b675 100644 > --- a/tests/qapi-schema/flat-union-invalid-discriminator.err > +++ b/tests/qapi-schema/flat-union-invalid-discriminator.err > @@ -1 +1 @@ > -<stdin>:13: Discriminator 'enum_wrong' is not a member of base type 'TestBase' > +tests/qapi-schema/flat-union-invalid-discriminator.json:13: Discriminator 'enum_wrong' is not a member of base type 'TestBase' > diff --git a/tests/qapi-schema/flat-union-no-base.err b/tests/qapi-schema/flat-union-no-base.err > index e2d7443..a59749e 100644 > --- a/tests/qapi-schema/flat-union-no-base.err > +++ b/tests/qapi-schema/flat-union-no-base.err > @@ -1 +1 @@ > -<stdin>:7: Flat union 'TestUnion' must have a base field > +tests/qapi-schema/flat-union-no-base.json:7: Flat union 'TestUnion' must have a base field > diff --git a/tests/qapi-schema/flat-union-string-discriminator.err b/tests/qapi-schema/flat-union-string-discriminator.err > index 8748270..200016b 100644 > --- a/tests/qapi-schema/flat-union-string-discriminator.err > +++ b/tests/qapi-schema/flat-union-string-discriminator.err > @@ -1 +1 @@ > -<stdin>:13: Discriminator 'kind' must be of enumeration type > +tests/qapi-schema/flat-union-string-discriminator.json:13: Discriminator 'kind' must be of enumeration type > diff --git a/tests/qapi-schema/funny-char.err b/tests/qapi-schema/funny-char.err > index d3dd293..bfc890c 100644 > --- a/tests/qapi-schema/funny-char.err > +++ b/tests/qapi-schema/funny-char.err > @@ -1 +1 @@ > -<stdin>:2:36: Stray ";" > +tests/qapi-schema/funny-char.json:2:36: Stray ";" > diff --git a/tests/qapi-schema/missing-colon.err b/tests/qapi-schema/missing-colon.err > index 9f2a355..d9d66b3 100644 > --- a/tests/qapi-schema/missing-colon.err > +++ b/tests/qapi-schema/missing-colon.err > @@ -1 +1 @@ > -<stdin>:1:10: Expected ":" > +tests/qapi-schema/missing-colon.json:1:10: Expected ":" > diff --git a/tests/qapi-schema/missing-comma-list.err b/tests/qapi-schema/missing-comma-list.err > index 4fe0700..e73d277 100644 > --- a/tests/qapi-schema/missing-comma-list.err > +++ b/tests/qapi-schema/missing-comma-list.err > @@ -1 +1 @@ > -<stdin>:2:20: Expected "," or "]" > +tests/qapi-schema/missing-comma-list.json:2:20: Expected "," or "]" > diff --git a/tests/qapi-schema/missing-comma-object.err b/tests/qapi-schema/missing-comma-object.err > index b0121b5..52b3a8a 100644 > --- a/tests/qapi-schema/missing-comma-object.err > +++ b/tests/qapi-schema/missing-comma-object.err > @@ -1 +1 @@ > -<stdin>:2:3: Expected "," or "}" > +tests/qapi-schema/missing-comma-object.json:2:3: Expected "," or "}" > diff --git a/tests/qapi-schema/non-objects.err b/tests/qapi-schema/non-objects.err > index a6c2dc2..334f0c9 100644 > --- a/tests/qapi-schema/non-objects.err > +++ b/tests/qapi-schema/non-objects.err > @@ -1 +1 @@ > -<stdin>:1:1: Expected "{" > +tests/qapi-schema/non-objects.json:1:1: Expected "{" > diff --git a/tests/qapi-schema/quoted-structural-chars.err b/tests/qapi-schema/quoted-structural-chars.err > index a6c2dc2..9b18384 100644 > --- a/tests/qapi-schema/quoted-structural-chars.err > +++ b/tests/qapi-schema/quoted-structural-chars.err > @@ -1 +1 @@ > -<stdin>:1:1: Expected "{" > +tests/qapi-schema/quoted-structural-chars.json:1:1: Expected "{" > diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py > index 5a26ef3..634ef2d 100644 > --- a/tests/qapi-schema/test-qapi.py > +++ b/tests/qapi-schema/test-qapi.py > @@ -12,10 +12,11 @@ > > from qapi import * > from pprint import pprint > +import os > import sys > > try: > - exprs = parse_schema(sys.stdin) > + exprs = parse_schema(sys.argv[1]) > except SystemExit: > raise > This one reports missing input file name argument as "IndexError: list index out of range". Again, fits right in. [...]
Markus Armbruster writes: > Lluís Vilanova <vilanova@ac.upc.edu> writes: >> Use an explicit input file on the command-line instead of reading from standard input > Please limit commit message line length to 70 characters. > Worth mentioning that this commit improves error messages! > <stdin>:123: Borked > becomes > qapi-schema.json:123: Borked > which enables Emacs to jump to the error. >> >> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu> >> --- >> Makefile | 12 ++++++------ >> docs/qapi-code-gen.txt | 4 ++-- >> scripts/qapi-commands.py | 9 ++++++--- >> scripts/qapi-types.py | 9 ++++++--- >> scripts/qapi-visit.py | 9 ++++++--- >> scripts/qapi.py | 5 +++-- >> tests/Makefile | 11 ++++++----- >> tests/qapi-schema/duplicate-key.err | 2 +- >> .../qapi-schema/flat-union-invalid-branch-key.err | 2 +- >> .../flat-union-invalid-discriminator.err | 2 +- >> tests/qapi-schema/flat-union-no-base.err | 2 +- >> .../flat-union-string-discriminator.err | 2 +- >> tests/qapi-schema/funny-char.err | 2 +- >> tests/qapi-schema/missing-colon.err | 2 +- >> tests/qapi-schema/missing-comma-list.err | 2 +- >> tests/qapi-schema/missing-comma-object.err | 2 +- >> tests/qapi-schema/non-objects.err | 2 +- >> tests/qapi-schema/quoted-structural-chars.err | 2 +- >> tests/qapi-schema/test-qapi.py | 3 ++- >> tests/qapi-schema/trailing-comma-list.err | 2 +- >> tests/qapi-schema/trailing-comma-object.err | 2 +- >> tests/qapi-schema/unclosed-list.err | 2 +- >> tests/qapi-schema/unclosed-object.err | 2 +- >> tests/qapi-schema/unclosed-string.err | 2 +- >> tests/qapi-schema/union-invalid-base.err | 2 +- >> 25 files changed, 54 insertions(+), 42 deletions(-) >> >> diff --git a/Makefile b/Makefile >> index 84345ee..ac6a047 100644 >> --- a/Makefile >> +++ b/Makefile >> @@ -238,33 +238,33 @@ qapi-py = $(SRC_PATH)/scripts/qapi.py $(SRC_PATH)/scripts/ordereddict.py >> qga/qapi-generated/qga-qapi-types.c qga/qapi-generated/qga-qapi-types.h :\ >> $(SRC_PATH)/qga/qapi-schema.json $(SRC_PATH)/scripts/qapi-types.py $(qapi-py) >> $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py \ >> - $(gen-out-type) -o qga/qapi-generated -p "qga-" < $<, \ >> + $(gen-out-type) -o qga/qapi-generated -p "qga-" -i $<, \ >> " GEN $@") >> qga/qapi-generated/qga-qapi-visit.c qga/qapi-generated/qga-qapi-visit.h :\ >> $(SRC_PATH)/qga/qapi-schema.json $(SRC_PATH)/scripts/qapi-visit.py $(qapi-py) >> $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py \ >> - $(gen-out-type) -o qga/qapi-generated -p "qga-" < $<, \ >> + $(gen-out-type) -o qga/qapi-generated -p "qga-" -i $<, \ >> " GEN $@") >> qga/qapi-generated/qga-qmp-commands.h qga/qapi-generated/qga-qmp-marshal.c :\ >> $(SRC_PATH)/qga/qapi-schema.json $(SRC_PATH)/scripts/qapi-commands.py $(qapi-py) >> $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py \ >> - $(gen-out-type) -o qga/qapi-generated -p "qga-" < $<, \ >> + $(gen-out-type) -o qga/qapi-generated -p "qga-" -i $<, \ >> " GEN $@") >> >> qapi-types.c qapi-types.h :\ >> $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-types.py $(qapi-py) >> $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py \ >> - $(gen-out-type) -o "." -b < $<, \ >> + $(gen-out-type) -o "." -b -i $<, \ >> " GEN $@") >> qapi-visit.c qapi-visit.h :\ >> $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-visit.py $(qapi-py) >> $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py \ >> - $(gen-out-type) -o "." -b < $<, \ >> + $(gen-out-type) -o "." -b -i $<, \ >> " GEN $@") >> qmp-commands.h qmp-marshal.c :\ >> $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-commands.py $(qapi-py) >> $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py \ >> - $(gen-out-type) -o "." -m < $<, \ >> + $(gen-out-type) -o "." -m -i $<, \ >> " GEN $@") >> >> QGALIB_GEN=$(addprefix qga/qapi-generated/, qga-qapi-types.h qga-qapi-visit.h qga-qmp-commands.h) >> diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt >> index d78921f..63b03cf 100644 >> --- a/docs/qapi-code-gen.txt >> +++ b/docs/qapi-code-gen.txt >> @@ -221,7 +221,7 @@ created code. >> Example: >> >> mdroth@illuin:~/w/qemu2.git$ python scripts/qapi-types.py \ >> - --output-dir="qapi-generated" --prefix="example-" < example-schema.json >> + --output-dir="qapi-generated" --prefix="example-" --input-file=example-schema.json >> mdroth@illuin:~/w/qemu2.git$ cat qapi-generated/example-qapi-types.c >> /* AUTOMATICALLY GENERATED, DO NOT MODIFY */ >> >> @@ -291,7 +291,7 @@ $(prefix)qapi-visit.h: declarations for previously mentioned visitor >> Example: >> >> mdroth@illuin:~/w/qemu2.git$ python scripts/qapi-visit.py \ >> - --output-dir="qapi-generated" --prefix="example-" < example-schema.json >> + --output-dir="qapi-generated" --prefix="example-" --input-file=example-schema.json >> mdroth@illuin:~/w/qemu2.git$ cat qapi-generated/example-qapi-visit.c >> /* THIS FILE IS AUTOMATICALLY GENERATED, DO NOT MODIFY */ >> >> diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py >> index 9734ab0..8d9096f 100644 >> --- a/scripts/qapi-commands.py >> +++ b/scripts/qapi-commands.py >> @@ -369,9 +369,10 @@ def gen_command_def_prologue(prefix="", proxy=False): >> >> >> try: >> - opts, args = getopt.gnu_getopt(sys.argv[1:], "chp:o:m", >> + opts, args = getopt.gnu_getopt(sys.argv[1:], "chp:i:o:m", >> ["source", "header", "prefix=", >> - "output-dir=", "type=", "middle"]) >> + "input-file=", "output-dir=", >> + "type=", "middle"]) >> except getopt.GetoptError, err: >> print str(err) >> sys.exit(1) >> @@ -389,6 +390,8 @@ do_h = False >> for o, a in opts: >> if o in ("-p", "--prefix"): >> prefix = a >> + elif o in ("-i", "--input-file"): >> + input_file = a > Here, you don't initialize input_file. Missing -i is "diagnosed" as > "NameError: name 'input_file' is not defined". Since this fits right in > with the existing, disgusting command line error reporting, I'm not > asking you to do better. >> elif o in ("-o", "--output-dir"): >> output_dir = a + "/" >> elif o in ("-t", "--type"): > Not your fault: -t is missing in getopt.gnu_getopt()'s second argument > above. >> @@ -420,7 +423,7 @@ except os.error, e: >> if e.errno != errno.EEXIST: >> raise >> >> -exprs = parse_schema(sys.stdin) >> +exprs = parse_schema(input_file) >> commands = filter(lambda expr: expr.has_key('command'), exprs) >> commands = filter(lambda expr: not expr.has_key('gen'), commands) >> >> diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py >> index 10864ef..b463232 100644 >> --- a/scripts/qapi-types.py >> +++ b/scripts/qapi-types.py >> @@ -279,14 +279,15 @@ void qapi_free_%(type)s(%(c_type)s obj) >> >> >> try: >> - opts, args = getopt.gnu_getopt(sys.argv[1:], "chbp:o:", >> + opts, args = getopt.gnu_getopt(sys.argv[1:], "chbp:i:o:", >> ["source", "header", "builtins", >> - "prefix=", "output-dir="]) >> + "prefix=", "input-file=", "output-dir="]) >> except getopt.GetoptError, err: >> print str(err) >> sys.exit(1) >> >> output_dir = "" >> +input_file = "" >> prefix = "" >> c_file = 'qapi-types.c' >> h_file = 'qapi-types.h' > Here, you do initialize input_file. Missing -i is "diagnosed" as > "IOError: [Errno 2] No such file or directory: ''". Again, I'm not > asking you to do better. The inconsistency annoys me, though. If it > still annoys me next time I touch this piece of crap, I'll clean it up. >> @@ -298,6 +299,8 @@ do_builtins = False >> for o, a in opts: >> if o in ("-p", "--prefix"): >> prefix = a >> + elif o in ("-i", "--input-file"): >> + input_file = a >> elif o in ("-o", "--output-dir"): >> output_dir = a + "/" >> elif o in ("-c", "--source"): >> @@ -378,7 +381,7 @@ fdecl.write(mcgen(''' >> ''', >> guard=guardname(h_file))) >> >> -exprs = parse_schema(sys.stdin) >> +exprs = parse_schema(input_file) >> exprs = filter(lambda expr: not expr.has_key('gen'), exprs) >> >> fdecl.write(guardstart("QAPI_TYPES_BUILTIN_STRUCT_DECL")) >> diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py >> index 45ce3a9..c6579be 100644 >> --- a/scripts/qapi-visit.py >> +++ b/scripts/qapi-visit.py >> @@ -397,13 +397,14 @@ void visit_type_%(name)s(Visitor *m, %(name)s * obj, const char *name, Error **e >> name=name) >> >> try: >> - opts, args = getopt.gnu_getopt(sys.argv[1:], "chbp:o:", >> + opts, args = getopt.gnu_getopt(sys.argv[1:], "chbp:i:o:", >> ["source", "header", "builtins", "prefix=", >> - "output-dir="]) >> + "input-file=", "output-dir="]) >> except getopt.GetoptError, err: >> print str(err) >> sys.exit(1) >> >> +input_file = "" >> output_dir = "" >> prefix = "" >> c_file = 'qapi-visit.c' >> @@ -416,6 +417,8 @@ do_builtins = False >> for o, a in opts: >> if o in ("-p", "--prefix"): >> prefix = a >> + elif o in ("-i", "--input-file"): >> + input_file = a >> elif o in ("-o", "--output-dir"): >> output_dir = a + "/" >> elif o in ("-c", "--source"): >> @@ -494,7 +497,7 @@ fdecl.write(mcgen(''' >> ''', >> prefix=prefix, guard=guardname(h_file))) >> >> -exprs = parse_schema(sys.stdin) >> +exprs = parse_schema(input_file) >> >> # to avoid header dependency hell, we always generate declarations >> # for built-in types in our header files and simply guard them >> diff --git a/scripts/qapi.py b/scripts/qapi.py >> index b474c39..3a38e27 100644 >> --- a/scripts/qapi.py >> +++ b/scripts/qapi.py >> @@ -12,6 +12,7 @@ >> # See the COPYING file in the top-level directory. >> >> from ordereddict import OrderedDict >> +import os >> import sys >> >> builtin_types = [ >> @@ -263,9 +264,9 @@ def check_exprs(schema): >> if expr.has_key('union'): >> check_union(expr, expr_elem['info']) >> >> -def parse_schema(fp): >> +def parse_schema(input_path): > The parameter is not a path. I'd call it fname, or maybe input_file. >> try: >> - schema = QAPISchema(fp) >> + schema = QAPISchema(open(input_path, "r")) >> except QAPISchemaError, e: >> print >>sys.stderr, e >> exit(1) >> diff --git a/tests/Makefile b/tests/Makefile >> index 42ed652..6803e7b 100644 >> --- a/tests/Makefile >> +++ b/tests/Makefile >> @@ -217,17 +217,17 @@ tests/test-vmstate$(EXESUF): tests/test-vmstate.o \ >> tests/test-qapi-types.c tests/test-qapi-types.h :\ >> $(SRC_PATH)/tests/qapi-schema/qapi-schema-test.json $(SRC_PATH)/scripts/qapi-types.py >> $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py \ >> - $(gen-out-type) -o tests -p "test-" < $<, \ >> + $(gen-out-type) -o tests -p "test-" -i $<, \ >> " GEN $@") >> tests/test-qapi-visit.c tests/test-qapi-visit.h :\ >> $(SRC_PATH)/tests/qapi-schema/qapi-schema-test.json $(SRC_PATH)/scripts/qapi-visit.py >> $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py \ >> - $(gen-out-type) -o tests -p "test-" < $<, \ >> + $(gen-out-type) -o tests -p "test-" -i $<, \ >> " GEN $@") >> tests/test-qmp-commands.h tests/test-qmp-marshal.c :\ >> $(SRC_PATH)/tests/qapi-schema/qapi-schema-test.json $(SRC_PATH)/scripts/qapi-commands.py >> $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py \ >> - $(gen-out-type) -o tests -p "test-" < $<, \ >> + $(gen-out-type) -o tests -p "test-" -i $<, \ >> " GEN $@") >> >> tests/test-string-output-visitor$(EXESUF): tests/test-string-output-visitor.o $(test-qapi-obj-y) libqemuutil.a libqemustub.a >> @@ -370,13 +370,14 @@ check-tests/test-qapi.py: tests/test-qapi.py >> $(patsubst %, check-%, $(check-qapi-schema-y)): check-%.json: $(SRC_PATH)/%.json >> $(call quiet-command, PYTHONPATH=$(SRC_PATH)/scripts \ >> $(PYTHON) $(SRC_PATH)/tests/qapi-schema/test-qapi.py \ >> - <$^ \ >> + $^ \ >> >$*.test.out \ 2> $*.test.err; \ >> echo $$? >$*.test.exit, \ >> " TEST $*.out") >> @diff -q $(SRC_PATH)/$*.out $*.test.out >> - @diff -q $(SRC_PATH)/$*.err $*.test.err >> + @# Sanitize error messages (make them independent of build directory) >> + @sed 's|$(SRC_PATH)/||g' $*.test.err | diff -q $(SRC_PATH)/$*.err - >> @diff -q $(SRC_PATH)/$*.exit $*.test.exit >> > Breaks when $(SRC_PATH) interpreted as regexp matches anything but a > prefix of the source file part. In particular, it breaks when SRC_PATH > is ".." (which is common) and the error message contains "/". > Here's a safer version: > @perl -p -e 's|^\Q$(SRC_PATH)\E/||' $*.test.err | diff -q $(SRC_PATH)/$*.err - Not a perl fan, so I'll assume your line as correct. Also, can perl be assumed as present? >> # Consolidated targets >> diff --git a/tests/qapi-schema/duplicate-key.err b/tests/qapi-schema/duplicate-key.err >> index 0801c6a..768b276 100644 >> --- a/tests/qapi-schema/duplicate-key.err >> +++ b/tests/qapi-schema/duplicate-key.err >> @@ -1 +1 @@ >> -<stdin>:2:10: Duplicate key "key" >> +tests/qapi-schema/duplicate-key.json:2:10: Duplicate key "key" >> diff --git a/tests/qapi-schema/flat-union-invalid-branch-key.err b/tests/qapi-schema/flat-union-invalid-branch-key.err >> index 1125caf..ccf72d2 100644 >> --- a/tests/qapi-schema/flat-union-invalid-branch-key.err >> +++ b/tests/qapi-schema/flat-union-invalid-branch-key.err >> @@ -1 +1 @@ >> -<stdin>:13: Discriminator value 'value_wrong' is not found in enum 'TestEnum' >> +tests/qapi-schema/flat-union-invalid-branch-key.json:13: Discriminator value 'value_wrong' is not found in enum 'TestEnum' >> diff --git a/tests/qapi-schema/flat-union-invalid-discriminator.err b/tests/qapi-schema/flat-union-invalid-discriminator.err >> index cad9dbf..790b675 100644 >> --- a/tests/qapi-schema/flat-union-invalid-discriminator.err >> +++ b/tests/qapi-schema/flat-union-invalid-discriminator.err >> @@ -1 +1 @@ >> -<stdin>:13: Discriminator 'enum_wrong' is not a member of base type 'TestBase' >> +tests/qapi-schema/flat-union-invalid-discriminator.json:13: Discriminator 'enum_wrong' is not a member of base type 'TestBase' >> diff --git a/tests/qapi-schema/flat-union-no-base.err b/tests/qapi-schema/flat-union-no-base.err >> index e2d7443..a59749e 100644 >> --- a/tests/qapi-schema/flat-union-no-base.err >> +++ b/tests/qapi-schema/flat-union-no-base.err >> @@ -1 +1 @@ >> -<stdin>:7: Flat union 'TestUnion' must have a base field >> +tests/qapi-schema/flat-union-no-base.json:7: Flat union 'TestUnion' must have a base field >> diff --git a/tests/qapi-schema/flat-union-string-discriminator.err b/tests/qapi-schema/flat-union-string-discriminator.err >> index 8748270..200016b 100644 >> --- a/tests/qapi-schema/flat-union-string-discriminator.err >> +++ b/tests/qapi-schema/flat-union-string-discriminator.err >> @@ -1 +1 @@ >> -<stdin>:13: Discriminator 'kind' must be of enumeration type >> +tests/qapi-schema/flat-union-string-discriminator.json:13: Discriminator 'kind' must be of enumeration type >> diff --git a/tests/qapi-schema/funny-char.err b/tests/qapi-schema/funny-char.err >> index d3dd293..bfc890c 100644 >> --- a/tests/qapi-schema/funny-char.err >> +++ b/tests/qapi-schema/funny-char.err >> @@ -1 +1 @@ >> -<stdin>:2:36: Stray ";" >> +tests/qapi-schema/funny-char.json:2:36: Stray ";" >> diff --git a/tests/qapi-schema/missing-colon.err b/tests/qapi-schema/missing-colon.err >> index 9f2a355..d9d66b3 100644 >> --- a/tests/qapi-schema/missing-colon.err >> +++ b/tests/qapi-schema/missing-colon.err >> @@ -1 +1 @@ >> -<stdin>:1:10: Expected ":" >> +tests/qapi-schema/missing-colon.json:1:10: Expected ":" >> diff --git a/tests/qapi-schema/missing-comma-list.err b/tests/qapi-schema/missing-comma-list.err >> index 4fe0700..e73d277 100644 >> --- a/tests/qapi-schema/missing-comma-list.err >> +++ b/tests/qapi-schema/missing-comma-list.err >> @@ -1 +1 @@ >> -<stdin>:2:20: Expected "," or "]" >> +tests/qapi-schema/missing-comma-list.json:2:20: Expected "," or "]" >> diff --git a/tests/qapi-schema/missing-comma-object.err b/tests/qapi-schema/missing-comma-object.err >> index b0121b5..52b3a8a 100644 >> --- a/tests/qapi-schema/missing-comma-object.err >> +++ b/tests/qapi-schema/missing-comma-object.err >> @@ -1 +1 @@ >> -<stdin>:2:3: Expected "," or "}" >> +tests/qapi-schema/missing-comma-object.json:2:3: Expected "," or "}" >> diff --git a/tests/qapi-schema/non-objects.err b/tests/qapi-schema/non-objects.err >> index a6c2dc2..334f0c9 100644 >> --- a/tests/qapi-schema/non-objects.err >> +++ b/tests/qapi-schema/non-objects.err >> @@ -1 +1 @@ >> -<stdin>:1:1: Expected "{" >> +tests/qapi-schema/non-objects.json:1:1: Expected "{" >> diff --git a/tests/qapi-schema/quoted-structural-chars.err b/tests/qapi-schema/quoted-structural-chars.err >> index a6c2dc2..9b18384 100644 >> --- a/tests/qapi-schema/quoted-structural-chars.err >> +++ b/tests/qapi-schema/quoted-structural-chars.err >> @@ -1 +1 @@ >> -<stdin>:1:1: Expected "{" >> +tests/qapi-schema/quoted-structural-chars.json:1:1: Expected "{" >> diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py >> index 5a26ef3..634ef2d 100644 >> --- a/tests/qapi-schema/test-qapi.py >> +++ b/tests/qapi-schema/test-qapi.py >> @@ -12,10 +12,11 @@ >> >> from qapi import * >> from pprint import pprint >> +import os >> import sys >> >> try: >> - exprs = parse_schema(sys.stdin) >> + exprs = parse_schema(sys.argv[1]) >> except SystemExit: >> raise >> > This one reports missing input file name argument as "IndexError: list > index out of range". Again, fits right in. > [...] AFAIR, Eric said that it'd be better to leave it as simple as possible, since no one else uses this script. Same applies to the argument parsing of the other scripts (since no one is sufficiently annoyed to rewrite it with argparse or similar). Lluis
On 04/29/2014 02:20 PM, Lluís Vilanova wrote: > Markus Armbruster writes: > >> Lluís Vilanova <vilanova@ac.upc.edu> writes: >>> Use an explicit input file on the command-line instead of reading from standard input > >> Please limit commit message line length to 70 characters. > >> Worth mentioning that this commit improves error messages! > >> <stdin>:123: Borked > >> becomes > >> qapi-schema.json:123: Borked > >> which enables Emacs to jump to the error. >>> @diff -q $(SRC_PATH)/$*.out $*.test.out >>> - @diff -q $(SRC_PATH)/$*.err $*.test.err >>> + @# Sanitize error messages (make them independent of build directory) >>> + @sed 's|$(SRC_PATH)/||g' $*.test.err | diff -q $(SRC_PATH)/$*.err - >>> @diff -q $(SRC_PATH)/$*.exit $*.test.exit >>> > >> Breaks when $(SRC_PATH) interpreted as regexp matches anything but a >> prefix of the source file part. In particular, it breaks when SRC_PATH >> is ".." (which is common) and the error message contains "/". > >> Here's a safer version: > >> @perl -p -e 's|^\Q$(SRC_PATH)\E/||' $*.test.err | diff -q $(SRC_PATH)/$*.err - I guess there's other ways to avoid the problems without using perl (such as sanitizing all metacharacters before passing a munged $(SRC_PATH) to sed), but it gets hairy. > > Not a perl fan, so I'll assume your line as correct. Also, can perl be assumed > as present? Makefile already uses perl to generate documentation from .pod files; although this appears to be the first use of perl in the testsuite. > >> This one reports missing input file name argument as "IndexError: list >> index out of range". Again, fits right in. > >> [...] > > AFAIR, Eric said that it'd be better to leave it as simple as possible, since no > one else uses this script. > > Same applies to the argument parsing of the other scripts (since no one is > sufficiently annoyed to rewrite it with argparse or similar). Careful - I think that keeping _this patch_ simple is okay, but I would also _welcome_ further patches in this (or other) series that further improves the error message quality and consistency when using the various .py generators from the command line. I think the point we are making here is that your code has very ugly inconsistent results when detecting errors across the various scripts, but that 1) it's no uglier than pre-patch, and 2) in the normal case, 'make' and 'make check' will not trigger these ugly error paths.
Eric Blake <eblake@redhat.com> writes: > On 04/29/2014 02:20 PM, Lluís Vilanova wrote: >> Markus Armbruster writes: >> >>> Lluís Vilanova <vilanova@ac.upc.edu> writes: >>>> Use an explicit input file on the command-line instead of reading >>>> from standard input >> >>> Please limit commit message line length to 70 characters. >> >>> Worth mentioning that this commit improves error messages! >> >>> <stdin>:123: Borked >> >>> becomes >> >>> qapi-schema.json:123: Borked >> >>> which enables Emacs to jump to the error. > >>>> @diff -q $(SRC_PATH)/$*.out $*.test.out >>>> - @diff -q $(SRC_PATH)/$*.err $*.test.err >>>> + @# Sanitize error messages (make them independent of build directory) >>>> + @sed 's|$(SRC_PATH)/||g' $*.test.err | diff -q $(SRC_PATH)/$*.err - >>>> @diff -q $(SRC_PATH)/$*.exit $*.test.exit >>>> >> >>> Breaks when $(SRC_PATH) interpreted as regexp matches anything but a >>> prefix of the source file part. In particular, it breaks when SRC_PATH >>> is ".." (which is common) and the error message contains "/". >> >>> Here's a safer version: >> >>> @perl -p -e 's|^\Q$(SRC_PATH)\E/||' $*.test.err | diff -q >>> $(SRC_PATH)/$*.err - > > I guess there's other ways to avoid the problems without using perl > (such as sanitizing all metacharacters before passing a munged > $(SRC_PATH) to sed), but it gets hairy. Indeed, and that's why I prefer the santizing to be done by the regexp package. Perl does it with escapes (anybody surprised?). Emacs provides a function regexp-quote. sed provides... nothing. Let me decipher the Perl regexp for you: ^ anchors to the beginning of the line \Q$(SRC_PATH)\E matches $(SRC_PATH) exactly (\Q disables pattern metacharacters until \E) / matches the / your script appends to $(SRC_PATH), which it receives as argument of -o Clear as mud? Feel free to adopt something else, just make it work :) >> Not a perl fan, so I'll assume your line as correct. Also, can perl be assumed >> as present? > > Makefile already uses perl to generate documentation from .pod files; > although this appears to be the first use of perl in the testsuite. There's one in tests/qemu-iotests/common.config's _readlink(), but it appears unused. We also have perl scripts checkpatch.pl and get_maintainer.pl. >>> This one reports missing input file name argument as "IndexError: list >>> index out of range". Again, fits right in. >> >>> [...] >> >> AFAIR, Eric said that it'd be better to leave it as simple as >> possible, since no >> one else uses this script. >> >> Same applies to the argument parsing of the other scripts (since no one is >> sufficiently annoyed to rewrite it with argparse or similar). > > Careful - I think that keeping _this patch_ simple is okay, but I would > also _welcome_ further patches in this (or other) series that further > improves the error message quality and consistency when using the > various .py generators from the command line. I think the point we are > making here is that your code has very ugly inconsistent results when > detecting errors across the various scripts, but that 1) it's no uglier > than pre-patch, and 2) in the normal case, 'make' and 'make check' will > not trigger these ugly error paths. Exactly.
diff --git a/Makefile b/Makefile index 84345ee..ac6a047 100644 --- a/Makefile +++ b/Makefile @@ -238,33 +238,33 @@ qapi-py = $(SRC_PATH)/scripts/qapi.py $(SRC_PATH)/scripts/ordereddict.py qga/qapi-generated/qga-qapi-types.c qga/qapi-generated/qga-qapi-types.h :\ $(SRC_PATH)/qga/qapi-schema.json $(SRC_PATH)/scripts/qapi-types.py $(qapi-py) $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py \ - $(gen-out-type) -o qga/qapi-generated -p "qga-" < $<, \ + $(gen-out-type) -o qga/qapi-generated -p "qga-" -i $<, \ " GEN $@") qga/qapi-generated/qga-qapi-visit.c qga/qapi-generated/qga-qapi-visit.h :\ $(SRC_PATH)/qga/qapi-schema.json $(SRC_PATH)/scripts/qapi-visit.py $(qapi-py) $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py \ - $(gen-out-type) -o qga/qapi-generated -p "qga-" < $<, \ + $(gen-out-type) -o qga/qapi-generated -p "qga-" -i $<, \ " GEN $@") qga/qapi-generated/qga-qmp-commands.h qga/qapi-generated/qga-qmp-marshal.c :\ $(SRC_PATH)/qga/qapi-schema.json $(SRC_PATH)/scripts/qapi-commands.py $(qapi-py) $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py \ - $(gen-out-type) -o qga/qapi-generated -p "qga-" < $<, \ + $(gen-out-type) -o qga/qapi-generated -p "qga-" -i $<, \ " GEN $@") qapi-types.c qapi-types.h :\ $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-types.py $(qapi-py) $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py \ - $(gen-out-type) -o "." -b < $<, \ + $(gen-out-type) -o "." -b -i $<, \ " GEN $@") qapi-visit.c qapi-visit.h :\ $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-visit.py $(qapi-py) $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py \ - $(gen-out-type) -o "." -b < $<, \ + $(gen-out-type) -o "." -b -i $<, \ " GEN $@") qmp-commands.h qmp-marshal.c :\ $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-commands.py $(qapi-py) $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py \ - $(gen-out-type) -o "." -m < $<, \ + $(gen-out-type) -o "." -m -i $<, \ " GEN $@") QGALIB_GEN=$(addprefix qga/qapi-generated/, qga-qapi-types.h qga-qapi-visit.h qga-qmp-commands.h) diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt index d78921f..63b03cf 100644 --- a/docs/qapi-code-gen.txt +++ b/docs/qapi-code-gen.txt @@ -221,7 +221,7 @@ created code. Example: mdroth@illuin:~/w/qemu2.git$ python scripts/qapi-types.py \ - --output-dir="qapi-generated" --prefix="example-" < example-schema.json + --output-dir="qapi-generated" --prefix="example-" --input-file=example-schema.json mdroth@illuin:~/w/qemu2.git$ cat qapi-generated/example-qapi-types.c /* AUTOMATICALLY GENERATED, DO NOT MODIFY */ @@ -291,7 +291,7 @@ $(prefix)qapi-visit.h: declarations for previously mentioned visitor Example: mdroth@illuin:~/w/qemu2.git$ python scripts/qapi-visit.py \ - --output-dir="qapi-generated" --prefix="example-" < example-schema.json + --output-dir="qapi-generated" --prefix="example-" --input-file=example-schema.json mdroth@illuin:~/w/qemu2.git$ cat qapi-generated/example-qapi-visit.c /* THIS FILE IS AUTOMATICALLY GENERATED, DO NOT MODIFY */ diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py index 9734ab0..8d9096f 100644 --- a/scripts/qapi-commands.py +++ b/scripts/qapi-commands.py @@ -369,9 +369,10 @@ def gen_command_def_prologue(prefix="", proxy=False): try: - opts, args = getopt.gnu_getopt(sys.argv[1:], "chp:o:m", + opts, args = getopt.gnu_getopt(sys.argv[1:], "chp:i:o:m", ["source", "header", "prefix=", - "output-dir=", "type=", "middle"]) + "input-file=", "output-dir=", + "type=", "middle"]) except getopt.GetoptError, err: print str(err) sys.exit(1) @@ -389,6 +390,8 @@ do_h = False for o, a in opts: if o in ("-p", "--prefix"): prefix = a + elif o in ("-i", "--input-file"): + input_file = a elif o in ("-o", "--output-dir"): output_dir = a + "/" elif o in ("-t", "--type"): @@ -420,7 +423,7 @@ except os.error, e: if e.errno != errno.EEXIST: raise -exprs = parse_schema(sys.stdin) +exprs = parse_schema(input_file) commands = filter(lambda expr: expr.has_key('command'), exprs) commands = filter(lambda expr: not expr.has_key('gen'), commands) diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py index 10864ef..b463232 100644 --- a/scripts/qapi-types.py +++ b/scripts/qapi-types.py @@ -279,14 +279,15 @@ void qapi_free_%(type)s(%(c_type)s obj) try: - opts, args = getopt.gnu_getopt(sys.argv[1:], "chbp:o:", + opts, args = getopt.gnu_getopt(sys.argv[1:], "chbp:i:o:", ["source", "header", "builtins", - "prefix=", "output-dir="]) + "prefix=", "input-file=", "output-dir="]) except getopt.GetoptError, err: print str(err) sys.exit(1) output_dir = "" +input_file = "" prefix = "" c_file = 'qapi-types.c' h_file = 'qapi-types.h' @@ -298,6 +299,8 @@ do_builtins = False for o, a in opts: if o in ("-p", "--prefix"): prefix = a + elif o in ("-i", "--input-file"): + input_file = a elif o in ("-o", "--output-dir"): output_dir = a + "/" elif o in ("-c", "--source"): @@ -378,7 +381,7 @@ fdecl.write(mcgen(''' ''', guard=guardname(h_file))) -exprs = parse_schema(sys.stdin) +exprs = parse_schema(input_file) exprs = filter(lambda expr: not expr.has_key('gen'), exprs) fdecl.write(guardstart("QAPI_TYPES_BUILTIN_STRUCT_DECL")) diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py index 45ce3a9..c6579be 100644 --- a/scripts/qapi-visit.py +++ b/scripts/qapi-visit.py @@ -397,13 +397,14 @@ void visit_type_%(name)s(Visitor *m, %(name)s * obj, const char *name, Error **e name=name) try: - opts, args = getopt.gnu_getopt(sys.argv[1:], "chbp:o:", + opts, args = getopt.gnu_getopt(sys.argv[1:], "chbp:i:o:", ["source", "header", "builtins", "prefix=", - "output-dir="]) + "input-file=", "output-dir="]) except getopt.GetoptError, err: print str(err) sys.exit(1) +input_file = "" output_dir = "" prefix = "" c_file = 'qapi-visit.c' @@ -416,6 +417,8 @@ do_builtins = False for o, a in opts: if o in ("-p", "--prefix"): prefix = a + elif o in ("-i", "--input-file"): + input_file = a elif o in ("-o", "--output-dir"): output_dir = a + "/" elif o in ("-c", "--source"): @@ -494,7 +497,7 @@ fdecl.write(mcgen(''' ''', prefix=prefix, guard=guardname(h_file))) -exprs = parse_schema(sys.stdin) +exprs = parse_schema(input_file) # to avoid header dependency hell, we always generate declarations # for built-in types in our header files and simply guard them diff --git a/scripts/qapi.py b/scripts/qapi.py index b474c39..3a38e27 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -12,6 +12,7 @@ # See the COPYING file in the top-level directory. from ordereddict import OrderedDict +import os import sys builtin_types = [ @@ -263,9 +264,9 @@ def check_exprs(schema): if expr.has_key('union'): check_union(expr, expr_elem['info']) -def parse_schema(fp): +def parse_schema(input_path): try: - schema = QAPISchema(fp) + schema = QAPISchema(open(input_path, "r")) except QAPISchemaError, e: print >>sys.stderr, e exit(1) diff --git a/tests/Makefile b/tests/Makefile index 42ed652..6803e7b 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -217,17 +217,17 @@ tests/test-vmstate$(EXESUF): tests/test-vmstate.o \ tests/test-qapi-types.c tests/test-qapi-types.h :\ $(SRC_PATH)/tests/qapi-schema/qapi-schema-test.json $(SRC_PATH)/scripts/qapi-types.py $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py \ - $(gen-out-type) -o tests -p "test-" < $<, \ + $(gen-out-type) -o tests -p "test-" -i $<, \ " GEN $@") tests/test-qapi-visit.c tests/test-qapi-visit.h :\ $(SRC_PATH)/tests/qapi-schema/qapi-schema-test.json $(SRC_PATH)/scripts/qapi-visit.py $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py \ - $(gen-out-type) -o tests -p "test-" < $<, \ + $(gen-out-type) -o tests -p "test-" -i $<, \ " GEN $@") tests/test-qmp-commands.h tests/test-qmp-marshal.c :\ $(SRC_PATH)/tests/qapi-schema/qapi-schema-test.json $(SRC_PATH)/scripts/qapi-commands.py $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py \ - $(gen-out-type) -o tests -p "test-" < $<, \ + $(gen-out-type) -o tests -p "test-" -i $<, \ " GEN $@") tests/test-string-output-visitor$(EXESUF): tests/test-string-output-visitor.o $(test-qapi-obj-y) libqemuutil.a libqemustub.a @@ -370,13 +370,14 @@ check-tests/test-qapi.py: tests/test-qapi.py $(patsubst %, check-%, $(check-qapi-schema-y)): check-%.json: $(SRC_PATH)/%.json $(call quiet-command, PYTHONPATH=$(SRC_PATH)/scripts \ $(PYTHON) $(SRC_PATH)/tests/qapi-schema/test-qapi.py \ - <$^ \ + $^ \ >$*.test.out \ 2>$*.test.err; \ echo $$? >$*.test.exit, \ " TEST $*.out") @diff -q $(SRC_PATH)/$*.out $*.test.out - @diff -q $(SRC_PATH)/$*.err $*.test.err + @# Sanitize error messages (make them independent of build directory) + @sed 's|$(SRC_PATH)/||g' $*.test.err | diff -q $(SRC_PATH)/$*.err - @diff -q $(SRC_PATH)/$*.exit $*.test.exit # Consolidated targets diff --git a/tests/qapi-schema/duplicate-key.err b/tests/qapi-schema/duplicate-key.err index 0801c6a..768b276 100644 --- a/tests/qapi-schema/duplicate-key.err +++ b/tests/qapi-schema/duplicate-key.err @@ -1 +1 @@ -<stdin>:2:10: Duplicate key "key" +tests/qapi-schema/duplicate-key.json:2:10: Duplicate key "key" diff --git a/tests/qapi-schema/flat-union-invalid-branch-key.err b/tests/qapi-schema/flat-union-invalid-branch-key.err index 1125caf..ccf72d2 100644 --- a/tests/qapi-schema/flat-union-invalid-branch-key.err +++ b/tests/qapi-schema/flat-union-invalid-branch-key.err @@ -1 +1 @@ -<stdin>:13: Discriminator value 'value_wrong' is not found in enum 'TestEnum' +tests/qapi-schema/flat-union-invalid-branch-key.json:13: Discriminator value 'value_wrong' is not found in enum 'TestEnum' diff --git a/tests/qapi-schema/flat-union-invalid-discriminator.err b/tests/qapi-schema/flat-union-invalid-discriminator.err index cad9dbf..790b675 100644 --- a/tests/qapi-schema/flat-union-invalid-discriminator.err +++ b/tests/qapi-schema/flat-union-invalid-discriminator.err @@ -1 +1 @@ -<stdin>:13: Discriminator 'enum_wrong' is not a member of base type 'TestBase' +tests/qapi-schema/flat-union-invalid-discriminator.json:13: Discriminator 'enum_wrong' is not a member of base type 'TestBase' diff --git a/tests/qapi-schema/flat-union-no-base.err b/tests/qapi-schema/flat-union-no-base.err index e2d7443..a59749e 100644 --- a/tests/qapi-schema/flat-union-no-base.err +++ b/tests/qapi-schema/flat-union-no-base.err @@ -1 +1 @@ -<stdin>:7: Flat union 'TestUnion' must have a base field +tests/qapi-schema/flat-union-no-base.json:7: Flat union 'TestUnion' must have a base field diff --git a/tests/qapi-schema/flat-union-string-discriminator.err b/tests/qapi-schema/flat-union-string-discriminator.err index 8748270..200016b 100644 --- a/tests/qapi-schema/flat-union-string-discriminator.err +++ b/tests/qapi-schema/flat-union-string-discriminator.err @@ -1 +1 @@ -<stdin>:13: Discriminator 'kind' must be of enumeration type +tests/qapi-schema/flat-union-string-discriminator.json:13: Discriminator 'kind' must be of enumeration type diff --git a/tests/qapi-schema/funny-char.err b/tests/qapi-schema/funny-char.err index d3dd293..bfc890c 100644 --- a/tests/qapi-schema/funny-char.err +++ b/tests/qapi-schema/funny-char.err @@ -1 +1 @@ -<stdin>:2:36: Stray ";" +tests/qapi-schema/funny-char.json:2:36: Stray ";" diff --git a/tests/qapi-schema/missing-colon.err b/tests/qapi-schema/missing-colon.err index 9f2a355..d9d66b3 100644 --- a/tests/qapi-schema/missing-colon.err +++ b/tests/qapi-schema/missing-colon.err @@ -1 +1 @@ -<stdin>:1:10: Expected ":" +tests/qapi-schema/missing-colon.json:1:10: Expected ":" diff --git a/tests/qapi-schema/missing-comma-list.err b/tests/qapi-schema/missing-comma-list.err index 4fe0700..e73d277 100644 --- a/tests/qapi-schema/missing-comma-list.err +++ b/tests/qapi-schema/missing-comma-list.err @@ -1 +1 @@ -<stdin>:2:20: Expected "," or "]" +tests/qapi-schema/missing-comma-list.json:2:20: Expected "," or "]" diff --git a/tests/qapi-schema/missing-comma-object.err b/tests/qapi-schema/missing-comma-object.err index b0121b5..52b3a8a 100644 --- a/tests/qapi-schema/missing-comma-object.err +++ b/tests/qapi-schema/missing-comma-object.err @@ -1 +1 @@ -<stdin>:2:3: Expected "," or "}" +tests/qapi-schema/missing-comma-object.json:2:3: Expected "," or "}" diff --git a/tests/qapi-schema/non-objects.err b/tests/qapi-schema/non-objects.err index a6c2dc2..334f0c9 100644 --- a/tests/qapi-schema/non-objects.err +++ b/tests/qapi-schema/non-objects.err @@ -1 +1 @@ -<stdin>:1:1: Expected "{" +tests/qapi-schema/non-objects.json:1:1: Expected "{" diff --git a/tests/qapi-schema/quoted-structural-chars.err b/tests/qapi-schema/quoted-structural-chars.err index a6c2dc2..9b18384 100644 --- a/tests/qapi-schema/quoted-structural-chars.err +++ b/tests/qapi-schema/quoted-structural-chars.err @@ -1 +1 @@ -<stdin>:1:1: Expected "{" +tests/qapi-schema/quoted-structural-chars.json:1:1: Expected "{" diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py index 5a26ef3..634ef2d 100644 --- a/tests/qapi-schema/test-qapi.py +++ b/tests/qapi-schema/test-qapi.py @@ -12,10 +12,11 @@ from qapi import * from pprint import pprint +import os import sys try: - exprs = parse_schema(sys.stdin) + exprs = parse_schema(sys.argv[1]) except SystemExit: raise diff --git a/tests/qapi-schema/trailing-comma-list.err b/tests/qapi-schema/trailing-comma-list.err index ff839a3..24c24b0 100644 --- a/tests/qapi-schema/trailing-comma-list.err +++ b/tests/qapi-schema/trailing-comma-list.err @@ -1 +1 @@ -<stdin>:2:36: Expected "{", "[" or string +tests/qapi-schema/trailing-comma-list.json:2:36: Expected "{", "[" or string diff --git a/tests/qapi-schema/trailing-comma-object.err b/tests/qapi-schema/trailing-comma-object.err index f540962..30bce5e 100644 --- a/tests/qapi-schema/trailing-comma-object.err +++ b/tests/qapi-schema/trailing-comma-object.err @@ -1 +1 @@ -<stdin>:2:38: Expected string +tests/qapi-schema/trailing-comma-object.json:2:38: Expected string diff --git a/tests/qapi-schema/unclosed-list.err b/tests/qapi-schema/unclosed-list.err index 0e837a7..fb41a86 100644 --- a/tests/qapi-schema/unclosed-list.err +++ b/tests/qapi-schema/unclosed-list.err @@ -1 +1 @@ -<stdin>:1:20: Expected "," or "]" +tests/qapi-schema/unclosed-list.json:1:20: Expected "," or "]" diff --git a/tests/qapi-schema/unclosed-object.err b/tests/qapi-schema/unclosed-object.err index e6dc950..db3deed 100644 --- a/tests/qapi-schema/unclosed-object.err +++ b/tests/qapi-schema/unclosed-object.err @@ -1 +1 @@ -<stdin>:1:21: Expected "," or "}" +tests/qapi-schema/unclosed-object.json:1:21: Expected "," or "}" diff --git a/tests/qapi-schema/unclosed-string.err b/tests/qapi-schema/unclosed-string.err index 948d883..12b1870 100644 --- a/tests/qapi-schema/unclosed-string.err +++ b/tests/qapi-schema/unclosed-string.err @@ -1 +1 @@ -<stdin>:1:11: Missing terminating "'" +tests/qapi-schema/unclosed-string.json:1:11: Missing terminating "'" diff --git a/tests/qapi-schema/union-invalid-base.err b/tests/qapi-schema/union-invalid-base.err index dd8e3d1..938f969 100644 --- a/tests/qapi-schema/union-invalid-base.err +++ b/tests/qapi-schema/union-invalid-base.err @@ -1 +1 @@ -<stdin>:7: Base 'TestBaseWrong' is not a valid type +tests/qapi-schema/union-invalid-base.json:7: Base 'TestBaseWrong' is not a valid type
Use an explicit input file on the command-line instead of reading from standard input Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu> --- Makefile | 12 ++++++------ docs/qapi-code-gen.txt | 4 ++-- scripts/qapi-commands.py | 9 ++++++--- scripts/qapi-types.py | 9 ++++++--- scripts/qapi-visit.py | 9 ++++++--- scripts/qapi.py | 5 +++-- tests/Makefile | 11 ++++++----- tests/qapi-schema/duplicate-key.err | 2 +- .../qapi-schema/flat-union-invalid-branch-key.err | 2 +- .../flat-union-invalid-discriminator.err | 2 +- tests/qapi-schema/flat-union-no-base.err | 2 +- .../flat-union-string-discriminator.err | 2 +- tests/qapi-schema/funny-char.err | 2 +- tests/qapi-schema/missing-colon.err | 2 +- tests/qapi-schema/missing-comma-list.err | 2 +- tests/qapi-schema/missing-comma-object.err | 2 +- tests/qapi-schema/non-objects.err | 2 +- tests/qapi-schema/quoted-structural-chars.err | 2 +- tests/qapi-schema/test-qapi.py | 3 ++- tests/qapi-schema/trailing-comma-list.err | 2 +- tests/qapi-schema/trailing-comma-object.err | 2 +- tests/qapi-schema/unclosed-list.err | 2 +- tests/qapi-schema/unclosed-object.err | 2 +- tests/qapi-schema/unclosed-string.err | 2 +- tests/qapi-schema/union-invalid-base.err | 2 +- 25 files changed, 54 insertions(+), 42 deletions(-)