Re: Make use of $inherited_linker_flags

[Ralf Wildenhues]

* Peter O'Gorman wrote on Tue, Jan 04, 2005 at 12:11:41AM CET:
> Albert Chin wrote:
> | On Sun, Dec 26, 2004 at 04:09:32PM +0000, Peter O'Gorman wrote:
> |
> |>I don't remember why I implemented the inherited_linker_flags like
> |>this, it may indeed be better to put them into complier_flags.
> |>Patches accepted.
> |
> | Should we call it inherited_compiler_flags then?
> 
> I don't know about that, here is a patch for HEAD which makes
> inherited_linker_flags actually do what it was meant to do in the first
> place. Okay to apply?

Minor glitches only, see below.  Other than the fact that I haven't
fully grokked the ltmain.m4sh logic, I think it's ok to apply.

> Thanks Albert! I was convinced that this already worked, but you forced me
> to test it for real.

Thanks for debugging this and adding a test!

Regards,
Ralf

> Index: ChangeLog
> 2004-12-31  Peter O'Gorman  <address@hidden>
> 
>       * config/ltmain.m4sh (func_mode_link): Fix inherited_linker_flags so 
>       it puts it on the link line when linking shared libraries as well as 
>       when linking executables. Thanks to Albert Chin for overcoming my
>       stubbornness in pointing out the bug.
>       * tests/inherited_flags.at: test inherited_linker_flags.
>       * tests/Makefile.am: Add test.
>       * tests/testsuite.at: Add test.
> 
> from  Ralf Wildenhues  <address@hidden>
> Index: config/ltmain.m4sh
> ===================================================================
> RCS file: /cvsroot/libtool/libtool/config/ltmain.m4sh,v
> retrieving revision 1.39
> diff -u -3 -p -u -r1.39 ltmain.m4sh
> --- config/ltmain.m4sh 28 Dec 2004 13:50:23 -0000 1.39
> +++ config/ltmain.m4sh 2 Jan 2005 00:00:22 -0000
> @@ -2188,6 +2188,7 @@ func_mode_link ()
>      dllsearchpath=
>      lib_search_path=`pwd`
>      inst_prefix_dir=
> +    new_inherited_linker_flags=
>  
>      avoid_version=no
>      dlfiles=
> @@ -3098,7 +3099,7 @@ func_mode_link ()
>       case $pass in
>       dlopen) libs="$dlfiles" ;;
>       dlpreopen) libs="$dlprefiles" ;;
> -     link) libs="$deplibs %DEPLIBS% $dependency_libs 
> $inherited_linker_flags" ;;
> +     link) libs="$deplibs %DEPLIBS% $dependency_libs" ;;
>       esac
>        fi
>        if test "$linkmode,$pass" = "lib,dlpreopen"; then
> @@ -3140,9 +3141,9 @@ func_mode_link ()
>         else
>           deplibs="$deplib $deplibs"
>           if test "$linkmode" = lib ; then
> -             case "$inherited_linker_flags " in
> +             case "$new_inherited_linker_flags " in
>                   *" $deplib "*) ;;
> -                 * ) inherited_linker_flags="$inherited_linker_flags 
> $deplib" ;;
> +                 * ) new_inherited_linker_flags="$new_inherited_linker_flags 
> $deplib" ;;
>               esac
>           fi
>         fi
> @@ -3222,9 +3223,9 @@ func_mode_link ()
>         else
>           deplibs="$deplib $deplibs"
>           if test "$linkmode" = lib ; then
> -             case "$inherited_linker_flags " in
> +             case "$new_inherited_linker_flags " in
>                   *" $deplib "*) ;;
> -                 * ) inherited_linker_flags="$inherited_linker_flags 
> $deplib" ;;
> +                 * ) new_inherited_linker_flags="$new_inherited_linker_flags 
> $deplib" ;;
>               esac
>           fi
>         fi
> @@ -3381,12 +3382,18 @@ func_mode_link ()
>       *-*-darwin*)
>         # Convert "-framework foo" to "foo.ltframework"
>         if test -n "$inherited_linker_flags"; then
> -         inherited_linker_flags=`$ECHO "X$inherited_linker_flags" | $Xsed -e 
> 's/-framework \([[^ $]]*\)/\1.ltframework/g'`
> +         tmp_inherited_linker_flags=`$ECHO "X$inherited_linker_flags" | 
> $Xsed -e 's/-framework \([[^ $]]*\)/\1.ltframework/g'`
> +         new_inherited_linker_flags="$new_inherited_linker_flags 
> $tmp_inherited_linker_flags"
>         fi
>         dependency_libs=`$ECHO "X $dependency_libs" | $Xsed -e 's% \([[^ 
> $]]*\).ltframework% -framework \1%g'`
>         ;;
>       esac
> -
> +     if test "$linkmode,$pass" = "prog,link"; then
> +       compile_deplibs="$inherited_linker_flags $compile_deplibs"
> +       finalize_deplibs="$inherited_linker_flags $finalize_deplibs"
> +     else
> +       compiler_flags="$compiler_flags $inherited_linker_flags"
> +     fi
>       if test "$linkmode,$pass" = "lib,link" ||
>          test "$linkmode,$pass" = "prog,scan" ||
>          { test "$linkmode" != prog && test "$linkmode" != lib; }; then
> @@ -3949,7 +3956,7 @@ func_mode_link ()
>  
>         if test "$link_all_deplibs" != no; then
>           # Add the search paths of all dependency libraries
> -         for deplib in $dependency_libs $inherited_linker_flags; do
> +         for deplib in $dependency_libs; do
>             case $deplib in
>             -L*) path="$deplib" ;;
>             *.la)
> @@ -4848,7 +4855,7 @@ EOF
>        case $host in
>       *-*-darwin*)
>         newdeplibs=`$ECHO "X $newdeplibs" | $Xsed -e 's% \([[^ 
> $]]*\).ltframework% -framework \1%g'`
> -       inherited_linker_flags=`$ECHO "X $inherited_linker_flags" | $Xsed -e 
> 's% \([[^ $]]*\).ltframework% -framework \1%g'`
> +       new_inherited_linker_flags=`$ECHO "X $new_inherited_linker_flags" | 
> $Xsed -e 's% \([[^ $]]*\).ltframework% -framework \1%g'`
>         deplibs=`$ECHO "X $deplibs" | $Xsed -e 's% \([[^ $]]*\).ltframework% 
> -framework \1%g'`
>         ;;
>        esac
> @@ -6431,7 +6438,7 @@ library_names='$library_names'
>  old_library='$old_library'
>  
>  # Linker flags that can not go in dependency_libs.
> -inherited_linker_flags='$inherited_linker_flags'
> +inherited_linker_flags='$new_inherited_linker_flags'
>  
>  # Libraries that this one depends upon.
>  dependency_libs='$dependency_libs'
> Index: tests/Makefile.am
> ===================================================================
> RCS file: /cvsroot/libtool/libtool/tests/Makefile.am,v
> retrieving revision 1.48
> diff -u -3 -p -u -r1.48 Makefile.am
> --- tests/Makefile.am 20 Dec 2004 15:03:39 -0000 1.48
> +++ tests/Makefile.am 2 Jan 2005 00:00:22 -0000
> @@ -24,7 +24,8 @@
>  TESTSUITE    = testsuite
>  TESTSUITE_AT = testsuite.at \
>                 am-subdir.at \
> -               functests.at
> +               functests.at \
> +               inherited_flags.at
>  
>  EXTRA_DIST   = $(TESTSUITE) $(TESTSUITE_AT) package.m4
>  
> Index: tests/inherited_flags.at
> ===================================================================
> RCS file: tests/inherited_flags.at
> diff -N tests/inherited_flags.at
> --- /dev/null 1 Jan 1970 00:00:00 -0000
> +++ tests/inherited_flags.at 2 Jan 2005 00:00:23 -0000
> @@ -0,0 +1,62 @@
> +# Hand crafted tests for GNU Libtool.                         -*- Autotest 
> -*-
> +# Copyright 2004 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 2, or (at your option)
> +# any later version.
> +
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, write to the Free Software
> +# Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA
> +# 02111-1307, USA.
> +
> +
> +AT_SETUP([inherited_linker_flags])
> +
> +cp ${abs_top_builddir}/libtool libtool

Is there a reason not to just do
  : ${LIBTOOL=$abs_top_builddir/libtool}
and then use ${LIBTOOL} everywhere below?

> +chmod +x libtool

Could kill this as well then.

> +AT_DATA([foo.c],
> +[
> +int foo() { return 1;}
> +])
> +
> +AT_DATA([bar.c],
> +[
> +int bar() { return 1;}
> +])
> +
> +AT_DATA([baz.c],
> +[
> +int baz() { return 1;}
> +])
> +
> +AT_DATA([main.c],
> +[
> +int main() { return 0;}
> +])
> +eval `grep ^CC libtool | sed -n -e '1p'`

I like the idiom
  eval `$LIBTOOL --config | grep ^CC`
better, because it's documented interface (who knows whether libtool
will be rewritten in a compiled language eventually?).

Otherwise, your line could still more easily be written as
  eval `sed -n '/^CC/{p;q;}' $LIBTOOL`

> +./libtool --mode=compile --tag=CC $CC -c -o foo.lo foo.c
> +./libtool --mode=compile --tag=CC $CC -c -o bar.lo bar.c
> +./libtool --mode=compile --tag=CC $CC -c -o baz.lo baz.c
> +./libtool --mode=compile --tag=CC $CC -c -o main.lo main.c
> +./libtool --mode=link --tag=CC $CC -o libfoo.la foo.lo -rpath /usr/local/lib
> +./libtool --mode=link --tag=CC $CC -o libbar.la bar.lo -rpath /usr/local/lib
> +
> +mv libfoo.la libfoo.la.bak
> +sed -e 
> 's/^inherited_linker_flags.*/inherited_linker_flags='-llt_inlikely_existing_lib'/g'
>  < libfoo.la.bak > libfoo.la

Minor nits:

  sed 's/^\(inherited_linker_flags\).*/\1=-llt_inlikely_existing_lib/' < 
libfoo.la.bak > libfoo.la

(your quoting makes little sense).  If you really want the quotes around
the right hand side, use something like (ugly)
  's/^\(inherited_linker_flags\).*/\1='"'-llt_inlikely_existing_lib'"'/'

but I intend to eventually launch another go at .la file specification
which would allow the first form (plus, it works with current libtool
versions).

> +rm libfoo.la.bak
> +
> +mv libbar.la libbar.la.bak
> +sed -e 
> 's/^inherited_linker_flags.*/inherited_linker_flags='-llt_unlikely_existing_lib'/g'
>  < libbar.la.bak > libbar.la

same here.

> +rm libbar.la.bak
> +
> +AT_CHECK([./libtool --mode=link --tag=CC $CC -o libbaz.la baz.lo -rpath 
> /usr/local/lib ./libfoo.la ./libbar.la | grep 
> 'llt_[[ui]]nlikely_existing_lib.*llt_[[ui]]nlikely_existing_lib'],[0],[ignore],[ignore])
> +AT_CHECK([./libtool --mode=link --tag=CC $CC -o main main.lo -rpath 
> /usr/local/lib  ./libfoo.la ./libbar.la | grep 
> 'llt_[[ui]]nlikely_existing_lib.*llt_[[ui]]nlikely_existing_lib'],[0],[ignore],[ignore])
> +
> +AT_CLEANUP
> Index: tests/testsuite.at
> ===================================================================
> RCS file: /cvsroot/libtool/libtool/tests/testsuite.at,v
> retrieving revision 1.3
> diff -u -3 -p -u -r1.3 testsuite.at
> --- tests/testsuite.at 20 Dec 2004 15:03:39 -0000 1.3
> +++ tests/testsuite.at 2 Jan 2005 00:00:23 -0000
> @@ -63,3 +63,4 @@ AT_INIT
>  # Torturing subdir-objects builds
>  m4_include([am-subdir.at])

Might be nice to add empty line plus comment here..

>  m4_include([functests.at])

..and here.

> +m4_include([inherited_flags.at])
> \ No newline at end of file

Please don't commit nonempty files without ending newline.  Better even,
disallow your editor to create such files.

Regards,
Ralf