libtool-patches
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: deplibs_check_method=test_compile


From: Ralf Wildenhues
Subject: Re: deplibs_check_method=test_compile
Date: Sun, 5 Feb 2006 11:32:56 +0100
User-agent: Mutt/1.5.9i

Hi Gary,

* Gary V. Vaughan wrote on Sat, Feb 04, 2006 at 02:38:53AM CET:
> Ralf Wildenhues wrote:
*snip*
> > -         if test "$name" != "" && test "$name" -ne "0"; then
> > +         if test "$name" != "" && test "$name" != 0; then
>                                                        ^^
> s/-ne/!=/ agreed... but I prefer to leave "0" alone.
*snip*
> > -         if test "$name" != "" && test "$name" != "0"; then
> > +         if test "$name" != "" && test "$name" != 0; then
>                                                     ^^
> Huh?  != is the string comparison operator, so I like that it was
> explicit with "0".  Does this really cause a spurious error?

No, of course the two changes you point out do not cause or fix any
errors.  So it is a matter of personal preference.

> Just nit-picking really.  If you would rather drop the quotes, I'm
> not overly fussed.  Yes, please apply.

Thanks.  I have applied the patch without the two changes, to HEAD and
branch-1-5, because the bugs were there, too.  Both shown below.


But I'd like to mention a couple of sentences of rationale for above.
Many times I have seen users code like this
   if test $parameter = "value" ...
rather than
   if test "$parameter" = value ...

, and your comment:
> != is the string comparison operator, so I like that it was
> explicit with "0".

is also indicative of a common and wide-spread misconception:
Quoting here is not done at all because of `test', but because parameter
expansion may result in more than one `word'.  `test' will never ever
get to see the quotes.  And the string `0' simply won't be changed by
any of shell expansions, so it will never result in more than one word.
The fact that `test' is often a builtin command rather than an external
program is completely irrelevant to this; and IMVHO the fact that
numeric comparisons are usually not done in the same command line with
parameters that expand to one word is a coincidence; quoting is also
used to prevent parameters from expanding to zero words; that '' is not
a valid number is another coincidence.  All IMVHO, of course.

I have come to prefer more minimal quoting in examples, in order to make
above difference a bit more explicit: it took me a long time to grasp
this peculiarity of shell syntax.

Cheers,
Ralf

HEAD:
        * libltdl/config/ltmain.m4sh (func_mode_link) < test_compile >:
        Fix a couple of instances where `test .. -ne ..' would possibly
        compare non-numbers.  Clean up a bit.

Index: libltdl/config/ltmain.m4sh
===================================================================
RCS file: /cvsroot/libtool/libtool/libltdl/config/ltmain.m4sh,v
retrieving revision 1.33
diff -u -r1.33 ltmain.m4sh
--- libltdl/config/ltmain.m4sh  3 Feb 2006 09:25:01 -0000       1.33
+++ libltdl/config/ltmain.m4sh  5 Feb 2006 10:18:10 -0000
@@ -4594,13 +4594,12 @@
          int main() { return 0; }
 EOF
          $opt_dry_run || $RM conftest
-         $LTCC $LTCFLAGS -o conftest conftest.c $deplibs
-         if test "$?" -eq 0 ; then
+         if $LTCC $LTCFLAGS -o conftest conftest.c $deplibs; then
            ldd_output=`ldd conftest`
            for i in $deplibs; do
              name=`expr $i : '-l\(.*\)'`
              # If $name is empty we are operating on a -L argument.
-             if test "$name" != "" && test "$name" -ne "0"; then
+             if test "$name" != "" && test "$name" != "0"; then
                if test "X$allow_libtool_libs_with_static_runtimes" = "Xyes" ; 
then
                  case " $predeps $postdeps " in
                  *" $i "*)
@@ -4639,9 +4638,7 @@
              # If $name is empty we are operating on a -L argument.
              if test "$name" != "" && test "$name" != "0"; then
                $opt_dry_run || $RM conftest
-               $LTCC $LTCFLAGS -o conftest conftest.c $i
-               # Did it work?
-               if test "$?" -eq 0 ; then
+               if $LTCC $LTCFLAGS -o conftest conftest.c $i; then
                  ldd_output=`ldd conftest`
                  if test "X$allow_libtool_libs_with_static_runtimes" = "Xyes" 
; then
                    case " $predeps $postdeps " in
@@ -4673,7 +4670,7 @@
                  droppeddeps=yes
                  $ECHO
                  $ECHO "*** Warning!  Library $i is needed by this library but 
I was not able to"
-                 $ECHO "***  make it link in!  You will probably need to 
install it or some"
+                 $ECHO "*** make it link in!  You will probably need to 
install it or some"
                  $ECHO "*** library that it depends on before this library 
will be fully"
                  $ECHO "*** functional.  Installing it before continuing would 
be even better."
                fi

branch-1-5:
        * ltmain.in (link mode) < test_compile >: Fix a couple of instances
        where `test .. -ne ..' would possibly compare non-numbers.  Clean up a
        bit.

Index: ltmain.in
===================================================================
RCS file: /cvsroot/libtool/libtool/Attic/ltmain.in,v
retrieving revision 1.334.2.119
diff -u -r1.334.2.119 ltmain.in
--- ltmain.in   3 Feb 2006 09:25:56 -0000       1.334.2.119
+++ ltmain.in   5 Feb 2006 10:18:43 -0000
@@ -3524,13 +3524,12 @@
          int main() { return 0; }
 EOF
          $rm conftest
-         $LTCC $LTCFLAGS -o conftest conftest.c $deplibs
-         if test "$?" -eq 0 ; then
+         if $LTCC $LTCFLAGS -o conftest conftest.c $deplibs; then
            ldd_output=`ldd conftest`
            for i in $deplibs; do
              name=`expr $i : '-l\(.*\)'`
              # If $name is empty we are operating on a -L argument.
-              if test "$name" != "" && test "$name" -ne "0"; then
+              if test "$name" != "" && test "$name" != "0"; then
                if test "X$allow_libtool_libs_with_static_runtimes" = "Xyes" ; 
then
                  case " $predeps $postdeps " in
                  *" $i "*)
@@ -3569,9 +3568,7 @@
              # If $name is empty we are operating on a -L argument.
               if test "$name" != "" && test "$name" != "0"; then
                $rm conftest
-               $LTCC $LTCFLAGS -o conftest conftest.c $i
-               # Did it work?
-               if test "$?" -eq 0 ; then
+               if $LTCC $LTCFLAGS -o conftest conftest.c $i; then
                  ldd_output=`ldd conftest`
                  if test "X$allow_libtool_libs_with_static_runtimes" = "Xyes" 
; then
                    case " $predeps $postdeps " in
@@ -3603,7 +3600,7 @@
                  droppeddeps=yes
                  $echo
                  $echo "*** Warning!  Library $i is needed by this library but 
I was not able to"
-                 $echo "***  make it link in!  You will probably need to 
install it or some"
+                 $echo "*** make it link in!  You will probably need to 
install it or some"
                  $echo "*** library that it depends on before this library 
will be fully"
                  $echo "*** functional.  Installing it before continuing would 
be even better."
                fi




reply via email to

[Prev in Thread] Current Thread [Next in Thread]