Re: mode=execute argument munging bug

[Ralf Wildenhues]

* Roberto Bagnara wrote on Wed, Mar 05, 2008 at 07:37:58AM CET:

 It is better now, but there is still the problem that, apparently,
 libtool redirects stdin for the program it is running.

Gosh.  How embarrassing.  I've applied this patch.

Thanks for testing!
Ralf

2008-03-05  Ralf Wildenhues  [EMAIL PROTECTED]

* libltdl/config/ltmain.m4sh (func_lalib_unsafe_p): redirect
and restore from stdin, not stdout.
* tests/execute-mode.at (execute mode): Adjust test to catch
this.
Report by Roberto Bagnara.

Index: libltdl/config/ltmain.m4sh
===
RCS file: /cvsroot/libtool/libtool/libltdl/config/ltmain.m4sh,v
retrieving revision 1.98
diff -u -r1.98 ltmain.m4sh
--- libltdl/config/ltmain.m4sh  4 Mar 2008 21:25:48 -   1.98
+++ libltdl/config/ltmain.m4sh  5 Mar 2008 20:12:28 -
@@ -648,7 +648,7 @@
 func_lalib_unsafe_p ()
 {
 lalib_p=no
-if test -r $1  exec 51 $1; then
+if test -r $1  exec 50 $1; then
for lalib_p_l in 1 2 3 4
do
read lalib_p_line
@@ -656,7 +656,7 @@
\#\ Generated\ by\ *$PACKAGE* ) lalib_p=yes; break;;
esac
done
-   exec 15 5-
+   exec 05 5-
 fi
 test $lalib_p = yes
 }
Index: tests/execute-mode.at
===
RCS file: /cvsroot/libtool/libtool/tests/execute-mode.at,v
retrieving revision 1.1
diff -u -r1.1 execute-mode.at
--- tests/execute-mode.at   4 Mar 2008 21:25:48 -   1.1
+++ tests/execute-mode.at   5 Mar 2008 20:12:28 -
@@ -51,6 +51,30 @@
 AT_DATA([lt-real],
 [[#! /bin/sh
 echo $@
+cat
+]])
+
+AT_DATA([libfakelib.la],
+[[# libfakelib.la - a libtool library file
+# Generated by ltmain.sh (GNU libtool 1.2605 2008/03/04 22:31:32) 2.3a
+#
+# Please DO NOT delete this file!
+# It is necessary for linking the library.
+
+dlname=''
+library_names=''
+old_library='libfakelib.a'
+inherited_linker_flags=''
+dependency_libs=''
+weak_library_names=''
+current=
+age=
+revision=
+installed=no
+shouldnotlink=yes
+dlopen=''
+dlpreopen=''
+libdir=''
 ]])
 
 mkdir sub
@@ -61,20 +85,26 @@
 AT_CHECK([$LIBTOOL --mode=execute sub/foo])
 AT_CHECK([$LIBTOOL --mode=execute ./foo foo], [], [foo
 ])
-AT_CHECK([$LIBTOOL --mode=execute ./lt-wrapper foo], [], [foo
+AT_CHECK([$LIBTOOL --mode=execute ./lt-wrapper foo /dev/null], [], [foo
 ])
 AT_CHECK([cd sub  $LIBTOOL --mode=execute ./foo ../foo], [], [../foo
 ])
 # suppose that ./foo is gdb, and lt-wrapper is the wrapper script.
-AT_CHECK([$LIBTOOL --mode=execute ./foo lt-wrapper bar baz], [],
+AT_CHECK([$LIBTOOL --mode=execute ./foo lt-wrapper bar baz /dev/null], [],
 [./lt-real bar baz
 ])
 
+# check that stdin works even with -dlopen.
+AT_CHECK([echo bar | $LIBTOOL --mode=execute -dlopen libfakelib.la 
./lt-wrapper foo],
+[], [foo
+bar
+])
+
 # Check that a missing real program causes an error.
 # The error message and code are likely to be 126,
 # No such file or directory but system-dependent.
 mv -f lt-real lt-backup
-AT_CHECK([$LIBTOOL --mode=execute ./lt-wrapper foo || exit 1],
+AT_CHECK([$LIBTOOL --mode=execute ./lt-wrapper foo /dev/null || exit 1],
 [1], [ignore], [ignore])
 mv -f lt-backup lt-real
 
@@ -82,7 +112,7 @@
 AT_CHECK([$LIBTOOL --mode=execute ./foo arg  with special chars: \$!*\`'()],
 [], [arg  with special chars: $!*`'()
 ])
-AT_CHECK([$LIBTOOL --mode=execute ./lt-wrapper arg  with special chars: 
\$!*\`'()],
+AT_CHECK([$LIBTOOL --mode=execute ./lt-wrapper arg  with special chars: 
\$!*\`'() /dev/null],
 [], [arg  with special chars: $!*`'()
 ])
 AT_CHECK([$LIBTOOL --mode=execute ./foo lt-wrapper arg  with special chars: 
\$!*\`'()],

Re: libtool generated by GNU $PACKAGE

[Ralf Wildenhues]

Hi Eric,

* Eric Blake wrote on Wed, Mar 05, 2008 at 04:28:51AM CET:
 According to Ralf Wildenhues on 3/4/2008 2:14 PM:
 | Fixed with the patch below.  I don't care much that, in the Libtool
 | package itself, the will result in a libtool script with the line
 | # Generated automatically by config.status (libtool 1.2600 2008/03/02 
 02:19:16) 2.3a
 |
 | instead of GNU libtool.

 Hmm.  While this avoids the bug, I'm not sure if it was the best fix.

Granted, it's certainly debatable.  IMHO it's a strict improvement over
what we had before, though.

 |  # `$ECHO $ofile | sed 's%^.*/%%'` - Provide generalized library-building 
 support services.
 | -# Generated automatically by $as_me (GNU $PACKAGE$TIMESTAMP) $VERSION
 | +# Generated automatically by $as_me ($PACKAGE$TIMESTAMP) $VERSION

 TIMESTAMP is a libtool invention, and is not documented in autoconf or
 automake as a typical variable.  At one point, M4 also tried using it, by
 parsing the CVS revision from ChangeLog, but ever since m4 switched to
 git, that aspect is broken (besides, I like what autoconf has achieved
 with intra-release versioning via the git-version-gen script).  Can we
 expect reasonable semantics from TIMESTAMP in all other libtoolized
 packages, if we don't document it?  Meanwhile, is there one of the
 Autoconf-provided variables, such as PACKAGE_NAME, which might fit better
 (and in the case of libtool, preserve the name 'GNU libtool')?

PACKAGE_NAME is currently not 'GNU libtool' for Libtool, also
PACKAGE_NAME is currently not available inside config.status (FWIW, I
feel that the fact that libtool even makes $PACKAGE available in
config.status is an ugly hack and a layering violation; OTOH you should
be able to decide at m4 time whether what we're bootstrapping is Libtool
or some third-party package).

Changing PACKAGE for the Libtool package requires somebody to audit all
code that matches comments in .lo and .la files and so on.  If TIMESTAMP
is empty because we are in a third party package, the above line has no
problem.  The exact timestamp and version of the Libtool used for
libtoolizing the third-party package is anyway found further down in the
libtool file, after the variable settings:

# Generated from ltmain.m4sh.

# ltmain.sh (GNU libtool 1.2600 2008/03/02 02:19:16) 2.3a

So frankly, the only thing that may be bothering at all is the missing
'GNU ' for the /usr/bin/libtool script which comes from the Libtool
package, and that TIMESTAMP may need adjusting when we move to git.
I let somebody motivated enough work on fixing the first, and will cross
the bridge for the second when I get to it.

Cheers,
Ralf