After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 760440 - libgoa-backend.so doesn't call bindtextdomain
libgoa-backend.so doesn't call bindtextdomain
Status: RESOLVED FIXED
Product: gnome-online-accounts
Classification: Core
Component: general
3.19.x
Other All
: Normal normal
: ---
Assigned To: GNOME Online Accounts maintainer(s)
GNOME Online Accounts maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2016-01-11 07:32 UTC by Ting-Wei Lan
Modified: 2017-06-16 18:17 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
backend: Setup gettext in goa_provider_class_init (1.10 KB, patch)
2016-04-23 07:00 UTC, Ting-Wei Lan
none Details | Review
backend: Use constructor to setup gettext (7.28 KB, patch)
2016-05-21 17:34 UTC, Ting-Wei Lan
committed Details | Review
backend: Use constructor to setup gettext (7.33 KB, patch)
2017-06-16 18:15 UTC, Debarshi Ray
committed Details | Review

Description Ting-Wei Lan 2016-01-11 07:32:53 UTC
gnome-online-accounts installs .mo files, but it doesn't call bindtextdomain to set the location of .mo files before using gettext. This causes gettext to open gnome-online-accounts.mo from its default prefix (usually /usr or /usr/local) even if gnome-online-accounts is built in a JHBuild prefix.
Comment 1 Ting-Wei Lan 2016-04-23 07:00:13 UTC
Created attachment 326588 [details] [review]
backend: Setup gettext in goa_provider_class_init

It seems that all users of the backend library have to use GoaProvider,
so calling bindtextdomain and bind_textdomain_codeset in its _class_init
should fix the translation problem when libintl and gnome-online-accounts
are installed in different prefixes.
Comment 2 Debarshi Ray 2016-04-26 16:15:31 UTC
Review of attachment 326588 [details] [review]:

I had been dragging my feet on this because I couldn't find an universally accepted, elegant solution. Some libraries (eg., libgdata) simply ignore this problem, while others stick it into random class_init functions [*]. Somehow I find that icky. :/

I like the way glib has solved it for its own internals by having a special _ macro that sets up gettext inside g_once_init_* guards, but it would be cumbersome for us. Maybe we should follow libsoup's lead by using G_DEFINE_CONSTRUCTOR and friends?

[*] I am ignoring libraries with an explicit init function.
Comment 3 Ting-Wei Lan 2016-04-26 17:12:23 UTC
(In reply to Debarshi Ray from comment #2)
> I like the way glib has solved it for its own internals by having a special
> _ macro that sets up gettext inside g_once_init_* guards, but it would be
> cumbersome for us. Maybe we should follow libsoup's lead by using
> G_DEFINE_CONSTRUCTOR and friends?
> 

I agree the best solution is G_DEFINE_CONSTRUCTOR. I also wanted to use it before making the patch that added code to _class_init. I think the only problem is that it is not available in GLib API and we have to copy a lot of code to use it. If you think it is acceptable to copy the constructor code, I can try to do it.
Comment 4 Debarshi Ray 2016-04-27 09:24:06 UTC
(In reply to Ting-Wei Lan from comment #3)
> I agree the best solution is G_DEFINE_CONSTRUCTOR. I also wanted to use it
> before making the patch that added code to _class_init. I think the only
> problem is that it is not available in GLib API and we have to copy a lot of
> code to use it. If you think it is acceptable to copy the constructor code,
> I can try to do it.

Yes, I am fine with copying the constructor code. Isn't it just glib/gconstructor.h? Let's just copy what libsoup does.
Comment 5 Ting-Wei Lan 2016-05-21 17:34:51 UTC
Created attachment 328319 [details] [review]
backend: Use constructor to setup gettext

To ensure the translation data are loaded from the correct prefix even
if libintl and gnome-online-accounts are installed in different
prefixes, we must call bindtextdomain and bind_textdomain_codeset
before translating messages.

To keep the code clean, gconstructor.h from GLib is copied to support
running code when the library is loaded, so we can have a constructor
to call gettext functions.

I copied the license text from a file in the same directory,
but it calls the license 'GNU Lesser General Public version 2', not
'GNU Library General Public version 2' or
'GNU Lesser General Public version 2.1'. Is this acceptable?
Comment 6 Ting-Wei Lan 2016-07-16 16:01:30 UTC
Can the new patch be reviewed now?
Comment 7 Ting-Wei Lan 2016-08-16 07:20:36 UTC
I hope this problem can be fixed in the next version of GNOME ... Can it be reviewed now?
Comment 8 Ting-Wei Lan 2017-06-08 14:27:21 UTC
Ping again ...
Comment 9 Debarshi Ray 2017-06-16 18:13:35 UTC
Review of attachment 328319 [details] [review]:

Sorry that this fell through the cracks. This looks good to me.

::: src/goabackend/goabackendinit.c
@@ +31,3 @@
+
+static void
+goa_backend_init (void)

Nitpick: I'd call it goa_backend_init_ctor for consistency with glib and libsoup.
Comment 10 Debarshi Ray 2017-06-16 18:15:30 UTC
Created attachment 353916 [details] [review]
backend: Use constructor to setup gettext
Comment 11 Debarshi Ray 2017-06-16 18:17:11 UTC
Pushed to master and gnome-3-24.