You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Raphael Kubo da Costa <ra...@FreeBSD.org> on 2012/09/02 12:02:59 UTC
[PATCH] Remove NLS check from kwallet.m4 (was Re: Does KWallet support really need NLS on?)
Daniel Shahaf <d....@daniel.shahaf.name> writes:
> Raphael Kubo da Costa wrote on Sat, Sep 01, 2012 at 11:05:02 -0300:
>> What else needs to be done for it to be decided whether it makes sense
>> to remove the check now?
>
> Send a patch that removes the check, ask people to test it on various
> platforms, commit it if no one complains?
Right, the patch below reverts r871309 by removing the NLS checks from
kwallet.m4 -- it worked fine here with KDE from git master/svn trunk and
gcc/libstdc++ 4.2.1 on FreeBSD. It'd be good to know if it works for
other people as well.
Re: [PATCH] Remove NLS check from kwallet.m4 (was Re: Does KWallet
support really need NLS on?)
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Daniel Shahaf wrote on Mon, Sep 03, 2012 at 19:44:45 +0100:
> Raphael Kubo da Costa wrote on Mon, Sep 03, 2012 at 08:15:11 -0300:
> > Daniel Shahaf <d....@daniel.shahaf.name> writes:
> >
> > > Arfrever Frehtes Taifersar Arahesis wrote on Sun, Sep 02, 2012 at 22:11:11 +0200:
> > >> We need to decide if we support GCC <4.3.0 on GNU systems.
> > >
> > > Can't kwallet.m4 only require --enable-nls if the compiler is gcc <4.3 ?
> >
> > Or if building some version of Arfrever's test code fails? Speaking of
> > that, isn't the problem of disabling NLS, including
> > svn_private_config.h and risking including something else which may use
> > libintl.h independent of the kwallet code?
>
> I think the correct fix is to include headers in the correct order ---
> in this case, to move the kwallet includes (which are "external library",
> ie, neither svn's code nor OS/libc) up within kwallet.cpp.
... and, rereading your mail, to ensure that we do that elsewhere in
Subversion, too.
Re: [PATCH] Remove NLS check from kwallet.m4 (was Re: Does KWallet
support really need NLS on?)
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Raphael Kubo da Costa wrote on Mon, Sep 03, 2012 at 08:15:11 -0300:
> Daniel Shahaf <d....@daniel.shahaf.name> writes:
>
> > Arfrever Frehtes Taifersar Arahesis wrote on Sun, Sep 02, 2012 at 22:11:11 +0200:
> >> We need to decide if we support GCC <4.3.0 on GNU systems.
> >
> > Can't kwallet.m4 only require --enable-nls if the compiler is gcc <4.3 ?
>
> Or if building some version of Arfrever's test code fails? Speaking of
> that, isn't the problem of disabling NLS, including
> svn_private_config.h and risking including something else which may use
> libintl.h independent of the kwallet code?
I think the correct fix is to include headers in the correct order ---
in this case, to move the kwallet includes (which are "external library",
ie, neither svn's code nor OS/libc) up within kwallet.cpp.
The current order is:
kwallet.cpp:#include <stdlib.h>
kwallet.cpp:#include <string.h>
kwallet.cpp:#include <unistd.h>
kwallet.cpp:#include <apr_pools.h>
kwallet.cpp:#include <apr_strings.h>
kwallet.cpp:#include "svn_auth.h"
kwallet.cpp:#include "svn_config.h"
kwallet.cpp:#include "svn_error.h"
kwallet.cpp:#include "svn_io.h"
kwallet.cpp:#include "svn_pools.h"
kwallet.cpp:#include "svn_string.h"
kwallet.cpp:#include "svn_version.h"
kwallet.cpp:#include "private/svn_auth_private.h"
kwallet.cpp:#include "svn_private_config.h"
kwallet.cpp:#include <dbus/dbus.h>
kwallet.cpp:#include <QtCore/QCoreApplication>
kwallet.cpp:#include <QtCore/QString>
kwallet.cpp:#include <kaboutdata.h>
kwallet.cpp:#include <kcmdlineargs.h>
kwallet.cpp:#include <kcomponentdata.h>
kwallet.cpp:#include <klocalizedstring.h>
kwallet.cpp:#include <kwallet.h>
Re: [PATCH] Remove NLS check from kwallet.m4 (was Re: Does KWallet support really need NLS on?)
Posted by Raphael Kubo da Costa <ra...@FreeBSD.org>.
Daniel Shahaf <d....@daniel.shahaf.name> writes:
> Raphael Kubo da Costa wrote on Mon, Sep 03, 2012 at 08:15:11 -0300:
>> that, isn't the problem of disabling NLS, including
>> svn_private_config.h and risking including something else which may use
>> libintl.h independent of the kwallet code?
>
> Are you talking about order of includes --- a header included after
> svn_private_config.h possibly redefining the '_' macro? Or are you
> talking about link-time or run-time issues?
The former, which (as I understood) is the reason Arfrever gave for not
removing the NLS checks altogether from kwallet.m4.
Re: [PATCH] Remove NLS check from kwallet.m4 (was Re: Does KWallet
support really need NLS on?)
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Raphael Kubo da Costa wrote on Mon, Sep 03, 2012 at 08:15:11 -0300:
> Daniel Shahaf <d....@daniel.shahaf.name> writes:
>
> > Arfrever Frehtes Taifersar Arahesis wrote on Sun, Sep 02, 2012 at 22:11:11 +0200:
> >> We need to decide if we support GCC <4.3.0 on GNU systems.
> >
> > Can't kwallet.m4 only require --enable-nls if the compiler is gcc <4.3 ?
>
> Or if building some version of Arfrever's test code fails? Speaking of
+1
> that, isn't the problem of disabling NLS, including
> svn_private_config.h and risking including something else which may use
> libintl.h independent of the kwallet code?
Are you talking about order of includes --- a header included after
svn_private_config.h possibly redefining the '_' macro? Or are you
talking about link-time or run-time issues?
Re: [PATCH] Remove NLS check from kwallet.m4 (was Re: Does KWallet support really need NLS on?)
Posted by Raphael Kubo da Costa <ra...@FreeBSD.org>.
Daniel Shahaf <d....@daniel.shahaf.name> writes:
> Arfrever Frehtes Taifersar Arahesis wrote on Sun, Sep 02, 2012 at 22:11:11 +0200:
>> We need to decide if we support GCC <4.3.0 on GNU systems.
>
> Can't kwallet.m4 only require --enable-nls if the compiler is gcc <4.3 ?
Or if building some version of Arfrever's test code fails? Speaking of
that, isn't the problem of disabling NLS, including
svn_private_config.h and risking including something else which may use
libintl.h independent of the kwallet code?
Re: [PATCH] Remove NLS check from kwallet.m4 (was Re: Does KWallet
support really need NLS on?)
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Arfrever Frehtes Taifersar Arahesis wrote on Sun, Sep 02, 2012 at 22:11:11 +0200:
> 2012-09-02 12:02:59 Raphael Kubo da Costa napisał(a):
> > > > What else needs to be done for it to be decided whether it makes sense
> > > > to remove the check now?
>
> We need to decide if we support GCC <4.3.0 on GNU systems.
Can't kwallet.m4 only require --enable-nls if the compiler is gcc <4.3 ?
Re: [PATCH] Remove NLS check from kwallet.m4 (was Re: Does KWallet support really need NLS on?)
Posted by Arfrever Frehtes Taifersar Arahesis <ar...@gmail.com>.
2012-09-02 12:02:59 Raphael Kubo da Costa napisał(a):
> Daniel Shahaf <d....@daniel.shahaf.name> writes:
> > Raphael Kubo da Costa wrote on Sat, Sep 01, 2012 at 11:05:02 -0300:
> > > c++locale.h does not seem to exist in
> > > libstdc++ 4.7.1 in a Linux machine I have access to, and it does not
> > > include libintl.h on libstdc++ 4.2.1 here on FreeBSD.
c++locale.h definitely still exists:
$ find /usr/lib/gcc -name c++locale.h
/usr/lib/gcc/x86_64-pc-linux-gnu/4.6.3/include/g++-v4/x86_64-pc-linux-gnu/32/bits/c++locale.h
/usr/lib/gcc/x86_64-pc-linux-gnu/4.6.3/include/g++-v4/x86_64-pc-linux-gnu/bits/c++locale.h
/usr/lib/gcc/x86_64-pc-linux-gnu/4.7.1/include/g++-v4/x86_64-pc-linux-gnu/32/bits/c++locale.h
/usr/lib/gcc/x86_64-pc-linux-gnu/4.7.1/include/g++-v4/x86_64-pc-linux-gnu/bits/c++locale.h
--disable-nls causes that subversion/svn_private_config.h defines gettext() and dgettext() macros.
If libintl.h was somewhere included, then compilation failure would still occur:
$ cat test.cpp
#define gettext(x) (x)
#define dgettext(domain, x) (x)
#include <libintl.h>
$ g++ -c test.cpp
In file included from test.cpp:3:0:
/usr/include/libintl.h:40:14: error: expected unqualified-id before ‘__const’
/usr/include/libintl.h:40:14: error: expected ‘)’ before ‘__const’
/usr/include/libintl.h:40:14: error: expected initializer before ‘__const’
/usr/include/libintl.h:45:14: error: expected unqualified-id before ‘__const’
/usr/include/libintl.h:45:14: error: expected ‘)’ before ‘__const’
/usr/include/libintl.h:45:14: error: expected initializer before ‘__const’
Build system of GCC creates bits/c++locale.h from libstdc++-v3/config/locale/gnu/c_locale.h on GNU systems and
from libstdc++-v3/config/locale/generic/c_locale.h on other systems (e.g. FreeBSD).
GCC r124803 [1] deleted inclusion of libintl.h in bits/c++locale.h on GNU systems.
This change was released in GCC 4.3.0 [2] and was never backported to gcc-4_2-branch branch [3].
[1] http://gcc.gnu.org/viewcvs?view=revision&revision=124803
[2] http://gcc.gnu.org/viewcvs?view=revision&revision=132392
[3] http://gcc.gnu.org/viewcvs/branches/gcc-4_2-branch/libstdc%2B%2B-v3/config/locale/gnu/c_locale.h?view=markup
> > > What else needs to be done for it to be decided whether it makes sense
> > > to remove the check now?
We need to decide if we support GCC <4.3.0 on GNU systems.
--
Arfrever Frehtes Taifersar Arahesis
Re: [PATCH] Remove NLS check from kwallet.m4 (was Re: Does KWallet
support really need NLS on?)
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Raphael Kubo da Costa wrote on Sun, Sep 02, 2012 at 07:02:59 -0300:
> Daniel Shahaf <d....@daniel.shahaf.name> writes:
>
> > Raphael Kubo da Costa wrote on Sat, Sep 01, 2012 at 11:05:02 -0300:
> >> What else needs to be done for it to be decided whether it makes sense
> >> to remove the check now?
> >
> > Send a patch that removes the check, ask people to test it on various
> > platforms, commit it if no one complains?
>
> Right, the patch below reverts r871309 by removing the NLS checks from
> kwallet.m4 -- it worked fine here with KDE from git master/svn trunk and
> gcc/libstdc++ 4.2.1 on FreeBSD. It'd be good to know if it works for
> other people as well.
>
For easier reviewing, here is the 'svn diff -x-w' version of the patch
you just sent:
% $svn di -x-w build/ac-macros/kwallet.m4
Index: build/ac-macros/kwallet.m4
===================================================================
--- build/ac-macros/kwallet.m4 (revision 1379958)
+++ build/ac-macros/kwallet.m4 (working copy)
@@ -35,7 +35,6 @@
AC_MSG_RESULT([yes])
if test "$svn_enable_shared" = "yes"; then
if test "$APR_HAS_DSO" = "yes"; then
- if test "$USE_NLS" = "yes"; then
if test -n "$PKG_CONFIG"; then
if test "$HAVE_DBUS" = "yes"; then
AC_MSG_CHECKING([for QtCore, QtDBus, QtGui])
@@ -101,9 +100,6 @@
AC_MSG_ERROR([cannot find pkg-config])
fi
else
- AC_MSG_ERROR([missing support for internationalization])
- fi
- else
AC_MSG_ERROR([APR does not have support for DSOs])
fi
else