279-gary-LT_CONFIG_LTDL_DIR.diff
Here is the first part of the patch series to fix the LT_WITH_INIT interfaces. Okay to commit? This changeset brings some sanity to tracking the directory name for libltdl sources. Old syntax still works, but the documentation now talks only about using the new LT_CONFIG_LTDL_DIR. If any of the old macros are still given a directory argument, it is set as before, only now autoconf will stop with an error if there is a conflict. NEWS |1 doc/libtool.texi | 74 +--- libltdl/configure.ac |1 libltdl/m4/ltdl.m4 | 225 --- libtoolize.m4sh | 74 +--- 5 files changed, 246 insertions(+), 129 deletions(-) Index: libtool--devo--1.0/NEWS === --- libtool--devo--1.0.orig/NEWS +++ libtool--devo--1.0/NEWS @@ -3,6 +3,7 @@ NEWS - list of user-visible changes betw New in 1.9h: 2005-??-??; CVS version 2.1a, Libtool team: * New tests for support of Automake subdir-objects. * Fix libltdl on static platforms. +* New LT_CONFIG_LTDL_DIR macro. * 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 between Index: libtool--devo--1.0/doc/libtool.texi === --- libtool--devo--1.0.orig/doc/libtool.texi +++ libtool--devo--1.0/doc/libtool.texi @@ -2243,12 +2243,11 @@ Display a help message and exit. @item --ltdl [EMAIL PROTECTED] Install libltdl in a the @var{TARGET-DIRECTORY-NAME} subdirectory of -your package. If you specify a subdirectory name, then you will need -to be careful to pass the same directory name to the autoconf macros: [EMAIL PROTECTED]; @code{LTDL_CONVENIENCE}; [EMAIL PROTECTED] (@pxref{Distributing libltdl}). Without -an argument, @samp{libltdl} is used as the default target directory -name. +your package. Normally, the directory is extracted from the argument +to @code{LT_CONFIG_LTDL_DIR} in @file{configure.ac}, though you can +also specify a subdirectory name here if you are not using Autoconf +for example. If @command{libtoolize} can't determine the target +directory, @samp{libltdl} is used as the default. @item --quiet @itemx -q @@ -4133,34 +4132,39 @@ release of libltdl. }. Having made the macros available, you must add a call to the @samp{LT_WITH_LTDL} macro to your package's @file{configure.ac} to perform the configure time checks required to build the library -correctly. This method has problems if you then try to link the -package binaries with an installed libltdl, or a library that depends -on libltdl: you will have problems with duplicate symbol definitions. +correctly. Unfortunately, this method has problems if you then try to +link the package binaries with an installed libltdl, or a library that +depends on libltdl, because of the duplicate symbol definitions. +Ensuring that only one copy of the libltdl sources are linked into any +program is left as an exercise for the reader. + [EMAIL PROTECTED] LT_CONFIG_LTDL_DIR (@var{DIRECTORY}) +Declare @var{DIRECTORY} to be the location of the @code{libltdl} +source files, for @command{libtoolize --ltdl} to place +them. @xref{Invoking libtoolize}, for more details. [EMAIL PROTECTED] defmac [EMAIL PROTECTED] LT_WITH_LTDL (@var{DIRECTORY}) [EMAIL PROTECTED] LT_WITH_LTDL Add the @option{--with-included-ltdl} option to the @file{configure} -script. By default, this macro will try to build @code{libltdl} in -a subdirectory named @file{libltdl}, which is where [EMAIL PROTECTED] --ltdl} will place the files unless directed -differently. @xref{Invoking libtoolize}, for how to do that. -Otherwise, use @var{DIRECTORY} to pass the location of the [EMAIL PROTECTED] source files. +script. This option will then allow the person who builds your +package to choose between linking against an already installed [EMAIL PROTECTED] (@option{--without-included-ltdl}), or the sources +shipped with the package in the subdirectory named by [EMAIL PROTECTED] (@option{--with-included-ltdl}). @end defmac [EMAIL PROTECTED] LTDL_INSTALLABLE (@var{DIRECTORY}) [EMAIL PROTECTED] LTDL_INSTALLABLE If there is an installed @code{libltdl}, then set @code{LIBLTDL} to the link flags needed to use it, and @code{LTDLINCL} to the preprocessor flags needed to find the installed headers. Otherwise, set them to point into an included version of @code{libltdl}, and install that. -By default, this macro will pass options to the @file{libltdl} -subdirectory @file{configure} to cause it to be built as an -installable library. If you named the @code{libltdl} source directory -differently, then pass the directory name relative to [EMAIL PROTECTED] as @var{DIRECTORY}. If you're not using automake, -you will need to define @code{top_builddir} and @code{top_srcdir} in -your makefile so that @code{LIBLTDL} an
279-gary-LT_CONFIG_LTDL_DIR.diff
Hallo Ralf, Thanks for the review. This fixes the inline LT_CONFIG_LTDL_DIR problem you report, but I'm not sure I understand how the aclocal.m4 problem came to be: Ralf Wildenhues wrote: > ++ cat aclocal.m4 ./foo/libltdl/configure.ac How did configure_ac get set to ./foo/libltdl/configure.ac? I suppose I need to duplicate that mechanism to find the correct aclocal.m4. This changeset brings some sanity to tracking the directory name for libltdl sources. Old syntax still works, but the documentation now talks only about using the new LT_CONFIG_LTDL_DIR. If any of the old macros are still given a directory argument, it is set as before, only now autoconf will stop with an error if there is a conflict. NEWS |1 doc/libtool.texi | 74 +--- libltdl/configure.ac |1 libltdl/m4/ltdl.m4 | 231 --- libtoolize.m4sh | 75 +--- 5 files changed, 253 insertions(+), 129 deletions(-) Index: libtool--devo--1.0/ChangeLog from Gary V. Vaughan <[EMAIL PROTECTED]> * libltdl/m4/ltdl.m4: Bump serial as we are changing the interface. (LT_CONFIG_LTDL_DIR): New macro to centralise setting the subdirectory used for libltdl. (LTDL_CONVENIENCE): Continue backwards compatibility support for declaring the libltdl source subdirectory with an argument, but defer to LT_CONFIG_LTDL_DIR. (AC_LIBLTDL_CONVENIENCE): Adjust to upgrade to the new style. (LTDL_INSTALLABLE, AC_LIBLTDL_INSTALLABLE): Ditto. (LTDL_INIT): lt_ltdl_dir is set by LT_CONFIG_LTDL_DIR now, and even `./' needs trailing slashes trimming! If the user didn't upgrade their configure.ac yet, call LT_CONFIG_LTDL_DIR for them. * libtoolize.m4sh (func_scan_files): If --ltdl option is given without a directory argument, use the value from LT_CONFIG_LTDL_DIR; if the argument is given, and there is also a value in LT_CONFIG_LTDL_DIR, ensure they are the same. * libltdl/configure.ac: Use it. * NEWS: Updated. Index: libtool--devo--1.0/NEWS === --- libtool--devo--1.0.orig/NEWS +++ libtool--devo--1.0/NEWS @@ -3,6 +3,7 @@ NEWS - list of user-visible changes betw New in 1.9h: 2005-??-??; CVS version 2.1a, Libtool team: * New tests for support of Automake subdir-objects. * Fix libltdl on static platforms. +* New LT_CONFIG_LTDL_DIR macro. * 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 between Index: libtool--devo--1.0/doc/libtool.texi === --- libtool--devo--1.0.orig/doc/libtool.texi +++ libtool--devo--1.0/doc/libtool.texi @@ -2243,12 +2243,11 @@ Display a help message and exit. @item --ltdl [EMAIL PROTECTED] Install libltdl in a the @var{TARGET-DIRECTORY-NAME} subdirectory of -your package. If you specify a subdirectory name, then you will need -to be careful to pass the same directory name to the autoconf macros: [EMAIL PROTECTED]; @code{LTDL_CONVENIENCE}; [EMAIL PROTECTED] (@pxref{Distributing libltdl}). Without -an argument, @samp{libltdl} is used as the default target directory -name. +your package. Normally, the directory is extracted from the argument +to @code{LT_CONFIG_LTDL_DIR} in @file{configure.ac}, though you can +also specify a subdirectory name here if you are not using Autoconf +for example. If @command{libtoolize} can't determine the target +directory, @samp{libltdl} is used as the default. @item --quiet @itemx -q @@ -4133,34 +4132,39 @@ release of libltdl. }. Having made the macros available, you must add a call to the @samp{LT_WITH_LTDL} macro to your package's @file{configure.ac} to perform the configure time checks required to build the library -correctly. This method has problems if you then try to link the -package binaries with an installed libltdl, or a library that depends -on libltdl: you will have problems with duplicate symbol definitions. +correctly. Unfortunately, this method has problems if you then try to +link the package binaries with an installed libltdl, or a library that +depends on libltdl, because of the duplicate symbol definitions. +Ensuring that only one copy of the libltdl sources are linked into any +program is left as an exercise for the reader. + [EMAIL PROTECTED] LT_CONFIG_LTDL_DIR (@var{DIRECTORY}) +Declare @var{DIRECTORY} to be the location of the @code{libltdl} +source files, for @command{libtoolize --ltdl} to place +them. @xref{Invoking libtoolize}, for more details. [EMAIL PROTECTED] defmac [EMAIL PROTECTED] LT_WITH_LTDL (@var{DIRECTORY}) [EMAIL PROTECTED] LT_WITH_LTDL Add the @option{--with-included-ltdl} option to the @file{configure} -script. By default, this macro will try to build @c
Re: 279-gary-LT_CONFIG_LTDL_DIR.diff
ChangeLog: Index: libtool--devo--1.0/ChangeLog from Gary V. Vaughan <[EMAIL PROTECTED]> * libltdl/m4/ltdl.m4: Bump serial as we are changing the interface. (LT_CONFIG_LTDL_DIR): New macro to centralise setting the subdirectory used for libltdl. (LTDL_CONVENIENCE): Continue backwards compatibility support for declaring the libltdl source subdirectory with an argument, but defer to LT_CONFIG_LTDL_DIR. (AC_LIBLTDL_CONVENIENCE): Adjust to upgrade to the new style. (LTDL_INSTALLABLE, AC_LIBLTDL_INSTALLABLE): Ditto. (LTDL_INIT): lt_ltdl_dir is set by LT_CONFIG_LTDL_DIR now, and even `./' needs trailing slashes trimming! If the user didn't upgrade their configure.ac yet, call LT_CONFIG_LTDL_DIR for them. * libtoolize.m4sh (func_scan_files): If --ltdl option is given without a directory argument, use the value from LT_CONFIG_LTDL_DIR; if the argument is given, and there is also a value in LT_CONFIG_LTDL_DIR, ensure they are the same. * libltdl/configure.ac: Use it. * NEWS: Updated. Sorry, Gary. -- Gary V. Vaughan ())_. [EMAIL PROTECTED],gnu.org} Research Scientist ( '/ http://tkd.kicks-ass.net GNU Hacker / )= http://www.gnu.org/software/libtool Technical Author `(_~)_ http://sources.redhat.com/autobook signature.asc Description: OpenPGP digital signature
Re: 279-gary-LT_CONFIG_LTDL_DIR.diff
Hi Gary, * Gary V. Vaughan wrote on Fri, Sep 23, 2005 at 02:26:24PM CEST: > Here is the first part of the patch series to fix the LT_WITH_INIT > interfaces. Okay to commit? > > This changeset brings some sanity to tracking the directory name for > libltdl sources. Old syntax still works, but the documentation now > talks only about using the new LT_CONFIG_LTDL_DIR. If any of the old > macros are still given a directory argument, it is set as before, only > now autoconf will stop with an error if there is a conflict. This patch doesn't look so bad at first sight. However, there are some nontrivial merge conflicts in ltdl.m4 against CVS HEAD. Could you be bothered to repost a patch against CVS HEAD plain, or, alternatively, just post how ltdl.m4 looks after application of the patch? Thank you. I'll really try to get through all your (and a couple of Peter's) pending patches this weekend, sorry. Cheers, Ralf
Re: 279-gary-LT_CONFIG_LTDL_DIR.diff
Ralf Wildenhues wrote: Hi Gary, Hallo Ralf, * Gary V. Vaughan wrote on Fri, Sep 23, 2005 at 02:26:24PM CEST: Here is the first part of the patch series to fix the LT_WITH_INIT interfaces. Okay to commit? This changeset brings some sanity to tracking the directory name for libltdl sources. Old syntax still works, but the documentation now talks only about using the new LT_CONFIG_LTDL_DIR. If any of the old macros are still given a directory argument, it is set as before, only now autoconf will stop with an error if there is a conflict. This patch doesn't look so bad at first sight. However, there are some nontrivial merge conflicts in ltdl.m4 against CVS HEAD. Could you be bothered to repost a patch against CVS HEAD plain, or, alternatively, just post how ltdl.m4 looks after application of the patch? It applies after my other pending patches. If the conflicts are non-trivial, probably best to wait until we've shepherded the other patches through first... otherwise it's like having another patch to test (the one that applies to HEAD now, and another that applies to HEAD after the earlier patches are applied). Even with arch and quilt, reordering patches that touch the same files is a difficult and error-prone process. I can post you a distchecked tarball of the whole lot if that will help your testing? Thank you. I'll really try to get through all your (and a couple of Peter's) pending patches this weekend, sorry. Not a problem. We are all plenty busy, it doesn't matter in the least if it takes a while to polish everything in the queue. If Peter and I get bored of waiting, we can always review each others' patches ;-) Cheers, Gary. -- Gary V. Vaughan ())_. [EMAIL PROTECTED],gnu.org} Research Scientist ( '/ http://tkd.kicks-ass.net GNU Hacker / )= http://www.gnu.org/software/libtool Technical Author `(_~)_ http://sources.redhat.com/autobook signature.asc Description: OpenPGP digital signature
Re: 279-gary-LT_CONFIG_LTDL_DIR.diff
Hi Gary, * Gary V. Vaughan wrote on Fri, Sep 23, 2005 at 02:26:24PM CEST: > Here is the first part of the patch series to fix the LT_WITH_INIT > interfaces. Okay to commit? No, unfortunately. Observed bug below. > This changeset brings some sanity to tracking the directory name for > libltdl sources. Old syntax still works, but the documentation now > talks only about using the new LT_CONFIG_LTDL_DIR. If any of the old > macros are still given a directory argument, it is set as before, only > now autoconf will stop with an error if there is a conflict. Using libtoolize with this patch on a third-party package (which has a subpackage that uses libltdl): $ eval `cat aclocal.m4 ./foo/libltdl/configure.ac | sed "$my_sed_traces" ` ++ sed 's,#.*$,,; s,^dnl .*$,,; s, dnl .*$,,; s,^.*AC_REQUIRE(.*$,,; s,^.*m4_require(.*$,,; s,^.*m4_define(.*$,,; s,^.*A[CU]_DEFUN(.*$,,; s,^.*m4_defun(.*$,,; /AC_CONFIG_AUX_DIR(/ { s,^.*AC_CONFIG_AUX_DIR([[ ]*\([^])]*\).*$,auxdir=\1,; p; }; /AC_CONFIG_MACRO_DIR(/ { s,^.*AC_CONFIG_MACRO_DIR([[ ]*\([^])]*\).*$,macrodir=\1,; p; }; /LT_CONFIG_LTDL_DIR(/ { s,^.*LT_CONFIG_LTDL_DIR([[ ]*\([^])]*\).*$,ac_ltdldir=\1,; p; }; /A[CM]_PROG_LIBTOOL/ { s,^.*$,seen_libtool=:,; p; }; /LT_INIT/{ s,^.*$,seen_libtool=:,; p; }; /LTDL_INIT/ { s,^.*$,seen_ltdl=:,; p; }; /LT_WITH_LTDL/ { s,^.*$,seen_ltdl=:,; p; }; /AC_LIB_LTDL/{ s,^.*$,seen_ltdl=:,; p; }; /AC_WITH_LTDL/ { s,^.*$,seen_ltdl=:,; p; }; d' ++ cat aclocal.m4 ./foo/libltdl/configure.ac + eval seen_libtool=: seen_libtool=: seen_ltdl=: seen_ltdl=: 'ac_ltdldir=$1' 'ac_ltdldir=$1' seen_ltdl=: 'ac_ltdldir=$1' 'ac_ltdldir=$1' ac_ltdldir=libltdl seen_ltdl=: seen_ltdl=: seen_ltdl=: 'ac_ltdldir=$1' 'ac_ltdldir=m4_default([$1' seen_ltdl=: seen_libtool=: seen_libtool=: seen_libtool=: seen_libtool=: seen_libtool=: seen_libtool=: seen_libtool=: seen_libtool=: seen_libtool=: seen_libtool=: seen_libtool=: seen_libtool=: seen_libtool=: auxdir=config macrodir=m4 ac_ltdldir=. seen_libtool=: seen_ltdl=: bash: syntax error near unexpected token `(' First, the aclocal.m4 is the wrong file (should've been foo/libltdl/aclocal.m4). Second, it's picking up the stuff from ltdl.m4 that ended up in aclocal.m4. I believe you can fix this by using an internal macro _LT_CONFIG_LTDL_DIR (with LT_CONFIG_LTDL_DIR then being only a wrapper around it) and not calling the external one from inside ltdl.m4. I can check more after this is fixed. Cheers, and thanks for your work on this, Ralf
Re: 279-gary-LT_CONFIG_LTDL_DIR.diff
Hi Gary, * Gary V. Vaughan wrote on Tue, Sep 27, 2005 at 03:36:43PM CEST: > > Thanks for the review. This fixes the inline LT_CONFIG_LTDL_DIR problem > you report, but I'm not sure I understand how the aclocal.m4 problem came > to be: This patch looks much better, but still has a bug, see below (The aclocal.m4 problem is orthogonal; let's discuss it elsewhere). > This changeset brings some sanity to tracking the directory name for > libltdl sources. Old syntax still works, but the documentation now > talks only about using the new LT_CONFIG_LTDL_DIR. If any of the old > macros are still given a directory argument, it is set as before, only > now autoconf will stop with an error if there is a conflict. First, LT_WITH_LTDL and --with-included-ltdl leads to LIBLTDL = ${top_builddir}/ltdl//libltdlc.la which is just a tad ugly (because of the doubled slash). Can we avoid it, and just omit all those trailing slashes added? Then, there's a bug: --without-included-ltdl leads to | checking for ltdl.h... yes | checking for lt_dlcaller_register in -lltdl... yes | checking whether to use included libltdl... no and | LIBLTDL = -lltdl ... | LTDLINCL = -I${top_srcdir}/ltdl/ which is wrong -- the include path should not be changed here. I'll provide a few more data points to reproduce this (it's ok if you want to fix this in a separate patch). cat >configure.ac < Index: libtool--devo--1.0/ChangeLog > from Gary V. Vaughan <[EMAIL PROTECTED]> > * libltdl/m4/ltdl.m4: Bump serial as we are changing the > interface. > (LT_CONFIG_LTDL_DIR): New macro to centralise setting the > subdirectory used for libltdl. > (LTDL_CONVENIENCE): Continue backwards compatibility support for > declaring the libltdl source subdirectory with an argument, but > defer to LT_CONFIG_LTDL_DIR. > (AC_LIBLTDL_CONVENIENCE): Adjust to upgrade to the new style. > (LTDL_INSTALLABLE, AC_LIBLTDL_INSTALLABLE): Ditto. > (LTDL_INIT): lt_ltdl_dir is set by LT_CONFIG_LTDL_DIR now, and > even `./' needs trailing slashes trimming! If the user didn't > upgrade their configure.ac yet, call LT_CONFIG_LTDL_DIR for them. > * libtoolize.m4sh (func_scan_files): If --ltdl option is given > without a directory argument, use the value from > LT_CONFIG_LTDL_DIR; if the argument is given, and there is also a > value in LT_CONFIG_LTDL_DIR, ensure they are the same. > * libltdl/configure.ac: Use it. > * NEWS: Updated. > > Index: libtool--devo--1.0/NEWS > === > --- libtool--devo--1.0.orig/NEWS > +++ libtool--devo--1.0/NEWS > @@ -3,6 +3,7 @@ NEWS - list of user-visible changes betw > New in 1.9h: 2005-??-??; CVS version 2.1a, Libtool team: > * New tests for support of Automake subdir-objects. > * Fix libltdl on static platforms. > +* New LT_CONFIG_LTDL_DIR macro. > * 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 between > Index: libtool--devo--1.0/doc/libtool.texi > === > --- libtool--devo--1.0.orig/doc/libtool.texi > +++ libtool--devo--1.0/doc/libtool.texi > @@ -2243,12 +2243,11 @@ Display a help message and exit. > > @item --ltdl [EMAIL PROTECTED] > Install libltdl in a the @var{TARGET-DIRECTORY-NAME} subdirectory of > -your package. If you specify a subdirectory name, then you will need > -to be careful to pass the same directory name to the autoconf macros: > [EMAIL PROTECTED]; @code{LTDL_CONVENIENCE}; > [EMAIL PROTECTED] (@pxref{Distributing libltdl}). Without > -an argument, @samp{libltdl} is used as the default target directory > -name. > +your package. Normally, the directory is extracted from the argument > +to @code{LT_CONFIG_LTDL_DIR} in @file{configure.ac}, though you can > +also specify a subdirectory name here if you are not using Autoconf > +for example. If @command{libtoolize} can't determine the target > +directory, @samp{libltdl} is used as the default. > > @item --quiet > @itemx -q > @@ -4133,34 +4132,39 @@ release of libltdl. > }. Having made the macros available, you must add a call to the > @samp{LT_WITH_LTDL} macro to your package's @file{configure.ac} to > perform the configure time checks required to build the library > -correctly. This method has problems if you then try to link the > -package binaries with an installed libltdl, or a library that depends > -on libltdl: you will have problems with duplicate symbol definitions. > +correctly. Unfortunately, this method has problems if you then try to > +link the package binaries with an installed libltdl, or a library that > +depends on libltdl, because of the duplicate symbol definitions. > +Ensuring that only one copy of the libltdl sources are linked into any > +program is left as an exercise
Re: 279-gary-LT_CONFIG_LTDL_DIR.diff
Ralf Wildenhues wrote: Hi Gary, Hallo Ralf, Thanks for the review. New version of the patch attached. * Gary V. Vaughan wrote on Tue, Sep 27, 2005 at 03:36:43PM CEST: I'm not sure I understand how the aclocal.m4 problem came to be: (The aclocal.m4 problem is orthogonal; let's discuss it elsewhere). Okay. I'd like to fix this in a separate patch. First, LT_WITH_LTDL and --with-included-ltdl leads to LIBLTDL = ${top_builddir}/ltdl//libltdlc.la which is just a tad ugly (because of the doubled slash). Can we avoid it, and just omit all those trailing slashes added? Yes. Fixed. Then, there's a bug: --without-included-ltdl leads to | checking for ltdl.h... yes | checking for lt_dlcaller_register in -lltdl... yes | checking whether to use included libltdl... no and | LIBLTDL = -lltdl ... | LTDLINCL = -I${top_srcdir}/ltdl/ which is wrong -- the include path should not be changed here. I'll provide a few more data points to reproduce this (it's ok if you want to fix this in a separate patch). I've fixed it here. Nice catch btw. :-) +correctly. Unfortunately, this method has problems if you then try to +link the package binaries with an installed libltdl, or a library that +depends on libltdl, because of the duplicate symbol definitions. +Ensuring that only one copy of the libltdl sources are linked into any +program is left as an exercise for the reader. Hmm. You meant "so don't do that then", right? IOW: linking simultaneously against two different libltdl versions is bad, and linking simultaneously against an installed and a local libltdl (be that convenience or not) is bad, too. Improved the wording. Thanks. + [m4_fatal([multiple libltdl directories: `]_LTDL_DIR[', `$1`])])]) "quotes" don't match: `$1'. Ahh.. D'oh. I've changed this a bit in the new patch, but fixed the quote too. Thanks. + # If the included ltdl is not to be used. then Use the wrong punctuation and capitalization. Fixed. +# -l, --ltdl[=DIR] install libltdl sources [[default: libltdl]] AFAICS the double quotes are superfluous. Yes, they were. Removed. Also, I'm not so sure `-l' as abbreviation is such a good idea for `--ltdl'. What if we later want to --list something, or have an idea of something local or long? --ltdl is not much to type, and the list of short option abbreviations in GCS does not list a similar precedent. Okay. Removed -l. +# If neither --ltdl nor LT_CONFIG_LTDL_DIR are specified, default to +# `libltdl'. If both are specified, they must be the same. Otherwise, +# take the one that is given! (If LT_CONFIG_LTDL_DIR is not specified +# we suggest adding it later in this code.) +case x$ac_ltdldir,x$ltdldir in + x,x) ltdldir=libltdl ;; + x*,x)ltdldir=$ac_ltdldir ;; + x,x*)ltdldir=$ltdldir;; + *) +test x"$ac_ltdldir" = x"$ltdldir" || \ + func_fatal_error "--ltdl='$ltdldir' does not match LT_CONFIG_LTDL_DIR($ac_ltdldir)" Hmm. If I LT_CONFIG_LTDL_DIR(ltdl/) and libtoolize --ltdl=ltdl/ then it'll barf. Probably ok, I just noticed it. Oh, good point. Fixed. Okay to commit? Cheers, Gary. -- Gary V. Vaughan ())_. [EMAIL PROTECTED],gnu.org} Research Scientist ( '/ http://tkd.kicks-ass.net GNU Hacker / )= http://www.gnu.org/software/libtool Technical Author `(_~)_ http://sources.redhat.com/autobook This changeset brings some sanity to tracking the directory name for libltdl sources. Old syntax still works, but the documentation now talks only about using the new LT_CONFIG_LTDL_DIR. If any of the old macros are still given a directory argument, it is set as before, only now autoconf will stop with an error if there is a conflict. NEWS |1 doc/libtool.texi | 74 +--- libltdl/configure.ac |1 NEWS |1 doc/libtool.texi | 76 --- libltdl/configure.ac |1 libltdl/m4/ltdl.m4 | 251 --- libtoolize.m4sh | 83 +--- 5 files changed, 271 insertions(+), 141 deletions(-) Index: libtool--devo--1.0/ChangeLog from Gary V. Vaughan <[EMAIL PROTECTED]> * libltdl/m4/ltdl.m4: Bump serial as we are changing the interface. (LT_CONFIG_LTDL_DIR): New macro to centralise setting the subdirectory used for libltdl. (LTDL_CONVENIENCE): Continue backwards compatibility support for declaring the libltdl source subdirectory with an argument, but defer to LT_CONFIG_LTDL_DIR. (AC_LIBLTDL_CONVENIENCE): Adjust to upgrade to the new style. (LTDL_INSTALLABLE, AC_LIBLTDL_INSTALLABLE): Ditto. (LTDL_INIT): lt_ltdl_dir is set by LT_CONFIG_LTDL_DIR now, and even `./' needs trailing slashes trimming! If the user didn't upgrade their configure.ac yet, call LT_CONFIG_LTDL_DIR for them. * libtoolize.m4sh (func_scan_files): If --ltdl option is given without a directory argument, use the value from
Re: 279-gary-LT_CONFIG_LTDL_DIR.diff
Hi Gary, * Gary V. Vaughan wrote on Thu, Sep 29, 2005 at 11:19:41AM CEST: > Ralf Wildenhues wrote: > >* Gary V. Vaughan wrote on Tue, Sep 27, 2005 at 03:36:43PM CEST: > >>I'm not sure I understand how the aclocal.m4 problem came > >>to be: > > > >(The aclocal.m4 problem is orthogonal; let's discuss it elsewhere). > > Okay. I'd like to fix this in a separate patch. I had previously thought this wasn't a Libtool bug at all.. *snip* > >Then, there's a bug: --without-included-ltdl leads to > >| checking for ltdl.h... yes > >| checking for lt_dlcaller_register in -lltdl... yes > >| checking whether to use included libltdl... no > > > >and > >| LIBLTDL = -lltdl > >... > >| LTDLINCL = -I${top_srcdir}/ltdl/ > > > >which is wrong -- the include path should not be changed here. > >I'll provide a few more data points to reproduce this (it's ok if you > >want to fix this in a separate patch). > > I've fixed it here. Nice catch btw. :-) Thank you. *big snip of all fixed items* > Okay to commit? I really don't want this to be a neverending story, ;) but there are some more issues. - One is the following (only happens if sub/ltdl does not exist): $ libtoolize --copy --ltdl *snip* libtoolize: copying file `sub/ltdl/config/ltmain.sh' libtoolize: `./ltmain.sh' is already up to date. libtoolize: putting macros in AC_CONFIG_MACRO_DIR, `sub/ltdl/m4'. libtoolize: copying file `sub/ltdl/m4/libtool.m4' /bin/sed: can't read sub/ltdl/m4/argz.m4: No such file or directory /bin/sed: can't read sub/ltdl/m4/ltdl.m4: No such file or directory I believe it is fixed with the following patch. OK to apply? (By the way, the use of global variables prefixed with my_ really sucks here; I also needed some time to realize that you are actually not using them wrongly.) * libtoolize.m4sh (func_included_files): Do not recurse non-existent files. Index: libtoolize.m4sh === RCS file: /cvsroot/libtool/libtool/libtoolize.m4sh,v retrieving revision 1.34 diff -u -r1.34 libtoolize.m4sh --- libtoolize.m4sh 27 Sep 2005 13:09:20 - 1.34 +++ libtoolize.m4sh 29 Sep 2005 14:04:51 - @@ -453,13 +500,13 @@ if test -f "$my_searchfile"; then $ECHO "X$my_searchfile" | $Xsed -fi -# Only recurse when we don't care if all the variables we use get -# trashed, since they are in global scope. -for my_filename in `$SED "$my_sed_include" "$my_searchfile"`; do - func_included_files $my_filename -done + # Only recurse when we don't care if all the variables we use get + # trashed, since they are in global scope. + for my_filename in `$SED "$my_sed_include" "$my_searchfile"`; do + func_included_files $my_filename + done +fi } - The other problems are connected to the AC_CONFIG_SUBDIRS call in LT_WITH_LTDL, is both wrong there, and does not work. Wrong in the sense that, should non-subpackage libltdl ever work, it should still be possible to use LT_WITH_LTDL (this is what Bob complained about originally). IOW: the decision whether libltdl is to be a subpackage or not must be done in a new macro. And the default should of course be that libltdl _is_ a subpackge (backwards compatible). This may be fixed in an another patch though, it's not a regression introduced by this one. The other thing is, that calling toplevel configure does not recurse into the subdir: To reproduce: cat >configure.ac
Re: 279-gary-LT_CONFIG_LTDL_DIR.diff
Ralf Wildenhues wrote: Hi Gary, Hallo Ralf! I think I can discount the following points... plus an okay for your patch... I'll repost 279 later when I've worked out the remaining known bugs. * Gary V. Vaughan wrote on Thu, Sep 29, 2005 at 11:19:41AM CEST: Ralf Wildenhues wrote: * Gary V. Vaughan wrote on Tue, Sep 27, 2005 at 03:36:43PM CEST: I'm not sure I understand how the aclocal.m4 problem came to be: (The aclocal.m4 problem is orthogonal; let's discuss it elsewhere). Okay. I'd like to fix this in a separate patch. I had previously thought this wasn't a Libtool bug at all.. Agreed. I had thought you were saying it was a bug in my patch. But now that I've looked at it, I think it is either a bug in the package you tested with, or a stale file picked up by mistake somewhere in the bootstrap process. but there are some more issues. - One is the following (only happens if sub/ltdl does not exist): $ libtoolize --copy --ltdl *snip* libtoolize: copying file `sub/ltdl/config/ltmain.sh' libtoolize: `./ltmain.sh' is already up to date. libtoolize: putting macros in AC_CONFIG_MACRO_DIR, `sub/ltdl/m4'. libtoolize: copying file `sub/ltdl/m4/libtool.m4' /bin/sed: can't read sub/ltdl/m4/argz.m4: No such file or directory /bin/sed: can't read sub/ltdl/m4/ltdl.m4: No such file or directory I believe it is fixed with the following patch. OK to apply? > > * libtoolize.m4sh (func_included_files): Do not recurse > non-existent files. Looks okay to me. (By the way, the use of global variables prefixed with my_ really sucks here; I also needed some time to realize that you are actually not using them wrongly.) Patches always welcome ;-) - The other problems are connected to the AC_CONFIG_SUBDIRS call in LT_WITH_LTDL, is both wrong there, and does not work. Wrong in the sense that, should non-subpackage libltdl ever work, it should still be possible to use LT_WITH_LTDL (this is what Bob complained about originally). IOW: the decision whether libltdl is to be a subpackage or not must be done in a new macro. And the default should of course be that libltdl _is_ a subpackge (backwards compatible). This may be fixed in an another patch though, it's not a regression introduced by this one. This patch is just to introduce LT_CONFIG_LTDL_DIR without regressions. The rest of the fixes for LT_WITH_LTDL are yet to be split into separate patches and posted for review. - The fifth thing is, that I can't seem to disable the included ltdl: --without-included-ltdl --with-included-ltdl=no -without-included-ltdl -with-included-ltdl=no all end up with | checking whether to use included libltdl... yes I believe this worked in the last iteration of your patch. The previous iteration was checking for lt_dlcaller_register which is a different API to what we have now. This patch now checks for lt_dlinterface_register, which I guess your installed libltdl doesn't have? For clarity, I've changed the messages to read something like: checking for lt_dlinterface_register in -lltdl... no checking whether to use included libltdl... yes It's a twisted maze. :-/ That's why it's taken me a month to fix it :-( At least breaking it into pieces for the commit is giving it an excellent review though. Thanks! Cheers, Gary. -- Gary V. Vaughan ())_. [EMAIL PROTECTED],gnu.org} Research Scientist ( '/ http://tkd.kicks-ass.net GNU Hacker / )= http://www.gnu.org/software/libtool Technical Author `(_~)_ http://sources.redhat.com/autobook signature.asc Description: OpenPGP digital signature
Re: 279-gary-LT_CONFIG_LTDL_DIR.diff
Hi Gary, * Gary V. Vaughan wrote on Thu, Sep 29, 2005 at 06:32:32PM CEST: > Ralf Wildenhues wrote: > > >I believe it is fixed with the following patch. OK to apply? > > > > * libtoolize.m4sh (func_included_files): Do not recurse > > non-existent files. > > Looks okay to me. Applied. > >(By the way, the use of global variables prefixed with my_ really sucks > >here; I also needed some time to realize that you are actually not using > >them wrongly.) > > Patches always welcome ;-) I know. > This patch is just to introduce LT_CONFIG_LTDL_DIR without regressions. > The rest of the fixes for LT_WITH_LTDL are yet to be split into separate > patches and posted for review. OK. Good point. > > -with-included-ltdl=no > >all end up with > >| checking whether to use included libltdl... yes > > > >I believe this worked in the last iteration of your patch. > > The previous iteration was checking for lt_dlcaller_register which is a > different API to what we have now. This patch now checks for > lt_dlinterface_register, which I guess your installed libltdl doesn't > have? D'oh, d'oh, d'oh. > For clarity, I've changed the messages to read something like: > > checking for lt_dlinterface_register in -lltdl... no > checking whether to use included libltdl... yes Thank you! Man, did I go blind or what.. > >It's a twisted maze. :-/ > > That's why it's taken me a month to fix it :-( At least breaking it into > pieces for the commit is giving it an excellent review though. Thanks! Surely. Cheers, Ralf
Re: 279-gary-LT_CONFIG_LTDL_DIR.diff
Ralf Wildenhues wrote: Hi Gary, Hallo Ralf, Thanks for the re-review ;-) Okay to commit the attached? I really don't want this to be a neverending story, ;) but there are some more issues. Not at all. Here's a possible reason: grep _LTDL_DIR Makefile configure | Makefile:subdirs = _LTDL_DIR | configure:ac_subdirs_all='_LTDL_DIR' | configure:subdirs="$subdirs _LTDL_DIR" Yep, that was the reason for all the pertinent failures. Fixed in this version. I've made your test case into a new Autotest case, which I'll post separately. What is the status of this patch btw?: 277-gary-rename-remaining-troublesome-ltdl-apis.diff Cheers, Gary. -- Gary V. Vaughan ())_. [EMAIL PROTECTED],gnu.org} Research Scientist ( '/ http://tkd.kicks-ass.net GNU Hacker / )= http://www.gnu.org/software/libtool Technical Author `(_~)_ http://sources.redhat.com/autobook This changeset brings some sanity to tracking the directory name for libltdl sources. Old syntax still works, but the documentation now talks only about using the new LT_CONFIG_LTDL_DIR. If any of the old macros are still given a directory argument, it is set as before, only now autoconf will stop with an error if there is a conflict. NEWS |1 doc/libtool.texi | 74 +--- libltdl/configure.ac |1 NEWS |1 doc/libtool.texi | 76 --- libltdl/configure.ac |1 libltdl/m4/ltdl.m4 | 255 --- libtoolize.m4sh | 83 +--- 5 files changed, 275 insertions(+), 141 deletions(-) Index: libtool--devo--1.0/ChangeLog from Gary V. Vaughan <[EMAIL PROTECTED]> * libltdl/m4/ltdl.m4: Bump serial as we are changing the interface. (LT_CONFIG_LTDL_DIR): New macro to centralise setting the subdirectory used for libltdl. (LTDL_CONVENIENCE): Continue backwards compatibility support for declaring the libltdl source subdirectory with an argument, but defer to LT_CONFIG_LTDL_DIR. (AC_LIBLTDL_CONVENIENCE): Adjust to upgrade to the new style. (LTDL_INSTALLABLE, AC_LIBLTDL_INSTALLABLE): Ditto. (LTDL_INIT): lt_ltdl_dir is set by LT_CONFIG_LTDL_DIR now, and even `./' needs trailing slashes trimming! If the user didn't upgrade their configure.ac yet, call LT_CONFIG_LTDL_DIR for them. * libtoolize.m4sh (func_scan_files): If --ltdl option is given without a directory argument, use the value from LT_CONFIG_LTDL_DIR; if the argument is given, and there is also a value in LT_CONFIG_LTDL_DIR, ensure they are the same. * libltdl/configure.ac: Use it. * NEWS: Updated. Index: libtool--devo--1.0/NEWS === --- libtool--devo--1.0.orig/NEWS +++ libtool--devo--1.0/NEWS @@ -3,6 +3,7 @@ NEWS - list of user-visible changes betw New in 1.9h: 2005-??-??; CVS version 2.1a, Libtool team: * New tests for support of Automake subdir-objects. * Fix libltdl on static platforms. +* New LT_CONFIG_LTDL_DIR macro. * 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 between Index: libtool--devo--1.0/doc/libtool.texi === --- libtool--devo--1.0.orig/doc/libtool.texi +++ libtool--devo--1.0/doc/libtool.texi @@ -2243,12 +2243,11 @@ Display a help message and exit. @item --ltdl [EMAIL PROTECTED] Install libltdl in a the @var{TARGET-DIRECTORY-NAME} subdirectory of -your package. If you specify a subdirectory name, then you will need -to be careful to pass the same directory name to the autoconf macros: [EMAIL PROTECTED]; @code{LTDL_CONVENIENCE}; [EMAIL PROTECTED] (@pxref{Distributing libltdl}). Without -an argument, @samp{libltdl} is used as the default target directory -name. +your package. Normally, the directory is extracted from the argument +to @code{LT_CONFIG_LTDL_DIR} in @file{configure.ac}, though you can +also specify a subdirectory name here if you are not using Autoconf +for example. If @command{libtoolize} can't determine the target +directory, @samp{libltdl} is used as the default. @item --quiet @itemx -q @@ -4133,34 +4132,41 @@ release of libltdl. }. Having made the macros available, you must add a call to the @samp{LT_WITH_LTDL} macro to your package's @file{configure.ac} to perform the configure time checks required to build the library -correctly. This method has problems if you then try to link the -package binaries with an installed libltdl, or a library that depends -on libltdl: you will have problems with duplicate symbol definitions. +correctly. Unfortunately, this method has problems if you then try to +link the package binaries with an installed libltdl, or a library that +depends on libltdl, because of the duplicate symbol definitions. For +example, ultimately linking against two different versions of libltdl,
Re: 279-gary-LT_CONFIG_LTDL_DIR.diff
Hi Gary, * Ralf Wildenhues wrote on Thu, Sep 29, 2005 at 06:39:28PM CEST: > * Gary V. Vaughan wrote on Thu, Sep 29, 2005 at 06:32:32PM CEST: > > Ralf Wildenhues wrote: > > > > This patch is just to introduce LT_CONFIG_LTDL_DIR without regressions. > > The rest of the fixes for LT_WITH_LTDL are yet to be split into separate > > patches and posted for review. > > OK. Good point. (There should be a way for the user to specify "I am also content with an older installed libltdl version"; for example, by allowing to test for a different library function.) * Gary V. Vaughan wrote on Fri, Sep 30, 2005 at 12:05:29PM CEST: > > Thanks for the re-review ;-) Okay to commit the attached? Yes, please commit, after accompanying the m4_pushdef with a m4_popdef at the end of the respective macro. Thank you! The BSD make failure should be fixed separately. There is some other stuff going on which I haven't understood fully yet, but let's fix that separately, too. (I'll post a report when I have sorted out that it's not again a failure of mine. :) > >Here's a possible reason: > >grep _LTDL_DIR Makefile configure > >| Makefile:subdirs = _LTDL_DIR > >| configure:ac_subdirs_all='_LTDL_DIR' > >| configure:subdirs="$subdirs _LTDL_DIR" > > Yep, that was the reason for all the pertinent failures. Fixed in > this version. While I can't see the difference in the patch versions that actually made this work, it seems to work now. :-)) > I've made your test case into a new Autotest case, which I'll post > separately. I'll soon post patches necessary to make the test work. > What is the status of this patch btw?: > > 277-gary-rename-remaining-troublesome-ltdl-apis.diff Since luckily that one is not part of the larger chain of patch dependencies (and it's nontrivial and introduces new code), I'd like to postpone it until the queue has been flushed a bit more.. Cheers, and thanks, Ralf
Re: 279-gary-LT_CONFIG_LTDL_DIR.diff
Ralf Wildenhues wrote: Hi Gary, Hallo Ralf, (There should be a way for the user to specify "I am also content with an older installed libltdl version"; for example, by allowing to test for a different library function.) That's a whole other feature. I'm on the fence. Let's discuss it more after the next alpha release. First reaction is that even if we do this, it is post-2.0... Yes, please commit, after accompanying the m4_pushdef with a m4_popdef at the end of the respective macro. Thank you! Will do! Cheers, Gary. -- Gary V. Vaughan ())_. [EMAIL PROTECTED],gnu.org} Research Scientist ( '/ http://tkd.kicks-ass.net GNU Hacker / )= http://www.gnu.org/software/libtool Technical Author `(_~)_ http://sources.redhat.com/autobook signature.asc Description: OpenPGP digital signature