Re: MSVC: Find potential libs regardless of case.

[Ralf Wildenhues]

Hi Peter,

* Peter Rosin wrote on Mon, Jun 14, 2010 at 01:26:04PM CEST:
> Here's the next patch on the MSVC branch. I decided to merge in
> a couple of things further ahead, no point in reviewing stuff
> when an improvement is around the corner, right?

this patch is ok for master, provided that it introduces no regressions
for MinGW with GCC, and you fix the trivial nits below.  You decide how
much rechecking is needed for that, and no need to ask for another
approval for the fixed patch.  I don't care when and in what order you
apply it, as long as it's not accidentally during a release freeze.  So
you might as well want to do it now and forget about it.  ;-)

> What I'm looking for is a go-ahead that this patch is ready for
> master, but then delay pushing it until enough of the MSVC support

However you like.  Thanks.

You need a short NEWS entry for the semantic change on MinGW that libs
are searched case-insensitively now.

> diff --git a/doc/libtool.texi b/doc/libtool.texi
> index 051aec3..035cb38 100644
> --- a/doc/libtool.texi
> +++ b/doc/libtool.texi
> @@ -5981,6 +5981,16 @@ and relinking at install time is triggered.  This also 
> means that @var{DESTDIR}
>  installation does not work as expected.
>  @end defvar
>  
> address@hidden file_magic_glob
> +How to find potential files when @var{deplibs_check_method} is
> address@hidden @var{file_magic_glob} is a @code{sed} expression,
> +and the @code{sed} instance is fed potential files, that is
> +transformed by the @var{file_magic_glob} expression. Useful when the
> +shell does not support the shell option @code{nocaseglob}, making
> address@hidden inappropriate. Normally disabled (i.e.
> address@hidden empty).
> address@hidden defvar

I find this paragraph hard to understand.  Maybe
  s/fed potential files/fed potential file names/   ?
  s/empty/is &/   ?

@var is not for variables, it is for metasyntactic variables only, which
are not needed anywhere in this and the other added paragraph.  Use
@samp or @code for file_magic_glob and want_nocaseglob.

> address@hidden want_nocaseglob
> +Find potential files using the shell option @code{nocaseglob} when

comma before when

> address@hidden is @samp{file_magic}. Normally set to
> address@hidden, set to @samp{yes} to enable the @code{nocaseglob} shell

Period after no, not comma; capitalize 'Set'.

> +option when looking for potential files.

when looking for potential file names in a case-insensitive manner.

> address@hidden defvar

> --- a/libltdl/m4/libtool.m4
> +++ b/libltdl/m4/libtool.m4
> @@ -3163,6 +3163,21 @@ tpf*)
>    ;;
>  esac
>  ])
> +
> +file_magic_glob=
> +want_nocaseglob=no
> +if test "$build" = "$host"; then
> +  case $host_os in
> +  mingw* | pw32*)
> +    if ( shopt | grep nocaseglob ) >/dev/null 2>&1; then
> +      want_nocaseglob=yes
> +    else
> +      file_magic_glob=`echo 
> aAbBcCdDeEfFgGhHiIjJkKlLmMnNoOpPqQrRsStTuUvVwWxXyYzZ | $SED -e 
> "s/\(..\)/s\/[[\1]]\/[[\1]]\/g;/g"`
> +    fi
> +    ;;
> +  esac
> +fi
> +
>  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
> @@ -3171,6 +3186,10 @@ _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"])

Please use = not == in shell script comments.  Another instance below,
and yes, I see the one that was there previously, might as well fix that
too while you're at it.  Thanks.

> +_LT_DECL([], [want_nocaseglob], [1],
> +    [Find potential files using nocaseglob when deplibs_check_method == 
> "file_magic"])
>  ])# _LT_CHECK_MAGIC_METHOD

> --- /dev/null
> +++ b/tests/nocase.at
> @@ -0,0 +1,82 @@
> +# nocase.at --  test for nocase lib search  -*- Autotest -*-
> +#
> +#   Copyright (C) 2007 Free Software Foundation, Inc.

Oops.

> +#   Written by Peter Rosin, 2007

> +AT_SETUP([nocase library search])

> +AT_DATA([foo/Foo.c],
> +[
> +int Foo(void) { return 1; }

Space before open parenthesis.  Several instances.  (Yes, I know we are
not consistent in the test suite, and it's harmless if you don't fix
this, but when I notice deviations from GCS I'll mention them.  ;-)

> +$LIBTOOL --mode=compile --tag=CC $CC $CPPFLAGS $CFLAGS -c -o foo/Foo.lo 
> foo/Foo.c
> +AT_CHECK([$LIBTOOL --mode=link --tag=CC $CC $CFLAGS $LDFLAGS -o 
> foo/libFoo.la foo/Foo.lo ]dnl
> +      [-no-undefined -version-info 1:0:0 -rpath $libdir],
> +      [], [ignore], [ignore])
> +AT_CHECK([$LIBTOOL --mode=install cp foo/libFoo.la $libdir],
> +      [], [ignore], [ignore])
> +
> +rm -f $libdir/libFoo.la

Don't you also want this here to avoid picking up uninstalled libFoo?
  AT_CHECK([$LIBTOOL --mode=clean rm -f foo/libFoo.la], [], [ignore])

> +$LIBTOOL --mode=compile --tag=CC $CC $CPPFLAGS $CFLAGS -c -o bar/bar.lo 
> bar/bar.c
> +AT_CHECK([$LIBTOOL --mode=link --tag=CC $CC $CFLAGS $LDFLAGS -o 
> bar/libbar.la bar/bar.lo ]dnl
> +      [-L$libdir -lfoo -no-undefined -version-info 1:0:0 -rpath $libdir],
> +      [], [ignore], [ignore])
> +AT_CHECK([$LIBTOOL --mode=install cp bar/libbar.la $libdir],
> +      [], [ignore], [ignore])
> +
> +eval `$EGREP '^(old_library)=' < $libdir/libbar.la`

This is probably lacking double-quoting around the `...` text.  Can you
verify?

> +libbar=$old_library
> +rm -f $libdir/$libbar
> +
> +$LIBTOOL --mode=compile --tag=CC $CC $CPPFLAGS $CFLAGS -c -o main.$OBJEXT 
> main.c
> +
> +AT_CHECK([$LIBTOOL --mode=link $CC $CFLAGS $LDFLAGS -o main main.$OBJEXT 
> -L$libdir -lbar],
> +      [], [ignore], [ignore])
> +
> +AT_CLEANUP

Thanks for your patience,
Ralf