Ralf Wildenhues wrote:
> This patch is ok to commit with nits below addressed as you see fit;
> no need for further review.

Thanks for the review.

> 
>> --- a/libltdl/config/ltmain.m4sh
>> +++ b/libltdl/config/ltmain.m4sh

>>  
>> -const char *program_name = NULL;
>> +const char *program_name = "wrapper"; /* in case xstrdup fails */
> 
> "libtool wrapper" might be clearer.  You decide.

Fixed.

> 
>> @@ -2880,7 +2867,8 @@ char *chase_symlinks (const char *pathspec);
>>  int make_executable (const char *path);
>>  int check_executable (const char *path);
>>  char *strendzap (char *str, const char *pat);
>> -void lt_fatal (const char *message, ...);
>> +void lt_debugprintf (const char *file, int line, const char *fmt, ...);
>> +void lt_fatal (const char *file, int line, const char *message, ...);
> 
> It'd be nicer to wrap these two in a macro that expands __FILE__ and
> __LINE__ automatically, but varargs macros are not yet portable, and
> multiple defines are somewhat ugly too,
> 
>   #define lt_fatal0 (message) lt_fatal_in (__FILE__, __LINE__, message)
>   #define lt_fatal1 (message, arg1) \
>           lt_fatal_in (__FILE__, __LINE__, message, arg1)
>   #define lt_fatal2 (message, arg1, arg2) \
>           lt_fatal_in (__FILE__, __LINE__, message, arg1, arg2)
>   ...
> 
> so maybe the current code is a good compromise.

Yeah, the original LTWRAPPER_DEBUGPRINTF was also a workaround for the
lack of vararg macros, but it forced you to use (()) which I hated.  I
really didn't want to create N different macros for *both*
lt_debugprintf and lt_fatal (especially lt_debugprintf, which
coincidentally also needs N=2 support macros of this type).  Of course,
N=2 /now/ -- and we could just go with "use these macros, and if you
every need N=3... add the macros then.

But it's so UGLY.

So, yeah, the current code is an explicit compromise to work around the
lack of vararg macro support.  I think explicitly invoking __FILE__ and
__LINE__ is marginally less ugly than lt_foo_N() macros...

>> +      if (strcmp (argv[i], ltwrapper_option_prefix) == 0)
>> +        {

[THIS]:

>> +          /* however, if there is an option in the LTWRAPPER_OPTION_PREFIX
>> +             namespace, but it is not one of the ones we know about and
>> +             have already dealt with, above (inluding dump-script), then
>> +             report an error. Otherwise, targets might begin to believe
>> +             they are allowed to use options in the LTWRAPPER_OPTION_PREFIX
>> +             namespace. The first time any user complains about this, we'll
>> +             need to make LTWRAPPER_OPTION_PREFIX a configure-time option
>> +             or a configure.ac-settable value.
>> +           */
> 
> The problem being that we cannot please this user without providing an
> upgrade, or letting her change libtool code.  Maybe warn instead of fail
> hard?  I'm really torn here; we suddenly grab in-band namespace without
> an easy way to avoid it through some option.

Well, this code has been in the cygwin libtool, in one form or another,
for almost two years.  It's been used to build all of KDE and about 1400
total open source packages (cygwinports project) as well about 2GB
(compressed) source packages in the main cygwin distribution.

If none of THOSE used --lt-* I think we're pretty safe in 'grabbing' it.

> This issue should not be addressed in this patch, it is not new here,
> and we first should get consensus on the right way to address it.
> (Have we done that elsewhere already?)

I believe the consensus was, yeah...the RTTD is to add an ugly configure
option to ltoptions like --libtool-wrapper-option-prefix which defaults
to "--lt-" (*), and use that value when emitting the cwrapper source and
shwrapper.

http://lists.gnu.org/archive/html/libtool-patches/2008-05/msg00066.html
But Eric said
http://lists.gnu.org/archive/html/libtool-patches/2008-05/msg00067.html
"I don't want to spend any effort coding around it unless someone (and
it won't be me) demonstrates a real need for the extra flexability."

FWIW, that big comment block ([THIS], above) is really just a
placeholder/reminder that we can/should implement some sort of 'easy way
to avoid it' mechanism...

(*) Specifying the value this way requires the '=' form on the configure
line;  --libtool-wrapper-option-prefix=--lt-, but if you 'assume' the
double-dash, then you might be able to get away with
--libtool-wrapper-option-prefix lt-
and no '='

>> +          lt_fatal (__FILE__, __LINE__,
>> +                "Unrecognized %s option: '%s'",
>> +                    ltwrapper_option_prefix, argv[i]);
> 
>> +        cat <<EOF
>> +  /* first use of lt_debugprintf AFTER parsing options */
> 
> Not sure what this comment aims at.  Is this a statement of a nontrivial
> fact?

The important thing is that the very first output, when in debug mode
and no errors have occured (eg. bad arguments, etc), be the "banner" so
that cwrapper.at is happy.  I'll reword the comment.

/* The GNU banner must be the first non-error debug message */


>> +  lt_debugprintf (__FILE__, __LINE__, "libtool wrapper (GNU 
>> $PACKAGE$TIMESTAMP) $VERSION\n");
>> +EOF
>> +        cat <<"EOF"
>> +  lt_debugprintf (__FILE__, __LINE__, "(main) argv[0]: %s\n", argv[0]);
>> +  lt_debugprintf (__FILE__, __LINE__, "(main) program_name: %s\n", 
>> program_name);
> 
>>    actual_cwrapper_path = chase_symlinks (tmp_pathspec);
>> -  LTWRAPPER_DEBUGPRINTF (("(main) found exe (after symlink chase) at : 
>> %s\n",
>> -                      actual_cwrapper_path));
>> +  lt_debugprintf (__FILE__, __LINE__,
>> +                  "(main) found exe (after symlink chase) at: %s\n",
>> +              actual_cwrapper_path);
>>    XFREE (tmp_pathspec);
>>  
>>    actual_cwrapper_name = xstrdup( base_name (actual_cwrapper_path));
> 
> This last line existed before the patch, but has spacing wrong,
> according to GCS.  Fix it or not, however you like.

Fixed.

>> @@ -3099,7 +3102,9 @@ EOF
>>    if (rval == -1)
>>      {
>>        /* failed to start process */
>> -      LTWRAPPER_DEBUGPRINTF (("(main) failed to launch target \"%s\": errno 
>> = %d\n", lt_argv_zero, errno));
>> +      lt_debugprintf (__FILE__, __LINE__,
>> +                  "(main) failed to launch target \"%s\": errno = %d\n",
> 
> s/(main) //  ?  In all debug messages?  Hmm, maybe not.

Well, is __func__ portable enough (at least as far as mingw, cygwin,
(msvc?) compilers are concerned)?  I'd've preferred
  foo("(%s)...", __func__', ...)
but was afraid to use it, which is why I hardcoded the function names
into (almost) all the debug messages, WAY back when. OTOH, I don't think
it makes sense for debug messages from functions other than main()
include their function name, but that ones in main() itself don't, so I
don't think s/(main // is the way to go.

If desired, I'll put together a separate single-purpose patch to change
"(func) ..." to '"(%s) ...", __func__' throughout, so that it can be
committed (and reverted if we discover it causes problems) as a single
commit.

>> +                  lt_argv_zero, errno);
> 
> strerror (errno) if it's non-NULL, instead of errno = %d?
> 
>>        return 127;
>>      }
>>    return rval;
>> @@ -3121,7 +3126,7 @@ xmalloc (size_t num)
>>  {
>>    void *p = (void *) malloc (num);
>>    if (!p)
>> -    lt_fatal ("Memory exhausted");
>> +    lt_fatal (__FILE__, __LINE__, "Memory exhausted");
> 
> No first-word capitalization of errors, in all error messages.

Fixed.

>> @@ -3155,8 +3160,8 @@ check_executable (const char *path)
>>  {
>>    struct stat st;
>>  
>> -  LTWRAPPER_DEBUGPRINTF (("(check_executable)  : %s\n",
>> -                      path ? (*path ? path : "EMPTY!") : "NULL!"));
>> +  lt_debugprintf (__FILE__, __LINE__, "(check_executable): %s\n",
>> +              path ? (*path ? path : "EMPTY!") : "NULL!");
> 
> s/EMPTY!/(empty)/;  s/NULL!/(null)/   here and below?  No need to scream
> at the user, and (null) is what glibc uses.  Similar for FATAL and
> <NULL> below.  Actually, it might even be more compact to just use
> 
>   static const char *nonnull (const char *s)
>   {
>     return s ? s : "(null)";
>   }
> 
>   static const char *nonempty (const char *s)
>   {
>     return (s && !*s) ? "(empty)" : nonnull (s);
>   }
> 
> throughout.

I like it. Fixed.


>> @@ -3254,7 +3259,7 @@ find_executable (const char *wrapper)
>>              {
>>                /* empty path: current directory */
>>                if (getcwd (tmp, LT_PATHMAX) == NULL)
>> -                lt_fatal ("getcwd failed");
>> +                lt_fatal (__FILE__, __LINE__, "getcwd failed");
> 
> getcwd sets errno, so strerror (errno) might be interesting here
> and below.

Is strerror/errno portable enough to use? I know it's supported by
cygwin and mingw, but msvc?

http://msdn.microsoft.com/en-us/library/zc53h9bh(VS.80).aspx
says it is declared in string.h and supported all the way back to Win95,
so I guess so.

...and never mind. We're already using it elsewhere. So, fixed.

>> --- a/tests/cwrapper.at
>> +++ b/tests/cwrapper.at
>> @@ -79,5 +79,57 @@ for restrictive_flags in '-Wall -Werror' '-std=c89 -Wall 
>> -Werror' '-std=c99 -Wal
>>    LT_AT_EXEC_CHECK([./usea], [0], [ignore], [ignore], [])
>>  done
>>  
>> +
>> +# Test RUN-TIME activation of wrapper debugging
> 
> s/RUN-TIME/run-time/.  Missing period.  Similar below for COMPILE-TIME.

Fixed.

>> +# This is not part of the loop above, because we
>> +# need to check, not ignore, the output.
>> +CFLAGS="$orig_CFLAGS"
>> +LIBTOOL=$orig_LIBTOOL
>> +
>> +AT_CHECK([$LIBTOOL --mode=compile $CC $CPPFLAGS $CFLAGS -c liba.c],
>> +         [], [ignore], [ignore])
>> +AT_CHECK([$LIBTOOL --mode=link $CC $CFLAGS $LDFLAGS -version-info=0.0.0 
>> -no-undefined -o liba.la -rpath /foo liba.lo],
> 
> -version-info needs ':' as separator, not '.'.  You can also just omit
> it, though.

D'oh. Fixed.

>> +# We structure this test as a loop, so that we can 'break' out of it
>> +# if necessary -- even though the loop by design executes only once.
>> +for debugwrapper_flags in '-DLT_DEBUGWRAPPER'; do
>> +  CFLAGS="$orig_CFLAGS $debugwrapper_flags"
>> +  sed "s/LTCFLAGS=.*/&' $debugwrapper_flags'/" < "$orig_LIBTOOL" |
>> +  sed "s/^lt_option_debug=/lt_option_debug=1/" > ./libtool
> 
> Can be just one sed command, but hey, however you like.

Fixed.

> It took me a few seconds to realize that you don't need chmod here
> because you overwrite an existing script here ...

Yeah, but it is confusing. I can add a redundant chmod +x here if you
think it necessary, but that's a one-liner for later on.

So, open issues, to be addressed if necessary in additional patches:
1) "(func) ..." ---> '"(%s) ...", __func__' ?
2) chmod in cwrapper.at?

As pushed.


--
Chuck
diff --git a/libltdl/config/ltmain.m4sh b/libltdl/config/ltmain.m4sh
index 80a1ff3..cb0672b 100644
--- a/libltdl/config/ltmain.m4sh
+++ b/libltdl/config/ltmain.m4sh
@@ -2727,10 +2727,6 @@ func_emit_cwrapperexe_src ()
 
    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* "$SHELL $output",
-   but could eventually absorb all of the scripts functionality and
-   exec $objdir/$outputname directly.
 */
 EOF
 	    cat <<"EOF"
@@ -2855,22 +2851,13 @@ int setenv (const char *, const char *, int);
   if (stale) { free ((void *) stale); stale = 0; } \
 } while (0)
 
-#undef LTWRAPPER_DEBUGPRINTF
-#if defined LT_DEBUGWRAPPER
-# define LTWRAPPER_DEBUGPRINTF(args) ltwrapper_debugprintf args
-static void
-ltwrapper_debugprintf (const char *fmt, ...)
-{
-    va_list args;
-    va_start (args, fmt);
-    (void) vfprintf (stderr, fmt, args);
-    va_end (args);
-}
+#if defined(LT_DEBUGWRAPPER)
+static int lt_debug = 1;
 #else
-# define LTWRAPPER_DEBUGPRINTF(args)
+static int lt_debug = 0;
 #endif
 
-const char *program_name = NULL;
+const char *program_name = "libtool-wrapper"; /* in case xstrdup fails */
 
 void *xmalloc (size_t num);
 char *xstrdup (const char *string);
@@ -2880,7 +2867,10 @@ char *chase_symlinks (const char *pathspec);
 int make_executable (const char *path);
 int check_executable (const char *path);
 char *strendzap (char *str, const char *pat);
-void lt_fatal (const char *message, ...);
+void lt_debugprintf (const char *file, int line, const char *fmt, ...);
+void lt_fatal (const char *file, int line, const char *message, ...);
+static const char *nonnull (const char *s);
+static const char *nonempty (const char *s);
 void lt_setenv (const char *name, const char *value);
 char *lt_extend_str (const char *orig_value, const char *add, int to_end);
 void lt_update_exe_path (const char *name, const char *value);
@@ -2932,12 +2922,10 @@ EOF
 	    cat <<"EOF"
 
 #define LTWRAPPER_OPTION_PREFIX         "--lt-"
-#define LTWRAPPER_OPTION_PREFIX_LENGTH  5
 
-static const size_t opt_prefix_len         = LTWRAPPER_OPTION_PREFIX_LENGTH;
 static const char *ltwrapper_option_prefix = LTWRAPPER_OPTION_PREFIX;
-
 static const char *dumpscript_opt       = LTWRAPPER_OPTION_PREFIX "dump-script";
+static const char *debug_opt            = LTWRAPPER_OPTION_PREFIX "debug";
 
 int
 main (int argc, char *argv[])
@@ -2954,10 +2942,13 @@ main (int argc, char *argv[])
   int i;
 
   program_name = (char *) xstrdup (base_name (argv[0]));
-  LTWRAPPER_DEBUGPRINTF (("(main) argv[0]      : %s\n", argv[0]));
-  LTWRAPPER_DEBUGPRINTF (("(main) program_name : %s\n", program_name));
+  newargz = XMALLOC (char *, argc + 1);
 
-  /* very simple arg parsing; don't want to rely on getopt */
+  /* very simple arg parsing; don't want to rely on getopt
+   * also, copy all non cwrapper options to newargz, except
+   * argz[0], which is handled differently
+   */
+  newargc=0;
   for (i = 1; i < argc; i++)
     {
       if (strcmp (argv[i], dumpscript_opt) == 0)
@@ -2974,21 +2965,54 @@ EOF
 	  lt_dump_script (stdout);
 	  return 0;
 	}
+      if (strcmp (argv[i], debug_opt) == 0)
+	{
+          lt_debug = 1;
+          continue;
+	}
+      if (strcmp (argv[i], ltwrapper_option_prefix) == 0)
+        {
+          /* however, if there is an option in the LTWRAPPER_OPTION_PREFIX
+             namespace, but it is not one of the ones we know about and
+             have already dealt with, above (inluding dump-script), then
+             report an error. Otherwise, targets might begin to believe
+             they are allowed to use options in the LTWRAPPER_OPTION_PREFIX
+             namespace. The first time any user complains about this, we'll
+             need to make LTWRAPPER_OPTION_PREFIX a configure-time option
+             or a configure.ac-settable value.
+           */
+          lt_fatal (__FILE__, __LINE__,
+		    "unrecognized %s option: '%s'",
+                    ltwrapper_option_prefix, argv[i]);
+        }
+      /* otherwise ... */
+      newargz[++newargc] = xstrdup (argv[i]);
     }
+  newargz[++newargc] = NULL;
+
+EOF
+	    cat <<EOF
+  /* The GNU banner must be the first non-error debug message */
+  lt_debugprintf (__FILE__, __LINE__, "libtool wrapper (GNU $PACKAGE$TIMESTAMP) $VERSION\n");
+EOF
+	    cat <<"EOF"
+  lt_debugprintf (__FILE__, __LINE__, "(main) argv[0]: %s\n", argv[0]);
+  lt_debugprintf (__FILE__, __LINE__, "(main) program_name: %s\n", program_name);
 
-  newargz = XMALLOC (char *, argc + 1);
   tmp_pathspec = find_executable (argv[0]);
   if (tmp_pathspec == NULL)
-    lt_fatal ("Couldn't find %s", argv[0]);
-  LTWRAPPER_DEBUGPRINTF (("(main) found exe (before symlink chase) at : %s\n",
-			  tmp_pathspec));
+    lt_fatal (__FILE__, __LINE__, "couldn't find %s", argv[0]);
+  lt_debugprintf (__FILE__, __LINE__,
+                  "(main) found exe (before symlink chase) at: %s\n",
+		  tmp_pathspec);
 
   actual_cwrapper_path = chase_symlinks (tmp_pathspec);
-  LTWRAPPER_DEBUGPRINTF (("(main) found exe (after symlink chase) at : %s\n",
-			  actual_cwrapper_path));
+  lt_debugprintf (__FILE__, __LINE__,
+                  "(main) found exe (after symlink chase) at: %s\n",
+		  actual_cwrapper_path);
   XFREE (tmp_pathspec);
 
-  actual_cwrapper_name = xstrdup( base_name (actual_cwrapper_path));
+  actual_cwrapper_name = xstrdup (base_name (actual_cwrapper_path));
   strendzap (actual_cwrapper_path, actual_cwrapper_name);
 
   /* wrapper name transforms */
@@ -3006,8 +3030,9 @@ EOF
   target_name = tmp_pathspec;
   tmp_pathspec = 0;
 
-  LTWRAPPER_DEBUGPRINTF (("(main) libtool target name: %s\n",
-			  target_name));
+  lt_debugprintf (__FILE__, __LINE__,
+		  "(main) libtool target name: %s\n",
+		  target_name);
 EOF
 
 	    cat <<EOF
@@ -3060,32 +3085,12 @@ EOF
   lt_update_lib_path (LIB_PATH_VARNAME, LIB_PATH_VALUE);
   lt_update_exe_path (EXE_PATH_VARNAME, EXE_PATH_VALUE);
 
-  newargc=0;
-  for (i = 1; i < argc; i++)
-    {
-      if (strncmp (argv[i], ltwrapper_option_prefix, opt_prefix_len) == 0)
-        {
-          /* however, if there is an option in the LTWRAPPER_OPTION_PREFIX
-             namespace, but it is not one of the ones we know about and
-             have already dealt with, above (inluding dump-script), then
-             report an error. Otherwise, targets might begin to believe
-             they are allowed to use options in the LTWRAPPER_OPTION_PREFIX
-             namespace. The first time any user complains about this, we'll
-             need to make LTWRAPPER_OPTION_PREFIX a configure-time option
-             or a configure.ac-settable value.
-           */
-          lt_fatal ("Unrecognized option in %s namespace: '%s'",
-                    ltwrapper_option_prefix, argv[i]);
-        }
-      /* otherwise ... */
-      newargz[++newargc] = xstrdup (argv[i]);
-    }
-  newargz[++newargc] = NULL;
-
-  LTWRAPPER_DEBUGPRINTF     (("(main) lt_argv_zero : %s\n", (lt_argv_zero ? lt_argv_zero : "<NULL>")));
+  lt_debugprintf (__FILE__, __LINE__, "(main) lt_argv_zero: %s\n",
+		  nonnull (lt_argv_zero));
   for (i = 0; i < newargc; i++)
     {
-      LTWRAPPER_DEBUGPRINTF (("(main) newargz[%d]   : %s\n", i, (newargz[i] ? newargz[i] : "<NULL>")));
+      lt_debugprintf (__FILE__, __LINE__, "(main) newargz[%d]: %s\n",
+		      i, nonnull (newargz[i]));
     }
 
 EOF
@@ -3099,7 +3104,9 @@ EOF
   if (rval == -1)
     {
       /* failed to start process */
-      LTWRAPPER_DEBUGPRINTF (("(main) failed to launch target \"%s\": errno = %d\n", lt_argv_zero, errno));
+      lt_debugprintf (__FILE__, __LINE__,
+		      "(main) failed to launch target \"%s\": %s\n",
+		      lt_argv_zero, strerror (errno));
       return 127;
     }
   return rval;
@@ -3121,7 +3128,7 @@ xmalloc (size_t num)
 {
   void *p = (void *) malloc (num);
   if (!p)
-    lt_fatal ("Memory exhausted");
+    lt_fatal (__FILE__, __LINE__, "memory exhausted");
 
   return p;
 }
@@ -3155,8 +3162,8 @@ check_executable (const char *path)
 {
   struct stat st;
 
-  LTWRAPPER_DEBUGPRINTF (("(check_executable)  : %s\n",
-			  path ? (*path ? path : "EMPTY!") : "NULL!"));
+  lt_debugprintf (__FILE__, __LINE__, "(check_executable): %s\n",
+                  nonempty (path));
   if ((!path) || (!*path))
     return 0;
 
@@ -3173,8 +3180,8 @@ make_executable (const char *path)
   int rval = 0;
   struct stat st;
 
-  LTWRAPPER_DEBUGPRINTF (("(make_executable)   : %s\n",
-			  path ? (*path ? path : "EMPTY!") : "NULL!"));
+  lt_debugprintf (__FILE__, __LINE__, "(make_executable): %s\n",
+                  nonempty (path));
   if ((!path) || (!*path))
     return 0;
 
@@ -3200,8 +3207,8 @@ find_executable (const char *wrapper)
   int tmp_len;
   char *concat_name;
 
-  LTWRAPPER_DEBUGPRINTF (("(find_executable)   : %s\n",
-			  wrapper ? (*wrapper ? wrapper : "EMPTY!") : "NULL!"));
+  lt_debugprintf (__FILE__, __LINE__, "(find_executable): %s\n",
+                  nonempty (wrapper));
 
   if ((wrapper == NULL) || (*wrapper == '\0'))
     return NULL;
@@ -3254,7 +3261,8 @@ find_executable (const char *wrapper)
 		{
 		  /* empty path: current directory */
 		  if (getcwd (tmp, LT_PATHMAX) == NULL)
-		    lt_fatal ("getcwd failed");
+		    lt_fatal (__FILE__, __LINE__, "getcwd failed: %s",
+                              strerror (errno));
 		  tmp_len = strlen (tmp);
 		  concat_name =
 		    XMALLOC (char, tmp_len + 1 + strlen (wrapper) + 1);
@@ -3279,7 +3287,7 @@ find_executable (const char *wrapper)
     }
   /* Relative path | not found in path: prepend cwd */
   if (getcwd (tmp, LT_PATHMAX) == NULL)
-    lt_fatal ("getcwd failed");
+    lt_fatal (__FILE__, __LINE__, "getcwd failed: %s", strerror (errno));
   tmp_len = strlen (tmp);
   concat_name = XMALLOC (char, tmp_len + 1 + strlen (wrapper) + 1);
   memcpy (concat_name, tmp, tmp_len);
@@ -3305,8 +3313,9 @@ chase_symlinks (const char *pathspec)
   int has_symlinks = 0;
   while (strlen (tmp_pathspec) && !has_symlinks)
     {
-      LTWRAPPER_DEBUGPRINTF (("checking path component for symlinks: %s\n",
-			      tmp_pathspec));
+      lt_debugprintf (__FILE__, __LINE__,
+		      "checking path component for symlinks: %s\n",
+		      tmp_pathspec);
       if (lstat (tmp_pathspec, &s) == 0)
 	{
 	  if (S_ISLNK (s.st_mode) != 0)
@@ -3328,8 +3337,9 @@ chase_symlinks (const char *pathspec)
 	}
       else
 	{
-	  char *errstr = strerror (errno);
-	  lt_fatal ("Error accessing file %s (%s)", tmp_pathspec, errstr);
+	  lt_fatal (__FILE__, __LINE__,
+		    "error accessing file \"%s\": %s",
+		    tmp_pathspec, strerror (errno));
 	}
     }
   XFREE (tmp_pathspec);
@@ -3342,7 +3352,8 @@ chase_symlinks (const char *pathspec)
   tmp_pathspec = realpath (pathspec, buf);
   if (tmp_pathspec == 0)
     {
-      lt_fatal ("Could not follow symlinks for %s", pathspec);
+      lt_fatal (__FILE__, __LINE__,
+		"could not follow symlinks for %s", pathspec);
     }
   return xstrdup (tmp_pathspec);
 #endif
@@ -3368,11 +3379,25 @@ strendzap (char *str, const char *pat)
   return str;
 }
 
+void
+lt_debugprintf (const char *file, int line, const char *fmt, ...)
+{
+  va_list args;
+  if (lt_debug)
+    {
+      (void) fprintf (stderr, "%s:%s:%d: ", program_name, file, line);
+      va_start (args, fmt);
+      (void) vfprintf (stderr, fmt, args);
+      va_end (args);
+    }
+}
+
 static void
-lt_error_core (int exit_status, const char *mode,
+lt_error_core (int exit_status, const char *file,
+	       int line, const char *mode,
 	       const char *message, va_list ap)
 {
-  fprintf (stderr, "%s: %s: ", program_name, mode);
+  fprintf (stderr, "%s:%s:%d: %s: ", program_name, file, line, mode);
   vfprintf (stderr, message, ap);
   fprintf (stderr, ".\n");
 
@@ -3381,20 +3406,32 @@ lt_error_core (int exit_status, const char *mode,
 }
 
 void
-lt_fatal (const char *message, ...)
+lt_fatal (const char *file, int line, const char *message, ...)
 {
   va_list ap;
   va_start (ap, message);
-  lt_error_core (EXIT_FAILURE, "FATAL", message, ap);
+  lt_error_core (EXIT_FAILURE, file, line, "FATAL", message, ap);
   va_end (ap);
 }
 
+static const char *
+nonnull (const char *s)
+{
+  return s ? s : "(null)";
+}
+
+static const char *
+nonempty (const char *s)
+{
+  return (s && !*s) ? "(empty)" : nonnull (s);
+}
+
 void
 lt_setenv (const char *name, const char *value)
 {
-  LTWRAPPER_DEBUGPRINTF (("(lt_setenv) setting '%s' to '%s'\n",
-                          (name ? name : "<NULL>"),
-                          (value ? value : "<NULL>")));
+  lt_debugprintf (__FILE__, __LINE__,
+		  "(lt_setenv) setting '%s' to '%s'\n",
+                  nonnull (name), nonnull (value));
   {
 #ifdef HAVE_SETENV
     /* always make a copy, for consistency with !HAVE_SETENV */
@@ -3442,9 +3479,9 @@ lt_extend_str (const char *orig_value, const char *add, int to_end)
 void
 lt_update_exe_path (const char *name, const char *value)
 {
-  LTWRAPPER_DEBUGPRINTF (("(lt_update_exe_path) modifying '%s' by prepending '%s'\n",
-                          (name ? name : "<NULL>"),
-                          (value ? value : "<NULL>")));
+  lt_debugprintf (__FILE__, __LINE__,
+		  "(lt_update_exe_path) modifying '%s' by prepending '%s'\n",
+                  nonnull (name), nonnull (value));
 
   if (name && *name && value && *value)
     {
@@ -3463,9 +3500,9 @@ lt_update_exe_path (const char *name, const char *value)
 void
 lt_update_lib_path (const char *name, const char *value)
 {
-  LTWRAPPER_DEBUGPRINTF (("(lt_update_lib_path) modifying '%s' by prepending '%s'\n",
-                          (name ? name : "<NULL>"),
-                          (value ? value : "<NULL>")));
+  lt_debugprintf (__FILE__, __LINE__,
+		  "(lt_update_lib_path) modifying '%s' by prepending '%s'\n",
+                  nonnull (name), nonnull (value));
 
   if (name && *name && value && *value)
     {
diff --git a/tests/cwrapper.at b/tests/cwrapper.at
index 42f8d0f..babd91e 100644
--- a/tests/cwrapper.at
+++ b/tests/cwrapper.at
@@ -79,5 +79,58 @@ for restrictive_flags in '-Wall -Werror' '-std=c89 -Wall -Werror' '-std=c99 -Wal
   LT_AT_EXEC_CHECK([./usea], [0], [ignore], [ignore], [])
 done
 
+
+# Test run-time activation of wrapper debugging.
+# This is not part of the loop above, because we
+# need to check, not ignore, the output.
+CFLAGS="$orig_CFLAGS"
+LIBTOOL=$orig_LIBTOOL
+
+AT_CHECK([$LIBTOOL --mode=compile $CC $CPPFLAGS $CFLAGS -c liba.c],
+         [], [ignore], [ignore])
+AT_CHECK([$LIBTOOL --mode=link $CC $CFLAGS $LDFLAGS -no-undefined -o liba.la -rpath /foo liba.lo],
+         [], [ignore], [ignore])
+AT_CHECK([test -f liba.la])
+
+AT_CHECK([$CC $CPPFLAGS $CFLAGS -c usea.c],
+         [], [ignore], [ignore])
+AT_CHECK([$LIBTOOL --mode=link $CC $CFLAGS $LDFLAGS -o usea$EXEEXT usea.$OBJEXT liba.la],
+         [], [ignore], [ignore])
+LT_AT_EXEC_CHECK([./usea], [0], [ignore], [stderr], [--lt-debug])
+LT_AT_UNIFY_NL([stderr])
+AT_CHECK([grep 'libtool wrapper' stderr], [0], [ignore], [ignore])
+
+
+# Test compile-time activation of wrapper debugging.
+# We structure this test as a loop, so that we can 'break' out of it
+# if necessary -- even though the loop by design executes only once.
+for debugwrapper_flags in '-DLT_DEBUGWRAPPER'; do
+  CFLAGS="$orig_CFLAGS $debugwrapper_flags"
+  sed -e "s/LTCFLAGS=.*/&' $debugwrapper_flags'/" \
+      -e "s/^lt_option_debug=/lt_option_debug=1/" \
+    < "$orig_LIBTOOL" > ./libtool
+  LIBTOOL=./libtool
+
+  # make sure $debugwrapper_flags do not cause a failure
+  # themselves (e.g. because a non-gcc compiler doesn't
+  # understand them)
+  $LIBTOOL --mode=compile $CC $CPPFLAGS $CFLAGS -c trivial.c || continue
+
+  AT_CHECK([$LIBTOOL --mode=compile $CC $CPPFLAGS $CFLAGS -c liba.c],
+           [], [ignore], [ignore])
+  AT_CHECK([$LIBTOOL --mode=link $CC $CFLAGS $LDFLAGS -no-undefined -o liba.la -rpath /foo liba.lo],
+           [], [ignore], [ignore])
+  AT_CHECK([test -f liba.la])
+
+  AT_CHECK([$CC $CPPFLAGS $CFLAGS -c usea.c],
+           [], [ignore], [ignore])
+  AT_CHECK([$LIBTOOL --mode=link $CC $CFLAGS $LDFLAGS -o usea$EXEEXT usea.$OBJEXT liba.la],
+           [], [ignore], [ignore])
+  LT_AT_EXEC_CHECK([./usea], [0], [ignore], [stderr], [])
+  LT_AT_UNIFY_NL([stderr])
+  AT_CHECK([grep 'libtool wrapper' stderr], [0], [ignore], [ignore])
+done
+
+
 AT_CLEANUP
 

Reply via email to