279-gary-LT_CONFIG_LTDL_DIR.diff

2005-09-23 Thread Gary V. Vaughan
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

2005-09-27 Thread Gary V. Vaughan
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

2005-09-23 Thread Gary V. Vaughan

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

2005-09-23 Thread Ralf Wildenhues
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

2005-09-23 Thread Gary V. Vaughan

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

2005-09-26 Thread Ralf Wildenhues
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

2005-09-28 Thread Ralf Wildenhues
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

2005-09-29 Thread Gary V. Vaughan

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

2005-09-29 Thread Ralf Wildenhues
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

2005-09-29 Thread Gary V. Vaughan

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

2005-09-29 Thread Ralf Wildenhues
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

2005-09-30 Thread Gary V. Vaughan

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

2005-10-02 Thread Ralf Wildenhues
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

2005-10-03 Thread Gary V. Vaughan

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