Re: [Freeciv-Dev] Silliness in FC_CHECK_X_LIB

[Greg Wooledge <wooledge@xxxxxxxxxxx>]

Sverker Wiberg (sverkerw@xxxxxxxxxxxx) wrote:

> dnl FC_CHECK_X_LIB(LIBRARY, FUNCTION [, ACTION-IF-FOUND [,
> dnl   ACTION-IF-NOT-FOUND]])
> dnl
> dnl This macro is intended to search for X11-related libraries.  It takes the
> dnl following variables for input:
> dnl   LIBS              -- prefixed to all linker lines
> dnl   X_LIBS            -- prefixed to all linker lines (after LIBS)
> dnl   X_EXTRA_LIBS      -- suffixed to all linker lines
> dnl Thus, the trial linker line will be "$LIBS $X_LIBS -l$1 $X_EXTRA_LIBS".

I wrote that, based on AC_CHECK_LIB, after much trial and error.  I
take responsibility for any problems with it that aren't also present
in AC_CHECK_LIB.

> This means that listing the resolver in LIBS will link the X libraries
> *after* and *without* the resolver, causing spurious link failures. Of
> course, I can configure it by listing the resolver in X_EXTRA_LIBS
> instead, but I don't see why FreeCiv should be the only package to
> require that. In short, I believe that LIBS should be appended to
> linker lines instead of prepended (LDFLAGS is prepended, btw).

LDFLAGS is prepended because it should never contain any libraries, only
flags like -s or -L/dir.

When you start playing with the LIBS and X_LIBS and such, though, it
becomes a royal mess. :-(

> A simple fix is in the diff below, but I think it can be done even
> better.

I ask only that the comments (quoted above) be kept up to date with the
code.

(I sincerely wish that the m4 macros supplied with autoconf had even
half that much documentation.  The damned info pages just say "oh, it
adds parameter_1 to $VARIABLE", but they *never* say whether it gets
added to the beginning or the end, and which other variables are used
in the process.  I'd like to get FC_CHECK_X_LIB, and any other macros
we develop, to be general enough that they can be used in all packages.
Currently I don't know of *anything* that works right. :-( )

Here's some of what I'm looking at which prompted this to be done the way
it was.  During my testing, I added a simple echo to the configure.in
following the call to AC_PATH_XTRA.  It looks like this (on Debian
GNU/Linux):

  After AC PATH XTRA: LIBS=, X_LIBS= -L/usr/X11R6/lib, X_PRE_LIBS= -lSM -lICE, 
X_EXTRA_LIBS=

See, they've put -L/dir into X_LIBS, which normally goes *after*
X_PRE_LIBS, which contains the libraries that -L/dir points to.
The whole thing's a big mess... the only reason it works at all on some
systems is because GNU binutils's ld is smart enough to realize that it
should rearrange its arguments as needed.  Most Unix-vendor ld's don't,
of course, as you noticed. :-/

Then, to make matters worse, AC_CHECK_LIB (which is what was used before)
puts the library being checked for onto the front of $LIBS if it finds it.
So, previously, we had this in configure.in:

  AC_PATH_XTRA
  LIBS="$LIBS $X_LIBS $X_PRE_LIBS $X_EXTRA_LIBS"
  AC_CHECK_LIB(X11, XOpenDisplay, , AC_MSG_ERROR("Need X11"), $LIBS)

which means that after a few checks, configure was executing links
like this:

  configure:2037: gcc -o conftest -g -O2  -I/usr/X11R6/include   conftest.c 
-lICE  -lXext -lX11   -L/usr/X11R6/lib  -lSM -lICE  1>&5

which *is* silly.  (And which also breaks horribly on non-GNU-binutils
systems.)

I couldn't come up with any good way to make AC_CHECK_LIB work.  But in
the process of trying to do so, I had rearranged the order of the various
*LIBS variables a few times.

> RCS file: /home/freeciv/CVS/freeciv/acinclude.m4,v
> retrieving revision 1.2
> diff -c -r1.2 acinclude.m4
> *** acinclude.m4        1998/11/18 12:10:28     1.2
> --- acinclude.m4        1998/11/19 14:14:34

Attached is a patch which includes your change but also updates the
documentation.  It makes absolutely no difference on my system, since
LIBS is empty during all of the FC_CHECK_X_LIB checks.  I hope it helps
other people out.

-- 
"Daddy, why do those people have to    |   Greg Wooledge
  use Microsoft Windows?"              |   wooledge@xxxxxxxxxxx
"Don't stare, son; it's not polite."   |   http://www.kellnet.com/wooledge/

fc-acinclude.m4-patch
Description: Text document