Re: FYI: 333-gary-refactor-LTDL_INIT.patch

[Ralf Wildenhues]

Hi Gary,

* Gary V. Vaughan wrote on Thu, Jan 17, 2008 at 05:29:17AM CET:
> On 16 Jan 2008, at 14:22, Ralf Wildenhues wrote:
> >OK to apply?
> 
> I have some nits first.  Please commit once they are addressed.

Addressed and committed as noted below.  Thanks for your quick review.

> >     * NEWS: Update.
> 
> Except for a couple of instances in the latest ChangeLog rotation, we've
> always used past tense here.  Super picky, I know, but we might as well
> strive for consistency.

Actually, it's a noun, or just active instead of passive voice.
FWIW, the GCS uses present tense in all their examples.  But hey,
I'm not attached to that.

> I'm also not sure about the name LIBLTDL_DEP, as it is visibly different
> to the other two, and also to existing PREOPEN_DEPENDENCIES and  
> INFO_DEPS,
> and gnulibs use of the '_DEPENDENCIES' suffix. I'd prefer either  
> LTDLDEPS
> (vis LTDLINCL) or LIBLTDL_DEPENDENCIES for consistency with prior art.

Be it LTDLDEPS then.

> Actually, now that I think about it, the user doesn't see these as shell
> variables at all, but as make macros.  It might be better to simply  
> refer to them as make macros to avoid any confusion.

But that would then be another independent change, yes?  Because it not
only touches the issue I'm fixing now, but also the other instances of
"shell variables" that were there earlier, from which I merely copied.
Would you please write this patch for me?  Thanks.

> >+AT_CHECK([$MAKE -q main$EXEEXT || exit 1], [1], [ignore], [ignore])
> >+
> 
> -q might not work with some of the makes that we support.  Safer to use:
> 
>   AT_CHECK([$MAKE main$EXEEXT >/dev/null 2>&1 || exit 1], ...

'make -q' is POSIX and works with make implementations from AIX 4.3.3
and up, HP-UX 10.20 and up, IRIX 6.5, Tru64 4.0D, FreeBSD, NetBSD,
OpenBSD, GNU, Solaris 6 and up.  Do you know one that doesn't support
it?  It should be mentioned in the Autoconf manual.  Note 'make -q' is
not like 'grep -q'.

Cheers,
Ralf

2008-01-17  Ralf Wildenhues  <address@hidden>

        New variable LTDLDEPS for use in output_DEPENDENCIES.

        * libltdl/m4/ltdl.m4 (_LTDL_CONVENIENCE, _LTDL_INSTALLABLE)
        (LTDL_INIT): Set and substitute LTDLDEPS according to chosen
        method.
        * tests/configure-iface.at (installable libltdl)
        (--with-ltdl-include/lib, --with-included-ltdl): Test it.
        * doc/libtool.texi (Distributing libltdl): Document LTDLDEPS.
        * NEWS: Updated.
        Dependency bug exposed by Eric Blake in M4, found by Bob Proulx
        build daemon.

Index: NEWS
===================================================================
RCS file: /cvsroot/libtool/libtool/NEWS,v
retrieving revision 1.214
diff -u -r1.214 NEWS
--- NEWS        12 Jan 2008 17:00:51 -0000      1.214
+++ NEWS        17 Jan 2008 05:25:22 -0000
@@ -52,6 +52,7 @@
     The Libtool package itself builds libltdl nonrecursively.
   - The 'nonrecursive', 'recursive' and 'subproject' libltdl build
     modes are given as LTDL_INIT options.
+  - New make variable LTDLDEPS for use in output_DEPENDENCIES.
   - New multi-module-loader safe libltdl handle iteration APIs:
     lt_dlhandle_iterate, lt_dlhandle_fetch, lt_dlhandle_map.
   - New lt_dlinterface_register to maintain separation of concerns
Index: doc/libtool.texi
===================================================================
RCS file: /cvsroot/libtool/libtool/doc/libtool.texi,v
retrieving revision 1.236
diff -u -r1.236 libtool.texi
--- doc/libtool.texi    17 Jan 2008 05:05:59 -0000      1.236
+++ doc/libtool.texi    17 Jan 2008 05:25:25 -0000
@@ -4444,10 +4444,11 @@
 @option{--with-included-ltdl}.
 
 If an installed @code{libltdl} is found, then @code{LIBLTDL} is set to
-the link flags needed to use it, and @code{LTDLINCL} to the
-preprocessor flags needed to find the installed headers.  Note,
-however, that no version checking is performed.  You should manually
-check for the @code{libltdl} features you need in @file{configure.ac}:
+the link flags needed to use it, and @code{LTDLINCL} to the preprocessor
+flags needed to find the installed headers, and @code{LTDLDEPS} will
+be empty.  Note, however, that no version checking is performed.  You
+should manually check for the @code{libltdl} features you need in
address@hidden:
 
 @example
 LT_INIT([dlopen])
@@ -4588,7 +4589,7 @@
 to cause it to be built as a convenience library.  If you're not
 using automake, you will need to define @code{top_build_prefix},
 @code{top_builddir}, and @code{top_srcdir} in your makefile so that
address@hidden and @code{LTDLINCL} expand correctly.
address@hidden, @code{LTDLDEPS}, and @code{LTDLINCL} expand correctly.
 
 One advantage of the convenience library is that it is not installed,
 so the fact that you use @code{libltdl} will not be apparent to the
@@ -4612,42 +4613,46 @@
 to cause it to be built as an installable library.  If you're not
 using automake, you will need to define @code{top_build_prefix},
 @code{top_builddir} and @code{top_srcdir} in your makefile so that
address@hidden and @code{LTDLINCL} are expanded properly.
address@hidden, @code{LTDLDEPS}, and @code{LTDLINCL} are expanded
+properly.
 
 Be aware that you could overwrite another @code{libltdl} already
 installed to the same directory if you use this option.
 @end table
 @end defmac
 
-Whatever method you use, @samp{LTDL_INIT} will define both the shell
-variable @var{LIBLTDL} to the link flag that you should use to link
-with @code{libltdl}, and the shell variable @var{LTDLINCL} to the
-preprocessor flag that you should use to compile programs that
-include @file{ltdl.h}. So, when you want to link a program with
-libltdl, be it a convenience, installed or installable library, just
-use @samp{$(LTDLINCL)} for preprocessing and compilation, and
address@hidden(LIBLTDL)} for linking.
+Whatever method you use, @samp{LTDL_INIT} will define the shell variable
address@hidden to the link flag that you should use to link with
address@hidden, the shell variable @var{LTDLDEPS} to the files that
+can be used as a dependency in @file{Makefile} rules, and the shell
+variable @var{LTDLINCL} to the preprocessor flag that you should use to
+compile programs that include @file{ltdl.h}. So, when you want to link a
+program with libltdl, be it a convenience, installed or installable
+library, just use @samp{$(LTDLINCL)} for preprocessing and compilation,
+and @samp{$(LIBLTDL)} for linking.
 
 @itemize @bullet
 @item
-If your package is built using an installed version of
address@hidden, @var{LIBLTDL} will be set to the compiler flags
-needed to link against the installed library, and
address@hidden will be set to the compiler flags needed to find
-the @code{libltdl} header files.
+If your package is built using an installed version of @code{libltdl},
address@hidden will be set to the compiler flags needed to link against
+the installed library, @var{LTDLDEPS} will be empty, and @var{LTDLINCL}
+will be set to the compiler flags needed to find the @code{libltdl}
+header files.
 
 @item
 If your package is built using the convenience libltdl, @var{LIBLTDL}
-will be the pathname for the convenience version of libltdl
-(starting with @address@hidden@}/} or @address@hidden@}})
-and @var{LTDLINCL} will be @option{-I} followed by the directory that
-contains @file{ltdl.h} (starting with @address@hidden@}/}).
+and @var{LTDLDEPS} will be the pathname for the convenience version of
+libltdl (starting with @address@hidden@}/} or
address@hidden@address@hidden) and @var{LTDLINCL} will be @option{-I}
+followed by the directory that contains @file{ltdl.h} (starting with
address@hidden@address@hidden/}).
 
 @item
 If an installable version of the included @code{libltdl} is being
 built, its pathname starting with @address@hidden@}/} or
address@hidden@address@hidden, will be stored in @var{LIBLTDL}, and
address@hidden will be set just like in the case of convenience library.
address@hidden@address@hidden, will be stored in @var{LIBLTDL} and
address@hidden, and @var{LTDLINCL} will be set just like in the case of
+convenience library.
 @end itemize
 
 You should probably also use the @samp{dlopen} option to @code{LT_INIT}
@@ -4687,7 +4692,7 @@
 
 myprog_LDFLAGS = -export-dynamic
 myprog_LDADD = $(LIBLTDL) -dlopen self -dlopen foo1.la
-myprog_DEPENDENCIES = $(LIBLTDL) foo1.la
+myprog_DEPENDENCIES = $(LTDLDEPS) foo1.la
 ...
 @end example
 
Index: libltdl/m4/ltdl.m4
===================================================================
RCS file: /cvsroot/libtool/libtool/libltdl/m4/ltdl.m4,v
retrieving revision 1.41
diff -u -r1.41 ltdl.m4
--- libltdl/m4/ltdl.m4  16 Jan 2008 18:35:05 -0000      1.41
+++ libltdl/m4/ltdl.m4  17 Jan 2008 05:25:25 -0000
@@ -96,9 +96,11 @@
       ac_configure_args="$ac_configure_args --enable-ltdl-convenience" ;;
 esac
 LIBLTDL='_LT_BUILD_PREFIX'"${lt_ltdl_dir+$lt_ltdl_dir/}libltdlc.la"
+LTDLDEPS=$LIBLTDL
 LTDLINCL='-I${top_srcdir}'"${lt_ltdl_dir+/$lt_ltdl_dir}"
 
 AC_SUBST([LIBLTDL])
+AC_SUBST([LTDLDEPS])
 AC_SUBST([LTDLINCL])
 
 # For backwards non-gettext consistent compatibility...
@@ -165,16 +167,19 @@
 case $enable_ltdl_install in
   no) ac_configure_args="$ac_configure_args --enable-ltdl-install=no"
       LIBLTDL="-lltdl"
+      LTDLDEPS=
       LTDLINCL=
       ;;
   *)  enable_ltdl_install=yes
       ac_configure_args="$ac_configure_args --enable-ltdl-install"
       LIBLTDL='_LT_BUILD_PREFIX'"${lt_ltdl_dir+$lt_ltdl_dir/}libltdl.la"
+      LTDLDEPS=$LIBLTDL
       LTDLINCL='-I${top_srcdir}'"${lt_ltdl_dir+/$lt_ltdl_dir}"
       ;;
 esac
 
 AC_SUBST([LIBLTDL])
+AC_SUBST([LTDLDEPS])
 AC_SUBST([LTDLINCL])
 
 # For backwards non-gettext consistent compatibility...
@@ -282,6 +287,7 @@
        AC_DEFINE([HAVE_LTDL], [1],
          [Define this if a modern libltdl is already installed])
        LIBLTDL=-lltdl
+       LTDLDEPS=
        LTDLINCL=
        ;;
   ,no*,no,*)
@@ -289,6 +295,7 @@
        ;;
   *)   with_included_ltdl=no
        LIBLTDL="-L$with_ltdl_lib -lltdl"
+       LTDLDEPS=
        LTDLINCL="-I$with_ltdl_include"
        ;;
 esac
Index: tests/configure-iface.at
===================================================================
RCS file: /cvsroot/libtool/libtool/tests/configure-iface.at,v
retrieving revision 1.5
diff -u -r1.5 configure-iface.at
--- tests/configure-iface.at    12 Jan 2008 14:07:02 -0000      1.5
+++ tests/configure-iface.at    17 Jan 2008 05:25:25 -0000
@@ -132,6 +132,7 @@
 bin_PROGRAMS           = main
 main_LDFLAGS           = -dlopen libmodule.la
 main_LDADD             = $(LIBLTDL)
+main_DEPENDENCIES      = $(LTDLDEPS)
 ]])
 
 # Install everything!
@@ -144,6 +145,10 @@
 AT_CHECK([test -f $prefix/lib/libltdl.la])
 AT_CHECK([test -f $prefix/include/ltdl.h])
 
+# Check that main is rebuilt if libltdl.la is newer
+rm -f libltdl/libltdl.la
+AT_CHECK([$MAKE -q main$EXEEXT || exit 1], [1], [ignore], [ignore])
+
 AT_CLEANUP
 
 
@@ -196,6 +201,7 @@
 bin_PROGRAMS           = main
 main_LDFLAGS           = -dlopen libmodule.la
 main_LDADD             = $(LIBLTDL)
+main_DEPENDENCIES      = $(LTDLDEPS)
 ]])
 
 # We don't use 'libtoolize --ltdl', so that we get an error if the test
@@ -249,6 +255,7 @@
 bin_PROGRAMS           = main
 main_LDFLAGS           = -dlopen libmodule.la
 main_LDADD             = $(LIBLTDL)
+main_DEPENDENCIES      = $(LTDLDEPS)
 ]])
 
 LT_AT_BOOTSTRAP([--ltdl], [-I libltdl/m4], [], [--add-missing], [],
@@ -262,6 +269,10 @@
 ## previously installed system libltdl.
 LT_AT_NOINST_EXEC_CHECK([./main], [-dlopen libmodule.la], [], [expout], [])
 
+# Check that main is rebuilt if libltdlc.la is newer
+rm -f libltdl/libltdlc.la
+AT_CHECK([$MAKE -q main$EXEEXT || exit 1], [1], [ignore], [ignore])
+
 AT_CLEANUP
 
 dnl Be careful not to let the definition leak into other tests