Re: osf5.1 + cc + -pthread

[Ralf Wildenhues]

Hi Peter,

* Peter O'Gorman wrote on Fri, Jan 14, 2005 at 02:52:47PM CET:
> 
> Can someone give a second opinion on this? Whose patch is "better"?
> I'm obviously biased, so shouldn't decide. Some kind of fix is required 
> though.

I'll give it a try.

> Albert Chin wrote:
> |
> | Patch below. Unlike the previous patch, we don't exclude based on $LD
> | but include based on $CC.
> 
> Albert's patch:
> Index: ltmain.in
> ===================================================================
> RCS file: /cvsroot/libtool/libtool/Attic/ltmain.in,v
> retrieving revision 1.334.2.42
> diff -u -3 -p -r1.334.2.42 ltmain.in
> - --- ltmain.in       9 Dec 2004 18:00:47 -0000       1.334.2.42
> +++ ltmain.in 11 Dec 2004 22:19:00 -0000
> @@ -1033,6 +1042,7 @@ EOF
> ~     finalize_shlibpath=
> ~     convenience=
> ~     old_convenience=
> +    add_to_deplibs=
> ~     deplibs=
> ~     old_deplibs=
> ~     compiler_flags=
> @@ -1498,7 +1508,7 @@ EOF
> ~     ;;
> 
> ~      -mt|-mthreads|-kthread|-Kthread|-pthread|-pthreads|--thread-safe)
> - -   deplibs="$deplibs $arg"
> +     add_to_deplibs="$add_to_deplibs $arg"
> ~     continue
> ~     ;;
> 
> @@ -1888,6 +1898,25 @@ EOF
> ~     *) linkmode=prog ;; # Anything else should be a program.
> ~     esac
> 
> +    # arguments we can add to $deplibs except when creating a
> +    # libtool library where the compiler driver is different
> +    # from the linker driver.
> +    if test "X$add_to_deplibs" != "X"; then
> +      if test "$linkmode" = "lib"; then
> +     case "`eval $echo $archive_cmds`" in
> +     "$CC"*)

I don't like the eval here.
Can't we do like in the other patch and match against
   *"\$CC"*
?  Note also the * before $CC because I see not reason
to restrict ourselves unnecessarily to the beginning here.

> +       for deplib in $add_to_deplibs; do
> +         deplibs="$deplibs $deplib"
> +       done

I fail to see the difference of these three lines and
  deplips="$deplibs $add_to_deplibs"
other than whitespace normalization
(which should be unnecessary in this case),
plus the latter is much shorter.

> +       ;;
> +     esac
> +      else
> +     for deplib in $add_to_deplibs; do
> +      deplibs="$deplibs $deplib"
> +     done

Same here.

> +      fi
> +    fi
> +
> ~     case $host in
> ~     *cygwin* | *mingw* | *pw32*)
> ~       # don't eliminate duplications in $postdeps and $predeps


> Mine:
> diff -u -3 -p -u -r1.334.2.41 ltmain.in
> - --- ltmain.in 1 Dec 2004 18:00:58 -0000 1.334.2.41
> +++ ltmain.in 5 Dec 2004 11:48:30 -0000
> @@ -1488,7 +1488,10 @@ EOF
> ~        ;;
> 
> ~      -mt|-mthreads|-kthread|-Kthread|-pthread|-pthreads|--thread-safe)
> - -       deplibs="$deplibs $arg"
> +       case "$archive_cmds" in
> +         *"\$LD"*) ;;
> +         *) deplibs="$deplibs $arg";;
> +       esac
> ~        continue
> ~        ;;
> 
> @@ -1976,7 +1979,10 @@ EOF
> ~            compile_deplibs="$deplib $compile_deplibs"
> ~            finalize_deplibs="$deplib $finalize_deplibs"
> ~          else
> - -           deplibs="$deplib $deplibs"
> +           case "$archive_cmds" in
> +             *"\$LD"*) ;;
> +             *) deplibs="$deplib $deplibs";;
> +           esac
> ~          fi
> ~          continue
> ~          ;;

Both tests fail if there ever is a case where $archive_expsyms_cmds uses
a different linker driver than $archive_cmds.  This might be an academic
issue.

Other than that, after my proposed changes:
- both patches have around the same size,
- I like the "inclusion because of $CC" argument
- but Albert's patch messes up the order of the arguments.
  IIRC that's done anyway, right?

Can we have your patch, Peter, and have it test for \$CC instead of
\$LD?

Just my .02 euro cents and by no means a strong opinion,
Ralf