Re: [patch 04/19] 286-gary-libtoolize-recursive-ltdl.diff Queue

[Ralf Wildenhues]

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  <address@hidden>
>         * 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.