GNOME Bugzilla – Bug 363033
Bonobo redefines i18n macros
Last modified: 2009-09-01 18:29:16 UTC
Bonobo redefines the i18n macros that GLib already provides. #include <bonobo.h> brings in <bonobo/bonobo-i18n.h> by way of <bonobo/bonobo-generic-factory.h>. <bonobo/bonobo-i18n.h> should simply #include <glib/gi18n-lib.h>.
Created attachment 74912 [details] [review] Proposed patch I've attempted to preserve backward-compatibility w.r.t. BONOBO_EXPLICIT_TRANSLATION_DOMAIN usage.
Kjartan - hope that's ok, I lost touch with what / whom / where _() should be defined long ago ;-)
Come to think of it, perhaps <bonobo.h> should not be exposing i18n stuff at all. Someone wanting to create a Bonobo-based *application* would want to #include <gi18n.h> as opposed to <gi18n-lib.h>. <bonobo/bonobo-generic-factory.h> does not need i18n macros (I checked), so perhaps <bonobo/bonobo-i18n.h> inclusion should be limited to Bonobo's internal C files as needed. That might constitute an API change, although I doubt anyone is depending on Bonobo for i18n features.
well - of course, the libgnome* headers include bonobo-i18n.h too - so ... beware.
any news here ?
Created attachment 82575 [details] [review] Revised patch I took another look at this, comparing <gi18n.h> vs <gi18n-lib.h> vs <bonobo/bonobo-i18n.h> with an eye towards the following requirements: 1) Preserve Bonobo's i18n API 2) Defer to GLib for the actual gettext definitions 3) Be safe to use for both libraries and applications I believe the following logic should meet all three: /* for backward compatibility */ #if !defined(GETTEXT_PACKAGE) && defined(BONOBO_EXPLICIT_TRANSLATION_DOMAIN) #define GETTEXT_PACKAGE BONOBO_EXPLICIT_TRANSLATION_DOMAIN #endif #ifdef GETTEXT_PACKAGE #include <glib/gi18n-lib.h> #else #include <glib/gi18n.h> #endif
Going to test this. Michael, does this calm your nerves wrt api stability?
sure, looks allright - it's a source compatibility issue of course, but - assuming we get that right, looks fine [ worth checking a full gnome build with this in-place ]
Worked for me. Commited.
This broke nautilus build for me: In file included from nautilus-application.c:70: /opt/gnome218/include/glib-2.0/glib/gi18n.h:25:1: error: "_" redefined In file included from /opt/gnome218/include/libbonobo-2.0/bonobo/bonobo-i18n.h:40, from /opt/gnome218/include/libbonobo-2.0/bonobo/bonobo-generic-factory.h:16, from nautilus-application.h:31, from nautilus-application.c:29: /opt/gnome218/include/glib-2.0/glib/gi18n-lib.h:30:1: error: this is the location of the previous definition In file included from nautilus-application.c:70: /opt/gnome218/include/glib-2.0/glib/gi18n.h:26:1: error: "Q_" redefined In file included from /opt/gnome218/include/libbonobo-2.0/bonobo/bonobo-i18n.h:40, from /opt/gnome218/include/libbonobo-2.0/bonobo/bonobo-generic-factory.h:16, from nautilus-application.h:31, from nautilus-application.c:29: /opt/gnome218/include/glib-2.0/glib/gi18n-lib.h:31:1: error: this is the location of the previous definition Reverting http://svn.gnome.org/viewcvs/libbonobo/trunk/bonobo/bonobo-i18n.h?r1=2355&r2=3325 fixes the problem for me.
Confirming. The problem is that GETTEXT_PACKAGE is defined, so the bonobo-i18n include includes gi18n-lib.h while nautilus itself of course includes gi18n.h. Maybe the test should be more like this: if BONOBO_EXPLICIT_TRANSLATION_DOMAIN is defined, we use it as gettext domain (including glib-i18n.h), else include gi18n.h ? BTW, google codesearch leads me to believe that just about nobody uses/used BONOBO_EXPLICIT_TRANSLATION_DOMAIN... Also I think we should deprecate the whole header, too.
Hmm actually in this case to build bonobo itself correctly, we'd need to define BONOBO_EXPLICIT_TRANSLATION_DOMAIN in bonobo's build system correctly....
Is it safe to assume that if BONOBO_EXPLICIT_TRANSLATION_DOMAIN is defined then GETTEXT_PACKAGE is also defined? If so, the logic could be as simple as: #ifdef BONOBO_EXPLICIT_TRANSLATION_DOMAIN #include <glib/gi18n-lib.h> #else #include <glib/gi18n.h> #endif Otherwise we're looking at something like: #ifdef BONOBO_EXPLICIT_TRANSLATION_DOMAIN #ifndef GETTEXT_PACKAGE #define GETTEXT_PACKAGE BONOBO_EXPLICIT_TRANSLATION_DOMAIN #endif #include <glib/gi18n-lib.h> #else #include <glib/gi18n.h> #endif But it looks like that's not necessary for compiling Bonobo itself.
Created attachment 83072 [details] [review] Patch against the previous patch
I'm able to build Rhythmbox now with the latest patch.
Yeah, nautilus builds here with it too. But it reintroduces warnings in gtkhtml: gcc -DHAVE_CONFIG_H -I. -I../.. -I../../src -I. -I/opt/gnome2/include -DORBIT2=1 -pthread -I/opt/gnome2/include/libgnomeui-2.0 -I/opt/gnome2/include/libgnome-2.0 -I/opt/gnome2/include/libgnomecanvas-2.0 -I/opt/gnome2/include/gtk-2.0 -I/opt/gnome2/include/libart-2.0 -I/opt/gnome2/include/gconf/2 -I/opt/gnome2/include/libbonoboui-2.0 -I/opt/gnome2/include/gnome-vfs-2.0 -I/opt/gnome2/lib/gnome-vfs-2.0/include -I/opt/gnome2/include/gnome-keyring-1 -I/opt/gnome2/include/orbit-2.0 -I/opt/gnome2/include/glib-2.0 -I/opt/gnome2/lib/glib-2.0/include -I/opt/gnome2/include/libbonobo-2.0 -I/opt/gnome2/include/bonobo-activation-2.0 -I/opt/gnome2/include/pango-1.0 -I/opt/gnome2/include -I/usr/include/freetype2 -I/opt/gnome2/lib/gtk-2.0/include -I/opt/gnome2/include/atk-1.0 -I/opt/gnome2/include/cairo -I/opt/gnome2/include/libxml2 -I/opt/gnome2/include/gtk-unix-print-2.0 -I/opt/gnome2/include/libglade-2.0 -I/usr/include/libpng12 -I/opt/gnome2/include/libsoup-2.2 -I/opt/gnome2/include/glib-2.0 -I/opt/gnome2/lib/glib-2.0/include -I/opt/gnome2/include/libxml2 -I/opt/gnome2/include -DGNOME_EXPLICIT_TRANSLATION_DOMAIN=\"gtkhtml-3.8\" -DG_LOG_DOMAIN=\"gtkhtml\" -DSRCDIR=\".\" -DPREFIX=\"/opt/gnome2\" -DORBIT2=1 -pthread -I/opt/gnome2/include/libgnomeui-2.0 -I/opt/gnome2/include/libgnome-2.0 -I/opt/gnome2/include/libgnomecanvas-2.0 -I/opt/gnome2/include/gtk-2.0 -I/opt/gnome2/include/libart-2.0 -I/opt/gnome2/include/gconf/2 -I/opt/gnome2/include/libbonoboui-2.0 -I/opt/gnome2/include/gnome-vfs-2.0 -I/opt/gnome2/lib/gnome-vfs-2.0/include -I/opt/gnome2/include/gnome-keyring-1 -I/opt/gnome2/include/orbit-2.0 -I/opt/gnome2/include/glib-2.0 -I/opt/gnome2/lib/glib-2.0/include -I/opt/gnome2/include/libbonobo-2.0 -I/opt/gnome2/include/bonobo-activation-2.0 -I/opt/gnome2/include/pango-1.0 -I/opt/gnome2/include -I/usr/include/freetype2 -I/opt/gnome2/lib/gtk-2.0/include -I/opt/gnome2/include/atk-1.0 -I/opt/gnome2/include/cairo -I/opt/gnome2/include/libxml2 -I/opt/gnome2/include/gtk-unix-print-2.0 -I/opt/gnome2/include/libglade-2.0 -I/usr/include/libpng12 -DICONDIR=\"/opt/gnome2/share/gtkhtml-3.8/icons\" -DGTKHTML_DATADIR=\"/opt/gnome2/share/gtkhtml-3.8\" -DGNOMELOCALEDIR=\"/opt/gnome2/share/locale\" -DGLADE_DATADIR=\"/opt/gnome2/share/gtkhtml-3.8\" -DGDK_DISABLE_DEPRECATED=1 -DG_DISABLE_DEPRECATED=1 -DPREFIX=\"/opt/gnome2\" -DSYSCONFDIR=\"/opt/gnome2/etc\" -DDATADIR=\"/opt/gnome2/share\" -DLIBDIR=\"/opt/gnome2/share\" -DBONOBO_DISABLE_DEPRECATED=1 -g -O2 -Wall -Wmissing-prototypes -MT search.lo -MD -MP -MF .deps/search.Tpo -c search.c -fPIC -DPIC -o .libs/search.o In file included from /opt/gnome2/include/libbonobo-2.0/bonobo/bonobo-i18n.h:37, from /opt/gnome2/include/libbonobo-2.0/bonobo/bonobo-generic-factory.h:16, from /opt/gnome2/include/libbonobo-2.0/libbonobo.h:36, from /opt/gnome2/include/libbonoboui-2.0/libbonoboui.h:14, from /opt/gnome2/include/libbonoboui-2.0/bonobo.h:14, from control-data.h:32, from search.h:29, from search.c:29: /opt/gnome2/include/glib-2.0/glib/gi18n.h:25:1: warning: "_" redefined In file included from search.c:25: /opt/gnome2/include/glib-2.0/glib/gi18n-lib.h:30:1: warning: this is the location of the previous definition In file included from /opt/gnome2/include/libbonobo-2.0/bonobo/bonobo-i18n.h:37, from /opt/gnome2/include/libbonobo-2.0/bonobo/bonobo-generic-factory.h:16, from /opt/gnome2/include/libbonobo-2.0/libbonobo.h:36, from /opt/gnome2/include/libbonoboui-2.0/libbonoboui.h:14, from /opt/gnome2/include/libbonoboui-2.0/bonobo.h:14, from control-data.h:32, from search.h:29, from search.c:29: /opt/gnome2/include/glib-2.0/glib/gi18n.h:26:1: warning: "Q_" redefined In file included from search.c:25: /opt/gnome2/include/glib-2.0/glib/gi18n-lib.h:31:1: warning: this is the location of the previous definition
Right, this is because Bonobo is now always grabbing <gi18n.h>, which is incorrect for libraries but works for applications (e.g. Nautilus and Rhythmbox). But the patch is equivalent to Bonobo's previous logic, so I think it's fine. I think the remaining issue is the fact that <bonobo.h> pulls in <bonobo/bonobo-i18n.h> by way of <bonobo/bonobo-generic-factory.h>, which translates the console message "Could not initialize Bonobo" in its BONOBO_FACTORY_INIT macro. Can we drop this translation so that we can remove the inclusion of i18n macros in a public header file, or would doing so constitute an API change? bonobo/bonobo-generic-factory.h ------------------------------- #include <bonobo/bonobo-i18n.h> ... #if defined (__BONOBO_UI_MAIN_H__) || defined (LIBBONOBOUI_H) #define BONOBO_FACTORY_INIT(descr, version, argcp, argv) \ if (!bonobo_ui_init (descr, version, argcp, argv)) \ g_error (_("Could not initialize Bonobo")); #else #define BONOBO_FACTORY_INIT(desc, version, argcp, argv) \ if (!bonobo_init (argcp, argv)) \ g_error (_("Could not initialize Bonobo")); #endif
I tried removing the _() around those and also the header include, but that gave me a lot of undefined references to _ later on...
Created attachment 83226 [details] [review] patch on top of the previous This is Matthew's patch plus removal of translation of the error messages in that header plus fixes for the fallout of that.
But this breaks compilation of gtkhtml: paragraph-style.c:33: warning: implicit declaration of function 'N_' paragraph-style.c:33: error: initializer element is not constant paragraph-style.c:33: error: (near initialization for 'paragraph_style_data[0].description') paragraph-style.c:36: error: initializer element is not constant paragraph-style.c:36: error: (near initialization for 'paragraph_style_data[1].description') paragraph-style.c:39: error: initializer element is not constant paragraph-style.c:39: error: (near initialization for 'paragraph_style_data[2].description') paragraph-style.c:42: error: initializer element is not constant paragraph-style.c:42: error: (near initialization for 'paragraph_style_data[3].description') paragraph-style.c:45: error: initializer element is not constant paragraph-style.c:45: error: (near initialization for 'paragraph_style_data[4].description') paragraph-style.c:48: error: initializer element is not constant paragraph-style.c:48: error: (near initialization for 'paragraph_style_data[5].description') paragraph-style.c:51: error: initializer element is not constant paragraph-style.c:51: error: (near initialization for 'paragraph_style_data[6].description') paragraph-style.c:54: error: initializer element is not constant paragraph-style.c:54: error: (near initialization for 'paragraph_style_data[7].description') paragraph-style.c:57: error: initializer element is not constant paragraph-style.c:57: error: (near initialization for 'paragraph_style_data[8].description') paragraph-style.c:60: error: initializer element is not constant paragraph-style.c:60: error: (near initialization for 'paragraph_style_data[9].description') paragraph-style.c:63: error: initializer element is not constant paragraph-style.c:63: error: (near initialization for 'paragraph_style_data[10].description') paragraph-style.c:66: error: initializer element is not constant paragraph-style.c:66: error: (near initialization for 'paragraph_style_data[11].description') paragraph-style.c:69: error: initializer element is not constant paragraph-style.c:69: error: (near initialization for 'paragraph_style_data[12].description') paragraph-style.c: In function 'paragraph_style_get_store': paragraph-style.c:94: warning: implicit declaration of function 'gettext' paragraph-style.c:94: warning: incompatible implicit declaration of built-in function 'gettext'
So, to summarize: 1) Preventing i18n definitions from being included with <bonobo.h> does break API, although it's unclear if i18n definitions were intended to be included with <bonobo.h> or if this is an accidental side-effect. I suspect the resulting breakage among libraries and applications that link against Bonobo is due to the occasional oversight in the code, since many of them do explicitly include i18n definitions in other compilation units. It might be a worthwhile GNOME Goal to get this cleaned up. 2) The patch in comment #14, which keeps the preprocessor logic equivalent to the original code but defers to GLib for the actual gettext macros, works well for applications but still causes redefinition warnings for libraries. BONOBO_EXPLICIT_TRANSLATION_DOMAIN, which when defined would include the proper i18n definitions for libraries, is undocumented. My suggestion for the time-being is: 1) Commit Kjartan's patch except for the removal of <bonobo/bonobo-i18n.h> from <bonobo/bonobo-generic-factory.h>. 2) Deprecate <bonobo/bonobo-i18n.h> and direct readers to GLib's i18n macros in the "libbonobo Reference Manual". Then, if we really want to be ambitious (or as a possible GNOME Goal), fix on a case-by-case basis any GNOME libraries or applications (e.g. GtkHTML) that would break from removing i18n definitions from <bonobo.h>. Then rinse and repeat this whole process for <libgnome/gnome-i18n.h>.
I commited what I had now with the change in 1)
I'm trying to jhbuild gnome-2-18, but I'm getting he following error with libbonoboui: gcc -DHAVE_CONFIG_H -I. -I. -I.. -I.. -I.. -Wall -Wmissing-prototypes -Wnested-externs -Wpointer-arith -Wno-sign-compare -DORBIT2=1 -pthread -I/opt/gnome2/include/libgnomecanvas-2.0 -I/opt/gnome2/include/libart-2.0 -I/opt/gnome2/include/gtk-2.0 -I/opt/gnome2/lib/gtk-2.0/include -I/opt/gnome2/include/atk-1.0 -I/opt/gnome2/include/cairo -I/opt/gnome2/include/pango-1.0 -I/opt/gnome2/include/glib-2.0 -I/opt/gnome2/lib/glib-2.0/include -I/opt/gnome2/include/libbonobo-2.0 -I/opt/gnome2/include/orbit-2.0 -I/opt/gnome2/include/bonobo-activation-2.0 -I/opt/gnome2/include/libgnome-2.0 -I/opt/gnome2/include/libxml2 -I/opt/gnome2/include/gconf/2 -I/opt/gnome2/include/gnome-vfs-2.0 -I/opt/gnome2/lib/gnome-vfs-2.0/include -I/opt/gnome2/include/gnome-vfs-module-2.0 -DPREFIX=\"/opt/gnome2\" -DPLUGIN_DIR=\"\" -DBONOBO_UI_INTERNAL -DBONOBO_UIDIR=\"/opt/gnome2/share/gnome-2.0/ui/\" -DBONOBO_LIBDIR=\"/opt/gnome2/lib\" -DBONOBO_DATADIR=\"/opt/gnome2/share\" -DBONOBO_PIXMAPDIR=\"/opt/gnome2/share/pixmaps\" -DBONOBO_BINDIR=\"/opt/gnome2/bin\" -DBONOBO_LOCALSTATEDIR=\"/opt/gnome2/var\" -DBONOBO_LOCALEDIR=\"/opt/gnome2/share/locale\" -DG_LOG_DOMAIN=\"Bonobo\" -DVERSION=\"2.17.91\" -DBONOBO_EXPLICIT_TRANSLATION_DOMAIN=\"libbonoboui-2.0\" -DG_DISABLE_DEPRECATED -DGDK_DISABLE_DEPRECATED -DGDK_PIXBUF_DISABLE_DEPRECATED -DPANGO_DISABLE_DEPRECATED -DGTK_DISABLE_DEPRECATED -DGCONF_DISABLE_DEPRECATED -DBONOBO_DISABLE_DEPRECATED -DBONOBO_UI_DISABLE_DEPRECATED -DGNOME_VFS_DISABLE_DEPRECATED -DGNOME_DISABLE_DEPRECATED -DLIBGLADE_DISABLE_DEPRECATED -UBONOBO_DISABLE_DEPRECATED -UBONOBO_UI_DISABLE_DEPRECATED -g -O0 -MT bonobo-ui-type-builtins.lo -MD -MP -MF .deps/bonobo-ui-type-builtins.Tpo -c bonobo-ui-type-builtins.c -fPIC -DPIC -o .libs/bonobo-ui-type-builtins.o In file included from /opt/gnome2/include/libbonobo-2.0/bonobo/bonobo-i18n.h:35, from /opt/gnome2/include/libbonobo-2.0/bonobo/bonobo-generic-factory.h:16, from /opt/gnome2/include/libbonobo-2.0/libbonobo.h:36, from ../libbonoboui.h:14, from bonobo-ui-type-builtins.c:4: /opt/gnome2/include/glib-2.0/glib/gi18n-lib.h:27:2: error: #error You must define GETTEXT_PACKAGE before including gi18n-lib.h. make[2]: *** [bonobo-ui-type-builtins.lo] Error 1 make[2]: Leaving directory `/home/devel/cvs/gnome2/libbonoboui/bonobo' This problem doesn't occur with the previous revision 3327.
*** Bug 411405 has been marked as a duplicate of this bug. ***
Seeing this in Gentoo, also.
Does this still occur on svn trunk ?
not for me.
Also see bug 504401 - it looks like the Glade 2 build is broken by these bonobo changes. (I haven't got a recent svn build so can't test it myself.)
Closing this as WONTFIX now that bonobo has been deprecated. Doesn't look like this can be fixed ever without breaking some other obscure part of the stack.