Charles Wilson wrote:
> Ralf Wildenhues wrote:
>> * Charles Wilson wrote on Mon, Jun 22, 2009 at 03:50:42AM CEST:
>>> * libltdl/config/ltmain.m4sh (func_emit_cwrapperexe_src)
>>> [ltwrapper_debugprintf]: Renamed to...
>> I think functions should still be put in (parens) in the ChangeLog
>> entry, not in [brackets], according to GCS.
[... other review comments ...]
Okay, here's a followon patch to
"Enable runtime cwrapper debugging; add tests"
http://lists.gnu.org/archive/html/libtool-patches/2009-12/msg00014.html
The first patch, in addition to addressing various points raised in this
thread, obsoletes the "extra" mingw fix:
http://lists.gnu.org/archive/html/libtool-patches/2009-12/msg00017.html
Assuming these two patches are approved, I'll squash them together into
a single commit (and update the commit history) before pushing. I just
figured it was easier to review, by presenting the response to the
review comments separately.
I haven't done a full test run; but I have tested a few of the old tests
and the cwrapper.at tests individually, with this change. Full test run
on both cygwin and linux in progress [*]
This sequence doesn't address the issue with regards to the /shell/
wrapper failing to pass the cwrapper tests (for instance, on linux).
I've got a followon patch to
"Re: [PATCH] Add --lt-* options to shell wrapper"
http://lists.gnu.org/archive/html/libtool-patches/2009-12/msg00015.html
which addresses that issue and will post it shortly, but we should
discuss that in the other thread.
[*] Testing with these two patches, plus the two shell wrapper patches
referenced above, plus the wrapper documentation patch:
http://lists.gnu.org/archive/html/libtool-patches/2009-12/msg00016.html
...
actually, the linux test already finished. Results:
Old: All 94 tests passed
New: Only two unexpected results (in particular, the cwrapper test passed)
21: passing CXX flags through libtool FAILED (flags.at:24)
100: Run tests with low max_cmd_len FAILED
(cmdline_wrap.at:43)
Err.. 21 should have been skipped, because I haven't installed g++ on
the linux box yet. And 100 is just a repeat of 21.
--
Chuck
Update cwrapper and tests per review comments
* libltdl/config/ltmain.m4sh (func_emit_cwrapperexe_src):
Update comments. Initialize program_name. Eliminate _LENGTH
variables for string constants. In debug mode, print a
banner with known content before any other output.
(func_emit_cwrapperexe_src:main): Use strcmp rather than
strncmp, where safe.
(func_emit_cwrapperexe_src:lt_debugprintf): Modify signature
and implementation to conform to GCS.
(func_emit_cwrapperexe_src:lt_fatal): Ditto.
(func_emit_cwrapperexe_src:lt_error_core): Ditto.
(func_emit_cwrapperexe_src): Update all callers to lt_fatal
and lt_debugprintf.
* tests/cwrapper.at: Clarify comments. Avoid unnecessarily
copying libtool. Adjust debug tests to search for wrapper
script banner message.
diff --git a/libltdl/config/ltmain.m4sh b/libltdl/config/ltmain.m4sh
index f09b323..4549023 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"
@@ -2861,7 +2857,7 @@ static int lt_debug = 1;
static int lt_debug = 0;
#endif
-const char *program_name = NULL;
+const char *program_name = "wrapper"; /* in case xstrdup fails */
void *xmalloc (size_t num);
char *xstrdup (const char *string);
@@ -2871,15 +2867,14 @@ 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_debugprintf (const char *fmt, ...);
-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, ...);
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);
void lt_update_lib_path (const char *name, const char *value);
char **prepare_spawn (char **argv);
void lt_dump_script (FILE *f);
-
EOF
cat <<EOF
@@ -2925,16 +2920,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 size_t dumpscript_opt_len = LTWRAPPER_OPTION_PREFIX_LENGTH + 11;
-
static const char *debug_opt = LTWRAPPER_OPTION_PREFIX "debug";
-static const size_t debug_opt_len = LTWRAPPER_OPTION_PREFIX_LENGTH + 5;
int
main (int argc, char *argv[])
@@ -2960,7 +2949,7 @@ main (int argc, char *argv[])
newargc=0;
for (i = 1; i < argc; i++)
{
- if (strncmp (argv[i], dumpscript_opt, dumpscript_opt_len) == 0)
+ if (strcmp (argv[i], dumpscript_opt) == 0)
{
EOF
case "$host" in
@@ -2974,12 +2963,12 @@ EOF
lt_dump_script (stdout);
return 0;
}
- if (strncmp (argv[i], debug_opt, debug_opt_len) == 0)
+ if (strcmp (argv[i], debug_opt) == 0)
{
lt_debug = 1;
continue;
}
- if (strncmp (argv[i], ltwrapper_option_prefix, opt_prefix_len) == 0)
+ 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
@@ -2990,7 +2979,8 @@ EOF
need to make LTWRAPPER_OPTION_PREFIX a configure-time option
or a configure.ac-settable value.
*/
- lt_fatal ("Unrecognized option in %s namespace: '%s'",
+ lt_fatal (__FILE__, __LINE__,
+ "Unrecognized %s option: '%s'",
ltwrapper_option_prefix, argv[i]);
}
/* otherwise ... */
@@ -2998,18 +2988,25 @@ EOF
}
newargz[++newargc] = NULL;
+EOF
+ cat <<EOF
/* first use of lt_debugprintf AFTER parsing options */
- lt_debugprintf ("(main) argv[0] : %s\n", argv[0]);
- lt_debugprintf ("(main) program_name : %s\n", program_name);
+ 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);
tmp_pathspec = find_executable (argv[0]);
if (tmp_pathspec == NULL)
- lt_fatal ("Couldn't find %s", argv[0]);
- lt_debugprintf ("(main) found exe (before symlink chase) at : %s\n",
+ 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);
- lt_debugprintf ("(main) found exe (after symlink chase) at : %s\n",
+ lt_debugprintf (__FILE__, __LINE__,
+ "(main) found exe (after symlink chase) at: %s\n",
actual_cwrapper_path);
XFREE (tmp_pathspec);
@@ -3031,7 +3028,8 @@ EOF
target_name = tmp_pathspec;
tmp_pathspec = 0;
- lt_debugprintf ("(main) libtool target name: %s\n",
+ lt_debugprintf (__FILE__, __LINE__,
+ "(main) libtool target name: %s\n",
target_name);
EOF
@@ -3085,10 +3083,12 @@ EOF
lt_update_lib_path (LIB_PATH_VARNAME, LIB_PATH_VALUE);
lt_update_exe_path (EXE_PATH_VARNAME, EXE_PATH_VALUE);
- lt_debugprintf ("(main) lt_argv_zero : %s\n", (lt_argv_zero ? lt_argv_zero : "<NULL>"));
+ lt_debugprintf (__FILE__, __LINE__, "(main) lt_argv_zero: %s\n",
+ (lt_argv_zero ? lt_argv_zero : "<NULL>"));
for (i = 0; i < newargc; i++)
{
- lt_debugprintf ("(main) newargz[%d] : %s\n", i, (newargz[i] ? newargz[i] : "<NULL>"));
+ lt_debugprintf (__FILE__, __LINE__, "(main) newargz[%d]: %s\n",
+ i, (newargz[i] ? newargz[i] : "<NULL>"));
}
EOF
@@ -3102,7 +3102,9 @@ EOF
if (rval == -1)
{
/* failed to start process */
- lt_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",
+ lt_argv_zero, errno);
return 127;
}
return rval;
@@ -3124,7 +3126,7 @@ xmalloc (size_t num)
{
void *p = (void *) malloc (num);
if (!p)
- lt_fatal ("Memory exhausted");
+ lt_fatal (__FILE__, __LINE__, "Memory exhausted");
return p;
}
@@ -3158,7 +3160,7 @@ check_executable (const char *path)
{
struct stat st;
- lt_debugprintf ("(check_executable) : %s\n",
+ lt_debugprintf (__FILE__, __LINE__, "(check_executable): %s\n",
path ? (*path ? path : "EMPTY!") : "NULL!");
if ((!path) || (!*path))
return 0;
@@ -3176,7 +3178,7 @@ make_executable (const char *path)
int rval = 0;
struct stat st;
- lt_debugprintf ("(make_executable) : %s\n",
+ lt_debugprintf (__FILE__, __LINE__, "(make_executable): %s\n",
path ? (*path ? path : "EMPTY!") : "NULL!");
if ((!path) || (!*path))
return 0;
@@ -3203,7 +3205,7 @@ find_executable (const char *wrapper)
int tmp_len;
char *concat_name;
- lt_debugprintf ("(find_executable) : %s\n",
+ lt_debugprintf (__FILE__, __LINE__, "(find_executable): %s\n",
wrapper ? (*wrapper ? wrapper : "EMPTY!") : "NULL!");
if ((wrapper == NULL) || (*wrapper == '\0'))
@@ -3257,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");
tmp_len = strlen (tmp);
concat_name =
XMALLOC (char, tmp_len + 1 + strlen (wrapper) + 1);
@@ -3282,7 +3284,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");
tmp_len = strlen (tmp);
concat_name = XMALLOC (char, tmp_len + 1 + strlen (wrapper) + 1);
memcpy (concat_name, tmp, tmp_len);
@@ -3308,7 +3310,8 @@ chase_symlinks (const char *pathspec)
int has_symlinks = 0;
while (strlen (tmp_pathspec) && !has_symlinks)
{
- lt_debugprintf ("checking path component for symlinks: %s\n",
+ lt_debugprintf (__FILE__, __LINE__,
+ "checking path component for symlinks: %s\n",
tmp_pathspec);
if (lstat (tmp_pathspec, &s) == 0)
{
@@ -3332,7 +3335,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, errstr);
}
}
XFREE (tmp_pathspec);
@@ -3345,7 +3350,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
@@ -3372,11 +3378,12 @@ strendzap (char *str, const char *pat)
}
void
-lt_debugprintf (const char *fmt, ...)
+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);
@@ -3384,10 +3391,11 @@ lt_debugprintf (const char *fmt, ...)
}
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");
@@ -3396,18 +3404,19 @@ 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);
}
void
lt_setenv (const char *name, const char *value)
{
- lt_debugprintf ("(lt_setenv) setting '%s' to '%s'\n",
+ lt_debugprintf (__FILE__, __LINE__,
+ "(lt_setenv) setting '%s' to '%s'\n",
(name ? name : "<NULL>"),
(value ? value : "<NULL>"));
{
@@ -3457,7 +3466,8 @@ lt_extend_str (const char *orig_value, const char *add, int to_end)
void
lt_update_exe_path (const char *name, const char *value)
{
- lt_debugprintf ("(lt_update_exe_path) modifying '%s' by prepending '%s'\n",
+ lt_debugprintf (__FILE__, __LINE__,
+ "(lt_update_exe_path) modifying '%s' by prepending '%s'\n",
(name ? name : "<NULL>"),
(value ? value : "<NULL>"));
@@ -3478,7 +3488,8 @@ lt_update_exe_path (const char *name, const char *value)
void
lt_update_lib_path (const char *name, const char *value)
{
- lt_debugprintf ("(lt_update_lib_path) modifying '%s' by prepending '%s'\n",
+ lt_debugprintf (__FILE__, __LINE__,
+ "(lt_update_lib_path) modifying '%s' by prepending '%s'\n",
(name ? name : "<NULL>"),
(value ? value : "<NULL>"));
diff --git a/tests/cwrapper.at b/tests/cwrapper.at
index 30a583c..66c9ce1 100644
--- a/tests/cwrapper.at
+++ b/tests/cwrapper.at
@@ -80,12 +80,11 @@ for restrictive_flags in '-Wall -Werror' '-std=c89 -Wall -Werror' '-std=c99 -Wal
done
-# Make sure wrapper debugging works, when activated at runtime
+# 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"
-cat "$orig_LIBTOOL" > ./libtool
-LIBTOOL=./libtool
+LIBTOOL=$orig_LIBTOOL
AT_CHECK([$LIBTOOL --mode=compile $CC $CPPFLAGS $CFLAGS -c liba.c],
[], [ignore], [ignore])
@@ -99,15 +98,16 @@ AT_CHECK([$LIBTOOL --mode=link $CC $CFLAGS $LDFLAGS -o usea$EXEEXT usea.$OBJEXT
[], [ignore], [ignore])
LT_AT_EXEC_CHECK([./usea], [0], [ignore], [stderr], [--lt-debug])
LT_AT_UNIFY_NL([stderr])
-AT_CHECK([grep '^(main) argv\[[0\]][[ \t]]*: \./usea' stderr], [0], [ignore], [ignore])
+AT_CHECK([grep 'libtool wrapper' stderr], [0], [ignore], [ignore])
-# Make sure wrapper debugging works, when activated at compile time.
+# 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 "s/LTCFLAGS=.*/&' $debugwrapper_flags'/" < "$orig_LIBTOOL" > ./libtool
+ sed "s/LTCFLAGS=.*/&' $debugwrapper_flags'/" < "$orig_LIBTOOL" |
+ sed "s/^lt_option_debug=/lt_option_debug=1/" > ./libtool
LIBTOOL=./libtool
# make sure $debugwrapper_flags do not cause a failure
@@ -127,7 +127,7 @@ for debugwrapper_flags in '-DLT_DEBUGWRAPPER'; do
[], [ignore], [ignore])
LT_AT_EXEC_CHECK([./usea], [0], [ignore], [stderr], [])
LT_AT_UNIFY_NL([stderr])
- AT_CHECK([grep '^(main) argv\[[0\]][[ \t]]*: \./usea' stderr], [0], [ignore], [ignore])
+ AT_CHECK([grep 'libtool wrapper' stderr], [0], [ignore], [ignore])
done
