Re: [Qemu-devel] [PATCH v3 42/50] qapi: add a -u/--unit option to specify which unit to visit

[Markus Armbruster]

Marc-André Lureau <address@hidden> writes:

> Allow to filter expressions based on unit name.
>
> By default, only default units are processed (unspecified pragma).
>
> 'all' will include all units. Anything else will filter by unit name.

You therefore can't have a unit called 'all'.  Hardly a loss, but I
suspect documenting and guarding against it is more trouble than the
feature's worth.

If we want to keep it, the previous patch needs to reject { 'pragma': {
'unit': 'all' } }.

> (add a FIXME to make implicit array types use the element type unit,
> not the unit of the first expression using that array type. This isn't
> necessary for now, and I am not sure how to best do it yet)
>
> Signed-off-by: Marc-André Lureau <address@hidden>
> ---
>  scripts/qapi.py      | 14 ++++++++++++--
>  scripts/qapi2texi.py |  1 +
>  2 files changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index 1d0defd638..7778585819 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 to consider for the visit, 'all' for all units
> +visit_unit = None
> +
>  enum_types = {}
>  struct_types = {}
>  union_types = {}
> @@ -1796,6 +1799,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')

@unit is str, ensured by QAPISchemaParser.

> +            if visit_unit != 'all' and visit_unit != unit:
> +                continue

If visit_unit is still None, we don't visit anything.

>              if visitor.visit_needed(entity):
>                  entity.visit(visitor)
>          visitor.visit_end()
> @@ -2103,13 +2110,14 @@ def parse_command_line(extra_options='', 
> extra_long_options=[]):
>  
>      try:
>          opts, args = getopt.gnu_getopt(sys.argv[1:],
> -            'chp:o:i:' + extra_options,
> +            '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_unit
>      output_dir = ''
>      prefix = ''
>      do_c = False
> @@ -2129,6 +2137,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_unit = a
>          elif o in ('-c', '--source'):
>              do_c = True
>          elif o in ('-h', '--header'):

If the user doesn't give -u, @visit_unit remains None, and
QAPISchema.visit() won't visit anything.

I recommend to dumb this down as follows.  No special unit name 'all'.
If you want all, don't specify -u.  @visit_unit remains None then.

> diff --git a/scripts/qapi2texi.py b/scripts/qapi2texi.py
> index 4e7b1cda87..6c856d4cb7 100755
> --- a/scripts/qapi2texi.py
> +++ b/scripts/qapi2texi.py
> @@ -293,6 +293,7 @@ def main(argv):
>          print >>sys.stderr, "%s: need exactly 1 argument: SCHEMA" % argv[0]
>          sys.exit(1)
>  
> +    qapi.visit_unit = 'all'
>      schema = qapi.QAPISchema(argv[1])
>      if not qapi.doc_required:
>          print >>sys.stderr, ("%s: need pragma 'doc-required' "

Bonus: this hunk won't be needed then, because the default value just
works.