[pulseaudio-discuss] [PATCH] Add windbg log target + Fix configure not honoring --without-nls

[Maarten Bosmans]

2011/6/23 Fritz Elfert <fritz at fritz-elfert.de>:
> Hi,
>
> Maarten suggested to hurry as 1.0 is on it's way, so attached are two
> more patches:
> [snip]

You see, the default behaviour of git send-email to have every commit
in a separate mail message is really a good thing. Now we have
discussions about two completely separate patches in one thread :-(

> 2. 0002-configure-honor-without-nls.patch (all platforms)
>
> Despite what configure --help says, the current setup does ignore this
> flag, always using NLS (and linking against libintl etc.) This is
> actually not a bug in pulseaudio itself but 2 bugs in intltool and
> glib's gettext M4 macros. Since intltool looks pretty dead to me (last
> commit ages ago) and I needed a fix *fast*, I chose to implement a
> workaround for that.

Indeed, this is broken. Thanks for looking into it.
Just out of curiosity, why would you want to disable this? It works
fine on Windows too.

>diff --git a/bootstrap.sh b/bootstrap.sh
>index c7c8582..1c7895b 100755
>--- a/bootstrap.sh
>+++ b/bootstrap.sh
>@@ -87,6 +87,12 @@ else
>     test "x$LIBTOOLIZE" = "x" && LIBTOOLIZE=libtoolize
>
>     intltoolize --copy --force --automake
>+    # There's a stupid bug in intltool:
>+    # It does not honor --disable-nls and still complains about missing/old
>+    # versions. Until that is fixed in upstream, we work around, by providing
>+    # our own fixed M4 file and therefore delete the one, generated by intltoolize.
>+    rm -f m4/intltool.m4
>+
>     "$LIBTOOLIZE" -c --force
>     run_versioned aclocal "$VERSION" -I m4
>     run_versioned autoconf 2.63 -Wall

This does not seem to be a problem for me. Are you sure that you have
an up-to-date intltool installed on your machine?
In general it seems like a bad idea to work around upstream bugs in
our try like that. (and Lennart seems to agree with me)

>diff --git a/configure.ac b/configure.ac
>index b79e0e0..a0b41f0 100644
>--- a/configure.ac
>+++ b/configure.ac
>@@ -555,7 +555,14 @@ IT_PROG_INTLTOOL([0.35.0])
> GETTEXT_PACKAGE=pulseaudio
> AC_SUBST([GETTEXT_PACKAGE])
> AC_DEFINE_UNQUOTED([GETTEXT_PACKAGE],["$GETTEXT_PACKAGE"],[Gettext package])
>-AM_GLIB_GNU_GETTEXT
>+
>+dnl AM_GLIB_GNU_GETTEXT has a nasty side effect:
>+dnl If it finds a working GNU gettext, it always
>+dnl sets USE_NLS=yes. This obviously renders the
>+dnl option --disable-nls useless.
>+dnl So: invoke it conditionally.
>+AS_IF([test x$USE_NLS = xyes],
>+      [AM_GLIB_GNU_GETTEXT])
>
> pulselocaledir='${prefix}/${DATADIRNAME}/locale'
> AC_SUBST(pulselocaledir)

This one is OK. That was (for me) the real problem of the not working
--disable-nls.

>diff --git a/src/pulsecore/proplist-util.c b/src/pulsecore/proplist-util.c
>index 642c498..ae47ba8 100644
>--- a/src/pulsecore/proplist-util.c
>+++ b/src/pulsecore/proplist-util.c
>@@ -25,7 +25,9 @@
>
> #include <string.h>
> #include <locale.h>
>+#ifdef ENABLE_NLS
> #include <libintl.h>
>+#endif
>
> #ifdef __APPLE__
> #include <crt_externs.h>
>@@ -207,10 +209,14 @@ void pa_init_proplist(pa_proplist *p) {
>     }
>
>     if (!pa_proplist_contains(p, PA_PROP_APPLICATION_LANGUAGE)) {
>+#ifdef ENABLE_NLS
>         const char *l;
>
>         if ((l = setlocale(LC_MESSAGES, NULL)))
>             pa_proplist_sets(p, PA_PROP_APPLICATION_LANGUAGE, l);
>+#else
>+        pa_proplist_sets(p, PA_PROP_APPLICATION_LANGUAGE, "C");
>+#endif
>     }
>
>     if (!pa_proplist_contains(p, PA_PROP_WINDOW_X11_DISPLAY)) {

And this is proof that no one bothered to have it disabled for a long time.

Summary: ACK for configue.ac and src/pulsecore/proplist-util.c hunks
and NACK for bootstrap.sh and m4/pa_intltool.m4 hunks.

Maarten