Opened 13 years ago

Closed 12 years ago

#916 closed Enhancement (fixed)

[PATCH] Make native language support (gettext) optional

Reported by: ncopa Owned by: charles
Priority: Normal Milestone: 1.50
Component: Transmission Version: 1.11
Severity: Normal Keywords: patch
Cc:

Description

It might be a good idea to have the --enable-nls/--disable-nls options to configure script.

Attachments (1)

configure-nls.patch (1.1 KB) - added by ncopa 12 years ago.

Download all attachments as: .zip

Change History (21)

comment:1 Changed 13 years ago by ncopa

  • Summary changed from Make native language support (gettext) optional to [PATCH] Make native language support (gettext) optional

comment:2 follow-up: Changed 13 years ago by charles

  • Resolution set to invalid
  • Status changed from new to closed

Some cursory Googling indicates that using gtk without NLS isn't supported. Before I check this in I'd like to see an explanation on (1) what problem this solves, (2) what gtk environments exist w/o nls, and (3) more information on gtk's level of support for non-nls environments.

Feel free to re-open with new information.

comment:3 follow-up: Changed 13 years ago by mezz

Charles is correct about that GTK+2 required gettext by default, so it doesn't make any sense to disable nls in gtk option.

I doubt your patch will working. If you use --disable-gtk with --disable-nls and I bet your patch will causing the configure failure something related with po/Makefile.in. Charles has tried to put nls only enable if gtk is enabled in past, but it doesn't work very well due to po/Makefile.in stuff that configure complained about. If you know the tricks and submit a patch then I won't be surpised for Charles might accept this patch instead.

By the way, if I recall it corrects is that the gettext string is in libtransmission (In 1.10, I had to add -lintl by manual in libtransmission but it's fixed in 1.11)? If I am right, then disable nls (without gettext) won't work if anyone only want cli or daemon? Or it will be ok?

comment:4 in reply to: ↑ 2 ; follow-up: Changed 13 years ago by ncopa

  • Resolution invalid deleted
  • Status changed from closed to reopened

Replying to charles:

Some cursory Googling indicates that using gtk without NLS isn't supported. Before I check this in I'd like to see an explanation on (1) what problem this solves, (2) what gtk environments exist w/o nls, and (3) more information on gtk's level of support for non-nls environments.

Feel free to re-open with new information.

  1. It makes it compile on uclibc environments withotu gettext/iconv. Typical installations are embedded boxes/routers with a web interface. In those environments it does make sense to disable NLS since size does matter. (It is technically possible to install gnu iconv and gettext for those but those are huge and cause serious problems with bootstrapping the toolchain (gcc links in libiconv if its there))
  1. none. this is only useful for the cli. headless boxes, mini servers etc. (for now)
  1. It is not supported upstream.

comment:5 in reply to: ↑ 3 Changed 13 years ago by ncopa

Replying to mezz:

Charles is correct about that GTK+2 required gettext by default, so it doesn't make any sense to disable nls in gtk option.

i just tried here. It works. gtk-2 uses (should use) the NLS stuff from glib so --disable-nls shoudl be irrelevant for gtk. (I tried here, it compiles just fine with --disable-nls --enable-gtk here)

I doubt your patch will working. If you use --disable-gtk with --disable-nls and I bet your patch will causing the configure failure something related with po/Makefile.in.

running intltoolize will fix this.

Charles has tried to put nls only enable if gtk is enabled in past, but it doesn't work very well due to po/Makefile.in stuff that configure complained about. If you know the tricks and submit a patch then I won't be surpised for Charles might accept this patch instead.

It seems like Charles have fixed it. It works.

ncopa@nc ~/src/Transmission $ ./configure  --disable-nls --disable-gtk
checking for a BSD-compatible install... /usr/bin/install -c
...
Configuration:

        Source code location:       .
        Compiler:                   g++
        Build Command-Line client:  yes
        Build Daemon:               yes
        Build BeOS client:          no
        Build GTK+ client:          no
          ... gio support:          no
          ... dbus-glib support:    no
          ... libnotify support:    no
	  ... NLS support:	    no
        Build OS X client:          no
        Build wxWidgets client:     no

Configure runs just fine. Making it:

...
gcc -pthread -g -Wall -W -O3 -funroll-loops -o transmissioncli transmissioncli.o  ../libtransmission/libtransmission.a ../third-party/libevent/.libs/libevent_core.a -lnsl -lrt -lresolv ../third-party/libnatpmp/libnatpmp.a ../third-party/miniupnp/libminiupnp.a -lssl -lcrypto -ldl /usr/lib64/libcurl.so /usr/lib64/libidn.so -L/usr/lib64 /usr/lib64/libgnutls.so -lz /usr/lib64/libtasn1.so /usr/lib64/libgcrypt.so /usr/lib64/libgpg-error.so -lm  
make[1]: Leaving directory `/home/ncopa/src/Transmission/cli'
make[1]: Entering directory `/home/ncopa/src/Transmission'
make[1]: Nothing to be done for `all-am'.
make[1]: Leaving directory `/home/ncopa/src/Transmission'

Also works.

By the way, if I recall it corrects is that the gettext string is in libtransmission (In 1.10, I had to add -lintl by manual in libtransmission but it's fixed in 1.11)? If I am right, then disable nls (without gettext) won't work if anyone only want cli or daemon? Or it will be ok?

It will be ok, infact, --disable-nls will only affect the cli or daemon as gtk uses nls via glib.

I have tested it on gentoo 64bit glibc and 32 bit uclibc. I had problem with autoconf on uclibc as i don't have the WX autoconf macros on my uclibc host, but generating a dist package (make dist) on the build box with wxwidgets installed seemed to work.

comment:6 Changed 13 years ago by mezz

I have tried your patch and it doesn't work here. I have ran autogen.sh that is supposed to run intltoolize for me.

./configure --disable-beos --disable-darwin --disable-wx --enable-cli --disable-daemon --disable-gtk --disable-nls --prefix=/usr/local
[...]
config.status: executing intltool commands
config.status: executing po/stamp-it commands
config.status: error: po/Makefile is not ready.
===>  Script "configure" failed unexpectedly.

Same error in past when Charles has tried to disable gettext stuff if gtk is disabled.

comment:7 in reply to: ↑ 4 Changed 13 years ago by mezz

Replying to ncopa:

  1. none. this is only useful for the cli. headless boxes, mini servers etc. (for now)
  1. It is not supported upstream.

Do you realized that your patch isn't right after you answered to 2 and 3 questions? It's why I am opposite on --disable-nls. It should be something like this:

if test "x$build_gtk" = "xyes"; then

    PKG_CHECK_MODULES([GIO],
[...]
    if test "x$use_dbus_glib" = "xyes"; then
        AC_DEFINE([HAVE_DBUS_GLIB], 1)
    fi

    AC_CHECK_HEADERS([libintl.h])
    GETTEXT_PACKAGE=transmission
    AC_SUBST(GETTEXT_PACKAGE)
    AC_DEFINE_UNQUOTED([GETTEXT_PACKAGE],["$GETTEXT_PACKAGE"],[Gettext package])
    AM_GLIB_GNU_GETTEXT
    transmissionlocaledir='${prefix}/${DATADIRNAME}/locale'
    AC_SUBST(transmissionlocaledir)
fi

IT_PROG_INTLTOOL([0.23],[no-xml])

But it still gets same error of "po/Makefile is not ready.". It will be great if someone can solve this. I would love this patch for FreeBSD ports tree too as I have splitted into three packages (transmission (cli), transmission-daemon, transmission-gtk2).

comment:8 Changed 13 years ago by charles

  • Resolution set to invalid
  • Status changed from reopened to closed

Just to follow-up on this: I'm okay with the idea of accepting a patch to disable nls for cases when you're only building cli and daemon, but the patch needs to not break the configure script for other users. ncopa, if you can tweak the patch s.t. it addresses mezz's comment above, and re-open this ticket, that would be great.

comment:9 Changed 12 years ago by ncopa

  • Resolution invalid deleted
  • Status changed from closed to reopened

Seems to me like the po/Makefile problem have been fixed somewhere else. The updated patch seems to work for me.

Thanks!

comment:10 Changed 12 years ago by charles

  • Keywords patch added

comment:11 follow-up: Changed 12 years ago by charles

mezz: could you test out this patch to see whether or not it works for you?

comment:12 in reply to: ↑ 11 ; follow-up: Changed 12 years ago by mezz

Replying to charles:

mezz: could you test out this patch to see whether or not it works for you?


This patch works and I don't see error anymore. Awsome.. I have a bit suggest to modify this patch. With current patch, it looks strange when you just do --disable-gtk alone.

Configuration:

        Source code location:       .
        Compiler:                   g++
        Build Command-Line client:  yes
        Build Daemon:               yes
        Build BeOS client:          no
        Build GTK+ client:          no
          ... gio support:          no
          ... dbus-glib support:    no
          ... libnotify support:    no
          ... NLS support:          yes <-- This one..
        Build OS X client:          no
        Build wxWidgets client:     no


I have a question. Shouldn't the --disable-nls include the '-DDISABLE_GETTEXT' or 'DISABLE_GETTEXT=1'?

comment:13 Changed 12 years ago by charles

ncopa: do you know the answer to mezz's question?

comment:14 in reply to: ↑ 12 Changed 12 years ago by ncopa

Replying to mezz:

Replying to charles:

mezz: could you test out this patch to see whether or not it works for you?


This patch works and I don't see error anymore. Awsome.. I have a bit suggest to modify this patch. With current patch, it looks strange when you just do --disable-gtk alone.

Agree. I dont think we need the NLS output at all. I also think that the libintl prog should be included with the rest of the gettext stuff. I'll make a new patch.

I have a question. Shouldn't the --disable-nls include the '-DDISABLE_GETTEXT' or 'DISABLE_GETTEXT=1'?

No. Those will be omitted:

 -DHAVE_LIBINTL_H=1 -DGETTEXT_PACKAGE=\"transmission\" -DHAVE_LOCALE_H=1 -DHAVE_LC_MESSAGES=1
 -DHAVE_BIND_TEXTDOMAIN_CODESET=1 -DHAVE_GETTEXT=1 -DHAVE_DCGETTEXT=1 -DENABLE_NLS=1

Seems like TR_EMBEDDED turns on DISABLE_GETTEXT, i'm not sure what those does but it looks like some kind of hack for embedded.

The missing HAVE_LIBINTL_H will define _() in libtransmission/util.h as it should.

I also noticed that --enable-gtk --disable-nls does not work. Optionally, we could just force --enable-nls if --enable-gtk, but i dont think its needed as no-one sane would combine --enable-gkt and --disable-nls.

Changed 12 years ago by ncopa

comment:15 Changed 12 years ago by charles

  • Milestone changed from None Set to 1.50
  • Resolution set to fixed
  • Status changed from reopened to closed

committed to trunk in r7258

comment:16 Changed 12 years ago by mezz

So I am mistake to patch in libtransmission/utils.h like this?

-/* #define DISABLE_GETTEXT */
+#define DISABLE_GETTEXT


It's what I have done in FreeBSD ports only on transmission-cli and transmission-daemon. I also patched the configure.ac to complete remove the check on gettext, intltool, gtk and etc.

http://www.freebsd.org/cgi/cvsweb.cgi/ports/net-p2p/transmission-cli/files/transmission-cli-configure.ac

Since now this patch has fixed, so I don't think I will need to patch in configure.ac anymore. But should I keep patch on libtransmission/utils.h when it ran --disable-nls/--disable-gtk or I shouldn't be touch the DISABLE_GETTEXT?

comment:18 Changed 12 years ago by charles

  • Resolution fixed deleted
  • Status changed from closed to reopened

comment:19 Changed 12 years ago by charles

  • Owner set to charles
  • Status changed from reopened to new

comment:20 Changed 12 years ago by charles

  • Resolution set to fixed
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.