Re: [Qemu-devel] [PATCH v4 42/51] qapi: add a 'unit' pragma

[Marc-André Lureau]

Hi

On Thu, Jan 11, 2018 at 10:32 PM, Marc-André Lureau
<address@hidden> 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 <address@hidden>
> ---
>  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