[PATCH xwininfo] Use AM_ICONV

[Gaetan Nadon]

On Tue, 2011-08-23 at 21:24 +0200, Guillem Jover wrote:

> Hi!
> 
> On Mon, 2011-08-22 at 14:34:45 -0500, Yaakov (Cygwin/X) wrote:
> > AC_SEARCH_LIBS does not detect GNU libiconv because its symbols are
> > exported in the "libiconv" namespace instead of "iconv".  The AM_ICONV
> > macro correctly detects both glibc and GNU libiconv, defines HAVE_ICONV,
> > ICONV_CONST, and LIBICONV depending on the system.  The config.rpath
> > file is required by this macro.
> > 
> > This adds a dependency on the aclocal macros from gettext (gettext-devel
> > in some distros) when building from git, but not when building from a
> > tarball.
> > ---
> >  config.rpath |  690 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  configure.ac |    5 +-
> 
> Needing to include config.rpath in the repository seems quite
> annoying to me, as it will get out of sync with the upstream version.
> The problem is that autoreconf and autopoint, do not handle packages
> just using AM_ICONV.
> 
> But this can be worked around by declaring we need AM_GNU_GETTEXT, which
> is a bit of a heavy hammer solution, but oh well...


I did notice this issue. We are using AM_ICONV from the gettext package,
not because we need the gettext package functionality, but because we
need the nice work done in AM_ICONV. That's how I understand it, I am no
expert. Using AM_ICONV is a workaround to begin with, so I thought
adding config.rpath isn't much of an issue. The AM_ICONV checks for the
presence of the file, I am not sure it even uses it.

> 
> > diff --git a/configure.ac b/configure.ac
> > index b86aa20..e712ba6 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -45,9 +45,7 @@ if test "x$ac_cv_func_strnlen_working" = xyes; then
> >    AC_DEFINE(HAVE_STRNLEN, 1, [Define to 1 if you have a working strnlen function.])
> >  fi
> >  
> > -# Check for iconv in libc, then libiconv
> > -AC_SEARCH_LIBS([iconv], [iconv], [AC_DEFINE([HAVE_ICONV], 1,
> > -	[Define to 1 if you have the iconv() function])])
> 
> # XXX: Declare that we need GNU gettext so that autopoint installs
> # config.rpath for us on autoreconf.
> AM_GNU_GETTEXT_VERSION([0.18])
> AM_GNU_GETTEXT([external])

Nothing wrong per say, other than being "heavier", but it creates an
additional dependency autopoint. On the plus side, the gettext package
is properly initialized and it can now be used for translation files.

And we would need to take appropriate action for this warning:

        AM_GNU_GETTEXT used but `po' not in SUBDIRS


Platforms will be able to configure --disable-nls (to prevent dragging
libs not available on target machines) and still use iconv.

Finally, I favor your suggestion. I'd rather see in the git repos use
the correct way, even if overkill. It's a tough call.

> 
> > +AM_ICONV
> >  
> >  # Allow using xcb-icccm, but don't make it the default while the API is
> >  # still being changed.
> 
> regards,
> guillem
> _______________________________________________
> xorg-devel at lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.x.org/archives/xorg-devel/attachments/20110823/65167e74/attachment.htm>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part
URL: <http://lists.x.org/archives/xorg-devel/attachments/20110823/65167e74/attachment.pgp>