Re: [patch] win32: eliminate wrapper script in main build dir
On Sat, Jul 07, 2007 at 09:01:43PM +0200, Peter Rosin wrote: > > >> char *tmp_pathspec; > > >> char *actual_cwrapper_path; > > >> char *shwrapper_name; > > >>+ intptr_t rval = 127; > The above breaks on my MinGW install, which is definitely later than > 2000. I need to #include to get it. Also, I think some > #include is needed when using the MS compiler (cl). We might as well use `int' here, then.
Re: [patch] win32: eliminate wrapper script in main build dir
On Sat, Jun 16, 2007 at 05:06:20AM -0400, Charles Wilson wrote: > Noah Misch wrote: > >>@@ -2561,6 +2620,7 @@ > >> char *tmp_pathspec; > >> char *actual_cwrapper_path; > >> char *shwrapper_name; > >>+ intptr_t rval = 127; > > > >Do all interesting versions of Cygwin and MinGW have intptr_t? > > Yes, going back at least to 2000. There have been issues with the > _INTPTR_T_DEFINED (__intptr_t_defined on cygwin) guards clashing (or, > rather, the absence of these guards causing the typedef itself to clash) > with the winapi headers. But as the wrapper.c code does not include any > winapi headers, that's not an issue here. The above breaks on my MinGW install, which is definitely later than 2000. I need to #include to get it. Also, I think some #include is needed when using the MS compiler (cl). Cheers, Peter PS. I got married and "suffered" a name change in the process, formerly known as Peter Ekberg...
[patch] Add self to AUTHORS [Was: [patch] win32: eliminate wrapper script in main build dir]
Peter O'Gorman wrote: Reminds me, please add yourself to AUTHORS and contribute.html. (see Noah's commits). Done, for AUTHORS. Concerning contribute.html, does the commit script work when called from a foreign directory (like the web cvs dir)? 2007-06-19 Charles Wilson <[EMAIL PROTECTED]> * AUTHORS: Add myself. -- Chuck Index: AUTHORS === RCS file: /sources/libtool/libtool/AUTHORS,v retrieving revision 1.17 diff -u -r1.17 AUTHORS --- AUTHORS 18 Jun 2007 03:49:19 - 1.17 +++ AUTHORS 19 Jun 2007 06:18:41 - @@ -27,3 +27,4 @@ Peter Ekberg [EMAIL PROTECTED] Noah Misch [EMAIL PROTECTED] +Charles Wilson [EMAIL PROTECTED]
Re: [patch] win32: eliminate wrapper script in main build dir
Charles Wilson wrote: This passes all expected tests on linux (native). On cygwin (native) it fails 14,16,32, and 54, which is the expected behavior. I'll test [with export INSTALL=/usr/bin/install.exe to ensure that I don't get spurious failures for 35 39 43 46] on mingw (native) before committing, and then commit tomorrow only if the failures are limited to the same as on cygwin, and there are no other objections. Having obtain the desired results on mingw, and hearing no other objections; committed to HEAD. -- Chuck
Re: [patch] win32: eliminate wrapper script in main build dir
Ralf Wildenhues wrote:
* Charles Wilson wrote on Sun, Jun 17, 2007 at 02:43:02AM CEST:
OK by me. Do you wat me to add your patch to mine and check it all in
at once (e.g. keep the new-testsuite .exe fixes to destdir.at together),
or will you check your changes in separately?
However you like.
It is included below.
Attached. Unless you beat me to it, I can try to go about a patch
sometime though.
In any case, I'd like to postpone the cross-compile old-testsuite .exe
fixes to a followup patch.
Sure.
OK.
Updated patch attached, incorporating (I hope) all extant comments from
Noah and Ralf, along with an updated ChangeLog.
This passes all expected tests on linux (native).
On cygwin (native) it fails 14,16,32, and 54, which is the expected
behavior.
I'll test
[with
export INSTALL=/usr/bin/install.exe
to ensure that I don't get spurious failures for 35 39 43 46]
on mingw (native) before committing, and then commit tomorrow only if
the failures are limited to the same as on cygwin, and there are no
other objections.
--
Chuck
2007-06-17 Charles Wilson <[EMAIL PROTECTED]>
* libltdl/config/ltmain.m4sh: Add new magic variable
for use with cwrapper.
(func_ltwrapper_script_p): New function.
(func_ltwrapper_executable_p): New function.
(func_ltwrapper_scriptname): New function.
(func_ltwrapper_p): Accomodate both wrapper scripts and
wrapper executables.
(func_mode_execute): Handle $file that is a wrapper
script and $file that is a wrapper executable differently.
(func_mode_install) [cygwin|mingw]: If $file is a wrapper
executable, use func_ltwrapper_scriptname to determine
wrapper script name. Afterwards, always use
func_ltwrapper_script_p instead of func_ltwrapper_p.
(func_emit_libtool_wrapper_script): Rename to...
(func_emit_wrapper): ...this. All callers changed.
(func_emit_libtool_cwrapperexe_source): Rename to...
(func_emit_cwrapperexe_src): ...this. All callers changed.
Embed new magic_exe variable into source. Private transient
wrapper script now called foo_ltshwrapperTMP, not
foo_ltshwrapper.
(func_emit_cwrapperexe_src) [main, mingw]: Use _spawnv
and return child's exit code manually rather than rely on
broken execv.
(func_mode_link) [cygwin|mingw]: Don't call dirname and
basename directly; use func_dirname and func_basename
when computing cwrapper names. Use cwrapper to generate
wrapper script, and use pathname returned by
func_ltwrapper_scriptname instead of $output.
(func_mode_link) [NOT cygwin|mingw]: move wrapper script
generation for non-win32 inside case statement, as default
case.
(func_mode_uninstall) [$name's extension != .lo|.la]:
'clean' mode must handle $file differently if it is a libtool
wrapper script, or if it is a libtool wrapper executable.
* tests/destdir.at [Simple DESTDIR install]: $EXEEXT fixups.
* tests/destdir.at [DESTDIR with in-package deplibs]: Ditto.
Index: libltdl/config/ltmain.m4sh
===
RCS file: /cvsroot/libtool/libtool/libltdl/config/ltmain.m4sh,v
retrieving revision 1.79
diff -u -r1.79 ltmain.m4sh
--- libltdl/config/ltmain.m4sh 9 Jun 2007 17:46:40 - 1.79
+++ libltdl/config/ltmain.m4sh 17 Jun 2007 18:02:16 -
@@ -135,7 +135,7 @@
fi
magic="%%%MAGIC variable%%%"
-
+magic_exe="%%%MAGIC EXE variable%%%"
# Global variables.
# $mode is unset
@@ -661,13 +661,55 @@
test "$lalib_p" = yes
}
+# func_ltwrapper_script_p file
+# True iff FILE is a libtool wrapper script
+# This function is only a basic sanity check; it will hardly flush out
+# determined imposters.
+func_ltwrapper_script_p ()
+{
+func_lalib_p "$1"
+}
+
+# func_ltwrapper_executable_p file
+# True iff FILE is a libtool wrapper executable
+# This function is only a basic sanity check; it will hardly flush out
+# determined imposters.
+func_ltwrapper_executable_p ()
+{
+func_ltwrapper_exec_suffix=
+case $1 in
+*.exe) ;;
+*) func_ltwrapper_exec_suffix=.exe ;;
+esac
+$GREP "$magic_exe" "$1$func_ltwrapper_exec_suffix" >/dev/null 2>&1
+}
+
+# func_ltwrapper_scriptname file
+# Assumes file is an ltwrapper_executable
+# uses $file to determine the appropriate filename for a
+# temporary ltwrapper_script.
+func_ltwrapper_scriptname ()
+{
+func_ltwrapper_scriptname_result=""
+if func_ltwrapper_executable_p "$1"; then
+ func_dirname "$1"
+ func_basename "$1"
+ func_stripname '' '.exe' "$func_basename_result"
+ if test -z "$func_dirname_result"; then
+
func_ltwrapper_scriptname_result="./$objdir/${func_stripname_result}_ltshwrapper"
+ else
+
func_ltwrapper_scriptname_result="$func_dirname_result/$objdir/${func_stripname_result}_ltshwrapp
Re: [patch] win32: eliminate wrapper script in main build dir
Hi Charles, * Charles Wilson wrote on Sun, Jun 17, 2007 at 08:01:20PM CEST: > Ralf Wildenhues wrote: > >* Charles Wilson wrote on Sun, Jun 17, 2007 at 02:43:02AM CEST: > > OK by me. Do you wat me to add your patch to mine and check it all in > at once (e.g. keep the new-testsuite .exe fixes to destdir.at together), > or will you check your changes in separately? However you like. > >Grepping the verbose output of the old HEAD testsuite for 'No such file > >or directory' shows several bits that need adjustment, too. > > Well, this is a bit more excusable: INSIDE libtool, we're allowed to > depend on internal libtool details. Even if it isn't a really good > idea, as in this case. > > Notwithstanding the answer to my question above concerning > new-testsuite/destdir.at, I'd appreciate it if you would (at least) > provide me with your cross-compile verbose log from the old testsuite, > or (at best) a followup patch to fix these issues -- since I don't have > a cross-compile setup, here. Attached. Unless you beat me to it, I can try to go about a patch sometime though. > In any case, I'd like to postpone the cross-compile old-testsuite .exe > fixes to a followup patch. Sure. Cheers, Ralf checklog-old-suite.bz2 Description: Binary data
Re: [patch] win32: eliminate wrapper script in main build dir
Ralf Wildenhues wrote: * Charles Wilson wrote on Sun, Jun 17, 2007 at 02:43:02AM CEST: Now, in this current patch, we only use _spawnv() if $host == mingw. Maybe we could be more clever than that, and only use _spawnv if mingw and native. However, we'd then need to do something about this: # we should really use a build-platform specific compiler # here, but OTOH, the wrappers (shell script and this C one) # are only useful if you want to execute the "real" binary. # Since the "real" binary is built for $host, then this # wrapper might as well be built for $host, too. $opt_dry_run || $LTCC $LTCFLAGS -s -o $cwrapper $cwrappersource Ah, yes. Good, looks like your patch doesn't make things worse here. OK; I'll ignore this issue for now. The new suite fails tests 25 26 32 54 now, which it didn't back on 2007-02-24, which is when I last tested it. I got those failures (native), until I fixed tests/destdir.at to include $EXEEXT on $bindir/m: [...] Perhaps that "fix" breaks cross? No, the failure is earlier: the install command does not find the programs. I can "fix" them (make them skip again) by means of the patch below. OK? OK by me. Do you wat me to add your patch to mine and check it all in at once (e.g. keep the new-testsuite .exe fixes to destdir.at together), or will you check your changes in separately? (Seeing that we need this change, I wonder how many packages not using Automake (and how many even using Automake) will break by the new semantics of not having 'prog' but only 'prog.exe' at least in the cross-compile case...) Probably a fair many. However, IMO those packages were improperly ported to support (cross-) compiling for mingw anyway: they were relying on an internal implementation detail of libtool, in that it happened to create a wrapper script with the same full name as the executable created by the compiler on 'regular' *nix. Grepping the verbose output of the old HEAD testsuite for 'No such file or directory' shows several bits that need adjustment, too. Well, this is a bit more excusable: INSIDE libtool, we're allowed to depend on internal libtool details. Even if it isn't a really good idea, as in this case. Notwithstanding the answer to my question above concerning new-testsuite/destdir.at, I'd appreciate it if you would (at least) provide me with your cross-compile verbose log from the old testsuite, or (at best) a followup patch to fix these issues -- since I don't have a cross-compile setup, here. In any case, I'd like to postpone the cross-compile old-testsuite .exe fixes to a followup patch. I could use some advice on the following: trying to add a new magic string to the executable, I did the obvious (shown after variable replacement, etc: static const char *MAGIC_EXE = "%%%MAGIC EXE variable%%%"; But the compiler stripped the symbol and the initialization string out of the executable. For now just drop the static. This will work until LTO (link time optimization) is deployed. Not sure what we'll need with that in place, but I suppose either make the variable volatile (another non-fix) or add a stub volatile access to it in main. OK. I've already tested that and it does work -- at least at present. I'll post my current patch in a little while, although I still need to test it on (native) mingw, and linux. -- Chuck
Re: [patch] win32: eliminate wrapper script in main build dir
* Charles Wilson wrote on Sun, Jun 17, 2007 at 02:43:02AM CEST: > Ralf Wildenhues wrote: > > > >Is this code ever to be run on a different system than MinGW? What > >about cross-compile setups? (Maybe you've addressed this in one of > >the comments of all your patches and I've overlooked it now?) > > In one of the previous patches, we added this bit of support > > + if test -n "$TARGETSHELL" ; then > + # no path translation at all > + lt_newargv0=$TARGETSHELL > + else > + case "$host" in > + *mingw* ) > < lots of ugly code to sniff out the 'cmd shell' > > + ;; > + * ) lt_newargv0=$SHELL ;; > + esac > + fi > > for cross-compiling in general (but mainly linux->mingw/wine cross, IIRC). > > Now, in this current patch, we only use _spawnv() if $host == mingw. > Maybe we could be more clever than that, and only use _spawnv if mingw > and native. However, we'd then need to do something about this: > > # we should really use a build-platform specific compiler > # here, but OTOH, the wrappers (shell script and this C one) > # are only useful if you want to execute the "real" binary. > # Since the "real" binary is built for $host, then this > # wrapper might as well be built for $host, too. > $opt_dry_run || $LTCC $LTCFLAGS -s -o $cwrapper $cwrappersource Ah, yes. Good, looks like your patch doesn't make things worse here. > >The new suite fails tests 25 26 32 54 now, which it > >didn't back on 2007-02-24, which is when I last tested it. > > I got those failures (native), until I fixed tests/destdir.at to include > $EXEEXT on $bindir/m: [...] > Perhaps that "fix" breaks cross? No, the failure is earlier: the install command does not find the programs. I can "fix" them (make them skip again) by means of the patch below. OK? (Seeing that we need this change, I wonder how many packages not using Automake (and how many even using Automake) will break by the new semantics of not having 'prog' but only 'prog.exe' at least in the cross-compile case...) Grepping the verbose output of the old HEAD testsuite for 'No such file or directory' shows several bits that need adjustment, too. > I could use some advice on the following: trying to add a new magic > string to the executable, I did the obvious (shown after variable > replacement, etc: > > static const char *MAGIC_EXE = "%%%MAGIC EXE variable%%%"; > > But the compiler stripped the symbol and the initialization string out > of the executable. For now just drop the static. This will work until LTO (link time optimization) is deployed. Not sure what we'll need with that in place, but I suppose either make the variable volatile (another non-fix) or add a stub volatile access to it in main. Cheers, Ralf Index: tests/destdir.at === RCS file: /cvsroot/libtool/libtool/tests/destdir.at,v retrieving revision 1.5 diff -u -r1.5 destdir.at --- tests/destdir.at25 Mar 2007 12:12:43 - 1.5 +++ tests/destdir.at17 Jun 2007 08:09:50 - @@ -60,7 +60,7 @@ mkdir $DESTDIR$libdir $DESTDIR$bindir AT_CHECK([$LIBTOOL --mode=install cp liba.la $DESTDIR$libdir/liba.la], [], [ignore], [ignore]) -AT_CHECK([$LIBTOOL --mode=install cp m $DESTDIR$bindir/m], +AT_CHECK([$LIBTOOL --mode=install cp m$EXEEXT $DESTDIR$bindir/m$EXEEXT], [], [ignore], [ignore]) $LIBTOOL --mode=clean rm -f liba.la m LT_AT_MVDIR(["$DESTDIR$libdir"], ["$libdir"]) @@ -99,7 +99,7 @@ [], [ignore], [ignore]) AT_CHECK([$LIBTOOL --mode=install cp liba.la $DESTDIR$libdir/liba.la], [], [ignore], [ignore]) -AT_CHECK([$LIBTOOL --mode=install cp m $DESTDIR$bindir/m], +AT_CHECK([$LIBTOOL --mode=install cp m$EXEEXT $DESTDIR$bindir/m$EXEEXT], [], [ignore], [ignore]) $LIBTOOL --mode=clean rm -f liba1dep.la liba2dep.la liba.la m LT_AT_MVDIR(["$DESTDIR$libdir"], ["$libdir"])
Re: [patch] win32: eliminate wrapper script in main build dir
Ralf Wildenhues wrote:
Let's keep as much code once as possible, and kill some superfluous
quotes:
func_ltwrapper_executable_p ()
{
func_ltwrapper_exec_suffix=
case $1 in
*.exe) ;;
*) func_ltwrapper_exec_suffix=.exe ;;
esac
grep "$magic" "$1$func_ltwrapper_exec_suffix" >/dev/null
}
OK.
Of course this is treading in dangerous waters, as not all systems' grep
commands work well with non text files (and POSIX does not demand it
either). I suppose we should 2>&1 here to suppress error output (and
live with the occasional core file), just like we do in func_lalib_p.
Yes. I mentioned this as a possible portability problem here:
http://lists.gnu.org/archive/html/libtool-patches/2007-04/msg00052.html
But as you note above, and as I noted then, we're already relying on
grepping a binary file in func_ltwrapper_p/func_lalib_p.
I'll add the 2>&1 redirection.
Side note: there should be a function that does both dirname and
basename at once, for speed. (Not in this patch, but later.)
Another instance below.
Ack.
[...]
# func_ltwrapper_p file
-# True iff FILE is a libtool wrapper script.
+# True iff FILE is a libtool wrapper script or wrapper executable
# This function is only a basic sanity check; it will hardly flush out
# determined imposters.
func_ltwrapper_p ()
{
-func_lalib_p "$1"
+func_ltwrapper_script_p "$1" || func_ltwrapper_execuable_p "$1"
}
Typo: s/execuable/executable/
Hmm...thanks. I see why this typo didn't me a problem, but it should be
fixed.
There is no need for double quotes on the right hand side of
assignments (2 instances).
Ack.
@@ -2688,19 +2748,28 @@
case $host_os in
mingw*)
cat <+ LTWRAPPER_DEBUGPRINTF (("(main) failed to launch target \"$lt_newargv0\": errno = %d\n", errno));
+ return 127;
+}
+ return rval;
+}
EOF
;;
*)
cat <
Is this code ever to be run on a different system than MinGW? What
about cross-compile setups? (Maybe you've addressed this in one of
the comments of all your patches and I've overlooked it now?)
In one of the previous patches, we added this bit of support
+ if test -n "$TARGETSHELL" ; then
+ # no path translation at all
+ lt_newargv0=$TARGETSHELL
+ else
+ case "$host" in
+ *mingw* )
< lots of ugly code to sniff out the 'cmd shell' >
+ ;;
+ * ) lt_newargv0=$SHELL ;;
+ esac
+ fi
for cross-compiling in general (but mainly linux->mingw/wine cross, IIRC).
Now, in this current patch, we only use _spawnv() if $host == mingw.
Maybe we could be more clever than that, and only use _spawnv if mingw
and native. However, we'd then need to do something about this:
# we should really use a build-platform specific compiler
# here, but OTOH, the wrappers (shell script and this C one)
# are only useful if you want to execute the "real" binary.
# Since the "real" binary is built for $host, then this
# wrapper might as well be built for $host, too.
$opt_dry_run || $LTCC $LTCFLAGS -s -o $cwrapper $cwrappersource
@@ -6708,8 +6777,13 @@
esac
case $host in
*cygwin* | *mingw* )
- output_name=`basename $output`
- output_path=`dirname $output`
+ func_basename "$output"
+ output_name="$func_basename_result"
+ func_dirname "$output"
+ output_path="$func_dirname_result"
Superfluous quotes as above (2 instances).
Ack.
I would prefer some shorter names in this. Consider a followup patch to
s/func_emit_libtool_wrapper_script/func_emit_wrapper/g
s/func_emit_libtool_cwrapperexe_source/func_emit_cwrapperexe_src/g
as approved.
OK.
With this patch, and Debian etch's MinGW cross compiler package mingw32
I get some test failures in the new suite. The old suite has failed
demo-hardcode.test and demo-relink.test for a long time (let's not worry
about them now).
This pass native (FYI).
The new suite fails tests 25 26 32 54 now, which it
didn't back on 2007-02-24, which is when I last tested it.
I got those failures (native), until I fixed tests/destdir.at to include
$EXEEXT on $bindir/m:
-if test "$OBJDUMP" != false && ($OBJDUMP -p $bindir/m) >/dev/null 2>&1;
then
- AT_CHECK([$OBJDUMP -p $bindir/m | $EGREP -i "R(UN)?PATH.*$DESTDIR"], [1])
+if test "$OBJDUMP" != false && ($OBJDUMP -p $bindir/m$EXEEXT)
>/dev/null 2>&1; then
+ AT_CHECK([$OBJDUMP -p $bindir/m$EXEEXT | $EGREP -i
"R(UN)?PATH.*$DESTDIR"], [1])
Perhaps that "fix" breaks cross? Objdump complains very loudly if its
target does not exist. On native, $bindir/m doesn't exist, but
$bindir/m.exe does. I imagine on cross, if m is built using the host
compiler, the situation may be reversed, but that's just a guess.
I haven't
localized it any further yet, so it may be
Re: [patch] win32: eliminate wrapper script in main build dir
* Ralf Wildenhues wrote on Sat, Jun 16, 2007 at 12:31:10PM CEST: > The new suite fails tests 25 26 32 54 now, which it > didn't back on 2007-02-24, which is when I last tested it. I haven't > localized it any further yet, so it may be completely independent of > this patch. Oh, I forgot to attach the fail log. Apologies, here it is. Cheers, Ralf testsuite.log.bz2 Description: Binary data
Re: [patch] win32: eliminate wrapper script in main build dir
Hello Charles,
Some additional comments:
* Charles Wilson wrote on Sun, Jun 10, 2007 at 08:23:58PM CEST:
>
> --- libltdl/config/ltmain.m4sh9 Jun 2007 17:46:40 - 1.79
> +++ libltdl/config/ltmain.m4sh10 Jun 2007 07:22:05 -
> +func_ltwrapper_executable_p ()
> +{
> +func_ltwrapper_executable_p_result=no
> +if ! func_ltwrapper_script_p "$1" ; then
> + case "$1" in
> +*.exe ) if grep "$magic" "$1" >/dev/null ; then
> +func_ltwrapper_executable_p_result=yes
> +fi ;;
> +* ) if grep "$magic" "${1}.exe" >/dev/null ; then
> +func_ltwrapper_executable_p_result=yes
> +fi ;;
> + esac
> +fi
> +test "$func_ltwrapper_executable_p_result" = "yes"
> +}
Let's keep as much code once as possible, and kill some superfluous
quotes:
func_ltwrapper_executable_p ()
{
func_ltwrapper_exec_suffix=
case $1 in
*.exe) ;;
*) func_ltwrapper_exec_suffix=.exe ;;
esac
grep "$magic" "$1$func_ltwrapper_exec_suffix" >/dev/null
}
Of course this is treading in dangerous waters, as not all systems' grep
commands work well with non text files (and POSIX does not demand it
either). I suppose we should 2>&1 here to suppress error output (and
live with the occasional core file), just like we do in func_lalib_p.
> +# func_ltwrapper_temp_scriptname file
> +# Assumes file is an ltwrapper_executable
> +# uses $file to determine the appropriate filename for a
> +# temporary ltwrapper_script.
> +func_ltwrapper_temp_scriptname ()
> +{
> +func_ltwrapper_temp_scriptname_result=""
> +if func_ltwrapper_executable_p "$1"; then
> +func_dirname "$1"
> +func_basename "$1"
Side note: there should be a function that does both dirname and
basename at once, for speed. (Not in this patch, but later.)
Another instance below.
[...]
> # func_ltwrapper_p file
> -# True iff FILE is a libtool wrapper script.
> +# True iff FILE is a libtool wrapper script or wrapper executable
> # This function is only a basic sanity check; it will hardly flush out
> # determined imposters.
> func_ltwrapper_p ()
> {
> -func_lalib_p "$1"
> +func_ltwrapper_script_p "$1" || func_ltwrapper_execuable_p "$1"
> }
Typo: s/execuable/executable/
> @@ -2085,14 +2139,19 @@
> # Do a test to see if this is really a libtool program.
> case $host in
> *cygwin*|*mingw*)
> - func_stripname '' '.exe' "$file"
> - wrapper=$func_stripname_result
> +if func_ltwrapper_executable_p "$file"; then
> + func_ltwrapper_temp_scriptname "$file"
> + wrapper="$func_ltwrapper_temp_scriptname_result"
> +else
> + func_stripname '' '.exe' "$file"
> + wrapper="$func_stripname_result"
There is no need for double quotes on the right hand side of
assignments (2 instances).
> +fi
> ;;
> *)
> wrapper=$file
> ;;
> esac
> - if func_ltwrapper_p "$wrapper"; then
> + if func_ltwrapper_script_p "$wrapper"; then
> notinst_deplibs=
> relink_command=
>
> @@ -2561,6 +2620,7 @@
>char *tmp_pathspec;
>char *actual_cwrapper_path;
>char *shwrapper_name;
> + intptr_t rval = 127;
>FILE *shwrapper;
>
>const char *dumpscript_opt = "--lt-dump-script";
> @@ -2688,19 +2748,28 @@
> case $host_os in
> mingw*)
> cat < - execv ("$lt_newargv0", (const char * const *) newargz);
> + /* execv doesn't actually work on mingw as expected on unix */
> + rval = _spawnv (_P_WAIT, "$lt_newargv0", (const char * const *) newargz);
> + if (rval == -1)
> +{
> + /* failed to start process */
> + LTWRAPPER_DEBUGPRINTF (("(main) failed to launch target
> \"$lt_newargv0\": errno = %d\n", errno));
> + return 127;
> +}
> + return rval;
> +}
> EOF
> ;;
> *)
> cat + return rval; /* =127, but avoids unused variable warning */
> +}
Is this code ever to be run on a different system than MinGW? What
about cross-compile setups? (Maybe you've addressed this in one of
the comments of all your patches and I've overlooked it now?)
> @@ -6708,8 +6777,13 @@
> esac
> case $host in
> *cygwin* | *mingw* )
> - output_name=`basename $output`
> - output_path=`dirname $output`
> + func_basename "$output"
> + output_name="$func_basename_result"
> + func_dirname "$output"
> + output_path="$func_dirname_result"
Superfluous quotes as above (2 instances).
I would prefer some shorter names in this. Consider a followup patch to
s/func_emit_libtool_wrapper_script/func_emit_wrapper/g
s/func_emit_libtool_cwrapperexe_source/func_emit_cwrapperexe_src/g
as approved.
With this patch, and Debian etch's MinGW cross compiler package mingw32
I get some test failures in the new suite. T
Re: [patch] win32: eliminate wrapper script in main build dir
Noah Misch wrote:
Overall, the patch looks suitable. Some minor comments:
+func_ltwrapper_executable_p_result=no
+if ! func_ltwrapper_script_p "$1" ; then
The `!' operator is not portable; use `if X; then :; else'. You could instead
add a different magic string to executables, avoiding two forks for this test.
That's a good idea; I'll do that.
+fi
Trim trailing white space from this line.
done
+else
+ if func_ltwrapper_executable_p "$file"; then
Use `elif'.
done
+func_ltwrapper_temp_scriptname "$file"
+ func_source "$func_ltwrapper_temp_scriptname_result"
+ # Transform arg to wrapped name.
+ file="$progdir/$program"
+ fi
fi
Place `file="$progdir/$program"' here, rather than keeping a copy in each branch
of the control flow.
OK.
@@ -2561,6 +2620,7 @@
char *tmp_pathspec;
char *actual_cwrapper_path;
char *shwrapper_name;
+ intptr_t rval = 127;
Do all interesting versions of Cygwin and MinGW have intptr_t?
Yes, going back at least to 2000. There have been issues with the
_INTPTR_T_DEFINED (__intptr_t_defined on cygwin) guards clashing (or,
rather, the absence of these guards causing the typedef itself to clash)
with the winapi headers. But as the wrapper.c code does not include any
winapi headers, that's not an issue here.
@@ -6717,19 +6791,30 @@
func_emit_libtool_cwrapperexe_source > $cwrappersource
- # we should really use a build-platform specific compiler
- # here, but OTOH, the wrappers (shell script and this C one)
- # are only useful if you want to execute the "real" binary.
- # Since the "real" binary is built for $host, then this
- # wrapper might as well be built for $host, too.
- $opt_dry_run || $LTCC $LTCFLAGS -s -o $cwrapper $cwrappersource
+ # we should really use a build-platform specific compiler
+ # here, but OTOH, the wrappers (shell script and this C one)
+ # are only useful if you want to execute the "real" binary.
+ # Since the "real" binary is built for $host, then this
+ # wrapper might as well be built for $host, too.
+ $opt_dry_run || $LTCC $LTCFLAGS -s -o $cwrapper $cwrappersource
As far as I can see, this chunk just converts spaces to tabs. That may be a
worthy change, but it does not belong in this patch.
No, I didn't convert spaces to tabs (at least, not that I can tell. Each
line still begins with a tab both before the patch and after). The
difference is I corrected the actual indentation of a small section of
the block I was already changing for other reasons:
+
+ # Now, create the wrapper script for func_source use:
+ func_ltwrapper_temp_scriptname $cwrapper
+ $RM $func_ltwrapper_temp_scriptname_result
+ trap "$RM $func_ltwrapper_temp_scriptname_result; exit
$EXIT_FAILURE" 1 2 15
+ $opt_dry_run || {
+ $cwrapper --lt-dump-script >
$func_ltwrapper_temp_scriptname_result
+ }
+ chmod +x $func_ltwrapper_temp_scriptname_result
This script need not be executable, since we only source it.
Done.
I'm still testing the revised patch; I'll post it with an updated
changelog tomorrow.
--
Chuck
Re: [patch] win32: eliminate wrapper script in main build dir
Hi Charles,
Overall, the patch looks suitable. Some minor comments:
On Sun, Jun 10, 2007 at 02:23:58PM -0400, Charles Wilson wrote:
> 2007-04-22 Charles Wilson <[EMAIL PROTECTED]>
>
> * libltdl/config/ltmain.m4sh (func_ltwrapper_script_p):
> new function detects if $1 is a libtool sh wrapper
See `info standards doc change' for change log style information. The sentence
after the colon begins with a capital letter and ends with a period. It is
enough to write "New function." when that is the case.
> --- libltdl/config/ltmain.m4sh9 Jun 2007 17:46:40 - 1.79
> +++ libltdl/config/ltmain.m4sh10 Jun 2007 07:22:05 -
> @@ -661,13 +661,61 @@
> test "$lalib_p" = yes
> }
>
> +# func_ltwrapper_script_p file
> +# True iff FILE is a libtool wrapper script
> +# This function is only a basic sanity check; it will hardly flush out
> +# determined imposters.
> +func_ltwrapper_script_p ()
> +{
> +func_lalib_p "$1"
> +}
> +
> +# func_ltwrapper_executable_p file
> +# True iff FILE is a libtool wrapper executable
> +# This function is only a basic sanity check; it will hardly flush out
> +# determined imposters.
> +func_ltwrapper_executable_p ()
> +{
> +func_ltwrapper_executable_p_result=no
> +if ! func_ltwrapper_script_p "$1" ; then
The `!' operator is not portable; use `if X; then :; else'. You could instead
add a different magic string to executables, avoiding two forks for this test.
> + case "$1" in
> +*.exe ) if grep "$magic" "$1" >/dev/null ; then
> +func_ltwrapper_executable_p_result=yes
> +fi ;;
> +* ) if grep "$magic" "${1}.exe" >/dev/null ; then
> +func_ltwrapper_executable_p_result=yes
> +fi ;;
> + esac
> +fi
> +test "$func_ltwrapper_executable_p_result" = "yes"
> +}
> +
> +# func_ltwrapper_temp_scriptname file
> +# Assumes file is an ltwrapper_executable
> +# uses $file to determine the appropriate filename for a
> +# temporary ltwrapper_script.
> +func_ltwrapper_temp_scriptname ()
> +{
> +func_ltwrapper_temp_scriptname_result=""
> +if func_ltwrapper_executable_p "$1"; then
> +func_dirname "$1"
> +func_basename "$1"
> +func_stripname '' '.exe' "$func_basename_result"
> + if test -z "$func_dirname_result"; then
> +
> func_ltwrapper_temp_scriptname_result="./$objdir/${func_stripname_result}_ltshwrapperTMP"
> +else
> +
> func_ltwrapper_temp_scriptname_result="$func_dirname_result/$objdir/${func_stripname_result}_ltshwrapperTMP"
> +fi
> +fi
Trim trailing white space from this line.
> +}
> +
> # func_ltwrapper_p file
> -# True iff FILE is a libtool wrapper script.
> +# True iff FILE is a libtool wrapper script or wrapper executable
> # This function is only a basic sanity check; it will hardly flush out
> # determined imposters.
> func_ltwrapper_p ()
> {
> -func_lalib_p "$1"
> +func_ltwrapper_script_p "$1" || func_ltwrapper_execuable_p "$1"
> }
>
>
> @@ -1649,11 +1697,17 @@
>-*) ;;
>*)
> # Do a test to see if this is really a libtool program.
> - if func_ltwrapper_p "$file"; then
> - func_source "$file"
> -
> + if func_ltwrapper_script_p "$file"; then
> + func_source "$file"
> # Transform arg to wrapped name.
> file="$progdir/$program"
> +else
> + if func_ltwrapper_executable_p "$file"; then
Use `elif'.
> +func_ltwrapper_temp_scriptname "$file"
> + func_source "$func_ltwrapper_temp_scriptname_result"
> + # Transform arg to wrapped name.
> + file="$progdir/$program"
> + fi
> fi
Place `file="$progdir/$program"' here, rather than keeping a copy in each branch
of the control flow.
> @@ -2561,6 +2620,7 @@
>char *tmp_pathspec;
>char *actual_cwrapper_path;
>char *shwrapper_name;
> + intptr_t rval = 127;
Do all interesting versions of Cygwin and MinGW have intptr_t?
> @@ -6717,19 +6791,30 @@
>
> func_emit_libtool_cwrapperexe_source > $cwrappersource
>
> - # we should really use a build-platform specific compiler
> - # here, but OTOH, the wrappers (shell script and this C one)
> - # are only useful if you want to execute the "real" binary.
> - # Since the "real" binary is built for $host, then this
> - # wrapper might as well be built for $host, too.
> - $opt_dry_run || $LTCC $LTCFLAGS -s -o $cwrapper $cwrappersource
> + # we should really use a build-platform specific compiler
> + # here, but OTOH, the wrappers (shell script and this C one)
> + # are only useful if you want to execute the "real" binary.
> + # Since the "real" binary is built for $host, then this
> + # wrapper might as well be built for $host, too.
> + $opt_dry_run || $LTCC $LTCFLAGS -s -o $cwrapper $cwrappersource
As far as I can see, this
[patch] win32: eliminate wrapper script in main build dir
This patch is an updated version of the one posted here: http://lists.gnu.org/archive/html/libtool-patches/2007-04/msg00052.html It is updated to reflect ongoing changes in HEAD, and avoids the mingw regressions mentioned in that email. Theory: On cygwin/mingw, a wrapper executable is used which should invoke a wrapper script that is the typical libtool wrapper script used on other platforms. However, it should be hidden inside $objdir, and given a name distinct from its target. I chose: .libs/foo_ltshwrapper The exectuable wrapper generates this file, on the fly, every time it is invoked. Therefore, this wrapper script *does not exist* until the target executable is invoked. This behavior is already incorporated in libtool-HEAD. In general, libtool ALSO uses the wrapper script to "save" information about the target, and often sources it. On mingw/cygwin, this patch insures that a suitable wrapper script is generated at the same time the wrapper executable is created -- in fact, with this patch libtool now uses the wrapper executable to generate it. However, I choose a *different* name than the one above: .libs/foo_ltshwrapperTMP Of course, other platforms continue to create a wrapper script as ./foo and do not use the executable wrapper at all. Thus, with this patch, the '.' directory will contain only wrapper executables, and no wrapper scripts -- eliminating "foo.exe" vs "foo" confusion. In $objdir, there will exist .libs/foo.exe .libs/foo_ltshwrapperTMP .libs/foo_ltshwrapper [ recreated on-demand ] (okay, maybe the names are backwards, but that's a two line cosmetic patch) Now, it is possible that we could use the *same* .libs/foo_ltshwrapper file for both "execute" and "func_source" purposes. However: (a) I never want the executable wrapper to fail because the wrapper script is missing (b) I don't want the timestamp of the func_source()'ed wrapper script to depend on whether/when the target was executed (c) It is possible that, in the future, we will need to store different state in one of these two: for instance, the wrapper executable may eventually take another cmdline arg like "--lt-relink-cmd '.'". By using separate files, one "invisible" (created on the fly by the executable wrapper as a transparent part of its execv()-the-target proess), and one "visible" that is timestamped at the same time as the executable wrapper, we retain enough flexibility for the future. I'd like to be able to delete the "invisible" wrapper script after use, but that's hard to arrange since we're using execv(): there is no execv(target)-and-tell-target-to-delete-itself-afterwards. On mingw, we (now) use _spawnv() and wait for the child, so presumably on that platform we could delete the on-the-fly wrapper script after use. But (1) I don't want mingw and cygwin to behave differently (one cleans up the temporary wrapper, the other doesn't), and (2) I'm not ready to change the wrapper's behavior in general to be wait-for-child. I did it on mingw out of necessity (see below). In any case, the execv-vs-spawnv and delete-after-use behavior can be addressed later. Comments? Portability issue? Implementation of func_ltwrapper_executable_p() greps $1 for the expanded form of $magic. This works because the wrapper executable contains the wrapper script contents verbatim, and the wrapper script tests $libtool_install_magic against the expanded $magic. We're already grepping binary files, without this patch. func_ltwrapper_p() can (and often is) called with a binary as $1. However, it delegates to func_lalib_p(), which does: $SED -e 4q "$1" 2>/dev/null \ | $GREP "^# Generated by .*$PACKAGE" > /dev/null 2>&1 Mingw-specific The original version of this patch -- and my initial forward port of it to current HEAD -- induced new testsuite regressions on mingw: < PASS: tests/demo-relink.test > FAIL: tests/demo-relink.test < PASS: tests/depdemo-relink.test > FAIL: tests/depdemo-relink.test 26: DESTDIR with in-package deplibs FAILED (destdir.at:131) 29: C subdir-objectsFAILED (am-subdir.at:81) 50: config.status FAILED (early-libtool.at:115) 51: config.lt FAILED (early-libtool.at:220) These turned out to be side effects of the wrapper script no longer being present in the main build dir. Up to this point, the msys shell was always using the wrapper script and not the wrapper executable. However, with the wrapper script missing, msys was now forced to use the wrapper executable. Because on mingw, execv() doesn't actually replace the current process, all 'expect fail' tests were getting an exit code of 0 -- from the wrapp
