Re: [Qemu-devel] [PATCH v3] qapi: skip redundant includes

[Luiz Capitulino <lcapitulino@redhat.com>]

On Wed, 14 May 2014 16:07:49 +0200
Benoît Canet <benoit.canet@irqsave.net> wrote:

> The purpose of this change is to help create a json file containing
> common definitions; each bit of generated C code must be emitted
> only one time.
> 
> A second history global to all QAPISchema instances has been added
> to detect when a file is included more than one time and skip these
> includes.
> It does not act as a stack and the changes made to it by the
> __init__ function are propagated back to the caller so it's really
> a global state.

Shouldn't this behavior be noted somewhere? Eg. in writing-qmp-commands.txt
or qapi-code-gen.txt?

> 
> Signed-off-by: Benoit Canet <benoit@irqsave.net>
> ---
>  scripts/qapi.py                               | 14 +++++++++++---
>  tests/Makefile                                |  3 ++-
>  tests/qapi-schema/include-repetition-sub.json |  2 ++
>  tests/qapi-schema/include-repetition.err      |  0
>  tests/qapi-schema/include-repetition.exit     |  1 +
>  tests/qapi-schema/include-repetition.json     |  3 +++
>  tests/qapi-schema/include-repetition.out      |  3 +++
>  7 files changed, 22 insertions(+), 4 deletions(-)
>  create mode 100644 tests/qapi-schema/include-repetition-sub.json
>  create mode 100644 tests/qapi-schema/include-repetition.err
>  create mode 100644 tests/qapi-schema/include-repetition.exit
>  create mode 100644 tests/qapi-schema/include-repetition.json
>  create mode 100644 tests/qapi-schema/include-repetition.out
> 
> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index ec806aa..0265b40 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -73,13 +73,18 @@ class QAPIExprError(Exception):
>  
>  class QAPISchema:
>  
> -    def __init__(self, fp, input_relname=None, include_hist=[], parent_info=None):
> +    def __init__(self, fp, input_relname=None, include_hist=[],
> +                 previously_included=[], parent_info=None):
> +        """ include_hist is a stack used to detect inclusion cycles
> +            previously_included is a global state used to avoid multiple
> +                                inclusions of the same file"""
>          input_fname = os.path.abspath(fp.name)
>          if input_relname is None:
>              input_relname = fp.name
>          self.input_dir = os.path.dirname(input_fname)
>          self.input_file = input_relname
>          self.include_hist = include_hist + [(input_relname, input_fname)]
> +        previously_included.append(input_fname)
>          self.parent_info = parent_info
>          self.src = fp.read()
>          if self.src == '' or self.src[-1] != '\n':
> @@ -106,13 +111,16 @@ class QAPISchema:
>                         for elem in self.include_hist):
>                      raise QAPIExprError(expr_info, "Inclusion loop for %s"
>                                          % include)
> +                # skip multiple include of the same file
> +                if include_path in previously_included:
> +                    continue
>                  try:
>                      fobj = open(include_path, 'r')
>                  except IOError as e:
>                      raise QAPIExprError(expr_info,
>                                          '%s: %s' % (e.strerror, include))
> -                exprs_include = QAPISchema(fobj, include,
> -                                           self.include_hist, expr_info)
> +                exprs_include = QAPISchema(fobj, include, self.include_hist,
> +                                           previously_included, expr_info)
>                  self.exprs.extend(exprs_include.exprs)
>              else:
>                  expr_elem = {'expr': expr,
> diff --git a/tests/Makefile b/tests/Makefile
> index a8d45be..24c3d05 100644
> --- a/tests/Makefile
> +++ b/tests/Makefile
> @@ -182,7 +182,8 @@ check-qapi-schema-y := $(addprefix tests/qapi-schema/, \
>          flat-union-string-discriminator.json \
>          include-simple.json include-relpath.json include-format-err.json \
>          include-non-file.json include-no-file.json include-before-err.json \
> -        include-nested-err.json include-self-cycle.json include-cycle.json)
> +        include-nested-err.json include-self-cycle.json include-cycle.json \
> +        include-repetition.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-repetition-sub.json b/tests/qapi-schema/include-repetition-sub.json
> new file mode 100644
> index 0000000..6bfffdf
> --- /dev/null
> +++ b/tests/qapi-schema/include-repetition-sub.json
> @@ -0,0 +1,2 @@
> +{ 'include': 'comments.json' }
> +{ 'include': 'comments.json' }
> diff --git a/tests/qapi-schema/include-repetition.err b/tests/qapi-schema/include-repetition.err
> new file mode 100644
> index 0000000..e69de29
> diff --git a/tests/qapi-schema/include-repetition.exit b/tests/qapi-schema/include-repetition.exit
> new file mode 100644
> index 0000000..573541a
> --- /dev/null
> +++ b/tests/qapi-schema/include-repetition.exit
> @@ -0,0 +1 @@
> +0
> diff --git a/tests/qapi-schema/include-repetition.json b/tests/qapi-schema/include-repetition.json
> new file mode 100644
> index 0000000..ec329dd
> --- /dev/null
> +++ b/tests/qapi-schema/include-repetition.json
> @@ -0,0 +1,3 @@
> +{ 'include': 'comments.json' }
> +{ 'include': 'include-repetition-sub.json' }
> +{ 'include': 'comments.json' }
> diff --git a/tests/qapi-schema/include-repetition.out b/tests/qapi-schema/include-repetition.out
> new file mode 100644
> index 0000000..4ce3dcf
> --- /dev/null
> +++ b/tests/qapi-schema/include-repetition.out
> @@ -0,0 +1,3 @@
> +[OrderedDict([('enum', 'Status'), ('data', ['good', 'bad', 'ugly'])])]
> +[{'enum_name': 'Status', 'enum_values': ['good', 'bad', 'ugly']}]
> +[]