Re: [RP] Xft patch

[Bernhard R. Link]

* Shawn Betts <address@hidden> [080527 00:36]:
> I changed this part around to use the pkg-config ac macro. Can you
> verify that I got it right? I think I did...

I've taken a look and I suggest the following changes:

Move the different flags to Makefile:

diff --git a/src/Makefile.am b/src/Makefile.am
index 64e7163..715c47b 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -22,6 +22,9 @@
 bin_PROGRAMS           = ratpoison
 MAINTAINERCLEANFILES = Makefile.in config.h.in
 
+AM_LDFLAGS=${X_LDFLAGS} ${X_LIBS} ${X_EXTRA_LIBS} ${XFT_LIBS} ${HISTORY_LIBS}
+AM_CPPFLAGS=${X_CFLAGS} ${XFT_CFLAGS}
+
 ratpoison_SOURCES      = actions.c             \
                        actions.h               \
                        bar.c                   \

This has the advantage that the user can more easily overwrite part of
the flags when calling make (like setting new default CPPFLAGS without
having to add all the -I added by configure). And if ratpoison ever
gets some other binaries that do not need all of the libraries, it is
easier to only make some use some libraries.

In configure.in I'd suggest to put those flags in flags.
The --with-xft is changed to not call pkg-config with --without-xft
(as that macro is a bit buggy in some error cases and does not properly
end the line), to check for more errors of PKG_CHECK_MODULES macro
(XFT_PKG_ERRORS is only set for some errors and not for generic pkg-config
errors), and if there was an explicit --with-xft given, then it will
error out if there is none.

diff --git a/configure.in b/configure.in
index 927fe7c..eae4ac3 100644
--- a/configure.in
+++ b/configure.in
@@ -42,23 +42,36 @@ AC_ARG_WITH(xterm, [ --with-xterm=PROG        set the x 
terminal emulator used b
 term_prog=$withval, term_prog="xterm")
 AC_DEFINE_UNQUOTED(TERM_PROG, "$term_prog", X terminal emulator to use)
 
-dnl Configure Xft. If it's found then use it unless explicitely disabled
-PKG_CHECK_MODULES([XFT], [xft],,[noop=noop])
-
-if test -z "$XFT_PKG_ERRORS"; then
+XFT_CFLAGS=
+XFT_LIBS=
+AC_ARG_WITH(xft, AC_HELP_STRING([--without-xft], [Don't use the Xft library 
even if available]),
+                xft=$withval, xft=default)
+if test "x$xft" != "xno"; then
+    dnl Configure Xft.
+    PKG_CHECK_MODULES([XFT], [xft],,[noop=noop])
     AC_MSG_CHECKING(whether to use Xft)
-    AC_ARG_WITH(xft, AC_HELP_STRING([--without-xft], [Don't use the Xft 
library even if available]),
-                xft=$withval, xft=yes)
-
-    if test "x$xft" != "xno"; then
+    echo "pkg_failed is $pkg_failed" >&5
+dnl pkg_failed can be yes, no or untried (if there is no pkg-config or that 
does not work).
+    if test "$pkg_failed" = "no" ; then
        AC_DEFINE_UNQUOTED(USE_XFT_FONT, 1, [Define this to use Xft])
-       CFLAGS="$CFLAGS $XFT_CFLAGS"
-       LDFLAGS="$LDFLAGS $XFT_LIBS"
        AC_MSG_RESULT(yes)
+    elif test "x$xft" != "xdefault" ; then
+dnl if explicitly requested but not found, bail out:
+       AC_MSG_RESULT(no)
+       if test "$pkg_failed" = "untried" ; then
+           AC_MSG_ERROR([Not Xft library found (this needs a working 
pkg-config)!])
+       else
+           AC_MSG_ERROR([No Xft library found!])
+       fi
     else
-       AC_MSG_RESULT(no)
+       AC_MSG_RESULT(no)
     fi
+else
+    AC_MSG_CHECKING(whether to use Xft)
+    AC_MSG_RESULT(no)
 fi
+AC_SUBST(XFT_CFLAGS)
+AC_SUBST(XFT_LIBS)
 
 dnl Checks for programs.
 AC_CHECK_TOOL(CC, gcc)
@@ -91,28 +104,34 @@ AC_ARG_ENABLE(history,
         check_for_libhistory=no
     fi],[check_for_libhistory=yes])
 
+HISTORY_LIBS=""
 if test x$check_for_libhistory = xyes ; then
   AC_CHECK_HEADERS([readline/history.h],
     AC_CHECK_LIB(history, add_history,
-      [LIBS="$LIBS -lhistory"
+      [HISTORY_LIBS="-lhistory"
        AC_DEFINE_UNQUOTED(HAVE_HISTORY, 1, Define this to enable history)],
       AC_MSG_WARN([*** Can't find History lib. Install readline dev libs for 
history.])),
     AC_MSG_WARN([*** Can't find History header. Install readline dev libs for 
history.]))
 fi
+AC_SUBST(HISTORY_LIBS)
 
-LDFLAGS="$LDFLAGS $X_LDFLAGS $X_LIBS $X_EXTRA_LIBS"
-CFLAGS="$CFLAGS $X_CFLAGS"
-
-AC_CHECK_LIB(X11, XOpenDisplay,,
-       AC_MSG_ERROR([*** Can't find libX11]))
+AC_CHECK_LIB(X11, XOpenDisplay, [X_LIBS="-lX11 $X_LIBS"],
+       AC_MSG_ERROR([*** Can't find libX11]),$X_LIBS $X_EXTRA_LIBS)
 
+mysavedCPPFLAGS="$CPPFLAGS"
+CPPFLAGS="$CPPFLAGS $X_CFLAGS"
 AC_CHECK_HEADERS([X11/extensions/Xinerama.h], [], [], [
 #include <X11/Xlib.h>
 ])
-AC_CHECK_LIB(Xext, XMissingExtension, [LIBS="-lXext $LIBS"],, $CFLAGS $LDFLAGS)
-AC_CHECK_LIB(Xinerama, XineramaQueryScreens, [LIBS="-lXinerama $LIBS"; 
AC_DEFINE(HAVE_LIBXINERAMA,1,[Xinerama])],, $CFLAGS $LDFLAGS)
-AC_CHECK_LIB(Xtst, XTestFakeButtonEvent, [LIBS="-lXtst $LIBS"; 
AC_DEFINE(HAVE_LIBXTST,1,[Xtst])],, $CFLAGS $LDFLAGS)
+CPPFLAGS="$mysavedCPPFLAGS"
+
+AC_CHECK_LIB(Xext, XMissingExtension, [X_LIBS="-lXext $X_LIBS"],,$X_LIBS 
$X_EXTRA_LIBS)
+AC_CHECK_LIB(Xinerama, XineramaQueryScreens, [X_LIBS="-lXinerama $X_LIBS"; 
AC_DEFINE(HAVE_LIBXINERAMA,1,[Xinerama])],,$X_LIBS $X_EXTRA_LIBS)
+AC_CHECK_LIB(Xtst, XTestFakeButtonEvent, [X_LIBS="-lXtst $X_LIBS"; 
AC_DEFINE(HAVE_LIBXTST,1,[Xtst])],,$X_LIBS $X_EXTRA_LIBS)
 
+AC_SUBST(X_LIBS)
+AC_SUBST(X_EXTRA_LIBS)
+AC_SUBST(X_CFLAGS)
 
 dnl Check for electric fence library
 dnl AC_CHECK_LIB(efence,malloc,,)

Additionally, I'd suggest to use AC_HELP_STRING everywhere instead of
only the new code. (and if it used once, one does not even have to think
about if there are old versions without which would still support the
other features used here). That's not in the patch above as it is quite
a mechanical transformation.

May I apply this?

Hochachtungsvoll,
        Bernhard R. Link
-- 
"Never contain programs so few bugs, as when no debugging tools are available!"
        Niklaus Wirth