| Message ID | 20180111213250.16511-43-marcandre.lureau@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | Hi, | expand |
Hi On Thu, Jan 11, 2018 at 10:32 PM, Marc-André Lureau <marcandre.lureau@redhat.com> wrote: > Add a pragma that allows to tag the following expressions in the > schema with a unit name. By default, an expression has no unit name. > > See the docs/devel/qapi-code-gen.txt for more details. > I inadvertently merged the following patch "qapi: add a -u/--unit option to specify which unit to visit" with this one. Fixed in the github branch: https://github.com/elmarco/qemu/commits/qapi-if > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> > --- > scripts/qapi.py | 22 +++++++++++++++++++--- > docs/devel/qapi-code-gen.txt | 4 ++++ > tests/Makefile.include | 7 ++++++- > tests/qapi-schema/pragma-unit-invalid.err | 1 + > tests/qapi-schema/pragma-unit-invalid.exit | 1 + > tests/qapi-schema/pragma-unit-invalid.json | 3 +++ > tests/qapi-schema/pragma-unit-invalid.out | 0 > tests/qapi-schema/qapi-schema-test.json | 6 +++++- > tests/qapi-schema/qapi-schema-test.out | 2 ++ > 9 files changed, 41 insertions(+), 5 deletions(-) > create mode 100644 tests/qapi-schema/pragma-unit-invalid.err > create mode 100644 tests/qapi-schema/pragma-unit-invalid.exit > create mode 100644 tests/qapi-schema/pragma-unit-invalid.json > create mode 100644 tests/qapi-schema/pragma-unit-invalid.out > > diff --git a/scripts/qapi.py b/scripts/qapi.py > index f56460d028..07e738c3f1 100644 > --- a/scripts/qapi.py > +++ b/scripts/qapi.py > @@ -47,6 +47,9 @@ returns_whitelist = [] > # Whitelist of entities allowed to violate case conventions > name_case_whitelist = [] > > +# Unit names to include for the visit (default to all units) > +visit_units = [] > + > enum_types = {} > struct_types = {} > union_types = {} > @@ -269,11 +272,12 @@ class QAPISchemaParser(object): > self.exprs = [] > self.docs = [] > self.accept() > + self.unit = None > cur_doc = None > > while self.tok is not None: > info = {'file': self.fname, 'line': self.line, > - 'parent': self.incl_info} > + 'parent': self.incl_info, 'unit': self.unit} > if self.tok == '#': > self.reject_expr_doc(cur_doc) > cur_doc = self.get_doc(info) > @@ -362,6 +366,11 @@ class QAPISchemaParser(object): > "Pragma name-case-whitelist must be" > " a list of strings") > name_case_whitelist = value > + elif name == 'unit': > + if not isinstance(value, str): > + raise QAPISemError(info, > + "Pragma 'unit' must be string") > + self.unit = value > else: > raise QAPISemError(info, "Unknown pragma '%s'" % name) > > @@ -1827,6 +1836,10 @@ class QAPISchema(object): > def visit(self, visitor): > visitor.visit_begin(self) > for (name, entity) in sorted(self._entity_dict.items()): > + # FIXME: implicit array types should use element type unit > + unit = entity.info and entity.info.get('unit') > + if visit_units and unit not in visit_units: > + continue > if visitor.visit_needed(entity): > entity.visit(visitor) > visitor.visit_end() > @@ -2126,13 +2139,14 @@ def parse_command_line(extra_options='', extra_long_options=[]): > > try: > opts, args = getopt.gnu_getopt( > - sys.argv[1:], 'chp:o:i:' + extra_options, > + sys.argv[1:], 'chp:o:u:i:' + extra_options, > ['source', 'header', 'prefix=', 'output-dir=', > - 'include='] + extra_long_options) > + 'unit=', 'include='] + extra_long_options) > except getopt.GetoptError as err: > print >>sys.stderr, "%s: %s" % (sys.argv[0], str(err)) > sys.exit(1) > > + global visit_units > output_dir = '' > prefix = '' > do_c = False > @@ -2152,6 +2166,8 @@ def parse_command_line(extra_options='', extra_long_options=[]): > prefix = a > elif o in ('-o', '--output-dir'): > output_dir = a + '/' > + elif o in ('-u', '--unit'): > + visit_units.append(a) > elif o in ('-c', '--source'): > do_c = True > elif o in ('-h', '--header'): > diff --git a/docs/devel/qapi-code-gen.txt b/docs/devel/qapi-code-gen.txt > index 479b755609..31bcbdac58 100644 > --- a/docs/devel/qapi-code-gen.txt > +++ b/docs/devel/qapi-code-gen.txt > @@ -326,6 +326,10 @@ violate the rules on permitted return types. Default is none. > Pragma 'name-case-whitelist' takes a list of names that may violate > rules on use of upper- vs. lower-case letters. Default is none. > > +Pragma 'unit' takes a string value. It will set the unit name for the > +following expressions in the schema. Most code generators can filter > +based on a unit name. This allows to select a subset of the > +expressions. Default is none. > > === Struct types === > > diff --git a/tests/Makefile.include b/tests/Makefile.include > index 2b83f05954..2f39e2d5aa 100644 > --- a/tests/Makefile.include > +++ b/tests/Makefile.include > @@ -510,6 +510,7 @@ qapi-schema += pragma-extra-junk.json > qapi-schema += pragma-name-case-whitelist-crap.json > qapi-schema += pragma-non-dict.json > qapi-schema += pragma-returns-whitelist-crap.json > +qapi-schema += pragma-unit-invalid.json > qapi-schema += qapi-schema-test.json > qapi-schema += quoted-structural-chars.json > qapi-schema += redefined-builtin.json > @@ -665,13 +666,17 @@ $(SRC_PATH)/tests/qapi-schema/qapi-schema-test.json $(SRC_PATH)/scripts/qapi-com > tests/test-qapi-event.c tests/test-qapi-event.h :\ > $(SRC_PATH)/tests/qapi-schema/qapi-schema-test.json $(SRC_PATH)/scripts/qapi-event.py $(qapi-py) > $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-event.py \ > - $(gen-out-type) -o tests -p "test-" $<, \ > + $(gen-out-type) -o tests -p "test-" -u test-unit $<, \ > "GEN","$@") > +# check that another-unit is not included > + $(call quiet-command,[ $(gen-out-type) == -h ] || grep -v -q ANOTHER_EVENT $@,"TEST","$@") > tests/test-qmp-introspect.c tests/test-qmp-introspect.h :\ > $(SRC_PATH)/tests/qapi-schema/qapi-schema-test.json $(SRC_PATH)/scripts/qapi-introspect.py $(qapi-py) > $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-introspect.py \ > $(gen-out-type) -o tests -p "test-" $<, \ > "GEN","$@") > +# check all units are included > + $(call quiet-command,[ $(gen-out-type) == -h ] || grep -q ANOTHER_EVENT $@,"TEST","$@") > > tests/qapi-schema/doc-good.test.texi: $(SRC_PATH)/tests/qapi-schema/doc-good.json $(SRC_PATH)/scripts/qapi2texi.py $(qapi-py) > $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi2texi.py $< > $@,"GEN","$@") > diff --git a/tests/qapi-schema/pragma-unit-invalid.err b/tests/qapi-schema/pragma-unit-invalid.err > new file mode 100644 > index 0000000000..e22176d42e > --- /dev/null > +++ b/tests/qapi-schema/pragma-unit-invalid.err > @@ -0,0 +1 @@ > +tests/qapi-schema/pragma-unit-invalid.json:3: Pragma 'unit' must be string > diff --git a/tests/qapi-schema/pragma-unit-invalid.exit b/tests/qapi-schema/pragma-unit-invalid.exit > new file mode 100644 > index 0000000000..d00491fd7e > --- /dev/null > +++ b/tests/qapi-schema/pragma-unit-invalid.exit > @@ -0,0 +1 @@ > +1 > diff --git a/tests/qapi-schema/pragma-unit-invalid.json b/tests/qapi-schema/pragma-unit-invalid.json > new file mode 100644 > index 0000000000..636b53939a > --- /dev/null > +++ b/tests/qapi-schema/pragma-unit-invalid.json > @@ -0,0 +1,3 @@ > +# Value of 'unit' pragma must be a string > + > +{ 'pragma': { 'unit': {} } } > diff --git a/tests/qapi-schema/pragma-unit-invalid.out b/tests/qapi-schema/pragma-unit-invalid.out > new file mode 100644 > index 0000000000..e69de29bb2 > diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json > index cd33c084cb..2fb1d3449c 100644 > --- a/tests/qapi-schema/qapi-schema-test.json > +++ b/tests/qapi-schema/qapi-schema-test.json > @@ -8,7 +8,8 @@ > # Commands allowed to return a non-dictionary: > 'returns-whitelist': [ > 'guest-get-time', > - 'guest-sync' ] } } > + 'guest-sync' ], > + 'unit': 'test-unit' } } > > { 'struct': 'TestStruct', > 'data': { 'integer': {'type': 'int'}, 'boolean': 'bool', 'string': 'str' } } > @@ -224,3 +225,6 @@ > { 'foo': 'TestIfStruct', > 'bar': { 'type': 'TestIfEnum', 'if': 'defined(TEST_IF_EVT_BAR)' } }, > 'if': 'defined(TEST_IF_EVT) && defined(TEST_IF_STRUCT)' } > + > +{ 'pragma': { 'unit': 'another-unit' } } > +{ 'event': 'ANOTHER_EVENT' } > diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out > index 7215aeb4a6..cbc61fc3a2 100644 > --- a/tests/qapi-schema/qapi-schema-test.out > +++ b/tests/qapi-schema/qapi-schema-test.out > @@ -1,3 +1,5 @@ > +event ANOTHER_EVENT None > + boxed=False > alternate AltEnumBool > tag type > case e: EnumOne > -- > 2.16.0.rc1.1.gef27df75a1 > >
Marc-André Lureau <marcandre.lureau@gmail.com> writes: > Hi > > On Thu, Jan 11, 2018 at 10:32 PM, Marc-André Lureau > <marcandre.lureau@redhat.com> wrote: >> Add a pragma that allows to tag the following expressions in the >> schema with a unit name. By default, an expression has no unit name. >> >> See the docs/devel/qapi-code-gen.txt for more details. >> > > I inadvertently merged the following patch "qapi: add a -u/--unit > option to specify which unit to visit" with this one. > > Fixed in the github branch: https://github.com/elmarco/qemu/commits/qapi-if Looks like it's still messed up: the patch there adds the pragma to docs/devel/qapi-code-gen.txt, the test cases, but no code. Make check fails. The code appears to be in the next patch. Please advise. [...]
On Mon, Feb 5, 2018 at 7:13 PM, Markus Armbruster <armbru@redhat.com> wrote: > Marc-André Lureau <marcandre.lureau@gmail.com> writes: > >> Hi >> >> On Thu, Jan 11, 2018 at 10:32 PM, Marc-André Lureau >> <marcandre.lureau@redhat.com> wrote: >>> Add a pragma that allows to tag the following expressions in the >>> schema with a unit name. By default, an expression has no unit name. >>> >>> See the docs/devel/qapi-code-gen.txt for more details. >>> >> >> I inadvertently merged the following patch "qapi: add a -u/--unit >> option to specify which unit to visit" with this one. >> >> Fixed in the github branch: https://github.com/elmarco/qemu/commits/qapi-if > > Looks like it's still messed up: the patch there adds the pragma to > docs/devel/qapi-code-gen.txt, the test cases, but no code. Make check > fails. The code appears to be in the next patch. Please advise. > More rebase mistakes.. fixed in github. How do you want to proceed? The -u/-i options from this series seems unnecessary one we have the "modularize generated qapi code" you proposed. But -i/-u can easily be removed too later on, depending on what goes first. If you can take the first patches of the series, and tell me based on what I should rebase or what to expect, I can keep working on it. In the meantime, I am a bit stuck.
Marc-André Lureau <marcandre.lureau@gmail.com> writes: > On Mon, Feb 5, 2018 at 7:13 PM, Markus Armbruster <armbru@redhat.com> wrote: >> Marc-André Lureau <marcandre.lureau@gmail.com> writes: >> >>> Hi >>> >>> On Thu, Jan 11, 2018 at 10:32 PM, Marc-André Lureau >>> <marcandre.lureau@redhat.com> wrote: >>>> Add a pragma that allows to tag the following expressions in the >>>> schema with a unit name. By default, an expression has no unit name. >>>> >>>> See the docs/devel/qapi-code-gen.txt for more details. >>>> >>> >>> I inadvertently merged the following patch "qapi: add a -u/--unit >>> option to specify which unit to visit" with this one. >>> >>> Fixed in the github branch: https://github.com/elmarco/qemu/commits/qapi-if >> >> Looks like it's still messed up: the patch there adds the pragma to >> docs/devel/qapi-code-gen.txt, the test cases, but no code. Make check >> fails. The code appears to be in the next patch. Please advise. >> > > More rebase mistakes.. fixed in github. > > How do you want to proceed? The -u/-i options from this series seems > unnecessary one we have the "modularize generated qapi code" you > proposed. But -i/-u can easily be removed too later on, depending on > what goes first. Modularization first would be less churn. But it's not quite complete, yet. > If you can take the first patches of the series, and tell me based on > what I should rebase or what to expect, I can keep working on it. In > the meantime, I am a bit stuck. This series' clash with my modularization patches should not interfere much with reviewing it. Let me try and see how far I get.
diff --git a/scripts/qapi.py b/scripts/qapi.py index f56460d028..07e738c3f1 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -47,6 +47,9 @@ returns_whitelist = [] # Whitelist of entities allowed to violate case conventions name_case_whitelist = [] +# Unit names to include for the visit (default to all units) +visit_units = [] + enum_types = {} struct_types = {} union_types = {} @@ -269,11 +272,12 @@ class QAPISchemaParser(object): self.exprs = [] self.docs = [] self.accept() + self.unit = None cur_doc = None while self.tok is not None: info = {'file': self.fname, 'line': self.line, - 'parent': self.incl_info} + 'parent': self.incl_info, 'unit': self.unit} if self.tok == '#': self.reject_expr_doc(cur_doc) cur_doc = self.get_doc(info) @@ -362,6 +366,11 @@ class QAPISchemaParser(object): "Pragma name-case-whitelist must be" " a list of strings") name_case_whitelist = value + elif name == 'unit': + if not isinstance(value, str): + raise QAPISemError(info, + "Pragma 'unit' must be string") + self.unit = value else: raise QAPISemError(info, "Unknown pragma '%s'" % name) @@ -1827,6 +1836,10 @@ class QAPISchema(object): def visit(self, visitor): visitor.visit_begin(self) for (name, entity) in sorted(self._entity_dict.items()): + # FIXME: implicit array types should use element type unit + unit = entity.info and entity.info.get('unit') + if visit_units and unit not in visit_units: + continue if visitor.visit_needed(entity): entity.visit(visitor) visitor.visit_end() @@ -2126,13 +2139,14 @@ def parse_command_line(extra_options='', extra_long_options=[]): try: opts, args = getopt.gnu_getopt( - sys.argv[1:], 'chp:o:i:' + extra_options, + sys.argv[1:], 'chp:o:u:i:' + extra_options, ['source', 'header', 'prefix=', 'output-dir=', - 'include='] + extra_long_options) + 'unit=', 'include='] + extra_long_options) except getopt.GetoptError as err: print >>sys.stderr, "%s: %s" % (sys.argv[0], str(err)) sys.exit(1) + global visit_units output_dir = '' prefix = '' do_c = False @@ -2152,6 +2166,8 @@ def parse_command_line(extra_options='', extra_long_options=[]): prefix = a elif o in ('-o', '--output-dir'): output_dir = a + '/' + elif o in ('-u', '--unit'): + visit_units.append(a) elif o in ('-c', '--source'): do_c = True elif o in ('-h', '--header'): diff --git a/docs/devel/qapi-code-gen.txt b/docs/devel/qapi-code-gen.txt index 479b755609..31bcbdac58 100644 --- a/docs/devel/qapi-code-gen.txt +++ b/docs/devel/qapi-code-gen.txt @@ -326,6 +326,10 @@ violate the rules on permitted return types. Default is none. Pragma 'name-case-whitelist' takes a list of names that may violate rules on use of upper- vs. lower-case letters. Default is none. +Pragma 'unit' takes a string value. It will set the unit name for the +following expressions in the schema. Most code generators can filter +based on a unit name. This allows to select a subset of the +expressions. Default is none. === Struct types === diff --git a/tests/Makefile.include b/tests/Makefile.include index 2b83f05954..2f39e2d5aa 100644 --- a/tests/Makefile.include +++ b/tests/Makefile.include @@ -510,6 +510,7 @@ qapi-schema += pragma-extra-junk.json qapi-schema += pragma-name-case-whitelist-crap.json qapi-schema += pragma-non-dict.json qapi-schema += pragma-returns-whitelist-crap.json +qapi-schema += pragma-unit-invalid.json qapi-schema += qapi-schema-test.json qapi-schema += quoted-structural-chars.json qapi-schema += redefined-builtin.json @@ -665,13 +666,17 @@ $(SRC_PATH)/tests/qapi-schema/qapi-schema-test.json $(SRC_PATH)/scripts/qapi-com tests/test-qapi-event.c tests/test-qapi-event.h :\ $(SRC_PATH)/tests/qapi-schema/qapi-schema-test.json $(SRC_PATH)/scripts/qapi-event.py $(qapi-py) $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-event.py \ - $(gen-out-type) -o tests -p "test-" $<, \ + $(gen-out-type) -o tests -p "test-" -u test-unit $<, \ "GEN","$@") +# check that another-unit is not included + $(call quiet-command,[ $(gen-out-type) == -h ] || grep -v -q ANOTHER_EVENT $@,"TEST","$@") tests/test-qmp-introspect.c tests/test-qmp-introspect.h :\ $(SRC_PATH)/tests/qapi-schema/qapi-schema-test.json $(SRC_PATH)/scripts/qapi-introspect.py $(qapi-py) $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-introspect.py \ $(gen-out-type) -o tests -p "test-" $<, \ "GEN","$@") +# check all units are included + $(call quiet-command,[ $(gen-out-type) == -h ] || grep -q ANOTHER_EVENT $@,"TEST","$@") tests/qapi-schema/doc-good.test.texi: $(SRC_PATH)/tests/qapi-schema/doc-good.json $(SRC_PATH)/scripts/qapi2texi.py $(qapi-py) $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi2texi.py $< > $@,"GEN","$@") diff --git a/tests/qapi-schema/pragma-unit-invalid.err b/tests/qapi-schema/pragma-unit-invalid.err new file mode 100644 index 0000000000..e22176d42e --- /dev/null +++ b/tests/qapi-schema/pragma-unit-invalid.err @@ -0,0 +1 @@ +tests/qapi-schema/pragma-unit-invalid.json:3: Pragma 'unit' must be string diff --git a/tests/qapi-schema/pragma-unit-invalid.exit b/tests/qapi-schema/pragma-unit-invalid.exit new file mode 100644 index 0000000000..d00491fd7e --- /dev/null +++ b/tests/qapi-schema/pragma-unit-invalid.exit @@ -0,0 +1 @@ +1 diff --git a/tests/qapi-schema/pragma-unit-invalid.json b/tests/qapi-schema/pragma-unit-invalid.json new file mode 100644 index 0000000000..636b53939a --- /dev/null +++ b/tests/qapi-schema/pragma-unit-invalid.json @@ -0,0 +1,3 @@ +# Value of 'unit' pragma must be a string + +{ 'pragma': { 'unit': {} } } diff --git a/tests/qapi-schema/pragma-unit-invalid.out b/tests/qapi-schema/pragma-unit-invalid.out new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json index cd33c084cb..2fb1d3449c 100644 --- a/tests/qapi-schema/qapi-schema-test.json +++ b/tests/qapi-schema/qapi-schema-test.json @@ -8,7 +8,8 @@ # Commands allowed to return a non-dictionary: 'returns-whitelist': [ 'guest-get-time', - 'guest-sync' ] } } + 'guest-sync' ], + 'unit': 'test-unit' } } { 'struct': 'TestStruct', 'data': { 'integer': {'type': 'int'}, 'boolean': 'bool', 'string': 'str' } } @@ -224,3 +225,6 @@ { 'foo': 'TestIfStruct', 'bar': { 'type': 'TestIfEnum', 'if': 'defined(TEST_IF_EVT_BAR)' } }, 'if': 'defined(TEST_IF_EVT) && defined(TEST_IF_STRUCT)' } + +{ 'pragma': { 'unit': 'another-unit' } } +{ 'event': 'ANOTHER_EVENT' } diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out index 7215aeb4a6..cbc61fc3a2 100644 --- a/tests/qapi-schema/qapi-schema-test.out +++ b/tests/qapi-schema/qapi-schema-test.out @@ -1,3 +1,5 @@ +event ANOTHER_EVENT None + boxed=False alternate AltEnumBool tag type case e: EnumOne
Add a pragma that allows to tag the following expressions in the schema with a unit name. By default, an expression has no unit name. See the docs/devel/qapi-code-gen.txt for more details. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- scripts/qapi.py | 22 +++++++++++++++++++--- docs/devel/qapi-code-gen.txt | 4 ++++ tests/Makefile.include | 7 ++++++- tests/qapi-schema/pragma-unit-invalid.err | 1 + tests/qapi-schema/pragma-unit-invalid.exit | 1 + tests/qapi-schema/pragma-unit-invalid.json | 3 +++ tests/qapi-schema/pragma-unit-invalid.out | 0 tests/qapi-schema/qapi-schema-test.json | 6 +++++- tests/qapi-schema/qapi-schema-test.out | 2 ++ 9 files changed, 41 insertions(+), 5 deletions(-) create mode 100644 tests/qapi-schema/pragma-unit-invalid.err create mode 100644 tests/qapi-schema/pragma-unit-invalid.exit create mode 100644 tests/qapi-schema/pragma-unit-invalid.json create mode 100644 tests/qapi-schema/pragma-unit-invalid.out