Re: MSVC: Find potential libs regardless of case.

[Peter Rosin]

Hi Ralf,

Den 2010-06-14 23:01 skrev 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.  ;-)

That's extremely hard to resist, so I'm pushing right away with nits
taken care of...

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

ok

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

ok

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

ok, removed a comma before "that" as well...

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

ok

> > +option when looking for potential files.
>
> when looking for potential file names in a case-insensitive manner.

ok

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

ok

> > +_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.

Hmmm, as in "that's long ago" or "should be 2010" or "should be 2007, 2010"?

I went with number 2, i.e. just 2010.

> > +#   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.  ;-)

ok

> > +$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])

Doesn't hurt I suppose. ok.

> > +$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?

I went with Eric's variation.

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

Thanks for the review!
BTW, I found nothing unexpected in the testsuite with MinGW-gcc, the
failures are the same with and without the patch.

Cheers,
Peter