|
Description
Xavier Claessens
2012-05-02 07:26:46 UTC
I think before we do this, we should include <telepathy-glib/telepathy-glib-dbus.h> in master, and make telepathy-glib.h complete - so, I've done that. I was going to make exceptions for some of the larger cli- and svc- headers, but it turns out that using complete meta-headers, exclusively, slows down a complete Gabble compilation by less than 6 seconds - so, life's too short. Proposed structure: <telepathy-glib/debug-ansi.h> <telepathy-glib/properties-mixin.h> <telepathy-glib/text-mixin.h> Are included directly, or preferably not included. (Rationale: they are all deprecated.) <telepathy-glib/proxy-subclass.h> Is included directly. (Rationale: it's low-level.) <telepathy-glib/telepathy-glib.h> Includes all non-generated headers except those above, plus enums and errors from the spec (which I consider to be still part of our stable ABI). In master, also includes generated code for svc-generic, svc-client, interfaces, gtypes, and all cli- code. In next, these will go away. I might introduce a #define that makes these go away from master too. <telepathy-glib/telepathy-glib-dbus.h> Includes telepathy-glib.h and all generated code, including cli- and svc-. (In reply to comment #1) > I was going to make exceptions for some of the larger cli- and svc- headers, > but it turns out that using complete meta-headers, exclusively, slows down a > complete Gabble compilation by less than 6 seconds - so, life's too short. Method: build Gabble (to warm up cache), then repeat 3 times: * make -C src clean * time CCACHE_DISABLE=1 make CPPFLAGS=-D_TP_IGNORE_DEPRECATIONS With individual headers (current master): 80.91s user 9.95s system 93% cpu 1:37.38 total 81.62s user 10.01s system 94% cpu 1:36.83 total 80.69s user 9.93s system 94% cpu 1:35.77 total With single headers (the tp-glib and Gabble patches I'll attach): 86.25s user 11.30s system 95% cpu 1:42.64 total 86.55s user 11.22s system 94% cpu 1:42.97 total 88.77s user 11.20s system 94% cpu 1:46.21 total The complete builds (including Wocky etc., and from a semi-cold start) took around 2:50. Created attachment 61027 [details] [review] [master, 1/7] Add multiple-inclusion guards to generated client-side code --- This is necessary to be able to include them in more than one place. Created attachment 61028 [details] [review] [master, 2/7] Mark all -internal headers as /*<private_header>*/ This is a nicer way to get them out of the gtk-doc than listing them all in docs/reference/Makefile.am. (We still need to mention proxy-introspectable.h there, though - it's private from gtk-doc's point of view, but not gobject-introspection's.) --- I already did this on next, but the set of headers is different, so I can't just cherry-pick. Created attachment 61030 [details] [review] [master, 3/7] Define _TP_COMPILATION when compiling library and tests (but not examples) This will switch off single-inclusion checks. We don't want to use telepathy-glib/telepathy-glib.h within the library since it'll trigger mass recompilation whenever any header changes. For the examples, it seems acceptable to use the meta-header so that the examples will be exemplary. --- I haven't actually *done* single-inclusion checks; that'll come later. Created attachment 61031 [details] [review] [master, 4/7] Include nearly all headers in telepathy-glib.h Exceptions: * dbus-daemon.h is included via dbus.h * debug-ansi.h is deprecated * extra-gtkdoc.h is not really a header * properties-mixin.h is deprecated * proxy-subclass.h is sufficiently low-level that API users should "opt in" * svc-*.h wil be in telepathy-glib-dbus.h * text-mixin.h is deprecated * verify.h is included via util.h Created attachment 61032 [details] [review] [master, 5/7] Add <telepathy-glib/telepathy-glib-dbus.h> meta-header This includes all generated code, including the things that will be a separate library in 1.0. Created attachment 61033 [details] [review] [master, 6/7] All examples: follow the meta-header policy Created attachment 61034 [details] [review] [master, 7/7] Documentation: recommend including meta-headers Created attachment 61035 [details] [review] [Gabble, 1/2] Stop using deprecated debug-ansi.h Created attachment 61036 [details] [review] [Gabble, 2/2] Use meta-headers for everything --- This can't be merged until the tp-glib patches attached above have been released. For the configure check, I assumed that that would happen in 0.19.0. I think we should land these telepathy-glib patches, and release them in 0.19.0. After that, we can start making "single include" mandatory - perhaps using the new versioned deprecations stuff, so that it's mandatory if you target 0.20 as your minimum version. +1 for all of this, nice work :) I agree it's better to give a bit more time to apps to be ported to single-include before forcing it. Created attachment 62274 [details] [review] meta-headers: #define _TP_IN_META_HEADER for future single-include guards Created attachment 62275 [details] [review] define _TP_COMPILATION in gir scanner Otherwise upcoming single-include #error will break the scanner Created attachment 62276 [details] [review] Examples: do no use message-internal.h Created attachment 62277 [details] [review] Example extensions: include global header Created attachment 62278 [details] [review] Consider proxy-subclass.h as a meta header That header can be included directly, so proxy.h would #error. Created attachment 62279 [details] [review] Add single-include #error in all headers included from telepathy-glib(-dbus).h Created attachment 62280 [details] [review] Add single-include #error in generated headers Attached patches to make single-include mandatory. Simon: you merged your tp-glib patches but not yet gabble's ? (In reply to comment #21) > Simon: you merged your tp-glib patches but not yet gabble's ? 1/2 was superseded by a similar patch for another bug, and is no longer needed. Feel free to apply 2/2 if you can make it compile. In my build setup, Gabble currently doesn't build because tp_channel_manager_emit_new_channels is deprecated, despite Jonny adding a patch specifically to avoid that deprecation warning; that yak is too hairy for me to shave right now, since I'm meant to be doing something else. (In reply to comment #22) > Feel free to apply 2/2 if you can make it compile. In my build setup, Gabble > currently doesn't build because tp_channel_manager_emit_new_channels is > deprecated I found the right runes to force config.h to be regenerated, so I've applied that now. Fixed in (Gabble) git for 0.17.0 Comment on attachment 62280 [details] [review] Add single-include #error in generated headers Review of attachment 62280 [details] [review]: ----------------------------------------------------------------- Including _gen headers was never meant to be API, FWIW. Ok, I droped the patch for single include in _gen, since it was a bit ugly IMO. I've also added TP_DISABLE_SINGLE_INCLUDE that apps need to define to make it #error. Created attachment 62525 [details] [review] meta-headers: #define _TP_IN_META_HEADER for future single-include guards Created attachment 62526 [details] [review] define _TP_COMPILATION in gir scanner Otherwise upcoming single-include #error will break the scanner Created attachment 62527 [details] [review] Examples: do no use message-internal.h Created attachment 62528 [details] [review] Example extensions: include global header Created attachment 62529 [details] [review] Consider proxy-subclass.h as a meta header That header can be included directly, so proxy.h would #error. Created attachment 62530 [details] [review] Add single-include #error in all headers included from telepathy-glib(-dbus).h Created attachment 62531 [details] [review] Examples: disable single include Comment on attachment 62525 [details] [review] meta-headers: #define _TP_IN_META_HEADER for future single-include guards Review of attachment 62525 [details] [review]: ----------------------------------------------------------------- ++ Comment on attachment 62526 [details] [review] define _TP_COMPILATION in gir scanner Review of attachment 62526 [details] [review]: ----------------------------------------------------------------- ++ Comment on attachment 62527 [details] [review] Examples: do no use message-internal.h Review of attachment 62527 [details] [review]: ----------------------------------------------------------------- ++ Comment on attachment 62528 [details] [review] Example extensions: include global header Review of attachment 62528 [details] [review]: ----------------------------------------------------------------- ++ Comment on attachment 62529 [details] [review] Consider proxy-subclass.h as a meta header Review of attachment 62529 [details] [review]: ----------------------------------------------------------------- ++ Comment on attachment 62530 [details] [review] Add single-include #error in all headers included from telepathy-glib(-dbus).h Review of attachment 62530 [details] [review]: ----------------------------------------------------------------- ++ Comment on attachment 62531 [details] [review] Examples: disable single include Review of attachment 62531 [details] [review]: ----------------------------------------------------------------- ++ Yes, ship it. Thanks, merged :) |
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.