Re: [PATCH v4 1/3] scripts/qapi/gen.py: add FOO.trace-events output module

[John Snow]

On Fri, Jan 21, 2022 at 11:22 AM Vladimir Sementsov-Ogievskiy
<vsementsov@virtuozzo.com> wrote:
>
> We are going to generate trace events for qmp commands. We should
> generate both trace_*() function calls and trace-events files listing
> events for trace generator.
>
> So, add an output module FOO.trace-events for each FOO schema module.
>
> Still, we'll need these .trace-events files only for
> QAPISchemaGenCommandVisitor successor of QAPISchemaModularCVisitor.
> So, make this possibility optional, to avoid generating extra empty
> files for all other successors of QAPISchemaModularCVisitor.
>
> We can't simply add the new feature directly to
> QAPISchemaGenCommandVisitor: this means we'll have to reimplement
> a kind of ._module / .write() functionality of parent class in the
> successor, which seems worse than extending base class functionality.
>
> Currently nobody set add_trace_events to True, so new functionality is
> formally disabled. It will be enabled for QAPISchemaGenCommandVisitor
> in further commit.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  scripts/qapi/gen.py | 31 +++++++++++++++++++++++++++----
>  1 file changed, 27 insertions(+), 4 deletions(-)
>
> diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py
> index 995a97d2b8..def52f021e 100644
> --- a/scripts/qapi/gen.py
> +++ b/scripts/qapi/gen.py
> @@ -192,6 +192,11 @@ def _bottom(self) -> str:
>          return guardend(self.fname)
>
>
> +class QAPIGenTrace(QAPIGen):
> +    def _top(self):

-> str:

> +        return super()._top() + '# AUTOMATICALLY GENERATED, DO NOT 
> MODIFY\n\n'
> +
> +
>  @contextmanager
>  def ifcontext(ifcond: QAPISchemaIfCond, *args: QAPIGenCCode) -> 
> Iterator[None]:
>      """
> @@ -244,15 +249,18 @@ def __init__(self,
>                   what: str,
>                   user_blurb: str,
>                   builtin_blurb: Optional[str],
> -                 pydoc: str):
> +                 pydoc: str,
> +                 add_trace_events: bool = False):
>          self._prefix = prefix
>          self._what = what
>          self._user_blurb = user_blurb
>          self._builtin_blurb = builtin_blurb
>          self._pydoc = pydoc
>          self._current_module: Optional[str] = None
> -        self._module: Dict[str, Tuple[QAPIGenC, QAPIGenH]] = {}
> +        self._module: Dict[str, Tuple[QAPIGenC, QAPIGenH,
> +                                      Optional[QAPIGenTrace]]] = {}
>          self._main_module: Optional[str] = None
> +        self.add_trace_events = add_trace_events
>
>      @property
>      def _genc(self) -> QAPIGenC:
> @@ -264,6 +272,14 @@ def _genh(self) -> QAPIGenH:
>          assert self._current_module is not None
>          return self._module[self._current_module][1]
>
> +    @property
> +    def _gent(self) -> QAPIGenTrace:
> +        assert self.add_trace_events
> +        assert self._current_module is not None
> +        gent = self._module[self._current_module][2]
> +        assert gent is not None
> +        return gent
> +
>      @staticmethod
>      def _module_dirname(name: str) -> str:
>          if QAPISchemaModule.is_user_module(name):
> @@ -293,7 +309,12 @@ def _add_module(self, name: str, blurb: str) -> None:
>          basename = self._module_filename(self._what, name)
>          genc = QAPIGenC(basename + '.c', blurb, self._pydoc)
>          genh = QAPIGenH(basename + '.h', blurb, self._pydoc)
> -        self._module[name] = (genc, genh)

gent: Optional[QAPIGenTrace]

> +        if self.add_trace_events:
> +            gent = QAPIGenTrace(basename + '.trace-events')
> +        else:
> +            gent = None
> +
> +        self._module[name] = (genc, genh, gent)
>          self._current_module = name
>
>      @contextmanager
> @@ -304,11 +325,13 @@ def _temp_module(self, name: str) -> Iterator[None]:
>          self._current_module = old_module
>
>      def write(self, output_dir: str, opt_builtins: bool = False) -> None:
> -        for name, (genc, genh) in self._module.items():
> +        for name, (genc, genh, gent) in self._module.items():
>              if QAPISchemaModule.is_builtin_module(name) and not opt_builtins:
>                  continue
>              genc.write(output_dir)
>              genh.write(output_dir)
> +            if gent is not None:
> +                gent.write(output_dir)
>
>      def _begin_builtin_module(self) -> None:
>          pass
> --
> 2.31.1
>

... Sorry, I didn't finish typing this module 100%, so the scripts
aren't in the tree yet. I'll have to resume that project soon, I am
just trying to juggle a lot of things simultaneously. forgive me!

but, these should work:

> cd ~/src/qemu/scripts
> mypy --config-file=qapi/mypy.ini qapi/
> flake8 --config=qapi/.flake8 qapi/

pylint and isort has had some small regressions, so don't worry about
those as much:

> pylint --rcfile=qapi/pylintrc qapi/
************* Module qapi.events
qapi/events.py:112:8: C0103: Variable name "f" doesn't conform to
snake_case naming style (invalid-name)

--js