[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [cygwin] cwrapper emits wrapper script
|
From: |
Charles Wilson |
|
Subject: |
Re: [cygwin] cwrapper emits wrapper script |
|
Date: |
Mon, 23 Apr 2007 22:34:41 -0400 |
|
User-agent: |
Thunderbird 1.5.0.10 (Windows/20070221) |
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.exp mdemo.exeS.o
cygfoo2-0.dll.def libmlib.la@ mdemo_ltshwrapper*
cygmlib-0.dll* libmlib.lai mdemo_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.exp mdemo.exe.exp mlib.o
libfoo2.la@ mdemo.exe.nm sub.o
libfoo2.lai mdemo.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.exp mdemo.exe.exp
cygfoo2-0.dll.def libmlib.la@ mdemo.exe.nm
cygmlib-0.dll* libmlib.lai mdemo.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.exp ltsh-mdemo* mlib.o
libfoo2.la@ ltsh-mdemo_static* sub.o
libfoo2.lai mdemo.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/msg00008.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/msg00000.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 am-1.9.6 and
ac-2.[59|61|whatever] -- I expect these errors would go away. But those
versions of the autotools aren't available by default on my distro...
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>
(Not sure about this WRAPPER_SCRIPT_BELONGS_IN_OBJDIR thing yet.)
What's not to like? A wrapper script that lives in $objdir *must*
behave ever-so-slightly differently from a wrapper script that lives in
$objdir/.. -- they must compute the location of the target executable.
Couple that with wanting to use the same func_emit_wrapper_script()
method in both cases...
-/* -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?
Because I've always felt really stupid about my original choice: using
the incredibly common "DEBUG" symbol as a function-like macro? What was
I smoking?
+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 's/$/\\n"/'
You can merge the 3 sed scripts into one, either by
$SED 's/.../.../
s/.../.../
s/.../.../'
Okay, I'll take your word for it: I was worried about order of evaluation.
or by multiple -e. Also, after a pipe (|), there's no need to escape
the newline.
+ $ECHO ";"
Please use plain `echo' here. $ECHO is for stuff that can contain \t,
begin with -n, and the like.
oookay.
+
+ cat <<EOF
int
main (int argc, char *argv[])
{
char **newargz;
+ char *tmp_pathspec;
+ char *actual_cwrapper_path;
+ char *shwrapper_name;
+ FILE *shwrapperFILE;
FILE *shwrapper;
looks much less hungarian. ;-)
yEAH, yEAH, whatEvEr.
+
+ const char* dumpscript_opt = "-ltdumpscript";
+ size_t dumpscript_len = strlen(dumpscript_opt);
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);
+
+ /* very simple arg parsing; don't want to rely on getopt */
+ for (i=1; i<argc; i++)
+ {
+ if (strncmp(argv[i], dumpscript_opt, dumpscript_len) == 0)
Please use strcmp, and drop dumpscript_len.
okay. But don't come crying to me if somebody populates your argv[]
with non-null-terminated strings.
+ {
+ printf("%s", script_text);
+ return 0;
+ }
+ }
+
newargz = XMALLOC(char *, argc+2);
EOF
- cat <<EOF
+ case $host_os in
+ mingw*)
+ # such a shame msys has no cygpath-like program...
Let's simplify all this to something like this (untested!):
lt_mingwSHELL=`( cmd //c echo $SHELL ) 2>/dev/null || echo $SHELL`
case $lt_mingwSHELL in
*.exe | *.EXE) ;;
*) lt_mingwSHELL=$lt_mingwSHELL.exe ;;
esac
No problem.
Probably for the cross-compile + simulator case we'd need to allow for
some override. Not sure if we want to factor out the path translation
(also not sure if we couldn't just use $fix_srcfile_path for this, and
not distinguish between cygwin and mingw at all, but that would be a
more far-reaching change).
A halfway step:
if test -n "$TARGETSHELL" ; then
# no path translation at all
lt_newargv1="$TARGETSHELL"
else
case "$host" in
*mingw* )
lt_newargv1=`( cmd //c echo $SHELL ) 2>/dev/null || echo $SHELL`
case $lt_newargv1 in
*.exe | *.EXE) ;;
*) lt_newargv1=$lt_newargv1.exe ;;
esac
;;
* ) lt_newargv1="$SHELL" ;;
esac
fi
<< now use $lt_newargv1 unconditionally >>
[...]
+ case $host_os in
+ mingw*)
+ cat <<"EOF"
+ {
+ char* p = newargz[1];
+ while(*p!='\0') {
+ if (*p == '\\') {
strchr?
+ *p = '/';
+ }
+ p++;
+ }
+ }
Thought about it. But (a) I'd still need the while loop, because I need
to replace all '\\' chars, and (b) what else does strchr DO but iterate
thru the string? I'm already iterating...
{
char* p = newargz[1];
while(*p!='\0') {
if (*p == '\\') {
*p = '/';
}
p++;
}
}
may be a few lines longer, but avoids an extra function call...
{
char* p;
while( (p = strchr(newargz[1], '\\')) != NULL) {
*p = '/';
}
}
but it is no big deal to me. If you like #2, I can do #2.
+EOF
+ ;;
+ esac
+
+ cat <<"EOF"
+ XFREE( shwrapper_name );
+ XFREE( actual_cwrapper_path );
+
+ if ( (shwrapperFILE = fopen (newargz[1], "wb")) == 0 )
Why binary?
On cygwin, with latest bash, you have to jump thru hoops (set -igncr,
use text mounts, etc) to execute scripts that have \r in them, whereas
'\n' without '\r' will always work -- even on msys, even on cygwin 'text
mounts'. So, "wt" is *definitely* out. That leaves "w" which is a very
tempting target for someone to "optimize" into "wt" (scripts are text,
right?). So it was either "wb" which always works, or
/* note: do NOT use "wt" here! -- defer to underlying
* mount type on cygwin
*/
if ( (shwrapperFILE = fopen (newargz[1], "w")) == 0 )
Six of one, half-dozen of the other...
for (i=0; i<argc+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. I could do this,
instead:
#if (LTWRAPPER_DEBUGPRINTF)
for (i=0; i<argc+1; i++)
{
LTWRAPPER_DEBUGPRINTF("(main) newargz[%d] : %s\n",i,newargz[i]);
}
#endif
but I'm not sure you can use function-like macros that way.
Please use GCS formatting, as far as possible (several instances),
esp.: spaces before opening paren, no spaces after nor before closing.
Sure.
+ /* search backwards for last DIR_SEPARATOR */
+ p = tmp_pathspec + strlen(tmp_pathspec) - 1;
+ while ( (p > tmp_pathspec) && (! IS_DIR_SEPARATOR(*p)) )
+ p--;
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.
+ if ( (p == tmp_pathspec) && (! IS_DIR_SEPARATOR(*p)) )
[...]
ditto.
Apologies for being so picky.
Picky is good. It keeps crappy code out of the repository. Usually. <g>
Also please note that I haven't had a
chance to test this patch yet, so if you would like me to do it before
you work on it again, please say so.
Nah. I'm going to table this for a bit, and wait for your comments on
the argz fix
http://lists.gnu.org/archive/html/libtool-patches/2007-04/msg00048.html
first.
--
Chuck
Re: [cygwin] cwrapper emits wrapper script, Charles Wilson, 2007/04/27