From: Marc-André Lureau <marcandre.lureau@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.lureau@redhat.com>
---
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):
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])
+
+
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")
# 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)
--
2.37.0.rc0
marcandre.lureau@redhat.com writes:
> From: Marc-André Lureau <marcandre.lureau@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.lureau@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?
>
> # 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.
Hi
On Tue, Jun 21, 2022 at 6:14 PM Markus Armbruster <armbru@redhat.com> wrote:
>
> marcandre.lureau@redhat.com writes:
>
> > From: Marc-André Lureau <marcandre.lureau@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.lureau@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.
>
Marc-André Lureau <marcandre.lureau@redhat.com> writes:
> Hi
>
>
> On Tue, Jun 21, 2022 at 6:14 PM Markus Armbruster <armbru@redhat.com> wrote:
>>
>> marcandre.lureau@redhat.com writes:
>>
>> > From: Marc-André Lureau <marcandre.lureau@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.lureau@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).
Now I'm curious. Can you sketch what you have in mind?
> Imho, global tweaking of compilation is better done from the command line.
The command line is fine for straightforward configuration. It's not
suitable for injecting code.
Fine: cc -c, which tells the compiler to work in a certain way.
Still fine: cc -DFOO, which effectively prepends '#define FOO 1" to the
.c.
No longer fine: a hypothetical option to prepend arbitrary C code. Even
if it was occasionally useful.
Now watch this:
$ python qapi-gen.py -o t qapi/qapi-schema.json -i '"abc.h"
#define FOO'
$ head -n 16 t/qapi-types.c
/* AUTOMATICALLY GENERATED, DO NOT MODIFY */
/*
* Schema-defined QAPI types
*
* Copyright IBM, Corp. 2011
* Copyright (c) 2013-2018 Red Hat Inc.
*
* This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
* See the COPYING.LIB file in the top-level directory.
*/
#include "abc.h"
#define FOO
#include "qapi/dealloc-visitor.h"
Sure, nobody of sane mind would ever do this. The fact remains that
we're doing something on the command line that should not be done there.
Your -i enables code injection because it takes either a file name or a
#include argument. Can we dumb it down to just file name?
[...]
Hi
On Tue, Aug 2, 2022 at 5:28 PM Markus Armbruster <armbru@redhat.com> wrote:
> Marc-André Lureau <marcandre.lureau@redhat.com> writes:
>
> > Hi
> >
> >
> > On Tue, Jun 21, 2022 at 6:14 PM Markus Armbruster <armbru@redhat.com>
> wrote:
> >>
> >> marcandre.lureau@redhat.com writes:
> >>
> >> > From: Marc-André Lureau <marcandre.lureau@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.lureau@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).
>
> Now I'm curious. Can you sketch what you have in mind?
>
I simply made the pragma lists additive:
https://gitlab.com/marcandre.lureau/qemu/-/commit/1861964a317c2e74bea2d1f86944625e00df777f
I didn't think much about replacing pragmas with extra annotations. But it
could be for ex moving some pragmas to the declarations.
From:
{ 'pragma': {
# Command names containing '_'
'command-name-exceptions': [
'add_client',
...
{ 'command': 'add_client',
'data': { ... } }
To:
{ 'command': {
'name': 'add_client',
# Command name containing '_'
'name-exception': true },
'data': { ... } }
Or eventually to the comment:
# @add_client: (name-exception):
> > Imho, global tweaking of compilation is better done from the command
> line.
>
> The command line is fine for straightforward configuration. It's not
> suitable for injecting code.
>
> Fine: cc -c, which tells the compiler to work in a certain way.
>
> Still fine: cc -DFOO, which effectively prepends '#define FOO 1" to the
> .c.
>
> No longer fine: a hypothetical option to prepend arbitrary C code. Even
> if it was occasionally useful.
>
> Now watch this:
>
> $ python qapi-gen.py -o t qapi/qapi-schema.json -i '"abc.h"
> #define FOO'
>
> $ head -n 16 t/qapi-types.c
> /* AUTOMATICALLY GENERATED, DO NOT MODIFY */
>
> /*
> * Schema-defined QAPI types
> *
> * Copyright IBM, Corp. 2011
> * Copyright (c) 2013-2018 Red Hat Inc.
> *
> * This work is licensed under the terms of the GNU LGPL, version 2.1
> or later.
> * See the COPYING.LIB file in the top-level directory.
> */
>
> #include "abc.h"
> #define FOO
>
> #include "qapi/dealloc-visitor.h"
>
> Sure, nobody of sane mind would ever do this. The fact remains that
> we're doing something on the command line that should not be done there.
>
> Your -i enables code injection because it takes either a file name or a
> #include argument. Can we dumb it down to just file name?
>
>
I think that can work too. Users can include a header that itself includes
extra headers in different ways, if needed.
thanks
--
Marc-André Lureau
Marc-André Lureau <marcandre.lureau@gmail.com> writes:
> Hi
>
> On Tue, Aug 2, 2022 at 5:28 PM Markus Armbruster <armbru@redhat.com> wrote:
>
>> Marc-André Lureau <marcandre.lureau@redhat.com> writes:
>>
>> > Hi
>> >
>> >
>> > On Tue, Jun 21, 2022 at 6:14 PM Markus Armbruster <armbru@redhat.com> wrote:
[...]
>> >> 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).
>>
>> Now I'm curious. Can you sketch what you have in mind?
>>
>
> I simply made the pragma lists additive:
>
> https://gitlab.com/marcandre.lureau/qemu/-/commit/1861964a317c2e74bea2d1f86944625e00df777f
>
>
> I didn't think much about replacing pragmas with extra annotations. But it
> could be for ex moving some pragmas to the declarations.
>
> From:
>
> { 'pragma': {
> # Command names containing '_'
> 'command-name-exceptions': [
> 'add_client',
> ...
>
> { 'command': 'add_client',
> 'data': { ... } }
>
> To:
>
> { 'command': {
> 'name': 'add_client',
> # Command name containing '_'
> 'name-exception': true },
> 'data': { ... } }
>
> Or eventually to the comment:
>
> # @add_client: (name-exception):
Keeping the QAPI rule violation overrides separate is kind of awkward,
but 1. it makes rule violations easy to spot in review, and 2. making
rule violations awkward helps deter people from violating the rules.
I figure the point of making pragmas additive is to let us avoid
duplication as we go from single schema to multiple schemas sharing
stuff.
We already do that for the storage daemon, admittedly in a crude &
stupid way. We simply reuse the entire pragma.json. Possible because
unused ones get ignored.
>> > Imho, global tweaking of compilation is better done from the command
>> > line.
>>
>> The command line is fine for straightforward configuration. It's not
>> suitable for injecting code.
>>
>> Fine: cc -c, which tells the compiler to work in a certain way.
>>
>> Still fine: cc -DFOO, which effectively prepends '#define FOO 1" to the
>> .c.
>>
>> No longer fine: a hypothetical option to prepend arbitrary C code. Even
>> if it was occasionally useful.
>>
>> Now watch this:
>>
>> $ python qapi-gen.py -o t qapi/qapi-schema.json -i '"abc.h"
>> #define FOO'
>>
>> $ head -n 16 t/qapi-types.c
>> /* AUTOMATICALLY GENERATED, DO NOT MODIFY */
>>
>> /*
>> * Schema-defined QAPI types
>> *
>> * Copyright IBM, Corp. 2011
>> * Copyright (c) 2013-2018 Red Hat Inc.
>> *
>> * This work is licensed under the terms of the GNU LGPL, version 2.1
>> or later.
>> * See the COPYING.LIB file in the top-level directory.
>> */
>>
>> #include "abc.h"
>> #define FOO
>>
>> #include "qapi/dealloc-visitor.h"
>>
>> Sure, nobody of sane mind would ever do this. The fact remains that
>> we're doing something on the command line that should not be done there.
>>
>> Your -i enables code injection because it takes either a file name or a
>> #include argument. Can we dumb it down to just file name?
>>
>>
> I think that can work too. Users can include a header that itself includes
> extra headers in different ways, if needed.
Yes. It could even be named "qemu/osdep.h" ;)
Teasing aside, I'm okay with a simple option to override the name of the
header to include first.
© 2016 - 2026 Red Hat, Inc.