[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 (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

Hmm, I'm wondering whether we should keep prefixing within .libs.  Maybe
?  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        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         mdemo_static.exe.nm           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        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)


$ ls tests/mdemo/.libs
cygfoo2-0.dll*     libmlib.exp            mdemo.exe.exp
cygfoo2-0.dll.def            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             mdemo_static.exe.def           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        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,


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 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
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

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 (
 38: compiling copied libltdl      FAILED (
 39: installable libltdl           FAILED (
 40: compiling softlinked libltdl  FAILED (
 41: compiling copied libltdl      FAILED (
 42: installable libltdl           FAILED (

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
 #if defined DEBUGWRAPPER
-# define DEBUG(format, ...) fprintf(stderr, format, __VA_ARGS__)
+# define LTWRAPPER_DEBUGPRINTF(format, ...) fprintf(stderr, format, 
-# 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/.../.../

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.


+           cat <<EOF
 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);
- 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 ;;

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
  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 ;;
    * ) lt_newargv1="$SHELL" ;;
<< now use $lt_newargv1 unconditionally >>

+           case $host_os in
+             mingw*)
+           cat <<"EOF"
+  {
+     char* p = newargz[1];
+     while(*p!='\0') {
+       if (*p == '\\') {


+         *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 = '/';

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.

+           ;;
+           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:

   for (i=0; i<argc+1; i++)
     LTWRAPPER_DEBUGPRINTF("(main) newargz[%d]   : %s\n",i,newargz[i]);

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.


+            /* search backwards for last DIR_SEPARATOR */
+            p = tmp_pathspec + strlen(tmp_pathspec) - 1;
+            while ( (p > tmp_pathspec) && (! IS_DIR_SEPARATOR(*p)) )
+                p--;


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)) )


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


reply via email to

[Prev in Thread] Current Thread [Next in Thread]