Re: [cygwin] cwrapper emits wrapper script
Hello, * Charles Wilson wrote on Wed, Jun 06, 2007 at 04:31:02PM CEST: I didn't realize Ralf was on holiday [...] That and an unintended stay at hospital afterwards to un-break my collarbone again. Cheers, Ralf, typing one-handed ATM
Re: [cygwin] cwrapper emits wrapper script
Peter O'Gorman wrote: On Wed, 2007-06-06 at 10:25 -0400, Charles Wilson wrote: 2007-04-27 Charles Wilson [EMAIL PROTECTED] * ltmain.m4sh (func_emit_libtool_wrapper_script): add code block to handle cases when wrapper script is in $objdir. (func_emit_libtool_cwrapperexe_source): replace DEBUG() macro with namespace-safe LTWRAPPER_DEBUGPRINTF(). Call func_emit_libtool_wrapper_script() with appropriate filters to embed script text in C char* static variable. (f_e_l_c_s: main): add new option --lt-dump-script, parse argv[] for it, and take appropriate action. Call chase_symlinks() on argv[0], in case this.exe was launched via one. Use chased value to determine full absolute pathname of wrapper script, in $objdir. Unconditionally write script out to this pathname and set permission bits. Remove extraneous ';' from debugging loop. (f_e_l_c_s: main) [mingw]: DOS-ize $SHELL before populating newargv[0]; if $TARGETSHELL environment variable is set, use it instead of $SHELL and do not DOS-ize. Ensure newargv[1] (wrapper script absolute path) uses only '/', not '\'. Make sure to call execv() with DOS-ized $SHELL (or un-DOS-ized $TARGETSHELL, if set) (f_e_l_c_s: make_executable): new function (f_e_l_c_s: chase_symlinks): new function (no-op if !S_ISLNK) 2007-04-27 Charles Wilson [EMAIL PROTECTED] * ltmain.m4sh (func_emit_libtool_cwrapperexe_source): ensure that generated source follows GCS as nearly as possible. Ok. Causes no regressions for me on linux or darwin, and nothing jumps out at me, please feel free to exercise your commit bit on these :-) I am not, unfortunately, able to test on windows. Committed (both chunks, but separately). The funny thing is, the hardest part was setting up an MTA under cygwin so that the commit script could work. I ended up having to recompile cygwin's ssmtp package to add SSL support, since my mailhost doesn't support unencrypted... Noah gave a good suggestion about passing WRAPPER_SCRIPT_BELONGS_IN_OBJDIR as an argument, but, I agree, it can be left to a later patch. Yes, I will make that change. However, I'd like to get part 3 of my 3-patch sequence up for consideration first. It's the last step in finally eliminating the ./foo (sh wrapper) vs. ./foo.exe (C wrapper) in the same directory confusion on cygwin and mingw. I need to update original patch and run it thru my gamut of tests, but the old version is here: http://lists.gnu.org/archive/html/libtool-patches/2007-04/msg00052.html At some point it would be nice to emit func_emit_libtool_cwrapperexe_source from config.status, saving 550 lines of code from libtool on non-windows platforms. But I'm just dreaming, you can ignore that :) Good, because I wouldn't know where to start, pushing something into config.status. Thanks for your review and testing. -- Chuck
Re: [cygwin] cwrapper emits wrapper script
Charles Wilson libtool at cwilson.fastmail.fm writes:
Attached.
Some nits that you should fix, now that you have committed this.
-/* -DDEBUG is fairly common in CFLAGS. */
-#undef DEBUG
+#undef LTWRAPPER_DEBUGPRINTF
#if defined DEBUGWRAPPER
-# define DEBUG(format, ...) fprintf(stderr, format, __VA_ARGS__)
+# define LTWRAPPER_DEBUGPRINTF(format, ...) fprintf(stderr, format,
__VA_ARGS__)
Not your original issue, but __VA_ARGS__ is not C89. Sure, on cygwin, you are
relatively assured of gcc; but what about mingw with Microsofts' compiler? Or
are we assuming that nobody would define DEBUGWRAPPER unless they are
developing with gcc?
+int
+make_executable(const char * path)
+{
+ int rval = 0;
+ struct stat st;
+
+ /* MinGW native WIN32 do not support S_IXOTH or S_IXGRP */
+ int S_XFLAGS =
+#if defined (S_IXOTH)
+ S_IXOTH ||
+#endif
+#if defined (S_IXGRP)
+ S_IXGRP ||
+#endif
+ S_IXUSR;
Code bug. You meant |, not || (but since on cygwin S_IXOTH is 1, and since
world execute privileges are adequate, it happened to work in spite of your
bug). IMHO, it looks nicer like this (note that my rewrite follows the GCS,
while yours left operators on the end of lines - in general, the coding style I
have seen from coreutils and gnulib prefers to factor out preprocesor
conditionals so that they need not appear in the middle of expressions):
#ifndef S_IXOTH
# define S_IXOTH 0
#endif
#ifndef S_IXGRP
# define S_IXGRP 0
#endif
int S_XFLAGS = S_IXOTH | S_IXGRP | S_IXUSR;
--
Eric Blake
Re: [cygwin] cwrapper emits wrapper script
Peter O'Gorman wrote:
Could you please resend the patch itself, I am having issues with
stripping the html markup from these links. (well, I can strip the html,
but the resulting patch is not applying.)
Attached.
--
Chuck
2007-04-27 Charles Wilson [EMAIL PROTECTED]
* ltmain.m4sh (func_emit_libtool_wrapper_script): add
code block to handle cases when wrapper script is in $objdir.
(func_emit_libtool_cwrapperexe_source): replace DEBUG() macro
with namespace-safe LTWRAPPER_DEBUGPRINTF(). Call
func_emit_libtool_wrapper_script() with appropriate filters to
embed script text in C char* static variable.
(f_e_l_c_s: main): add new option --lt-dump-script, parse argv[]
for it, and take appropriate action. Call chase_symlinks()
on argv[0], in case this.exe was launched via one. Use chased
value to determine full absolute pathname of wrapper script, in
$objdir. Unconditionally write script out to this pathname and
set permission bits. Remove extraneous ';' from debugging loop.
(f_e_l_c_s: main) [mingw]: DOS-ize $SHELL before populating
newargv[0]; if $TARGETSHELL environment variable is set, use
it instead of $SHELL and do not DOS-ize. Ensure newargv[1]
(wrapper script absolute path) uses only '/', not '\'. Make
sure to call execv() with DOS-ized $SHELL (or un-DOS-ized
$TARGETSHELL, if set)
(f_e_l_c_s: make_executable): new function
(f_e_l_c_s: chase_symlinks): new function (no-op if !S_ISLNK)
2007-04-27 Charles Wilson [EMAIL PROTECTED]
* ltmain.m4sh (func_emit_libtool_cwrapperexe_source):
ensure that generated source follows GCS as nearly as
possible.
Index: libltdl/config/ltmain.m4sh
===
RCS file: /cvsroot/libtool/libtool/libltdl/config/ltmain.m4sh,v
retrieving revision 1.74
diff -u -r1.74 ltmain.m4sh
--- libltdl/config/ltmain.m4sh 2007-04-24 23:04:28.546875000 -0400
+++ libltdl/config/ltmain.m4sh 2007-04-27 03:52:20.109375000 -0400
@@ -2301,6 +2301,20 @@
file=\`ls -ld \\$thisdir/\$file\ | ${SED} -n 's/.*- //p'\`
done
+ # cygwin/mingw cwrapper will rewrite this line:
+ WRAPPER_SCRIPT_BELONGS_IN_OBJDIR=no
+ if test \\$WRAPPER_SCRIPT_BELONGS_IN_OBJDIR\ = \yes\; then
+# special case for '.'
+if test \\$thisdir\ = \.\; then
+ thisdir=\`pwd\`
+fi
+# remove .libs from thisdir
+case \\$thisdir\ in
+*[/]$objdir ) thisdir=\`\$ECHO \X\$thisdir\ | \$Xsed -e
's%[/][^/]*$%%'\` ;;
+$objdir ) thisdir=. ;;
+esac
+ fi
+
# Try to get the absolute directory name.
absdir=\`cd \\$thisdir\ pwd\`
test -n \\$absdir\ thisdir=\\$absdir\
@@ -2424,7 +2438,7 @@
This wrapper executable should never be moved out of the build directory.
If it is, it will not operate correctly.
- Currently, it simply execs the wrapper *script* /bin/sh $output,
+ Currently, it simply execs the wrapper *script* $SHELL $output,
but could eventually absorb all of the scripts functionality and
exec $objdir/$outputname directly.
*/
@@ -2438,6 +2452,7 @@
#include assert.h
#include string.h
#include ctype.h
+#include errno.h
#include sys/stat.h
#if defined(PATH_MAX)
@@ -2482,12 +2497,11 @@
if (stale) { free ((void *) stale); stale = 0; } \
} while (0)
-/* -DDEBUG is fairly common in CFLAGS. */
-#undef DEBUG
+#undef LTWRAPPER_DEBUGPRINTF
#if defined DEBUGWRAPPER
-# define DEBUG(format, ...) fprintf(stderr, format, __VA_ARGS__)
+# define LTWRAPPER_DEBUGPRINTF(format, ...) fprintf(stderr, format,
__VA_ARGS__)
#else
-# define DEBUG(format, ...)
+# define LTWRAPPER_DEBUGPRINTF(format, ...)
#endif
const char *program_name = NULL;
@@ -2496,42 +2510,145 @@
char * xstrdup (const char *string);
const char * base_name (const char *name);
char * find_executable(const char *wrapper);
+char * chase_symlinks(const char *pathspec);
+intmake_executable(const char *path);
intcheck_executable(const char *path);
char * strendzap(char *str, const char *pat);
void lt_fatal (const char *message, ...);
+static const char* script_text =
+EOF
+
+ func_emit_libtool_wrapper_script |
+ $SED -e 's/\([\\]\)/\\\1/g' \
+-e 's/\(WRAPPER_SCRIPT_BELONGS_IN_OBJDIR\)=.*/\1=yes/' \
+-e 's/^//' -e 's/$/\\n/'
+ echo ;
+
+ cat EOF
int
main (int argc, char *argv[])
{
char **newargz;
+ char *tmp_pathspec;
+ char *actual_cwrapper_path;
+ char *shwrapper_name;
+ FILE *shwrapper;
+
+ const char* dumpscript_opt = --lt-dump-script;
int i;
program_name = (char *) xstrdup (base_name (argv[0]));
- DEBUG((main) argv[0] : %s\n,argv[0]);
- DEBUG((main) program_name : %s\n,program_name);
+ LTWRAPPER_DEBUGPRINTF((main) argv[0] : %s\n,argv[0]);
+ LTWRAPPER_DEBUGPRINTF((main) program_name : %s\n,program_name);
+
+
Re: [cygwin] cwrapper emits wrapper script
On Wed, 2007-06-06 at 10:25 -0400, Charles Wilson wrote: Peter O'Gorman wrote: Could you please resend the patch itself, I am having issues with stripping the html markup from these links. (well, I can strip the html, but the resulting patch is not applying.) Attached. Thanks, I'll look again tonight (I hope). I'm lazy and would like to avoid work as much as possible, Gary has already asked if you'd like a commit bit, I'm hoping you'll agree, then all we'll need to do is say ok and you can commit your changes yourself. So, you have an account on savannah? (chawil?) Peter
Re: [cygwin] cwrapper emits wrapper script
On Wed, 06 Jun 2007 09:43:50 -0500, Peter O'Gorman said: I'm lazy and would like to avoid work as much as possible, Gary has already asked if you'd like a commit bit, I'm hoping you'll agree, then all we'll need to do is say ok and you can commit your changes yourself. As long as somebody other than me inspects/approves (...or rejects) my patches, I'm happy to turn the crank to check them in myself. -- Chuck
Re: [cygwin] cwrapper emits wrapper script
On Wed, 2007-06-06 at 10:25 -0400, Charles Wilson wrote: Peter O'Gorman wrote: Could you please resend the patch itself, I am having issues with stripping the html markup from these links. (well, I can strip the html, but the resulting patch is not applying.) Attached. -- Chuck 2007-04-27 Charles Wilson [EMAIL PROTECTED] * ltmain.m4sh (func_emit_libtool_wrapper_script): add code block to handle cases when wrapper script is in $objdir. (func_emit_libtool_cwrapperexe_source): replace DEBUG() macro with namespace-safe LTWRAPPER_DEBUGPRINTF(). Call func_emit_libtool_wrapper_script() with appropriate filters to embed script text in C char* static variable. (f_e_l_c_s: main): add new option --lt-dump-script, parse argv[] for it, and take appropriate action. Call chase_symlinks() on argv[0], in case this.exe was launched via one. Use chased value to determine full absolute pathname of wrapper script, in $objdir. Unconditionally write script out to this pathname and set permission bits. Remove extraneous ';' from debugging loop. (f_e_l_c_s: main) [mingw]: DOS-ize $SHELL before populating newargv[0]; if $TARGETSHELL environment variable is set, use it instead of $SHELL and do not DOS-ize. Ensure newargv[1] (wrapper script absolute path) uses only '/', not '\'. Make sure to call execv() with DOS-ized $SHELL (or un-DOS-ized $TARGETSHELL, if set) (f_e_l_c_s: make_executable): new function (f_e_l_c_s: chase_symlinks): new function (no-op if !S_ISLNK) 2007-04-27 Charles Wilson [EMAIL PROTECTED] * ltmain.m4sh (func_emit_libtool_cwrapperexe_source): ensure that generated source follows GCS as nearly as possible. Ok. Causes no regressions for me on linux or darwin, and nothing jumps out at me, please feel free to exercise your commit bit on these :-) I am not, unfortunately, able to test on windows. Noah gave a good suggestion about passing WRAPPER_SCRIPT_BELONGS_IN_OBJDIR as an argument, but, I agree, it can be left to a later patch. At some point it would be nice to emit func_emit_libtool_cwrapperexe_source from config.status, saving 550 lines of code from libtool on non-windows platforms. But I'm just dreaming, you can ignore that :) Thank you! Peter
Re: [cygwin] cwrapper emits wrapper script
On Jun 1, 2007, at 4:20 PM, Charles Wilson wrote: On Fri, 25 May 2007 11:27:08 -0400, Charles Wilson said: On May 4, 2007, Charles Wilson wrote: Ping? http://lists.gnu.org/archive/html/libtool-patches/2007-04/ msg00088.html Ping. If it's Friday, it must be time for: Could you please resend the patch itself, I am having issues with stripping the html markup from these links. (well, I can strip the html, but the resulting patch is not applying.) Thanks, Peter -- Peter O'Gorman http://pogma.com
Re: [cygwin] cwrapper emits wrapper script
On Sat, Jun 02, 2007 at 10:56:22AM -0400, Charles Wilson wrote: Noah Misch wrote: Long-term, we can revise this cumbersome division of work between the executable wrapper and the wrapper script. Ideally, the wrapper methodology for Unix should resemble that for Cygwin/MSYS, so we don't have an extra code path potentially subject to bitrot. This patch does not introduce those problems, however, and seems to improve the current situation. Well, yes, but libtool itself is nothing BUT a twisty maze of extra code paths -- specifically so that end-users can avoid extra code paths in their Makefiles and other build support. We keep track of the complexities so you don't have to, right? Correct. I was not proposing any change to the user interface for running an executable linked to uninstalled libraries. Certain changes to the internal interfaces/implementation might improve code quality. In any case, those concerns need not block your patch. Anyway, I wanted to keep the impact to not-Cygwin/Mingw as small as possible -- zero, hopefully. Anything beyond that is fodder for the next release cycle, definitely not this one. Agreed. ... But making all unix do it the cygwin way is not a good idea either: it's slower (extra fork/exec), there's no good reason to make 'em do that except consistency (which is not always a virtue), and it's a major change from existing behavior on those platforms. Unix can afford an extra fork. You may be right that consistency in this area provides little value. From a purely Unix-centric standpoint, I would pass the script to a shell via a pipe, rather than writing it out to a file. Is that too much for MSYS? Well, that would require more (risky?) changes in the part of libtool where the wrapper script is read. Other platforms just source the existing script; I don't know if you can source a script on stdin. And if you could, it'd still be doing it some weird way on cygwin/mingw (call wrapper.exe to create a temporary script first, then continue as normal is slightly weird, but at least it says 'continue as normal' there at the end). Making cygwin even weirder is contrary to your advice about at least /trying/ to keep things unixish and cygwin/mingw similar. I forgot that Libtool can source the wrapper script. Ignore this idea for now. This will be cleaner if you pass the value of WRAPPER_SCRIPT_BELONGS_IN_OBJDIR as an argument to func_emit_libtool_wrapper_script, rather than hard-coding the no and filtering it after the fact. That's a thought. I'd prefer to push that change off to a later (small) patch though, rather than rewrite this one again -- especially given the difficulty in regenerating the GCS half of this patch. That is reasonable. Thanks.
Re: [cygwin] cwrapper emits wrapper script
Noah Misch wrote: I don't speak for the Libtool maintainers, but I'll throw out my impressions of the patch, in case it might help move things along. Not using Cygwin or MSYS myself these days, I trust that the patch improves things there as you say it does. It seems fairly harmless from the perspective of all other platforms. Thanks for your comments: Long-term, we can revise this cumbersome division of work between the executable wrapper and the wrapper script. Ideally, the wrapper methodology for Unix should resemble that for Cygwin/MSYS, so we don't have an extra code path potentially subject to bitrot. This patch does not introduce those problems, however, and seems to improve the current situation. Well, yes, but libtool itself is nothing BUT a twisty maze of extra code paths -- specifically so that end-users can avoid extra code paths in their Makefiles and other build support. We keep track of the complexities so you don't have to, right? Anyway, I wanted to keep the impact to not-Cygwin/Mingw as small as possible -- zero, hopefully. Anything beyond that is fodder for the next release cycle, definitely not this one. Even so, my changes may or may not be appropriate for 2.0; I'd like to hear from the maintainers on /that/ score... Long term, at least on cygwin/mingw, it would be /nice/ to do away with the wrapper script entirely -- I'm just not sure that it is /practical/ without a complete redesign of the wrapper system on all platforms shudder. The script is used both to launch the target with the right environment settings, but also to pass information between libtool invocations. It's that second bit that's problematic -- and doing THAT bit differently on unix vs. cygwin is more of an extra code path than having a simple .exe wrapper IMO. But making all unix do it the cygwin way is not a good idea either: it's slower (extra fork/exec), there's no good reason to make 'em do that except consistency (which is not always a virtue), and it's a major change from existing behavior on those platforms. From a purely Unix-centric standpoint, I would pass the script to a shell via a pipe, rather than writing it out to a file. Is that too much for MSYS? Well, that would require more (risky?) changes in the part of libtool where the wrapper script is read. Other platforms just source the existing script; I don't know if you can source a script on stdin. And if you could, it'd still be doing it some weird way on cygwin/mingw (call wrapper.exe to create a temporary script first, then continue as normal is slightly weird, but at least it says 'continue as normal' there at the end). Making cygwin even weirder is contrary to your advice about at least /trying/ to keep things unixish and cygwin/mingw similar. One specific thought: + # cygwin/mingw cwrapper will rewrite this line: + WRAPPER_SCRIPT_BELONGS_IN_OBJDIR=no ... + func_emit_libtool_wrapper_script | + $SED -e 's/\([\\]\)/\\\1/g' \ +-e 's/\(WRAPPER_SCRIPT_BELONGS_IN_OBJDIR\)=.*/\1=yes/' \ +-e 's/^//' -e 's/$/\\n/' This will be cleaner if you pass the value of WRAPPER_SCRIPT_BELONGS_IN_OBJDIR as an argument to func_emit_libtool_wrapper_script, rather than hard-coding the no and filtering it after the fact. That's a thought. I'd prefer to push that change off to a later (small) patch though, rather than rewrite this one again -- especially given the difficulty in regenerating the GCS half of this patch. -- Chuck
Re: [cygwin] cwrapper emits wrapper script
On Fri, 25 May 2007 11:27:08 -0400, Charles Wilson said: On May 4, 2007, Charles Wilson wrote: Ping? http://lists.gnu.org/archive/html/libtool-patches/2007-04/msg00088.html Ping. If it's Friday, it must be time for: Ping * 3. -- Chuck
Re: [cygwin] cwrapper emits wrapper script
On Fri, Jun 01, 2007 at 05:20:05PM -0400, Charles Wilson wrote: On Fri, 25 May 2007 11:27:08 -0400, Charles Wilson said: On May 4, 2007, Charles Wilson wrote: http://lists.gnu.org/archive/html/libtool-patches/2007-04/msg00088.html Ping * 3. I don't speak for the Libtool maintainers, but I'll throw out my impressions of the patch, in case it might help move things along. Not using Cygwin or MSYS myself these days, I trust that the patch improves things there as you say it does. It seems fairly harmless from the perspective of all other platforms. Long-term, we can revise this cumbersome division of work between the executable wrapper and the wrapper script. Ideally, the wrapper methodology for Unix should resemble that for Cygwin/MSYS, so we don't have an extra code path potentially subject to bitrot. This patch does not introduce those problems, however, and seems to improve the current situation. From a purely Unix-centric standpoint, I would pass the script to a shell via a pipe, rather than writing it out to a file. Is that too much for MSYS? One specific thought: + # cygwin/mingw cwrapper will rewrite this line: + WRAPPER_SCRIPT_BELONGS_IN_OBJDIR=no ... + func_emit_libtool_wrapper_script | + $SED -e 's/\([\\]\)/\\\1/g' \ +-e 's/\(WRAPPER_SCRIPT_BELONGS_IN_OBJDIR\)=.*/\1=yes/' \ +-e 's/^//' -e 's/$/\\n/' This will be cleaner if you pass the value of WRAPPER_SCRIPT_BELONGS_IN_OBJDIR as an argument to func_emit_libtool_wrapper_script, rather than hard-coding the no and filtering it after the fact. Hope that's useful.
Re: [cygwin] cwrapper emits wrapper script
On May 4, 2007, Charles Wilson wrote: Ping? http://lists.gnu.org/archive/html/libtool-patches/2007-04/msg00088.html Ping. -- Chuck
Re: [cygwin] cwrapper emits wrapper script
Ping? http://lists.gnu.org/archive/html/libtool-patches/2007-04/msg00088.html -- Chuck
Re: [cygwin] cwrapper emits wrapper script
This is a revised version of the patch that first appeared here: http://lists.gnu.org/archive/html/libtool-patches/2007-04/msg00050.html Please refer to that message for a discussion of the theory of this patch, its effects, and justification. This patch is phase 2 in the 3-phase sequence of changes intended to eliminate the presence of both foo.exe and foo wrappers in the same directory, which (a) is confusing to humans (b) is sometimes confusing to cygwin/mingw tools, given evolving behavior with respect to .exe suffix presence/absence, and (c) is totally broken on cygwin if the 'transparent_exe' option is active. Hopefully this patch address the concerns raised by Ralf and others in response to the original post. Also note that this patch fixes a long-standing breakage in the executable wrapper on mingw -- although I could not use Ralf's suggesting directly: lt_mingwSHELL=`( cmd //c echo $SHELL ) 2/dev/null || echo $SHELL` because 'cmd' appends an empty space to the end of lt_mingwSHELL, eventually leading to C:/blah/blah/blah/sh .exe. Bad. I used this, instead: lt_sed_strip_trailing_spaces=s/[ ]*\$// lt_newargv0=`( cmd //c echo $SHELL | $SED -e $lt_sed_strip_trailing_spaces ) 2/dev/null || echo $SHELL + Technically, this patch is divided into two separate patches: the first implements the new functionality and addressed all but one of the criticisms (I hope). The second addresses this comment: Please use GCS formatting, as far as possible (several instances), To effect this change, I iteratively performed the following sequence of steps: (1) used libtool to generate a wrapped executable, and grabbed a copy of the lt-*.c executable wrapper source code. (2) using the options listed here: http://www.gnu.org/prep/standards/html_node/Formatting.html#Formatting invoked indent on the source code (3) generate a before-vs-after diff between the two versions of the source code (4) edit ltmain.m4sh (func_emit_libtool_cwrapperexe_source) to replicate these changes (obviously, the diff can't actually be applied to the .m4sh file) (5) repeat from (1) until the diff generated in (3) is empty [*]. [*] actually, it never did get completely empty, because indent insists on rearranging the last few lines of the embedded wrapper script: 216,219c216 exit 1\n fi\n fi\n ; --- exit 1\n fi\n fi\n; I figured that small deviation from GCS didn't matter. This limitation still applies: POSSIBLE LIMITATION: assumes that if S_ISLNK is defined, then $host has lstat() and realpath(). Since the only place this matters (at present) is cygwin and mingw, this assumption is safe: cygwin has all three, mingw has none. The new code [chase_symlinks()] which makes this assumption was added so that symlinks to the wrapper executable would work as expected (on systems where symlinks exist) just like symlinks to the wrapper script work. + TESTING: Ran full test suite on cygwin, mingw, and linux. No regressions from here: http://lists.gnu.org/archive/html/libtool-patches/2007-04/msg00081.html http://lists.gnu.org/archive/html/libtool-patches/2007-04/msg00073.html although the test numbers have changed. __ Tested on cygwin (full testsuite) with expected results: == All 115 tests passed (9 tests were not run) == 14: Java convenience archives FAILED (convenience.at:273) 16: Link order of deplibs. FAILED (link-order2.at:129) 52: Run tests with low max_cmd_len FAILED (cmdline_wrap.at:43) This is the expected output, for now. ___ Linux, full testsuite: All 106 tests passed 50 tests behaved as expected. 2 tests were skipped. ___ mingw, full testsuite: == All 115 tests passed (9 tests were not run) == 14: Java convenience archives FAILED (convenience.at:273) 16: Link order of deplibs.FAILED (link-order2.at:129) 25: DESTDIR with in-package deplibs FAILED (destdir.at:131) 33: installable libltdl FAILED (standalone.at:69) 37: installable libltdl FAILED (subproject.at:99) 41: installable libltdl FAILED (nonrecursive.at:143) 44: installable libltdl FAILED (recursive.at:113) 52: Run tests with low max_cmd_lenFAILED (cmdline_wrap.at:43) As noted previously, 33,37,41,44 appear to be due to msys trying to execute INSTALL as if it were install.exe. Not sure about 25, but 14/16/51 are the same as cygwin. None of these errors represent regressions from http://lists.gnu.org/archive/html/libtool-patches/2007-04/msg00081.html
Re: [cygwin] cwrapper emits wrapper script
* Charles Wilson wrote on Thu, Apr 26, 2007 at 12:41:08AM CEST: The only problem I see is if libtool-HEAD-after-2.0 (say, nearing the /next/ major release) begins requiring ac-2.61/am-1.10 (or even newer). You'll have my vote against that happening too soon. I suspect they will make more of an effort to keep up with current autotools, plus I think any future ac/am updates will be much less, err, issue-prone than the ac-2.13/ac-2.5x transition was. Let's also reverse that statement: the more GCC keeps up to date with autotools, the less there is a chance that they will regress wrt. the functionality that GCC needs. Of course if people can On Wed, 25 Apr 2007 23:01:10 +0200, Ralf Wildenhues Primary aim is to release Libtool 2. Effectively you are suggesting that Cygwin's transparent_exe feature, its argz bug, and the MinGW breakage of cwrapper be considered release blockers. The latter two, yes: see below. The first one: no. Only, if you ARE going to accept it before 2.0, then I'd prefer to get that done before the upcoming gcc import, rather than miss it by a few days. If you're NOT going to accept it pre-2.0, or if it takes a month to stabilize and we miss the gcc deadline by _weeks_, then no problem. We can try, but running toward a deadline that's not known in advance can be challenging. ;-) It was you who said in response, last week: http://cygwin.com/ml/cygwin/2007-04/msg00549.html ... I'd prefer to see such a patch before deciding when it's good to put it in. Yes, and I stand by that now. Because I understand that these bugs are important to fix. Count all my other inconsistencies wrt. what I said months ago as me being wrong about when we'd have version 2 finished. Let's try to be as pragmatic as possible. Hope that works out. Cheers, Ralf
Re: [cygwin] cwrapper emits wrapper script
Ralf Wildenhues wrote:
* Ralf Wildenhues wrote on Tue, Apr 24, 2007 at 08:53:46AM CEST:
* Charles Wilson wrote on Tue, Apr 24, 2007 at 04:34:41AM CEST:
Ralf Wildenhues wrote:
for (i=0; iargc+1; i++)
{
-DEBUG((main) newargz[%d] : %s\n,i,newargz[i]);
+LTWRAPPER_DEBUGPRINTF((main) newargz[%d] : %s\n,i,newargz[i]);
;
What's that extra ; for BTW?
If !DEBUGWRAPPER, then LTWRAPPER_DEBUGPRINTF() goes away completely, and
gcc complains about the empty body in the for loop.
D'oh. Thanks.
No, wrong. If LTWRAPPER_DEBUGPRINTF goes away, then the semicolon at
the end of the LTWRAPPER_DEBUGPRINTF line remains. I don't see how
there can be a warning with one but no warning with two semicolons.
FWIW, my GCC 4.0.3 doesn't warn at all (with -O2 -W -Wall).
Hmm. The original commit that added this DEBUG() stuff was here:
cvs diff -r 1.352 -r 1.353 ltmain.in
at libtool toplevel. It shows:
...
+
+ for (i=0; iargc+1; i++)
+ {
+DEBUG((main) newargz[%d] : %s\n,i,newargz[i]);
+;
+ }
+
EOF
So this one/two semicolon thing was committed all at once. [Aside:
cygwin's gcc at that time was 3.3.3]. I *distinctly* remember having to
add the second semicolon during my testing before I posted the patch.
Speculation: maybe I originally defined the DEBUG() macro as a
'semicolon-swallowing' form something like:
+#if defined DEBUGWRAPPER
+# define DEBUG(format, ...) \
+do { fprintf(stderr, format, __VA_ARGS__); } while(0)
+#else
+# define DEBUG(format, ...) \
+# do { ; } while(0)
+#endif
for whatever reason, so I needed the second semicolon. Or, maybe
+#if defined DEBUGWRAPPER
+# define DEBUG(args) printf(args); not fprintf! own semicolon
+#else
+# define DEBUG(args)
+#endif
and expected semicolon-less usage like
DEBUG( (format string, some, arguments) )
Then, maybe I later fixed the DEBUG() macro to more-or-less its current
form:
+#if defined DEBUGWRAPPER
+# define DEBUG(format, ...) fprintf(stderr, format, __VA_ARGS__)
+#else
+# define DEBUG(format, ...)
+#endif
but never removed the now-superfluous second semicolon. In any case, I
*know* the second one was necessary at some point, but perhaps only in
my private development.
You're right, tho: it certainly is not needed now.
--
Chuck
Re: [cygwin] cwrapper emits wrapper script
Ralf Wildenhues wrote: Certainly. I was merely trying to not infer that you'd have to do even more work than the lot that you're already doing. Of course if you're ambitious go for it. ;-) Thanks for fixing the MinGW case here. Sure. Hmm, maybe one after the `rm -f $prefix/bin/...' and one after the `$MAKE uninstall' line? Tabled. Don't want to tackle more than one thing in a patch, you know. g Yes, but that particular two-line change could go in before everything else, given that it fixes the failure. I'll take a look. During my most recent testsuite run on mingw, tho, I didn't see the problem (which is typical of this particular mingwism). Yes, yes. I was thinking out loud and it was late... Been there, done that, got the T-shirt. Hmm. OK. But since you undefine DEBUG anyway (and you've kept that for the changed macro), there was no underlying problem here, no? You are correct. Anyway, keeping this change is fine. Thanks. This is weird, tho. On the 1.5 branch (25 Nov 2005): cvs diff -r 1.334.2.104 -r 1.334.2.105 ltmain.in +/* -DDEBUG is fairly common in CFLAGS. */ +#undef DEBUG +#if defined DEBUGWRAPPER +# define DEBUG(format, ...) fprintf(stderr, format, __VA_ARGS__) +#else +# define DEBUG(format, ...) +#endif was committed all at once. On the trunk (6 Nov 2003) at cvs diff -r 1.352 -r 1.353 ltmain.in only the following hunk was committed +#if defined DEBUGWRAPPER +# define DEBUG(format, ...) fprintf(stderr, format, __VA_ARGS__) +#else +# define DEBUG(format, ...) +#endif and the brown-paper-bag fix came along a year later (1 Nov 2004) after the file had been moved from top/ltmain.in to top/config/ltmain.m4sh (but before it was moved AGAIN to top/libltdl/config/ltmain.m4sh): cvs diff -r 1.17 -r 1.18 ltmain.m4sh +/* -DDEBUG is fairly common in CFLAGS. */ +#undef DEBUG And it was a year after THAT (25 Nov 2005), that the now-improved version was committed to the 1.5 branch. I had forgotten that the exectuable wrappers went into HEAD first. This is because $ECHO can be much much more expensive than echo, forking and all. Not on w32 especially (unless you happen to use a shell such as dash), but in general, yes. Ack. okay. But don't come crying to me if somebody populates your argv[] with non-null-terminated strings. How would that be possible? ISO C guarantees them to be strings, i.e., null-terminated runs of characters. You must have missed my invisible sarcasm tag. if test -n $TARGETSHELL ; then # no path translation at all lt_newargv1=$TARGETSHELL else [...] esac fi now use $lt_newargv1 unconditionally Sounds viable. (picky note: on the right hand side of a shell assignment, there is no need for double quotes, as word splitting is disabled there.) Ack. I think using strchr is clearer. Okay. + if ( (shwrapperFILE = fopen (newargz[1], wb)) == 0 ) Why binary? [snip explanation] OK, thanks for the explanation. What's that extra ; for BTW? If !DEBUGWRAPPER, then LTWRAPPER_DEBUGPRINTF() goes away completely, and gcc complains about the empty body in the for loop. D'oh. Thanks. See side thread. strrchr? Nope, can't do that here. IS_DIR_SEPARATOR(c) checks whether c is '\\' OR '/'; trying to use strrchr in that situation is more confusing than it is worth. Ah, ok. Sorry. FYI, I'm going to focus on the argz patch before returning to this one. (I am pushing a bit, because if possible and acceptable to [Ralf|Gary|others], I want to get both the argz stuff and as much of the eliminate .exe/shell wrapper in same directory confusion work as I can, into libtool-HEAD before Steve Ellcey commits the updated libtool ToT files to gcc svn...which if I'm reading the tea leaves correctly is Real Soon Now) -- Chuck
Re: [cygwin] cwrapper emits wrapper script
Hello Charles,
Thanks for the patch again. First review:
* Charles Wilson wrote on Sat, Apr 21, 2007 at 03:03:02AM CEST:
This patch depends on this one:
http://lists.gnu.org/archive/html/libtool-patches/2007-04/msg00048.html
(unfortunately, due to idiocy on my part, that patch will have to go
through another iteration. Review to come up in 1-2 days, hopefully.)
With this patch, after a successful build the following files are created:
foo.exe -- the wrapper executable. It does NOT launch the
wrapper script 'foo' in thisdir (.)
foo -- a wrapper script. It could be used to launch the
target executable, but isn't normally used for that.
This wrapper is sourced by func_source() when
necessary.
.libs/foo.exe -- the target executable
When the wrapper foo.exe is launched, it generates a new wrapper script
.libs/foo_ltshwrapper
Hmm, I'm wondering whether we should keep prefixing within .libs. Maybe
.libs/ltsh-foo
? WDYT?
At present, the .libs/foo_ltwrappersh is recreated every single time the
wrapper executable is run; later, timestamps could be used to avoid
this, but that's an optimization.
Furthermore, the wrapper executable can be invoked with the
'-ltdumpscript' option, which will emit the script on *stdout*.
The interesting option name is to guard against valid program flags,
right? What do you think about --lt-dump-script? It would not fit in
with libtool's other flags, though. Or maybe an environment variable
rather than a flag? (I'm simply unsure myself, gathering opinions, not
telling you to change your code here.)
This patch ALSO fixes the wrapper executable on mingw, by
DOS-izing /bin/sh when emitting those lines of the executable
wrapper's source code.
Thanks, that should fix
http://lists.gnu.org/archive/html/bug-libtool/2007-03/msg8.html.
Please note though that translating a path with MSYS can be done like
this (when $build = $host):
cmd //c echo $srcfile
but in the cross-compile case, we need to do more work, see this report
http://lists.gnu.org/archive/html/bug-libtool/2007-02/msg0.html
(note I'm not asking you to do this work here; actually, I'd like to ask
you not to fix even more different things with one patch. Merely noting
it in case you're interested.)
37: compiling softlinked libltdl FAILED (nonrecursive.at:90)
38: compiling copied libltdl FAILED (nonrecursive.at:114)
39: installable libltdl FAILED (nonrecursive.at:140)
40: compiling softlinked libltdl FAILED (recursive.at:67)
41: compiling copied libltdl FAILED (recursive.at:87)
42: installable libltdl FAILED (recursive.at:109)
Ah, ok. Without your patch, I don't get these, but I have 2.61 and 1.10
installed in my MSYS/MinGW, which would explain this. I also don't
think they have to do with your patch, but will check.
Failed test was: tests/mdemo-dryrun.test -- false alarm:
$ diff before after
66c66
drwxr-xr-x2 cwilson Administ0 Apr 20 2007 bin
---
drwxr-xr-x2 cwilson Administ0 Apr 20 20:29 bin
Probably need another 'sleep 1' somewhere, but that's outside the scope
of this patch.
Hmm, maybe one after the `rm -f $prefix/bin/...' and one after the
`$MAKE uninstall' line?
--- libltdl/config/ltmain.m4sh2007-04-19 19:54:53.5 -0400
+++ libltdl/config/ltmain.m4sh2007-04-20 03:20:46.28125 -0400
@@ -2301,6 +2301,20 @@
file=\`ls -ld \\$thisdir/\$file\ | ${SED} -n 's/.*- //p'\`
done
+ # cygwin/mingw cwrapper will rewrite this line:
+ WRAPPER_SCRIPT_BELONGS_IN_OBJDIR=no
+ if test \\$WRAPPER_SCRIPT_BELONGS_IN_OBJDIR\ = \yes\; then
(Not sure about this WRAPPER_SCRIPT_BELONGS_IN_OBJDIR thing yet.)
+# special case for '.'
[...]
@@ -2482,12 +2497,11 @@
if (stale) { free ((void *) stale); stale = 0; } \
} while (0)
-/* -DDEBUG is fairly common in CFLAGS. */
-#undef DEBUG
+#undef LTWRAPPER_DEBUGPRINTF
#if defined DEBUGWRAPPER
-# define DEBUG(format, ...) fprintf(stderr, format, __VA_ARGS__)
+# define LTWRAPPER_DEBUGPRINTF(format, ...) fprintf(stderr, format,
__VA_ARGS__)
#else
-# define DEBUG(format, ...)
+# define LTWRAPPER_DEBUGPRINTF(format, ...)
What's the reason for this change?
@@ -2496,41 +2510,156 @@
char * xstrdup (const char *string);
const char * base_name (const char *name);
char * find_executable(const char *wrapper);
+char * chase_symlinks(const char *pathspec);
+intmake_executable(const char *path);
intcheck_executable(const char *path);
char * strendzap(char *str, const char *pat);
void lt_fatal (const char *message, ...);
+static const char* script_text =
+EOF
+
+ func_emit_libtool_wrapper_script |\
+ $SED -e 's/\([\\]\)/\\\1/g' |\
+ $SED -e 's/\(WRAPPER_SCRIPT_BELONGS_IN_OBJDIR\)=.*/\1=yes/' |\
+ $SED -e 's/^//' -e
Re: [cygwin] cwrapper emits wrapper script
Ralf Wildenhues wrote: * Charles Wilson wrote on Sat, Apr 21, 2007 at 03:03:02AM CEST: When the wrapper foo.exe is launched, it generates a new wrapper script .libs/foo_ltshwrapper Hmm, I'm wondering whether we should keep prefixing within .libs. Maybe .libs/ltsh-foo ? WDYT? Meh, I don't care. Which would you prefer: $ ls tests/mdemo/.libs cygfoo2-0.dll* libmlib.expmdemo.exeS.o cygfoo2-0.dll.def libmlib.la@mdemo_ltshwrapper* cygmlib-0.dll* libmlib.laimdemo_ltshwrapperTMP* cygmlib-0.dll.def libmlibS.c mdemo_static.exe* cygsub-0.dll* libmlibS.o mdemo_static.exe.def foo1.dll* libsub.dll.a mdemo_static.exe.exp foo1.dll.a libsub.la@ mdemo_static.exe.nm foo1.la@ libsub.lai mdemo_static.exe.nmS foo1.lai lt-mdemo.c mdemo_static.exeS.c foo1.o lt-mdemo_static.c mdemo_static.exeS.o foo2.o mdemo.exe* mdemo_static_ltshwrapper* libfoo2.dll.a mdemo.exe.def mdemo_static_ltshwrapperTMP* libfoo2.expmdemo.exe.exp mlib.o libfoo2.la@mdemo.exe.nm sub.o libfoo2.laimdemo.exe.nmS libmlib.dll.a mdemo.exeS.c (the above is after a run of mdemo-shared.conf, so even mdemo_static.exe is built shared) Or $ ls tests/mdemo/.libs cygfoo2-0.dll* libmlib.expmdemo.exe.exp cygfoo2-0.dll.def libmlib.la@mdemo.exe.nm cygmlib-0.dll* libmlib.laimdemo.exe.nmS cygmlib-0.dll.def libmlibS.c mdemo.exeS.c cygsub-0.dll* libmlibS.o mdemo.exeS.o foo1.dll* libsub.dll.a mdemo_static.exe* foo1.dll.a libsub.la@ mdemo_static.exe.def foo1.la@ libsub.lai mdemo_static.exe.exp foo1.lai lt-mdemo.c mdemo_static.exe.nm foo1.o lt-mdemo_static.c mdemo_static.exe.nmS foo2.o ltshTMP-mdemo* mdemo_static.exeS.c libfoo2.dll.a ltshTMP-mdemo_static* mdemo_static.exeS.o libfoo2.expltsh-mdemo*mlib.o libfoo2.la@ltsh-mdemo_static* sub.o libfoo2.laimdemo.exe* libmlib.dll.a mdemo.exe.def The interesting option name is to guard against valid program flags, right? Yes. What do you think about --lt-dump-script? It would not fit in with libtool's other flags, though. But this isn't libtool, per se. It's a specific wrapper executable. The special option could be %%%LIBTOOL%%%-dump:::script++ for all the difference it makes. We could standardize (hah!) on '--lt-' for this sort of thing if you like. Or maybe an environment variable rather than a flag? (I'm simply unsure myself, gathering opinions, not telling you to change your code here.) Oh, I really don't like having a envvar affect the wrapper that way: okay, see, if LTWRAPPER_NOT_WRAPPING_RIGHT_NOW is set in the environment, then running the wrapper executable will cause it to dump a wrapper script to stdout, and not run the target at all. Ick -- just wait until I set that envvar manually while debugging something, and then forget to unset it later...at least with a cmd-line option, I'm actively and explicitly putting the executable into the dumpscript mode with each invocation. This patch ALSO fixes the wrapper executable on mingw, by DOS-izing /bin/sh when emitting those lines of the executable wrapper's source code. Thanks, that should fix http://lists.gnu.org/archive/html/bug-libtool/2007-03/msg8.html. Please note though that translating a path with MSYS can be done like this (when $build = $host): cmd //c echo $srcfile Hmm. I didn't know about that. That's much simpler. but in the cross-compile case, we need to do more work, see this report http://lists.gnu.org/archive/html/bug-libtool/2007-02/msg0.html Right. I'm not sure what the best approach is. (note I'm not asking you to do this work here; actually, I'd like to ask you not to fix even more different things with one patch. Merely noting it in case you're interested.) Ok, but it my defense: NOT fixing the mingw case (e.g. leaving the wrapper.exe broken on mingw) + eliminate the wrapper script from '.' == break EVERYTHING on mingw. So, I kinda HAD to address this: and two patches that touch exactly the same region of code is rather silly. 37: compiling softlinked libltdl FAILED (nonrecursive.at:90) 38: compiling copied libltdl FAILED (nonrecursive.at:114) 39: installable libltdl FAILED (nonrecursive.at:140) 40: compiling softlinked libltdl FAILED (recursive.at:67) 41: compiling copied libltdl FAILED (recursive.at:87) 42: installable libltdl FAILED (recursive.at:109) Ah, ok. Without your patch, I don't get these, but I have 2.61 and 1.10 installed in my MSYS/MinGW, which would explain this. I also don't think they have to do with your patch, but will check. If I bootstrap the patched source on linux -- with
[cygwin] cwrapper emits wrapper script
This patch depends on this one: http://lists.gnu.org/archive/html/libtool-patches/2007-04/msg00048.html With this patch, on cygwin/mingw the wrapper script created by libtool in '.' is used only for reading back in via func_source(); THIS copy of the wrapper script is not used during the run the uninstalled target sequence *at all*. In existing libtool, on cygwin/mingw, the following files are created: foo.exe -- a wrapper executable, that launches... foo -- a wrapper scripts, which sets env vars and launches... .libs/foo.exe -- the actual target executable However, the presence of both foo and foo.exe in the same directory causes some problems with coreutils/shells' autodetect .exe's logic. And it's downright unusable when the cygwin transparent_exe option is active. So, eventually we want to eliminate ./foo entirely on these platforms, and rely entirely on ./foo.exe and .libs/foo.exe (with perhaps some other files in .libs/) With this patch, after a successful build the following files are created: foo.exe -- the wrapper executable. It does NOT launch the wrapper script 'foo' in thisdir (.) foo -- a wrapper script. It could be used to launch the target executable, but isn't normally used for that. This wrapper is sourced by func_source() when necessary. .libs/foo.exe -- the target executable When the wrapper foo.exe is launched, it generates a new wrapper script .libs/foo_ltshwrapper which is identical [1] to the one in the real directory (./foo). Then, the executable wrapper execs THAT new wrapper script, which sets the env vars and execs the real target. At present, the .libs/foo_ltwrappersh is recreated every single time the wrapper executable is run; later, timestamps could be used to avoid this, but that's an optimization. Furthermore, the wrapper executable can be invoked with the '-ltdumpscript' option, which will emit the script on *stdout*. This is intended for future progress toward eliminating the ./foo wrapper script entirely, on cygwin/mingw. [1] This patch changes the *all* wrapper scripts on all platforms by adding the following: WRAPPER_SCRIPT_BELONGS_IN_OBJDIR=no if test $WRAPPER_SCRIPT_BELONGS_IN_OBJDIR = yes; then ... remove /.libs from $thisdir ... fi The ./foo wrapper (and all wrappers on non-cygwin/mingw) looks just like the above, so the additional code is permanently disabled. However, the script emitted by the wrapper executable on cygwin/mingw is modified so that WRAPPER_SCRIPT_BELONGS_IN_OBJDIR=yes, activating the additional code. This is because the script needs to compute $thisdir as being the main directory, in which .libs/* exists. Also: as it happens, currently under cygwin AND mingw, the executable wrapper is *not used* unless explicitly invoked with the .exe ending. Since the tests do not do so, the executable wrappers have not been exercised on either platform for a while. They do still work on cygwin, but not mingw: calling the msvcrt.dll version of execv(/bin/sh, ...) doesn't work. (The testsuite on mingw has been passing because the MSYS shell launches the wrapper *script* directly, instead of the wrapper executable). This patch ALSO fixes the wrapper executable on mingw, by DOS-izing /bin/sh when emitting those lines of the executable wrapper's source code. POSSIBLE LIMITATION: assumes that if S_ISLNK is defined, then $host has lstat() and realpath(). Since the only place this matters (at present) is cygwin and mingw, this assumption is safe: cygwin has all three, mingw has none. The new code [chase_symlinks()] which makes this assumption was added so that symlinks to the wrapper executable would work as expected (on systems where symlinks exist) just like symlinks to the wrapper script work. TESTING: ___ Tested on cygwin (full testsuite) with expected results: == All 115 tests passed (9 tests were not run) [no gfortran] == 14: Java convenience archives FAILED (convenience.at:273) 16: Link order of deplibs. FAILED (link-order2.at:129) 49: Run tests with low max_cmd_len FAILED (cmdline_wrap.at:42) This is the expected output, for now. ___ Linux, full testsuite: == All 105 tests passed (1 tests were not run) == 37: compiling softlinked libltdl FAILED (nonrecursive.at:90) 38: compiling copied libltdl FAILED (nonrecursive.at:114) 39: installable libltdl FAILED (nonrecursive.at:140) 40: compiling softlinked libltdl FAILED (recursive.at:67) 41: compiling copied libltdl FAILED (recursive.at:87) 42: installable libltdl FAILED (recursive.at:109) Each of these looks like: configure:10874: error: possibly undefined macro:
