Hi Peter,
First, let me thank you for your hard work on this!
Then, let me give a couple of quick comments, and
sorry that I still haven't had a chance to test any
of it myself (so take the comments with a grain of salt).
* Peter Ekberg wrote on Thu, Jun 09, 2005 at 02:01:35PM CEST:
>
> I have now fixed at least some issues with the previous
> patch. I changed $libname_spec to plain $name for MSVC
> as that seems to be the naming convention used by MS.
Hmm. OK.
> I also fixed locating potential dependent libs without
> regard to the filename case (i.e -lwsock32 on the
> link line will be matched by the file WSOCK32.lib).
The file_magic_glob is really cute, I like it. :)
> I realize that this still is not in shape for a commit.
> There's not even a changelog :-), but I'd really
> appreciate any pointers on how to get it into shape.
Sure.
I'd really like it if there were no two different functions
func_extract_a_lib and func_extract_an_archive. Something like:
(watch out: func_show_eval is Libtool HEAD notation; untested)
func_extract_an_archive ()
{
$opt_debug
# save nonlocal variables
f_ex_an_ar_dir_save=$output_objdir
output_objdir=$1; shift
f_ex_an_ar_oldlib_save=$oldlib
oldlib=$1
f_ex_an_ar_name_save=$name
func_show_eval "$extract_old_archive_cmds"
if ( eval "$list_old_archive_cmd" | sort | sort -uc >/dev/null 2>&1); then
:
else
func_fatal_error "object name conflicts in archive: $output_objdir/$oldlib"
fi
# restore nonlocal variables
oldlib=$f_ex_an_ar_oldlib_save
name=$f_ex_an_ar_name_save
output_objdir=$f_ex_an_ar_dir_save
}
and then in libtool.m4:
# MSVC
AR='link -LIB -NOLOGO'
# for other MSVC versions that might be just 'LIB', ...
list_old_archive_cmd='$AR -LIST "$oldlib"'
extract_old_archive_cmds='$AR -LIST "$oldlib" | while read name; do
(cd $output_objdir && link -LIB -NOLOGO -EXTRACT:$name
$oldlib );
done'
# Default
list_old_archive_cmd='$AR t "$oldlib"'
extract_old_archive_cmds='$AR x "$oldlib"'
That would enable you to kill some more of the system-dependency of the
patch in ltmain. It would also enable us to test for and use `ar' if
it's available and working. Does the presumably quadratic loop for MSVC
hurt for large archives?
Also note that I intended to parametrize more of the $AR handling anyway
(for omitting $RANLIB).
More comments to be found inline (some are really intended as hints
rather than anything else).
Cheers,
Ralf
| Index: config/ltmain.m4sh
| ===================================================================
| RCS file: /cvsroot/libtool/libtool/config/ltmain.m4sh,v
| retrieving revision 1.1.2.54
| diff -u -u -5 -r1.1.2.54 ltmain.m4sh
| --- config/ltmain.m4sh 5 Jun 2005 17:35:11 -0000 1.1.2.54
| +++ config/ltmain.m4sh 9 Jun 2005 11:45:21 -0000
| @@ -923,10 +923,29 @@
| compile_command=`$ECHO "X$compile_command" | $Xsed -e "s% @[EMAIL
PROTECTED]"`
| finalize_command=`$ECHO "X$finalize_command" | $Xsed -e "s% @[EMAIL
PROTECTED]"`
| fi
| }
|
| +# func_extract_a_lib dir oldlib
| +func_extract_a_lib ()
| +{
| + $opt_debug
| + f_ex_a_lib_dir="$1"; shift
| + f_ex_a_lib_oldlib="$1"
| + link -LIB -NOLOGO -LIST "$f_ex_a_lib_oldlib" | while read name
| + do
| + $show "(cd $f_ex_a_lib_dir && link -LIB -NOLOGO -EXTRACT:$name
$f_ex_a_lib_oldlib)"
| + $run eval "(cd \$f_ex_a_lib_dir && link -LIB -NOLOGO -EXTRACT:\$name
\$f_ex_a_lib_oldlib)" || exit $?
| + done
| + if (link -LIB -NOLOGO -LIST "$f_ex_a_lib_oldlib" | sort | sort -uc
>/dev/null 2>&1); then
| + :
| + else
| + func_fatal_error "object name conflicts in archive:
$f_ex_a_lib_dir/$f_ex_a_lib_oldlib"
| + fi
| +}
| +
| +
| # func_extract_an_archive dir oldlib
| func_extract_an_archive ()
| {
| $opt_debug
| f_ex_an_ar_dir="$1"; shift
| @@ -1002,12 +1021,20 @@
| func_extract_an_archive "$my_xdir" "$my_xabs"
| fi # $darwin_arches
| fi # $run
| ;;
| *)
| - func_extract_an_archive "$my_xdir" "$my_xabs"
| + case $with_gcc/$host in
| + no/*-*-mingw* | /*-*-mingw* | no/*-*-cygwin* | /*-*-cygwin*)
| + # assume MSVC
| + func_extract_a_lib "$my_xdir" "$my_xabs"
| + ;;
| + *)
| + func_extract_an_archive "$my_xdir" "$my_xabs"
| ;;
| + esac
| + ;;
| esac
| my_oldobjs="$my_oldobjs "`find $my_xdir -name \*.$objext -print -o
-name \*.lo -print | $NL2SP`
| done
|
| func_extract_archives_result="$my_oldobjs"
Above two hunks should be dealt with above.
| @@ -2596,10 +2623,11 @@
| continue
| ;;
| *-*-mingw* | *-*-os2*)
| # These systems don't actually have a C library (as such)
| test "X$arg" = "X-lc" && continue
| + test "X$with_gcc" != "Xyes" -a "X$arg" = "X-lm" && continue
Does MSVC not have libm? Does anybody know how Borland fares here?
| ;;
| *-*-openbsd* | *-*-freebsd* | *-*-dragonfly*)
| # Do not include libc due to us having libc/libc_r.
| test "X$arg" = "X-lc" && continue
| ;;
| @@ -3149,10 +3177,27 @@
| fi
| break 2
| fi
| done
| done
| + if test "$found" != "yes"; then
| + case $linkmode/$with_gcc/$host in
| + prog/no/*-*-mingw* | prog//*-*-mingw* | prog/no/*-*-cygwin* |
prog//*-*-cygwin*)
I don't like this $host distinction here much. Let's see if we can kill
it.
| + # assume MSVC, which will look in $LIB for libraries
| + save_ifs="$IFS"; IFS=';'
We have $PATH_SEPARATOR.
| + for msvc_libdir in $LIB; do
| + IFS="$save_ifs"
| + # assume windows drives are accessible as /D, where D is a
drive letter.
I don't think you can make this assumption.
I believe we should probably try to generalize fix_srcfile_path (as in:
either you have `cygpath' available or you can make some assumption
about drive letters which is true for MinGW.
| + if test -f "/$($ECHO "X$msvc_libdir" | Xsed -e s.\\\\./.g -e
s.\\:..)/${name}.lib"; then
| + found=no
| + break
| + fi
| + done
| + IFS="$save_ifs"
| + ;;
| + esac
| + fi
| if test "$found" != yes; then
| # deplib doesn't seem to be a libtool library
| if test "$linkmode,$pass" = "prog,link"; then
| compile_deplibs="$deplib $compile_deplibs"
| finalize_deplibs="$deplib $finalize_deplibs"
| @@ -4075,10 +4120,21 @@
| case " $tmp_libs " in
| *" $deplib "*) ;;
| *) tmp_libs="$tmp_libs $deplib" ;;
| esac
| ;;
| + -l*)
| + if test "$linkmode" = prog -a "X$with_gcc" != Xyes; then
| + case $host in
| + *-*-mingw* | *-*-cygwin*)
| + # assume MSVC
| + deplib=`$ECHO "X${deplib}.lib" | $Xsed -e 's/^-l//'`
| + ;;
| + esac
| + fi
| + tmp_libs="$tmp_libs $deplib"
| + ;;
Don't like this system-dependency here either.
Wonder if we need another switch or $libname_spec would suffice here..
| *) tmp_libs="$tmp_libs $deplib" ;;
| esac
| done
| eval $var=\"$tmp_libs\"
| done # for var
| @@ -4656,12 +4712,17 @@
| ;;
| esac
| fi
| if test -n "$a_deplib" ; then
| libname=`eval "\\$ECHO \"$libname_spec\""`
| + if test -n "$file_magic_glob"; then
| + libnameglob=`$ECHO "X$libname" | $Xsed -e $file_magic_glob`
| + else
| + libnameglob=$libname
| + fi
| for i in $lib_search_path $sys_lib_search_path
$shlib_search_path; do
| - potential_libs=`ls $i/$libname[[.-]]* 2>/dev/null`
| + potential_libs=`ls $i/$libnameglob[[.-]]* 2>/dev/null`
| for potent_lib in $potential_libs; do
| # Follow soft links.
| if ls -lLd "$potent_lib" 2>/dev/null |
| $GREP " -> " >/dev/null; then
| continue
| @@ -4680,11 +4741,18 @@
| esac
| done
| if eval $file_magic_cmd \"\$potlib\" 2>/dev/null |
| $SED -e 10q |
| $EGREP "$file_magic_regex" > /dev/null; then
| - newdeplibs="$newdeplibs $a_deplib"
| + case $with_gcc/$host in
| + no/*-*-mingw* | /*-*-mingw* | no/*-*-cygwin* |
/*-*-cygwin*)
| + newdeplibs="$newdeplibs ${name}.lib"
| + ;;
| + *)
| + newdeplibs="$newdeplibs $a_deplib"
| + ;;
| + esac
| a_deplib=""
| break 2
| fi
| done
| done
| @@ -5680,11 +5748,13 @@
| */
| EOF
| cat >> $cwrappersource<<"EOF"
| #include <stdio.h>
| #include <stdlib.h>
| +#ifndef _MSC_VER
| #include <unistd.h>
| +#endif
We should probably put a check for unistd.h (and sys/stat.h?) in
libtool.m4 and use that. What do you think? Also check for stat()
function? Is this idea overkill?
| #include <malloc.h>
| #include <stdarg.h>
| #include <assert.h>
| #include <ctype.h>
| #include <string.h>
| @@ -5732,15 +5802,19 @@
| if (stale) { free ((void *) stale); stale = 0; } \
| } while (0)
|
| /* -DDEBUG is fairly common in CFLAGS. */
| #undef DEBUG
| +#ifdef _MSC_VER
| +# define DEBUG()
| +#else
| #if defined DEBUGWRAPPER
| # define DEBUG(format, ...) fprintf(stderr, format, __VA_ARGS__)
| #else
| # define DEBUG(format, ...)
| #endif
| +#endif
Is your MSVC version not C99 compliant?
Does _MSC_VER reveal the version of the compiler?
|
| const char *program_name = NULL;
|
| void * xmalloc (size_t num);
| char * xstrdup (const char *string);
| @@ -5844,10 +5918,13 @@
|
| DEBUG("(check_executable) : %s\n", path ? (*path ? path : "EMPTY!") :
"NULL!");
| if ((!path) || (!*path))
| return 0;
|
| +#ifdef _MSC_VER
| + return 1;
| +#else
| if ((stat (path, &st) >= 0) &&
| (
| /* MinGW & native WIN32 do not support S_IXOTH or S_IXGRP */
| #if defined (S_IXOTH)
| ((st.st_mode & S_IXOTH) == S_IXOTH) ||
| @@ -5858,10 +5935,11 @@
| ((st.st_mode & S_IXUSR) == S_IXUSR))
| )
| return 1;
| else
| return 0;
| +#endif
| }
|
| /* Searches for the full path of the wrapper. Returns
| newly allocated full path name if found, NULL otherwise */
| char *
| Index: m4/libtool.m4
| ===================================================================
| RCS file: /cvsroot/libtool/libtool/m4/libtool.m4,v
| retrieving revision 1.125.2.58
| diff -u -u -5 -r1.125.2.58 libtool.m4
| --- m4/libtool.m4 6 Jun 2005 16:13:23 -0000 1.125.2.58
| +++ m4/libtool.m4 9 Jun 2005 11:45:22 -0000
| @@ -1816,10 +1816,12 @@
| ;;
| esac
| ;;
|
| *)
| + # Assume MSVC
| + libname_spec='$name'
| library_names_spec='${libname}`echo ${release} | $SED -e
's/[[.]]/-/g'`${versuffix}${shared_ext} $libname.lib'
Doesn't library_names_spec need to be case-ignoring as well (I don't
know actually).
| ;;
| esac
| dynamic_linker='Win32 ld.exe'
| # FIXME: first we should search . and the directory the executable is in
| @@ -2648,18 +2650,30 @@
| tpf*)
| lt_cv_deplibs_check_method=pass_all
| ;;
| esac
| ])
| +
| +case $host_os in
| +cygwin* | mingw* | pw32*)
| + file_magic_glob=`echo aAbBcCdDeEfFgGhHiIjJkKlLmMnNoOpPqQrRsStTuUvVwWxXyYzZ
| sed -e "s/\(..\)/s\/[[\1]]\/[[\1]]\/g;/g"`
| + ;;
| +*)
| + file_magic_glob=
| + ;;
| +esac
| +
| file_magic_cmd=$lt_cv_file_magic_cmd
| deplibs_check_method=$lt_cv_deplibs_check_method
| test -z "$deplibs_check_method" && deplibs_check_method=unknown
|
| _LT_DECL([], [deplibs_check_method], [1],
| [Method to check whether dependent libraries are shared objects])
| _LT_DECL([], [file_magic_cmd], [1],
| [Command to use when deplibs_check_method == "file_magic"])
| +_LT_DECL([], [file_magic_glob], [1],
| + [How to find potential files when deplibs_check_method == "file_magic"])
| ])# _LT_CHECK_MAGIC_METHOD
|
|
| # LT_PATH_NM
| # ----------
| @@ -3941,17 +3955,20 @@
| # Tell ltmain to make .lib files, not .a files.
| libext=lib
| # Tell ltmain to make .dll files, not .so files.
| shrext_cmds=".dll"
| # FIXME: Setting linknames here is a bad hack.
| - _LT_TAGVAR(archive_cmds, $1)='$CC -o $lib $libobjs $compiler_flags
`$ECHO "X$deplibs" | $Xsed -e '\''s/ -lc$//'\''` -link -dll~linknames='
| + _LT_TAGVAR(archive_cmds, $1)='$CC -o $lib $libobjs $compiler_flags
`$ECHO "X$deplibs" | $Xsed -e '\''s/ -lc$//'\''` -link -dll~linknames=~mv
.libs/${libname}`$ECHO "X${release}" | $Xsed -e s/[.]/-/g`${versuffix}.lib
.libs/${libname}.lib'
In general, you can't use .libs, it fails on FAT volumes. We have
$objdir for this.
Also I wonder why you deemed this necessary.
Moreover, independently of your patch, I believe we see some redundancy
here as we filter out ` -lc' here and in ltmain. Wonder whether we can
remove one of them?
| + _LT_TAGVAR(module_cmds, $1)='$CC -o $lib $libobjs $compiler_flags
`$ECHO "X$deplibs" | $Xsed -e '\''s/ -lc$//'\''` -link -dll~linknames='
| # The linker will automatically build a .lib file if we build a DLL.
| _LT_TAGVAR(old_archive_from_new_cmds, $1)='true'
| - # FIXME: Should let the user specify the lib program.
| - _LT_TAGVAR(old_archive_cmds, $1)='lib /OUT:$oldlib$oldobjs$old_deplibs'
| - _LT_TAGVAR(fix_srcfile_path, $1)='`cygpath -w "$srcfile"`'
| + # FIXME: Should let the user specify the link program.
| + _LT_TAGVAR(old_archive_cmds, $1)='link -LIB -NOLOGO
-OUT:$oldlib$oldobjs$old_deplibs'
This should use $AR.
| + #_LT_TAGVAR(fix_srcfile_path, $1)='`cygpath -w "$srcfile"`'
Don't comment this out unconditionally. It's ok to have a test here or
parameterize according to cygwin vs mingw.
| _LT_TAGVAR(enable_shared_with_static_runtimes, $1)=yes
| + # Don't use ranlib
| + _LT_TAGVAR(old_postinstall_cmds, $1)='chmod 644 $oldlib'
Why is this necessary? If ranlib is not found ($RANLIB empty), it
should not be used anyway.
| ;;
|
| darwin* | rhapsody*)
| case $host_os in
| rhapsody* | darwin1.[[012]])
|
|
