Hi
On Tue, Jun 21, 2022 at 6:14 PM Markus Armbruster <arm...@redhat.com> wrote: > > marcandre.lur...@redhat.com writes: > > > From: Marc-André Lureau <marcandre.lur...@redhat.com> > > > > Replace hard-coded "qemu/osdep.h" include with a qapi-gen option to > > specify the headers to include. This will allow to substitute QEMU > > osdep.h with glib.h for example, for projects with different > > global headers. > > > > For historical reasons, we can keep the default as "qemu/osdep.h". > > > > Signed-off-by: Marc-André Lureau <marcandre.lur...@redhat.com> > > I wish we'd all agree on "config.h" (conventional with autoconf). But > we don't. > > Precedence for tweaking generated code with command line options: -p. > > I suspect that we'll always specify the same -p and -i for a given > schema. To me, that suggests they should both go into the schema > instead. Observation, not demand. > > > --- > > scripts/qapi/commands.py | 15 ++++++++++----- > > scripts/qapi/events.py | 17 +++++++++++------ > > scripts/qapi/gen.py | 17 +++++++++++++++++ > > scripts/qapi/introspect.py | 11 +++++++---- > > scripts/qapi/main.py | 17 +++++++++++------ > > scripts/qapi/types.py | 17 +++++++++++------ > > scripts/qapi/visit.py | 17 +++++++++++------ > > 7 files changed, 78 insertions(+), 33 deletions(-) > > > > diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py > > index 38ca38a7b9dd..781491b6390d 100644 > > --- a/scripts/qapi/commands.py > > +++ b/scripts/qapi/commands.py > > @@ -294,9 +294,9 @@ def gen_register_command(name: str, > > > > > > class QAPISchemaGenCommandVisitor(QAPISchemaModularCVisitor): > > - def __init__(self, prefix: str, gen_tracing: bool): > > + def __init__(self, prefix: str, include: List[str], gen_tracing: bool): > > Ignorant question: why ist this List[str], not str? Do multiple options > accumulate into a list? > > Alright, I'm back from further down: looks like they do. > > > super().__init__( > > - prefix, 'qapi-commands', > > + prefix, include, 'qapi-commands', > > ' * Schema-defined QAPI/QMP commands', None, __doc__, > > gen_tracing=gen_tracing) > > self._visited_ret_types: Dict[QAPIGenC, Set[QAPISchemaType]] = {} > > @@ -308,7 +308,8 @@ def _begin_user_module(self, name: str) -> None: > > types = self._module_basename('qapi-types', name) > > visit = self._module_basename('qapi-visit', name) > > self._genc.add(mcgen(''' > > -#include "qemu/osdep.h" > > +%(include)s > > + > > #include "qapi/compat-policy.h" > > #include "qapi/visitor.h" > > #include "qapi/qmp/qdict.h" > > @@ -318,6 +319,7 @@ def _begin_user_module(self, name: str) -> None: > > #include "%(commands)s.h" > > > > ''', > > + include=self.genc_include(), > > commands=commands, visit=visit)) > > > > if self._gen_tracing and commands != 'qapi-commands': > > @@ -344,7 +346,8 @@ def visit_begin(self, schema: QAPISchema) -> None: > > ''', > > c_prefix=c_name(self._prefix, protect=False))) > > self._genc.add(mcgen(''' > > -#include "qemu/osdep.h" > > +%(include)s > > + > > #include "%(prefix)sqapi-commands.h" > > #include "%(prefix)sqapi-init-commands.h" > > > > @@ -353,6 +356,7 @@ def visit_begin(self, schema: QAPISchema) -> None: > > QTAILQ_INIT(cmds); > > > > ''', > > + include=self.genc_include(), > > prefix=self._prefix, > > c_prefix=c_name(self._prefix, protect=False))) > > > > @@ -404,7 +408,8 @@ def visit_command(self, > > def gen_commands(schema: QAPISchema, > > output_dir: str, > > prefix: str, > > + include: List[str], > > gen_tracing: bool) -> None: > > - vis = QAPISchemaGenCommandVisitor(prefix, gen_tracing) > > + vis = QAPISchemaGenCommandVisitor(prefix, include, gen_tracing) > > schema.visit(vis) > > vis.write(output_dir) > > diff --git a/scripts/qapi/events.py b/scripts/qapi/events.py > > index 27b44c49f5e9..6e677d11d2e0 100644 > > --- a/scripts/qapi/events.py > > +++ b/scripts/qapi/events.py > > @@ -175,9 +175,9 @@ def gen_event_send(name: str, > > > > class QAPISchemaGenEventVisitor(QAPISchemaModularCVisitor): > > > > - def __init__(self, prefix: str): > > + def __init__(self, prefix: str, include: List[str]): > > super().__init__( > > - prefix, 'qapi-events', > > + prefix, include, 'qapi-events', > > ' * Schema-defined QAPI/QMP events', None, __doc__) > > self._event_enum_name = c_name(prefix + 'QAPIEvent', protect=False) > > self._event_enum_members: List[QAPISchemaEnumMember] = [] > > @@ -188,7 +188,8 @@ def _begin_user_module(self, name: str) -> None: > > types = self._module_basename('qapi-types', name) > > visit = self._module_basename('qapi-visit', name) > > self._genc.add(mcgen(''' > > -#include "qemu/osdep.h" > > +%(include)s > > + > > #include "%(prefix)sqapi-emit-events.h" > > #include "%(events)s.h" > > #include "%(visit)s.h" > > @@ -198,6 +199,7 @@ def _begin_user_module(self, name: str) -> None: > > #include "qapi/qmp-event.h" > > > > ''', > > + include=self.genc_include(), > > events=events, visit=visit, > > prefix=self._prefix)) > > self._genh.add(mcgen(''' > > @@ -209,9 +211,11 @@ def _begin_user_module(self, name: str) -> None: > > def visit_end(self) -> None: > > self._add_module('./emit', ' * QAPI Events emission') > > self._genc.preamble_add(mcgen(''' > > -#include "qemu/osdep.h" > > +%(include)s > > + > > #include "%(prefix)sqapi-emit-events.h" > > ''', > > + include=self.genc_include(), > > prefix=self._prefix)) > > self._genh.preamble_add(mcgen(''' > > #include "qapi/util.h" > > @@ -246,7 +250,8 @@ def visit_event(self, > > > > def gen_events(schema: QAPISchema, > > output_dir: str, > > - prefix: str) -> None: > > - vis = QAPISchemaGenEventVisitor(prefix) > > + prefix: str, > > + include: List[str]) -> None: > > + vis = QAPISchemaGenEventVisitor(prefix, include) > > schema.visit(vis) > > vis.write(output_dir) > > diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py > > index 113b49134de4..54a70a5ff516 100644 > > --- a/scripts/qapi/gen.py > > +++ b/scripts/qapi/gen.py > > @@ -17,6 +17,7 @@ > > from typing import ( > > Dict, > > Iterator, > > + List, > > Optional, > > Sequence, > > Tuple, > > @@ -45,6 +46,12 @@ def gen_special_features(features: > > Sequence[QAPISchemaFeature]) -> str: > > return ' | '.join(special_features) or '0' > > > > > > +def genc_include(include: List[str]) -> str: > > + return '\n'.join(['#include ' + > > + (f'"{inc}"' if inc[0] not in ('<', '"') else inc) > > + for inc in include]) > > This maps a list of file names / #include arguments to C code including > them, mapping file names to #include arguments by enclosing them in "". > > Option arguments of the form <foo.h> and "foo.h" need to be quoted in > the shell. The latter can be done without quoting as foo.h. > > Somewhat funky wedding of generality with convenience. > > > + > > + > > class QAPIGen: > > def __init__(self, fname: str): > > self.fname = fname > > @@ -228,16 +235,21 @@ def ifcontext(ifcond: QAPISchemaIfCond, *args: > > QAPIGenCCode) -> Iterator[None]: > > class QAPISchemaMonolithicCVisitor(QAPISchemaVisitor): > > def __init__(self, > > prefix: str, > > + include: List[str], > > what: str, > > blurb: str, > > pydoc: str): > > self._prefix = prefix > > + self._include = include > > self._what = what > > self._genc = QAPIGenC(self._prefix + self._what + '.c', > > blurb, pydoc) > > self._genh = QAPIGenH(self._prefix + self._what + '.h', > > blurb, pydoc) > > > > + def genc_include(self) -> str: > > + return genc_include(self._include) > > + > > def write(self, output_dir: str) -> None: > > self._genc.write(output_dir) > > self._genh.write(output_dir) > > @@ -246,12 +258,14 @@ def write(self, output_dir: str) -> None: > > class QAPISchemaModularCVisitor(QAPISchemaVisitor): > > def __init__(self, > > prefix: str, > > + include: List[str], > > what: str, > > user_blurb: str, > > builtin_blurb: Optional[str], > > pydoc: str, > > gen_tracing: bool = False): > > self._prefix = prefix > > + self._include = include > > self._what = what > > self._user_blurb = user_blurb > > self._builtin_blurb = builtin_blurb > > @@ -262,6 +276,9 @@ def __init__(self, > > self._main_module: Optional[str] = None > > self._gen_tracing = gen_tracing > > > > + def genc_include(self) -> str: > > + return genc_include(self._include) > > + > > @property > > def _genc(self) -> QAPIGenC: > > assert self._current_module is not None > > diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py > > index 67c7d89aae00..d965d1769447 100644 > > --- a/scripts/qapi/introspect.py > > +++ b/scripts/qapi/introspect.py > > @@ -170,9 +170,9 @@ def to_c_string(string: str) -> str: > > > > class QAPISchemaGenIntrospectVisitor(QAPISchemaMonolithicCVisitor): > > > > - def __init__(self, prefix: str, unmask: bool): > > + def __init__(self, prefix: str, include: List[str], unmask: bool): > > super().__init__( > > - prefix, 'qapi-introspect', > > + prefix, include, 'qapi-introspect', > > ' * QAPI/QMP schema introspection', __doc__) > > self._unmask = unmask > > self._schema: Optional[QAPISchema] = None > > @@ -180,10 +180,12 @@ def __init__(self, prefix: str, unmask: bool): > > self._used_types: List[QAPISchemaType] = [] > > self._name_map: Dict[str, str] = {} > > self._genc.add(mcgen(''' > > -#include "qemu/osdep.h" > > +%(include)s > > + > > #include "%(prefix)sqapi-introspect.h" > > > > ''', > > + include=self.genc_include(), > > prefix=prefix)) > > > > def visit_begin(self, schema: QAPISchema) -> None: > > @@ -384,7 +386,8 @@ def visit_event(self, name: str, info: > > Optional[QAPISourceInfo], > > > > > > def gen_introspect(schema: QAPISchema, output_dir: str, prefix: str, > > + include: List[str], > > opt_unmask: bool) -> None: > > - vis = QAPISchemaGenIntrospectVisitor(prefix, opt_unmask) > > + vis = QAPISchemaGenIntrospectVisitor(prefix, include, opt_unmask) > > schema.visit(vis) > > vis.write(output_dir) > > diff --git a/scripts/qapi/main.py b/scripts/qapi/main.py > > index fc216a53d32a..eba98cb9ace2 100644 > > --- a/scripts/qapi/main.py > > +++ b/scripts/qapi/main.py > > @@ -9,7 +9,7 @@ > > > > import argparse > > import sys > > -from typing import Optional > > +from typing import List, Optional > > > > from .commands import gen_commands > > from .common import must_match > > @@ -31,6 +31,7 @@ def invalid_prefix_char(prefix: str) -> Optional[str]: > > def generate(schema_file: str, > > output_dir: str, > > prefix: str, > > + include: List[str], > > unmask: bool = False, > > builtins: bool = False, > > gen_tracing: bool = False) -> None: > > @@ -48,11 +49,11 @@ def generate(schema_file: str, > > assert invalid_prefix_char(prefix) is None > > > > schema = QAPISchema(schema_file) > > - gen_types(schema, output_dir, prefix, builtins) > > - gen_visit(schema, output_dir, prefix, builtins) > > - gen_commands(schema, output_dir, prefix, gen_tracing) > > - gen_events(schema, output_dir, prefix) > > - gen_introspect(schema, output_dir, prefix, unmask) > > + gen_types(schema, output_dir, prefix, include, builtins) > > + gen_visit(schema, output_dir, prefix, include, builtins) > > + gen_commands(schema, output_dir, prefix, include, gen_tracing) > > + gen_events(schema, output_dir, prefix, include) > > + gen_introspect(schema, output_dir, prefix, include, unmask) > > > > > > def main() -> int: > > @@ -75,6 +76,9 @@ def main() -> int: > > parser.add_argument('-u', '--unmask-non-abi-names', > > action='store_true', > > dest='unmask', > > help="expose non-ABI names in introspection") > > + parser.add_argument('-i', '--include', nargs='*', > > + default=['qemu/osdep.h'], > > + help="top-level include headers") > > The option name --include doesn't really tell me what it is about. Is > it an include path for schema files? Or is it about including something > in generated C? Where in generated C? > > The help text provides clues: "headers" suggests .h, and "top-level" > suggests somewhere top-like. > > In fact, it's about inserting C code at the beginning of generated .c > files. For the uses we have in mind, the C code is a single #include. > The patch implements any number of #includes. > > More general and arguably less funky: a way to insert arbitrary C code. > > Except arbitrary C code on the command line is unwieldy. Better kept it > in the schema. Pragma? > > Thoughts? Pragmas are global currently. This doesn't scale well, as we would like to split the schemas. I have a following patch that will allow me to split/merge the pragmas. This is not optimal either, I would rather remove/replace them (using annotations). Imho, global tweaking of compilation is better done from the command line. > > > > > # Option --suppress-tracing exists so we can avoid solving build system > > # problems. TODO Drop it when we no longer need it. > > @@ -94,6 +98,7 @@ def main() -> int: > > generate(args.schema, > > output_dir=args.output_dir, > > prefix=args.prefix, > > + include=args.include, > > unmask=args.unmask, > > builtins=args.builtins, > > gen_tracing=not args.suppress_tracing) > > diff --git a/scripts/qapi/types.py b/scripts/qapi/types.py > > index 477d02700137..9617b7d4edfa 100644 > > --- a/scripts/qapi/types.py > > +++ b/scripts/qapi/types.py > > @@ -282,18 +282,20 @@ def gen_type_cleanup(name: str) -> str: > > > > class QAPISchemaGenTypeVisitor(QAPISchemaModularCVisitor): > > > > - def __init__(self, prefix: str): > > + def __init__(self, prefix: str, include: List[str]): > > super().__init__( > > - prefix, 'qapi-types', ' * Schema-defined QAPI types', > > + prefix, include, 'qapi-types', ' * Schema-defined QAPI types', > > ' * Built-in QAPI types', __doc__) > > > > def _begin_builtin_module(self) -> None: > > self._genc.preamble_add(mcgen(''' > > -#include "qemu/osdep.h" > > +%(include)s > > + > > #include "qapi/dealloc-visitor.h" > > #include "qapi/qapi-builtin-types.h" > > #include "qapi/qapi-builtin-visit.h" > > -''')) > > +''', > > + include=self.genc_include())) > > self._genh.preamble_add(mcgen(''' > > #include "qapi/util.h" > > ''')) > > @@ -302,11 +304,13 @@ def _begin_user_module(self, name: str) -> None: > > types = self._module_basename('qapi-types', name) > > visit = self._module_basename('qapi-visit', name) > > self._genc.preamble_add(mcgen(''' > > -#include "qemu/osdep.h" > > +%(include)s > > + > > #include "qapi/dealloc-visitor.h" > > #include "%(types)s.h" > > #include "%(visit)s.h" > > ''', > > + include=self.genc_include(), > > types=types, visit=visit)) > > self._genh.preamble_add(mcgen(''' > > #include "qapi/qapi-builtin-types.h" > > @@ -381,7 +385,8 @@ def visit_alternate_type(self, > > def gen_types(schema: QAPISchema, > > output_dir: str, > > prefix: str, > > + include: List[str], > > opt_builtins: bool) -> None: > > - vis = QAPISchemaGenTypeVisitor(prefix) > > + vis = QAPISchemaGenTypeVisitor(prefix, include) > > schema.visit(vis) > > vis.write(output_dir, opt_builtins) > > diff --git a/scripts/qapi/visit.py b/scripts/qapi/visit.py > > index 380fa197f589..1ff464c0360f 100644 > > --- a/scripts/qapi/visit.py > > +++ b/scripts/qapi/visit.py > > @@ -318,17 +318,19 @@ def gen_visit_object(name: str) -> str: > > > > class QAPISchemaGenVisitVisitor(QAPISchemaModularCVisitor): > > > > - def __init__(self, prefix: str): > > + def __init__(self, prefix: str, include: List[str]): > > super().__init__( > > - prefix, 'qapi-visit', ' * Schema-defined QAPI visitors', > > + prefix, include, 'qapi-visit', ' * Schema-defined QAPI > > visitors', > > ' * Built-in QAPI visitors', __doc__) > > > > def _begin_builtin_module(self) -> None: > > self._genc.preamble_add(mcgen(''' > > -#include "qemu/osdep.h" > > +%(include)s > > + > > #include "qapi/error.h" > > #include "qapi/qapi-builtin-visit.h" > > -''')) > > +''', > > + include=self.genc_include())) > > self._genh.preamble_add(mcgen(''' > > #include "qapi/visitor.h" > > #include "qapi/qapi-builtin-types.h" > > @@ -339,11 +341,13 @@ def _begin_user_module(self, name: str) -> None: > > types = self._module_basename('qapi-types', name) > > visit = self._module_basename('qapi-visit', name) > > self._genc.preamble_add(mcgen(''' > > -#include "qemu/osdep.h" > > +%(include)s > > + > > #include "qapi/error.h" > > #include "qapi/qmp/qerror.h" > > #include "%(visit)s.h" > > ''', > > + include=self.genc_include(), > > visit=visit)) > > self._genh.preamble_add(mcgen(''' > > #include "qapi/qapi-builtin-visit.h" > > @@ -408,7 +412,8 @@ def visit_alternate_type(self, > > def gen_visit(schema: QAPISchema, > > output_dir: str, > > prefix: str, > > + include: List[str], > > opt_builtins: bool) -> None: > > - vis = QAPISchemaGenVisitVisitor(prefix) > > + vis = QAPISchemaGenVisitVisitor(prefix, include) > > schema.visit(vis) > > vis.write(output_dir, opt_builtins) > > Patch is mostly plumbing. Looks reasonable. >