Re: [patch 04/19] 286-gary-libtoolize-recursive-ltdl.diff Queue
Hi Gary, * Gary V. Vaughan wrote on Thu, Oct 13, 2005 at 06:40:59PM CEST: > Ralf Wildenhues wrote: > >Do you want the (non)recursive/subproject info as argument to > > LT_WITH_LTDL > >or > > LT_CONFIG_LTDL_DIR > > Eek! That leaked in from the ancient past. My first implementation tried > to put the mode argument in LT_WITH_LTDL, but for various reasons that > turned out to be untenable, so I moved it to LT_CONFIG_LTDL_DIR (which > actually is conceptually nicer anyway). I've removed all evidence of > the earlier implemention for sure this time! Thanks! > >I have found more nits, see inline. Also, please remember: > >- adjust doc plus makefiles for whatever you agree on wrt. *_LTLIBRARIES > > DEFS, AM_CPPFLAGS, AM_CFLAGS: `+=' vs `='. > > Done. Although I've moved the contents of DEFS into AM_CPPFLAGS to reduce > the number of magic variables a little. OK, even better, although I have a hard time seeing whether this works in all cases.. > >And please actually do the commit only together with the followup > >patches needed to implement the functionality, so CVS HEAD is not in > >broken state for long. Thank you. > > Actually, this patch doesn't break libtool at all... the testsuite still > passes, it's just that recursive mode isn't yet implemented. D'oh. Thank you, I overlooked that! > >Now, how's that gonna work if the user uses a name different from > >`libltdl/' as subdir? The paths in `libltdl/Makefile.inc' are pretty > >hardcoded to start with `libltdl/'! > > > >No need to change the implementation IMHO, but it may be good to adjust > >above paragraph to match the limitation to firmly state: if you use > >nonrecursive, that subdir better be named `libltdl' and nothing else! > > I had started a patch to substitute in the correct value as libtoolize > copied Makefile.inc into the project tree. But figure that is icing. > I'd forgotten that you *must* use libltdl for nonrecursive. Good catch, > thanks! I'll also add to [298] a note that a LIBOBJDIR capable Automake > and Autoconf are required for this mode to work as it stands (as per > your review of that patch) -- we certainly have room to tweak libtoolize > to replicate the trick libtool itself uses to workaround the problem > later... OK, fine with me. * Gary V. Vaughan wrote on Fri, Oct 14, 2005 at 05:34:28PM CEST: > Tweaked this patch slightly to allow for the next one to work whether > or not SUBDIR_LIBOBJS are supported by the user's autotools. Okay to > commit? I have a couple of gripes with it: - First, LTCOMPILE is not published interface by Automake, - Second, you kill dependency tracking for the LTLIBOBJS, Considering this, maybe this is too high a price to pay for gaining unchanged AM_CPPFLAGS, DEFS, and AM_LDFLAGS. What do you think? Sorry for not thinking enough about this before, but now that I see this, I believe it needs to be addressed. A couple of trivial nits: - I believe SUBDIR_LIBOBJS is too generic a name to be used in third-party Makefiles. (Prepend LT_ or LTDL_ or so) - list libltdl/Makefile.inc before libltdl/Makefile.am in ltdldatafiles > Incidentally, this also removes the troublesome '## %%% BEGIN' magic from > Makefile.am. Nice! Could you please repost the patch without format=flowed, so I can apply it cleanly. Thank you. I'm seeing weirdnesses with libtoolize but want to make sure they are not from me messing up the merge. Cheers, Ralf > from Gary V. Vaughan <[EMAIL PROTECTED]> > * configure.ac: Move SUBDIR_LIBOBJS check from here... > * libltdl/m4/ltdl.m4 (_LT_CHECK_SUBDIR_LIBOBJS: ...to here. > (LT_INIT): Adjust. > * libltdl/Makefile.inc: New file, factored out of Makefile.am for > use in non-recursive libltdl installations. > * Makefile.am: include it. > (libltdl/Makefile.am): Adjust to build from the new > libltdl/Makefile.inc. > * bootstrap: Adjust. > * doc/libtool.texi (Invoking libtoolize): Document the new modes > and libtoolize option to select them. > * libtoolize.m4sh: Parse new options, --nonrecursive, --recursive > and --subproject. Install the appropriate files with --ltdl > according to the selected mode. > (func_scan_files): If --subproject, --recursive or --nonrecursive > options were not given, use the value from LT_CONFIG_LTDL_DIR; if > a mode was given, and there is also an argument to > LT_CONFIG_LTDL_DIR, ensure they are the same. > * NEWS: Updated.
Re: [patch 04/19] 286-gary-libtoolize-recursive-ltdl.diff Queue
Tweaked this patch slightly to allow for the next one to work whether or not SUBDIR_LIBOBJS are supported by the user's autotools. Okay to commit? Another step towards fixing LT_WITH_LTDL. Here we add an optional mode argument to libtoolize so that the user can tell libtoolize whether she is going to subconfigure libltdl as always, or use either a non-recursive automake (like libtool itself), or a recursive automake with a top-level configure. This patch *doesn't* attempt to clean up the m4 macros. Incidentally, this also removes the troublesome '## %%% BEGIN' magic from Makefile.am. Makefile.am | 137 +++- NEWS |4 + bootstrap|5 - configure.ac |6 - doc/libtool.texi | 63 ++ libltdl/Makefile.inc | 173 +++ libltdl/m4/ltdl.m4 | 12 +++ libtoolize.m4sh | 69 ++-- 8 files changed, 329 insertions(+), 140 deletions(-) Index: libtool--devo--1.0/ChangeLog from Gary V. Vaughan <[EMAIL PROTECTED]> * configure.ac: Move SUBDIR_LIBOBJS check from here... * libltdl/m4/ltdl.m4 (_LT_CHECK_SUBDIR_LIBOBJS: ...to here. (LT_INIT): Adjust. * libltdl/Makefile.inc: New file, factored out of Makefile.am for use in non-recursive libltdl installations. * Makefile.am: include it. (libltdl/Makefile.am): Adjust to build from the new libltdl/Makefile.inc. * bootstrap: Adjust. * doc/libtool.texi (Invoking libtoolize): Document the new modes and libtoolize option to select them. * libtoolize.m4sh: Parse new options, --nonrecursive, --recursive and --subproject. Install the appropriate files with --ltdl according to the selected mode. (func_scan_files): If --subproject, --recursive or --nonrecursive options were not given, use the value from LT_CONFIG_LTDL_DIR; if a mode was given, and there is also an argument to LT_CONFIG_LTDL_DIR, ensure they are the same. * NEWS: Updated. Index: libtool--devo--1.0/Makefile.am === --- libtool--devo--1.0.orig/Makefile.am +++ libtool--devo--1.0/Makefile.am @@ -25,6 +25,7 @@ ACLOCAL_AMFLAGS= -I libltdl/m4 DIST_SUBDIRS = . +EXTRA_DIST = BUILT_SOURCES = libtool @@ -32,15 +33,9 @@ CLEANFILES = MOSTLYCLEANFILES = DISTCLEANFILES = -EXTRA_DIST = libltdl/COPYING.LIB \ - libltdl/Makefile.am \ - libltdl/Makefile.in \ - libltdl/README \ - libltdl/config-h.in \ - libltdl/configure \ - libltdl/configure.ac \ - libltdl/aclocal.m4 \ - libltdl/m4/lt~obsolete.m4 +noinst_LTLIBRARIES = +lib_LTLIBRARIES= +EXTRA_LTLIBRARIES = auxdir = libltdl/config m4dir = libltdl/m4 @@ -196,19 +191,22 @@ $(srcdir)/$(auxdir)/ltmain.sh: $(sh_file chmod a-w $(auxdir)/ltmain.tmp; \ mv -f $(auxdir)/ltmain.tmp $(auxdir)/ltmain.sh -$(srcdir)/libltdl/Makefile.am: Makefile.am +$(srcdir)/libltdl/Makefile.am: $(srcdir)/libltdl/Makefile.inc cd $(srcdir); \ - in=Makefile.am; out=libltdl/Makefile.am; \ + in=libltdl/Makefile.inc; out=libltdl/Makefile.am; \ rm -f $$out; \ $(SED) -n '/^.. Makefile.am -- /,/^.. Boston, MA/p' $$in > $$out; \ { echo 'ACLOCAL_AMFLAGS = -I m4'; \ echo 'AUTOMAKE_OPTIONS = foreign'; \ echo 'BUILT_SOURCES ='; \ + echo 'noinst_LTLIBRARIES ='; \ + echo 'lib_LTLIBRARIES ='; \ + echo 'EXTRA_LTLIBRARIES ='; \ echo 'EXTRA_DIST ='; \ echo 'CLEANFILES ='; \ echo 'MOSTLYCLEANFILES ='; \ } >> $$out; \ - $(SED) -n '/^. %%% BEGIN /,/^. %%% END / \ + $(SED) -n '/^.. DO NOT REMOVE THIS LINE -- /,$$ \ { s,libltdl_,,; s,libltdl/,,; s,: libltdl/,: ,; \ s,\$$(libltdl_,$$(,; p; }' $$in >> $$out; chmod a-w $(srcdir)/libltdl/Makefile.am @@ -224,119 +222,7 @@ all-local: $(srcdir)/libltdl/Makefile.in ## Libltdl. ## ## ## -# %%% BEGIN libltdl/Makefile.am - -DEFS = -DLTDL -DHAVE_CONFIG_H -DLT_CONFIG_H='<$(LT_CONFIG_H)>' - -# -I$(srcdir) is needed for user that built libltdl with a sub-Automake -# (not as a sub-package!) using 'nostdinc': -AM_CPPFLAGS= -I. -I$(srcdir) -Ilibltdl -I$(srcdir)/libltdl \ - -I$(srcdir)/libltdl/libltdl -AM_LDFLAGS = -no-undefined -VERSION_INFO = -version-info 7:0:0 - -noinst_LTLIBRARIES = $(LT_DLLOADERS) - -if INSTALL_LTDL -ltdlincludedir = $(includedir)/libltdl -ltdlinclude_HEADERS= libltdl/li
Re: [patch 04/19] 286-gary-libtoolize-recursive-ltdl.diff Queue
Hallo Ralf, Thanks for the review! Ralf Wildenhues wrote: Very nice patch! But it mixes up two things, and at that quite heavily: Do you want the (non)recursive/subproject info as argument to LT_WITH_LTDL or LT_CONFIG_LTDL_DIR or possibly both? Please redo `libtoolize' output based on the answer of this (I did not finishing testing, but will do again after this is fixed). Eek! That leaked in from the ancient past. My first implementation tried to put the mode argument in LT_WITH_LTDL, but for various reasons that turned out to be untenable, so I moved it to LT_CONFIG_LTDL_DIR (which actually is conceptually nicer anyway). I've removed all evidence of the earlier implemention for sure this time! I have found more nits, see inline. Also, please remember: - adjust doc plus makefiles for whatever you agree on wrt. *_LTLIBRARIES DEFS, AM_CPPFLAGS, AM_CFLAGS: `+=' vs `='. Done. Although I've moved the contents of DEFS into AM_CPPFLAGS to reduce the number of magic variables a little. And please actually do the commit only together with the followup patches needed to implement the functionality, so CVS HEAD is not in broken state for long. Thank you. Actually, this patch doesn't break libtool at all... the testsuite still passes, it's just that recursive mode isn't yet implemented. [EMAIL PROTECTED] --nonrecursive +If passed in conjunction with @option{--ltdl}, this option will cause +the @command{libtoolize} installed @samp{libltdl} to be set up for use Hypen between `libtoolize' and `installed'? Alternatively: .. the libltdl installed by libtoolize..? Oops. Thanks. +with a non-recursive @command{automake} build. To make use of it, you +will need to add the following to the @file{Makefile.am} of the parent +project (adjusted to include the file from whatever directory you +installed libltdl to): + [EMAIL PROTECTED] +include libltdl/Makefile.inc [EMAIL PROTECTED] example Now, how's that gonna work if the user uses a name different from `libltdl/' as subdir? The paths in `libltdl/Makefile.inc' are pretty hardcoded to start with `libltdl/'! No need to change the implementation IMHO, but it may be good to adjust above paragraph to match the limitation to firmly state: if you use nonrecursive, that subdir better be named `libltdl' and nothing else! I had started a patch to substitute in the correct value as libtoolize copied Makefile.inc into the project tree. But figure that is icing. I'd forgotten that you *must* use libltdl for nonrecursive. Good catch, thanks! I'll also add to [298] a note that a LIBOBJDIR capable Automake and Autoconf are required for this mode to work as it stands (as per your review of that patch) -- we certainly have room to tweak libtoolize to replicate the trick libtool itself uses to workaround the problem later... Okay to commit? Makefile.am | 137 +++-- NEWS |4 + bootstrap|5 - doc/libtool.texi | 63 libltdl/Makefile.inc | 155 +++ libtoolize.m4sh | 69 -- 6 files changed, 299 insertions(+), 134 deletions(-) Index: libtool--devo--1.0/ChangeLog from Gary V. Vaughan <[EMAIL PROTECTED]> * libltdl/Makefile.inc: New file, factored out of Makefile.am for use in non-recursive libltdl installations. * Makefile.am: include it. (libltdl/Makefile.am): Adjust to build from the new libltdl/Makefile.inc. * bootstrap: Adjust. * doc/libtool.texi (Invoking libtoolize): Document the new modes and libtoolize option to select them. * libtoolize.m4sh: Parse new options, --nonrecursive, --recursive and --subproject. Install the appropriate files with --ltdl according to the selected mode. (func_scan_files): If --subproject, --recursive or --nonrecursive options were not given, use the value from LT_CONFIG_LTDL_DIR; if a mode was given, and there is also an argument to LT_CONFIG_LTDL_DIR, ensure they are the same. * NEWS: Updated. Index: libtool--devo--1.0/Makefile.am === --- libtool--devo--1.0.orig/Makefile.am +++ libtool--devo--1.0/Makefile.am @@ -25,6 +25,7 @@ ACLOCAL_AMFLAGS= -I libltdl/m4 DIST_SUBDIRS = . +EXTRA_DIST = BUILT_SOURCES = libtool @@ -32,15 +33,9 @@ CLEANFILES = MOSTLYCLEANFILES = DISTCLEANFILES = -EXTRA_DIST = libltdl/COPYING.LIB \ - libltdl/Makefile.am \ - libltdl/Makefile.in \ - libltdl/README \ - libltdl/config-h.in \ - libltdl/configure \ - libltdl/configure.ac \ - libltdl/aclocal.m4 \ -
Re: [patch 04/19] 286-gary-libtoolize-recursive-ltdl.diff Queue
Hi Gary, * Gary V. Vaughan wrote on Mon, Oct 10, 2005 at 12:26:28PM CEST: > Another step towards fixing LT_WITH_LTDL. Here we add an optional mode > argument to libtoolize so that the user can tell libtoolize whether she > is going to subconfigure libltdl as always, or use either a non-recursive > automake (like libtool itself), or a recursive automake with a top-level > configure. This patch *doesn't* attempt to clean up the m4 macros. > > Incidentally, this also removes the troublesome '## %%% BEGIN' magic from > Makefile.am. Very nice patch! But it mixes up two things, and at that quite heavily: Do you want the (non)recursive/subproject info as argument to LT_WITH_LTDL or LT_CONFIG_LTDL_DIR or possibly both? Please redo `libtoolize' output based on the answer of this (I did not finishing testing, but will do again after this is fixed). I have found more nits, see inline. Also, please remember: - adjust doc plus makefiles for whatever you agree on wrt. *_LTLIBRARIES DEFS, AM_CPPFLAGS, AM_CFLAGS: `+=' vs `='. And please actually do the commit only together with the followup patches needed to implement the functionality, so CVS HEAD is not in broken state for long. Thank you. Cheers, Ralf > Makefile.am | 122 ++--- > NEWS |4 + > bootstrap|5 + > doc/libtool.texi | 46 + > libltdl/Makefile.inc | 137 > +++ > libltdl/m4/ltdl.m4 |8 ++ > libtoolize.m4sh | 71 -- > 7 files changed, 269 insertions(+), 124 deletions(-) > > Index: libtool--devo--1.0/ChangeLog > from Gary V. Vaughan <[EMAIL PROTECTED]> > * libltdl/Makefile.inc: New file, factored out of Makefile.am for > use in non-recursive libltdl installations. > * Makefile.am: include it. > (libltdl/Makefile.am): Adjust to build from the new > libltdl/Makefile.inc. > * bootstrap: Adjust. > * doc/libtool.texi (Invoking libtoolize): Document the new modes > and libtoolize option to select them. > * libtoolize.m4sh: Parse new options, --nonrecursive, --recursive > and --subproject. Install the appropriate files with --ltdl > according to the selected mode. > (func_scan_files): If --subproject, --recursive or --nonrecursive > options were not given, use the value from LT_CONFIG_LTDL_DIR; if > a mode was given, and there is also an argument to > LT_CONFIG_LTDL_DIR, ensure they are the same. > * NEWS: Updated. First nit: the patch changes (a comment in) libltdl/m4/ltdl.m4, but the log entry does not mention this. *big snip* > Index: libtool--devo--1.0/doc/libtool.texi > === > --- libtool--devo--1.0.orig/doc/libtool.texi > +++ libtool--devo--1.0/doc/libtool.texi > @@ -2249,11 +2249,57 @@ also specify a subdirectory name here if > for example. If @command{libtoolize} can't determine the target > directory, @samp{libltdl} is used as the default. > > [EMAIL PROTECTED] --nonrecursive > +If passed in conjunction with @option{--ltdl}, this option will cause > +the @command{libtoolize} installed @samp{libltdl} to be set up for use Hypen between `libtoolize' and `installed'? Alternatively: .. the libltdl installed by libtoolize..? > +with a non-recursive @command{automake} build. To make use of it, you > +will need to add the following to the @file{Makefile.am} of the parent > +project (adjusted to include the file from whatever directory you > +installed libltdl to): > + > [EMAIL PROTECTED] > +include libltdl/Makefile.inc > [EMAIL PROTECTED] example Now, how's that gonna work if the user uses a name different from `libltdl/' as subdir? The paths in `libltdl/Makefile.inc' are pretty hardcoded to start with `libltdl/'! No need to change the implementation IMHO, but it may be good to adjust above paragraph to match the limitation to firmly state: if you use nonrecursive, that subdir better be named `libltdl' and nothing else! > + > @item --quiet > @itemx -q > Work silently. @samp{libtoolize --quiet} is used by @sc{gnu} Automake > to add libtool files to your package if necessary. > > [EMAIL PROTECTED] --recursive > +If passed in conjunction with @option{--ltdl}, this option will cause > +the @command{libtoolize} installed @samp{libltd} to be set up for use > +with a recursive @command{automake} build. To make use of it, you > +will need to adjust the parent project's @file{configure.ac}: > + > [EMAIL PROTECTED] > +AC_CONFIG_FILES([libltdl/Makefile]) > [EMAIL PROTECTED] example > + > [EMAIL PROTECTED] > +and @file{Makefile.am}: > + > [EMAIL PROTECTED] > +SUBDIRS += libltdl > [EMAIL PROTECTED] example > + > [EMAIL PROTECTED] --subproject > +If passed in conjunction with @option{--ltdl}, this option will cause > +the @command{libtoolize} installed @samp{libltd} to be set up for s/