From 90f0733858f42e595c499893ea70d5c78b86f8d8 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Mon, 2 Dec 2019 15:53:14 +0000 Subject: [PATCH 1/2] gdbus-codegen: Add a --glib-min-version argument MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This can be used by callers to opt-in to backwards-incompatible changes to the behaviour or output of `gdbus-codegen` in future. This commit doesn’t introduce any such changes, though. Documentation and unit tests included. Signed-off-by: Philip Withnall Helps: #1726 --- docs/reference/gio/gdbus-codegen.xml | 25 +++++++++++++++++++ gio/gdbus-2.0/codegen/codegen.py | 12 ++++++--- gio/gdbus-2.0/codegen/codegen_main.py | 27 +++++++++++++++++++++ gio/tests/codegen.py | 35 +++++++++++++++++++++++++++ 4 files changed, 95 insertions(+), 4 deletions(-) diff --git a/docs/reference/gio/gdbus-codegen.xml b/docs/reference/gio/gdbus-codegen.xml index fdd367bc5..a0615cb8b 100644 --- a/docs/reference/gio/gdbus-codegen.xml +++ b/docs/reference/gio/gdbus-codegen.xml @@ -50,6 +50,7 @@ VALUE + VERSION FILE FILE @@ -420,6 +421,30 @@ gdbus-codegen --c-namespace MyApp \ + + VERSION + + + Specifies the minimum version of GLib which the code generated by + gdbus-codegen can depend on. This may be used to + make backwards-incompatible changes in the output or behaviour of + gdbus-codegen in future, which users may opt in to + by increasing the value they pass for . + If this option is not passed, the output from gdbus-codegen + is guaranteed to be compatible with all versions of GLib from 2.30 + upwards, as that is when gdbus-codegen was first + released. + + + The version number must be of the form + MAJOR.MINOR.MICRO, + where all parts are integers. MINOR and + MICRO are optional. The version number may not be smaller + than 2.30. + + + + diff --git a/gio/gdbus-2.0/codegen/codegen.py b/gio/gdbus-2.0/codegen/codegen.py index 3b3afe611..92622e2c2 100644 --- a/gio/gdbus-2.0/codegen/codegen.py +++ b/gio/gdbus-2.0/codegen/codegen.py @@ -62,7 +62,7 @@ def generate_header_guard(header_name): class HeaderCodeGenerator: def __init__(self, ifaces, namespace, generate_objmanager, generate_autocleanup, header_name, input_files_basenames, - use_pragma, outfile): + use_pragma, glib_min_version, outfile): self.ifaces = ifaces self.namespace, self.ns_upper, self.ns_lower = generate_namespace(namespace) self.generate_objmanager = generate_objmanager @@ -70,6 +70,7 @@ class HeaderCodeGenerator: self.header_guard = generate_header_guard(header_name) self.input_files_basenames = input_files_basenames self.use_pragma = use_pragma + self.glib_min_version = glib_min_version self.outfile = outfile # ---------------------------------------------------------------------------------------------------- @@ -618,12 +619,13 @@ class HeaderCodeGenerator: # ---------------------------------------------------------------------------------------------------- class InterfaceInfoHeaderCodeGenerator: - def __init__(self, ifaces, namespace, header_name, input_files_basenames, use_pragma, outfile): + def __init__(self, ifaces, namespace, header_name, input_files_basenames, use_pragma, glib_min_version, outfile): self.ifaces = ifaces self.namespace, self.ns_upper, self.ns_lower = generate_namespace(namespace) self.header_guard = generate_header_guard(header_name) self.input_files_basenames = input_files_basenames self.use_pragma = use_pragma + self.glib_min_version = glib_min_version self.outfile = outfile # ---------------------------------------------------------------------------------------------------- @@ -671,11 +673,12 @@ class InterfaceInfoHeaderCodeGenerator: # ---------------------------------------------------------------------------------------------------- class InterfaceInfoBodyCodeGenerator: - def __init__(self, ifaces, namespace, header_name, input_files_basenames, outfile): + def __init__(self, ifaces, namespace, header_name, input_files_basenames, glib_min_version, outfile): self.ifaces = ifaces self.namespace, self.ns_upper, self.ns_lower = generate_namespace(namespace) self.header_name = header_name self.input_files_basenames = input_files_basenames + self.glib_min_version = glib_min_version self.outfile = outfile # ---------------------------------------------------------------------------------------------------- @@ -903,13 +906,14 @@ class InterfaceInfoBodyCodeGenerator: class CodeGenerator: def __init__(self, ifaces, namespace, generate_objmanager, header_name, - input_files_basenames, docbook_gen, outfile): + input_files_basenames, docbook_gen, glib_min_version, outfile): self.ifaces = ifaces self.namespace, self.ns_upper, self.ns_lower = generate_namespace(namespace) self.generate_objmanager = generate_objmanager self.header_name = header_name self.input_files_basenames = input_files_basenames self.docbook_gen = docbook_gen + self.glib_min_version = glib_min_version self.outfile = outfile # ---------------------------------------------------------------------------------------------------- diff --git a/gio/gdbus-2.0/codegen/codegen_main.py b/gio/gdbus-2.0/codegen/codegen_main.py index e77ad5569..a9dfe0428 100644 --- a/gio/gdbus-2.0/codegen/codegen_main.py +++ b/gio/gdbus-2.0/codegen/codegen_main.py @@ -167,6 +167,8 @@ def codegen_main(): help='Use "pragma once" as the inclusion guard') arg_parser.add_argument('--annotate', nargs=3, action='append', metavar='WHAT KEY VALUE', help='Add annotation (may be used several times)') + arg_parser.add_argument('--glib-min-version', metavar='VERSION', + help='Minimum version of GLib to be supported by the outputted code (default: 2.30)') group = arg_parser.add_mutually_exclusive_group() group.add_argument('--generate-c-code', metavar='OUTFILES', @@ -233,6 +235,27 @@ def codegen_main(): c_file = args.output header_name = os.path.splitext(os.path.basename(c_file))[0] + '.h' + # Check the minimum GLib version. The minimum --glib-min-version is 2.30, + # because that’s when gdbus-codegen was introduced. Support 1, 2 or 3 + # component versions, but ignore the micro component if it’s present. + if args.glib_min_version: + try: + parts = args.glib_min_version.split('.', 3) + glib_min_version = (int(parts[0]), + int(parts[1] if len(parts) > 1 else 0)) + # Ignore micro component, but still validate it: + _ = int(parts[2] if len(parts) > 2 else 0) + except (ValueError, IndexError): + print_error('Unrecognized --glib-min-version string ‘{}’'.format( + args.glib_min_version)) + + if glib_min_version[0] < 2 or \ + (glib_min_version[0] == 2 and glib_min_version[1] < 30): + print_error('Invalid --glib-min-version string ‘{}’: minimum ' + 'version is 2.30'.format(args.glib_min_version)) + else: + glib_min_version = (2, 30) + all_ifaces = [] input_files_basenames = [] for fname in sorted(args.files + args.xml_files): @@ -262,6 +285,7 @@ def codegen_main(): header_name, input_files_basenames, args.pragma_once, + glib_min_version, outfile) gen.generate() @@ -273,6 +297,7 @@ def codegen_main(): header_name, input_files_basenames, docbook_gen, + glib_min_version, outfile) gen.generate() @@ -283,6 +308,7 @@ def codegen_main(): header_name, input_files_basenames, args.pragma_once, + glib_min_version, outfile) gen.generate() @@ -292,6 +318,7 @@ def codegen_main(): args.c_namespace, header_name, input_files_basenames, + glib_min_version, outfile) gen.generate() diff --git a/gio/tests/codegen.py b/gio/tests/codegen.py index bffe6f342..6a01b2f00 100644 --- a/gio/tests/codegen.py +++ b/gio/tests/codegen.py @@ -362,6 +362,41 @@ G_END_DECLS # The output should be the same. self.assertEqual(result1.out, result2.out) + def test_glib_min_version_invalid(self): + """Test running with an invalid --glib-min-version.""" + with self.assertRaises(subprocess.CalledProcessError): + self.runCodegenWithInterface('', + '--output', '/dev/stdout', + '--body', + '--glib-min-version', 'hello mum') + + def test_glib_min_version_too_low(self): + """Test running with a --glib-min-version which is too low (and hence + probably a typo).""" + with self.assertRaises(subprocess.CalledProcessError): + self.runCodegenWithInterface('', + '--output', '/dev/stdout', + '--body', + '--glib-min-version', '2.6') + + def test_glib_min_version_major_only(self): + """Test running with a --glib-min-version which contains only a major version.""" + result = self.runCodegenWithInterface('', + '--output', '/dev/stdout', + '--header', + '--glib-min-version', '3') + self.assertEqual('', result.err) + self.assertNotEqual('', result.out.strip()) + + def test_glib_min_version_with_micro(self): + """Test running with a --glib-min-version which contains a micro version.""" + result = self.runCodegenWithInterface('', + '--output', '/dev/stdout', + '--header', + '--glib-min-version', '2.46.2') + self.assertEqual('', result.err) + self.assertNotEqual('', result.out.strip()) + if __name__ == '__main__': unittest.main(testRunner=taptestrunner.TAPTestRunner()) From e3f80b925405359ed70530f5a478d0e70ed78a4d Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Mon, 2 Dec 2019 16:20:16 +0000 Subject: [PATCH 2/2] gdbus-codegen: Emit GUnixFDLists if an arg has type `h` w/ min-version This is a reimplementation of commit 4aba03562bc1526a1baf70ad068a9395ec64bf64 from Will Thompson, but conditional on the caller passing `--glib-min-version 2.64` to `gdbus-codegen` to explicitly opt-in to the new behaviour. From the commit message for that commit: Previously, if a method was not annotated with org.gtk.GDBus.C.UnixFD then the generated code would never contain GUnixFDList parameters, even if the method has 'h' (file descriptor) parameters. However, in this case, the generated code is essentially useless: the method cannot be called or handled except in degenerate cases where the file descriptors are missing or ignored. Check the argument types for 'h', and if present, generate code as if org.gtk.GDBus.C.UnixFD annotation were specified. Includes a unit test too. Signed-off-by: Philip Withnall Fixes: #1726 --- gio/gdbus-2.0/codegen/codegen_main.py | 7 +++- gio/gdbus-2.0/codegen/dbustypes.py | 7 +++- gio/gdbus-2.0/codegen/parser.py | 11 +++--- gio/tests/codegen.py | 50 +++++++++++++++++++++++++++ 4 files changed, 69 insertions(+), 6 deletions(-) diff --git a/gio/gdbus-2.0/codegen/codegen_main.py b/gio/gdbus-2.0/codegen/codegen_main.py index a9dfe0428..aecacc76d 100644 --- a/gio/gdbus-2.0/codegen/codegen_main.py +++ b/gio/gdbus-2.0/codegen/codegen_main.py @@ -256,12 +256,17 @@ def codegen_main(): else: glib_min_version = (2, 30) + glib_min_version_is_2_64 = (glib_min_version[0] > 2 or + (glib_min_version[0] == 2 and + glib_min_version[1] >= 64)) + all_ifaces = [] input_files_basenames = [] for fname in sorted(args.files + args.xml_files): with open(fname, 'rb') as f: xml_data = f.read() - parsed_ifaces = parser.parse_dbus_xml(xml_data) + parsed_ifaces = parser.parse_dbus_xml(xml_data, + h_type_implies_unix_fd=glib_min_version_is_2_64) all_ifaces.extend(parsed_ifaces) input_files_basenames.append(os.path.basename(fname)) diff --git a/gio/gdbus-2.0/codegen/dbustypes.py b/gio/gdbus-2.0/codegen/dbustypes.py index 16364f9b7..415a5cc7a 100644 --- a/gio/gdbus-2.0/codegen/dbustypes.py +++ b/gio/gdbus-2.0/codegen/dbustypes.py @@ -252,8 +252,9 @@ class Arg: a.post_process(interface_prefix, cns, cns_upper, cns_lower, self) class Method: - def __init__(self, name): + def __init__(self, name, h_type_implies_unix_fd=True): self.name = name + self.h_type_implies_unix_fd = h_type_implies_unix_fd self.in_args = [] self.out_args = [] self.annotations = [] @@ -284,10 +285,14 @@ class Method: for a in self.in_args: a.post_process(interface_prefix, cns, cns_upper, cns_lower, arg_count) arg_count += 1 + if self.h_type_implies_unix_fd and 'h' in a.signature: + self.unix_fd = True for a in self.out_args: a.post_process(interface_prefix, cns, cns_upper, cns_lower, arg_count) arg_count += 1 + if self.h_type_implies_unix_fd and 'h' in a.signature: + self.unix_fd = True if utils.lookup_annotation(self.annotations, 'org.freedesktop.DBus.Deprecated') == 'true': self.deprecated = True diff --git a/gio/gdbus-2.0/codegen/parser.py b/gio/gdbus-2.0/codegen/parser.py index f49136d6e..7dcc73558 100644 --- a/gio/gdbus-2.0/codegen/parser.py +++ b/gio/gdbus-2.0/codegen/parser.py @@ -36,7 +36,7 @@ class DBusXMLParser: STATE_ANNOTATION = 'annotation' STATE_IGNORED = 'ignored' - def __init__(self, xml_data): + def __init__(self, xml_data, h_type_implies_unix_fd=True): self._parser = xml.parsers.expat.ParserCreate() self._parser.CommentHandler = self.handle_comment self._parser.CharacterDataHandler = self.handle_char_data @@ -53,6 +53,8 @@ class DBusXMLParser: self.doc_comment_last_symbol = '' + self._h_type_implies_unix_fd = h_type_implies_unix_fd + self._parser.Parse(xml_data) COMMENT_STATE_BEGIN = 'begin' @@ -163,7 +165,8 @@ class DBusXMLParser: elif self.state == DBusXMLParser.STATE_INTERFACE: if name == DBusXMLParser.STATE_METHOD: self.state = DBusXMLParser.STATE_METHOD - method = dbustypes.Method(attrs['name']) + method = dbustypes.Method(attrs['name'], + h_type_implies_unix_fd=self._h_type_implies_unix_fd) self._cur_object.methods.append(method) self._cur_object = method elif name == DBusXMLParser.STATE_SIGNAL: @@ -288,6 +291,6 @@ class DBusXMLParser: self.state = self.state_stack.pop() self._cur_object = self._cur_object_stack.pop() -def parse_dbus_xml(xml_data): - parser = DBusXMLParser(xml_data) +def parse_dbus_xml(xml_data, h_type_implies_unix_fd): + parser = DBusXMLParser(xml_data, h_type_implies_unix_fd) return parser.parsed_interfaces diff --git a/gio/tests/codegen.py b/gio/tests/codegen.py index 6a01b2f00..dc2c9ff1a 100644 --- a/gio/tests/codegen.py +++ b/gio/tests/codegen.py @@ -397,6 +397,56 @@ G_END_DECLS self.assertEqual('', result.err) self.assertNotEqual('', result.out.strip()) + def test_unix_fd_types_and_annotations(self): + """Test an interface with `h` arguments, no annotation, and GLib < 2.64. + + See issue #1726. + """ + interface_xml = ''' + + + + + + + + + + + + + + + + + + ''' + + # Try without specifying --glib-min-version. + result = self.runCodegenWithInterface(interface_xml, + '--output', '/dev/stdout', + '--header') + self.assertEqual('', result.err) + self.assertEqual(result.out.strip().count('GUnixFDList'), 6) + + # Specify an old --glib-min-version. + result = self.runCodegenWithInterface(interface_xml, + '--output', '/dev/stdout', + '--header', + '--glib-min-version', '2.32') + self.assertEqual('', result.err) + self.assertEqual(result.out.strip().count('GUnixFDList'), 6) + + # Specify a --glib-min-version ≥ 2.64. There should be more + # mentions of `GUnixFDList` now, since the annotation is not needed to + # trigger its use. + result = self.runCodegenWithInterface(interface_xml, + '--output', '/dev/stdout', + '--header', + '--glib-min-version', '2.64') + self.assertEqual('', result.err) + self.assertEqual(result.out.strip().count('GUnixFDList'), 18) + if __name__ == '__main__': unittest.main(testRunner=taptestrunner.TAPTestRunner())