Re: Fix linking against uninstalled libs again (1/n)

[Ralf Wildenhues]

Hello again,

* Jacob Meuser wrote on Sun, Jan 22, 2006 at 04:56:59AM CET:
> On Sat, Jan 21, 2006 at 09:37:27AM +0100, Ralf Wildenhues wrote:
> > I messed up pretty badly while trying to fix the "linking against
> > uninstalled deplibs" right before 1.5.22 -- guess that's what I get
> > for not testing extensively.  Please accept my apologies.
> > 
> > The changes applied to branch-1-5 had two flaws (at least):
> > a) They did not move all paths to uninstalled libs up front.
> > b) They removed some uninstalled libs in some cases.
> 
> c) the main flaw, not all deplibs paths are absolute, but the
>    notinst_path paths are.

Jacob and I discussed a bit more offline.  He then agreed that,
if the `Eliminate all temporary directoried' part was undone from
branch-1-5, then things would work on OpenBSD with my patch.
There was one inconsistency in my patch that would once add
the path with `.libs' added and once without; fixed below
(thanks to Marc and Jacob for pointing this out!).

Since then, several days passed, and the testsuite test was extended
by quite a bit.  Which in turn uncovered more issues, of course:

Those paths to uninstalled libraries will propagate to installed .la
files in some cases, which is of course bad.  It's not horrible,
after all, you're supposed to add in-tree deplibs like this:
  ../path/to/libdeplib.la
and not this:
  -L../path/to -ldeplib
or this:
  -L../path/to ../path/to/libdeplib.la
but it's better to fix this anyway.


To be more precise, there are more details to this:
- at link time of an uninstalled output, uninstalled link and run paths
  must be used
- uninstalled .la files should refer to uninstalled deplibs in
  `dependency_libs'
- after installation only installed .la files should be referred to in
  `dependency_libs'; also, link paths to uninstalled locations should be
  gone.

All of the above is fixed with the patch below.  Open issues:
- all of the above won't work for non-normalized absolute paths yet
  (stuff like `/foo/../bar').  Not so pressing IMHO, and actually a bit
  more difficult to fix right.
- at installation time, if we relink, the command line that relink mode
  outputs (as opposed to its input: `libtool --mode=relink') should not
  have uninstalled paths either.  Why is this necessary?  Some systems
  hardcode `-L/paths' automatically as the default location for the
  library.
  This is dangerous to fix though until we have cross compilation
  working better though.  The hardcode_minus_L=yes issue affects very
  few systems, and those not very badly, so I'm postponing this to after
  2.0.

The corresponding checks in the tests are commented out.

OK to apply?  Tested on GNU/Linux, OpenBSD.

If you agree, and if the next 2.0 alpha does not turn up any issues with
this, I intend to backport this to branch-1-5.  The test takes pretty
long, by the way.  It's probably useful to combine some of the tests
once we're fairly sure things work the way we like; for 1.9h I would
like it like this though.

Cheers,
Ralf,  looking forward to 1.9h  :-)

        * libltdl/config/ltmain.m4sh (func_mode_link): Also reorder the
        non-absolute paths to uninstalled deplibs, and also reorder the
        `$path/.libs' lists. 
        (Eliminate all temporary directories): This section has never
        worked correctly due to wrong shell quoting.  Conditionalize it
        for relink mode (as a hint for later that this is not useful
        in link mode), fix the quoting error, and comment out the
        complete section.
        (creation of the .lai file): Remove uninstalled paths here, for
        the installed .la file.
        * tests/uninstalled.at: New tests: test that adding link paths
        to uninstalled locations will be removed in installed .la files,
        that uninstalled paths will be preferred in link mode
        independent of the order in which the paths are given.  Test
        that uninstalled executables or wrappers use uninstalled
        libraries, and installed executables use installed paths.
        * Makefile.am: Adjusted.

Index: Makefile.am
===================================================================
RCS file: /cvsroot/libtool/libtool/Makefile.am,v
retrieving revision 1.192
diff -u -r1.192 Makefile.am
--- Makefile.am 9 Feb 2006 15:31:11 -0000       1.192
+++ Makefile.am 10 Feb 2006 17:38:55 -0000
@@ -389,6 +389,7 @@
                  tests/convenience.at \
                  tests/link-order.at \
                  tests/fail.at \
+                 tests/uninstalled.at \
                  tests/old-m4-iface.at \
                  tests/am-subdir.at \
                  tests/standalone.at \
Index: libltdl/config/ltmain.m4sh
===================================================================
RCS file: /cvsroot/libtool/libtool/libltdl/config/ltmain.m4sh,v
retrieving revision 1.35
diff -u -r1.35 ltmain.m4sh
--- libltdl/config/ltmain.m4sh  5 Feb 2006 11:06:31 -0000       1.35
+++ libltdl/config/ltmain.m4sh  10 Feb 2006 17:38:55 -0000
@@ -3523,12 +3523,12 @@
            dir="$ladir"
            absdir="$abs_ladir"
            # Remove this search path later
-           notinst_path="$notinst_path $abs_ladir"
+           notinst_path="$notinst_path $abs_ladir $ladir"
          else
            dir="$ladir/$objdir"
            absdir="$abs_ladir/$objdir"
            # Remove this search path later
-           notinst_path="$notinst_path $abs_ladir"
+           notinst_path="$notinst_path $abs_ladir $ladir"
          fi
        fi # $installed = yes
        func_stripname 'lib' '.la' "$laname"
@@ -4501,11 +4501,13 @@
       fi
 
       # Eliminate all temporary directories.
-      for path in $notinst_path; do
-       lib_search_path=`$ECHO "X$lib_search_path " | $Xsed -e 's% $path % %g'`
-       deplibs=`$ECHO "X$deplibs " | $Xsed -e 's% -L$path % %g'`
-       dependency_libs=`$ECHO "X$dependency_libs " | $Xsed -e 's% -L$path % 
%g'`
-      done
+#      if test "$mode" = relink; then
+#      for path in $notinst_path; do
+#        lib_search_path=`$ECHO "X$lib_search_path " | $Xsed -e "s% $path % 
%g"`
+#        deplibs=`$ECHO "X$deplibs " | $Xsed -e "s% -L$path % %g"`
+#        dependency_libs=`$ECHO "X$dependency_libs " | $Xsed -e "s% -L$path % 
%g"`
+#      done
+#      fi
 
       if test -n "$xrpath"; then
        # If the user specified any rpath flags, then add them.
@@ -4906,15 +4908,16 @@
       # installed libraries to the beginning of the library search list
       new_libs=
       for path in $notinst_path; do
-       case " $new_libs " in
-       *" -L$path/$objdir "*) ;;
-       *)
-         case " $deplibs " in
-         *" -L$path/$objdir "*)
-           new_libs="$new_libs -L$path/$objdir" ;;
+       for p in $path $path/$objdir; do
+         case " $new_libs " in
+         *" -L$p "*) ;;
+         *)
+           case " $deplibs " in
+           *" -L$p "*) new_libs="$new_libs -L$p" ;;
+           esac
+           ;;
          esac
-         ;;
-       esac
+       done
       done
       for deplib in $deplibs; do
        case $deplib in
@@ -5523,15 +5526,16 @@
       # installed libraries to the beginning of the library search list
       new_libs=
       for path in $notinst_path; do
-       case " $new_libs " in
-       *" -L$path/$objdir "*) ;;
-       *)
-         case " $compile_deplibs " in
-         *" -L$path/$objdir "*)
-           new_libs="$new_libs -L$path/$objdir" ;;
+       for p in $path $path/$objdir; do
+         case " $new_libs " in
+         *" -L$p "*) ;;
+         *)
+           case " $compile_deplibs " in
+           *" -L$p "*) new_libs="$new_libs -L$p" ;;
+           esac
+           ;;
          esac
-         ;;
-       esac
+       done
       done
       for deplib in $compile_deplibs; do
        case $deplib in
@@ -6530,6 +6534,16 @@
                  func_fatal_error "\`$deplib' is not a valid libtool archive"
                newdependency_libs="$newdependency_libs $libdir/$name"
                ;;
+             -L*)
+               for path in $notinst_path; do
+                 case " $deplib " in
+                 *"-L$path"* | *"-L$path/$objdir"*)
+                   break ;;
+                 *) newdependency_libs="$newdependency_libs $deplib"
+                   break ;;
+                 esac
+               done
+               ;;
              *) newdependency_libs="$newdependency_libs $deplib" ;;
              esac
            done
--- /dev/null   1970-01-01 00:00:01.000000000 +0000
+++ tests/uninstalled.at        2006-02-10 17:06:00.000000000 +0100
@@ -0,0 +1,186 @@
+# Hand crafted tests for GNU Libtool.                         -*- Autotest -*-
+# Copyright 2006 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., 51 Franklin Street, Fifth Floor, Boston, MA
+# 02110-1301, USA.
+
+AT_SETUP([linking uninstalled libraries])
+AT_KEYWORDS([libtool])
+
+LDFLAGS="$LDFLAGS -no-undefined"
+eval `$LIBTOOL --config | grep '^objdir='`
+
+prefix=`pwd`/inst
+lib_dir=$prefix/lib    # Do not use $libdir: sourcing `*.la' will overwrite it
+bin_dir=$prefix/bin
+mkdir src src/a src/a-old src/b src/m $prefix $lib_dir $bin_dir
+cd src
+
+# Create the old library.
+cd a-old
+echo "int a_old () { return 1; }" > a.c
+$LIBTOOL --mode=compile --tag=CC $CC $CPPFLAGS $CFLAGS -c a.c
+$LIBTOOL --mode=link --tag=CC $CC $CPPFLAGS $CFLAGS $LDFLAGS \
+        -L $lib_dir -rpath $lib_dir -o liba.la a.lo
+$LIBTOOL --mode=install cp liba.la $lib_dir/liba.la
+
+func_backup_libfiles ()
+{
+  mkdir backup
+  . "$1"
+  for name in $library_names $old_library; do
+    mv $objdir/$name backup
+    # create fake files in both the .libs dir and above:
+    # we want both paths eliminated from relink
+    $LN_S ../a-old/$objdir/$name $name
+    $LN_S ../a-old/$objdir/$name $objdir/$name
+  done
+}
+
+func_restore_libfiles ()
+{
+  . "$1"
+  for name in $library_names $old_library; do
+    rm -f $name $objdir/$name
+    mv backup/$name $objdir/$name
+  done
+  rmdir backup
+}
+
+# Create the new library: either link- or execution-time incompatible.
+for old in _old ""; do
+  cd ../a
+  echo "int a$old() { return 0; }" > a.c
+  $LIBTOOL --mode=compile --tag=CC $CC $CPPFLAGS $CFLAGS -c a.c
+  cd ../m
+  echo "extern int a$old(); int main() { return a$old(); }" > main.c
+  $CC $CPPFLAGS $CFLAGS -c main.c
+
+  # Whether we add a `-L' path pointing to the uninstalled or the installed
+  # directory, libtool should always link with the right library, remove
+  # the uninstalled paths in the installed files, and preserve manually added
+  # paths otherwise.
+  for flag_deplib in "" "-L$lib_dir"; do
+    for flag_instlib in "" "-L../a" "-L`pwd`/../a" "-L`cd ../a && pwd`" 
"-L$lib_dir"; do
+      for deplib_path in ../a "`pwd`/../a" "`cd ../a && pwd`"; do
+
+       cd ../a
+       linkline="$LIBTOOL --mode=link --tag=CC $CC $CFLAGS $LDFLAGS 
$flag_deplib \
+                 -rpath $lib_dir -o liba.la a.lo"
+       AT_CHECK([echo $linkline; $linkline], [0], [ignore], [ignore])
+       cd ../m
+       linkline="$LIBTOOL --mode=link $CC $CFLAGS $LDFLAGS $flag_instlib \
+                 -o main main.$OBJEXT $deplib_path/liba.la"
+       AT_CHECK([echo $linkline; $linkline], [0], [ignore], [ignore])
+       LT_AT_EXEC_CHECK([./main])
+
+       # install the library and the program, check that no build-tree
+       # references remain.  (Move the bogus installed lib temporarily.)
+       mv $lib_dir $lib_dir-backup
+       mkdir $lib_dir
+       AT_CHECK([$LIBTOOL --mode=install cp ../a/liba.la $lib_dir/liba.la], 
[0], [ignore], [ignore])
+       AT_CHECK([$LIBTOOL --mode=install cp main $bin_dir/main], [0], 
[ignore], [ignore])
+       AT_CHECK([. $lib_dir/liba.la; echo " $dependency_libs" | grep '/src/'], 
[1])
+       $LIBTOOL --mode=clean rm -f main ../a/liba.la
+       LT_AT_EXEC_CHECK([$bin_dir/main])
+       $LIBTOOL --mode=uninstall rm -f $bin_dir/main $lib_dir/liba.la
+       rmdir $lib_dir
+       mv $lib_dir-backup $lib_dir
+      done
+    done
+  done
+done
+
+# Now the same thing, but with a dependent library.
+for old in ""; do
+  cd ../a
+  echo "int a$old() { return 0; }" > a.c
+  $LIBTOOL --mode=compile --tag=CC $CC $CPPFLAGS $CFLAGS -c a.c
+  cd ../b
+  echo "extern int a$old(); int b() { return a$old(); }" > b.c
+  $LIBTOOL --mode=compile --tag=CC $CC $CPPFLAGS $CFLAGS -c b.c
+  cd ../m
+  echo "extern int a$old(), b(); int main() { return a$old() + b(); }" > main.c
+  $CC $CPPFLAGS $CFLAGS -c main.c
+
+  for flag_deplib in "" "-L$lib_dir"; do
+    # TODO: fix ltmain so in next line we can also use "-L`pwd`/../a" 
+    for flag_lib in "" "-L../a" "-L`cd ../a && pwd`" "-L$lib_dir"; do
+      # TODO: fix ltmain so in next line we can also use `pwd`/../a
+      for lib_path in ../a `cd ../a && pwd`; do
+       for flag_instlib in "" "-L$lib_dir"; do
+         cd ../a
+         linkline="$LIBTOOL --mode=link --tag=CC $CC $CFLAGS $LDFLAGS 
$flag_deplib \
+                   -rpath $lib_dir -o liba.la a.lo"
+         AT_CHECK([echo $linkline; $linkline], [0], [ignore], [ignore])
+         cd ../b
+         linkline="$LIBTOOL --mode=link --tag=CC $CC $CFLAGS $LDFLAGS 
$flag_lib \
+                   -rpath $lib_dir -o libb.la b.lo $lib_path/liba.la"
+         AT_CHECK([echo $linkline; $linkline], [0], [ignore], [ignore])
+         # The next test does not have to be true: we can also just remove
+         # those paths in relink mode itself
+         # AT_CHECK([. ./libb.la; echo " $relink_command " | grep '/a '], [1])
+         cd ../m
+         linkline="$LIBTOOL --mode=link $CC $CFLAGS $LDFLAGS $flag_instlib \
+                   -no-fast-install -o main main.$OBJEXT ../b/libb.la"
+         AT_CHECK([echo $linkline; $linkline], [0], [ignore], [ignore])
+         LT_AT_EXEC_CHECK([./main])
+
+         # install the library and the program, check that no build-tree
+         # references remain:
+         # - Move the bogus installed lib temporarily, so that we do not
+         #   overwrite it (we need it for later tests)
+         # - After installing liba, but before installing libb, replace
+         #   the actual uninstalled versions of liba with bogus ones (not
+         #   the liba.la file, just the shared/static library): this way
+         #   we find out if all uninstalled paths have been removed from
+         #   the command line that relink mode outputs.
+         #   This is necessary for systems with hardcode_minus_L=yes.
+         # We do the same with libb, for installing main.
+         mv $lib_dir $lib_dir-backup
+         mkdir $lib_dir
+         AT_CHECK([$LIBTOOL --mode=install cp ../a/liba.la $lib_dir/liba.la], 
[0], [ignore], [ignore])
+         ( cd ../a && func_backup_libfiles ./liba.la )
+         AT_CHECK([$LIBTOOL --mode=install cp ../b/libb.la $lib_dir/libb.la], 
[0], [stdout], [stderr])
+         # FIXME: we should not point to uninstalled deplibs in relink mode:
+         # AT_CHECK([grep -v 'mode=relink' stdout stderr | grep '/a '], [1], 
[], [ignore])
+         ( cd ../b && func_backup_libfiles ./libb.la )
+         AT_CHECK([$LIBTOOL --mode=install cp main $bin_dir/main], [0], 
[stdout], [stderr])
+         # FIXME: we should not point to uninstalled deplibs in relink mode:
+         # AT_CHECK([grep -v 'mode=relink' stdout stderr | grep '/a '], [1], 
[], [ignore])
+         AT_CHECK([. $lib_dir/liba.la; echo " $dependency_libs " | grep '/a 
'], [1])
+
+         # FIXME: fix ltmain so that the next line does not fail any more.
+         AT_CHECK([. $lib_dir/libb.la; echo " $dependency_libs " | grep '/a 
'], [1])
+         ( cd ../b && func_restore_libfiles ./libb.la )
+         ( cd ../a && func_restore_libfiles ./liba.la )
+         $LIBTOOL --mode=clean rm -f main ../b/libb.la ../a/liba.la
+         LT_AT_EXEC_CHECK([$bin_dir/main])
+         $LIBTOOL --mode=uninstall rm -f $bin_dir/main $lib_dir/libb.la 
$lib_dir/liba.la
+         rmdir $lib_dir
+         mv $lib_dir-backup $lib_dir
+
+       done
+      done
+    done
+  done
+done
+
+# TODO: Now the same thing, but also using a dlopen'ed module that itself has a
+# dependent library in yet another source directory.
+
+# TODO: We would like to also support non-libtool libraries in the build tree.
+
+AT_CLEANUP