GNOME Bugzilla – Bug 784853
glib-genmarshal python rewrite breaks compiling vte
Last modified: 2017-07-13 09:15:17 UTC
glib-genmarshal python rewrite breaks compiling vte: .libs/libvte_2_91_la-vtegtk.o: In function `vte_terminal_class_init': /home/hussam/cache/gnome/vte3/src/vte3/src/vtegtk.cc:827: undefined reference to `_vte_marshal_VOID__STRING_BOXED' /usr/bin/ld: .libs/libvte_2_91_la-vtegtk.o: relocation R_X86_64_PC32 against undefined hidden symbol `_vte_marshal_VOID__STRING_BOXED' can not be used when making a shared object /usr/bin/ld: final link failed: Bad value reverting to pre python rewrite fixes it. I have two sets of marshal.* files from vte build ("working" is pre python rewrite and "broken" is post python rewrite). Check the attached tarball.
Created attachment 355435 [details] gmarshal files.
Thanks for the bug report. The generated files are identical, except: - the header guard inside the C source - the C++ guards inside the C source The latter may be the culprit, but then it means this is a C++ build failure and VTE is literally building by sheer accident.
If I replace: --header --body with: --body --include-header="marshal.h" Then the build succeeds. I guess the problem is, indeed, that the prototypes break C++. I literally have no idea how to solve this, except telling glib-genmarshal to generate G_BEGIN_DECLS/G_END_DECLS if --prototypes is set, which is a very crappy thing to do.
Created attachment 355441 [details] [review] Fix the generation of the signal marshallers The generation of the signal marshallers is based on an undefined behaviour of the glib-genmarshal tool. Passing both the --header and --body command line arguments is undocumented and untested behaviour. The generated C source adds prototypes for the marshallers, and wraps the whole file between C++ guards.
Since VTE is relying on undefined behaviour, I took the liberty of fixing VTE. There's still the option of changing glib-genmarshal to generate C++ guards in the C source file if the `--prototypes` command line switch is used (and, thus, if projects use the deprecated `--header --body` pair), but I'd rather avoid that because it's gross. VTE is the only project I know that uses a C++ compiler to compile what is defined as C code with undefined side effects, so I'm perfectly okay with VTE requiring to be fixed.
Re-assigning to VTE for feedback on the patch. Again, I'd be okay with changing glib-genmarshal to allow VTE to build unmodified, but I'd rather not.
I guess we (VTE) should be okay with this -- Christian? However: Do we really _have_ to bump the required glib version? I'm not happy with that. As I understand, the fixed version should work with older glibs too, right?
Well, we do need /some/ fix :-) However, I really don't want to bump the glib version requirement. If this patch works with the older glib, then committing it without the version bump is ok. But I don't think it will work: my glib is 2.50 and its glib-genmarshal doesn't understand --include-header. (Also, looking on https://codesearch.debian.net/search?q=--header+--body there are five pages of results from various modules, so it's not just vte passing both options together, although it's probably the only C++ one.)
Created attachment 355476 [details] [review] Fix the generation of the signal marshallers The generation of the signal marshallers is based on an undefined behaviour of the glib-genmarshal tool. Passing both the --header and --body command line arguments is undocumented and untested behaviour. The generated C source adds prototypes for the marshallers, and wraps the whole file between C++ guards. Instead of relying on undocumented behaviour, we can achieve the same result by including the generated header in the generated source.
The new patch in attachment 355476 [details] [review] is backwards compatible with existing glib-genmarshal, and achieves the same result. This issue is not using `--header --body`: there are, indeed, enough outliers to justify supporting that behaviour, even if it's completely undocumented. The problem is that there is no guarantee that the generated hybrid C header/source is going to be safe to compile with a C++ compiler. Using `--header --body` as a hack to generate the missing prototypes is one thing; relying on that to also add the C++ declaration guards is another. Given that VTE is the only C++ project taking this assumption, I'd rather fix VTE than add yet another fallback to glib-genmarshal.
Created attachment 355482 [details] [review] Fix the generation of the signal marshallers The generation of the signal marshallers is based on an undefined behaviour of the glib-genmarshal tool. Passing both the --header and --body command line arguments is undocumented and untested behaviour. The generated C source adds prototypes for the marshallers, and wraps the whole file between C++ guards. Instead of relying on undocumented behaviour, we can achieve the same result by including the generated header in the generated source.
Comment on attachment 355482 [details] [review] Fix the generation of the signal marshallers Now that I like :-) Thanks!
Thanks for the review. Attachment 355482 [details] pushed as fa3bc86 - Fix the generation of the signal marshallers