Re: backport of duplicate members changes to branch-2-0

[Ralf Wildenhues]

Hi Peter,

* Peter O'Gorman wrote on Thu, May 05, 2005 at 03:02:03PM CEST:
>
> Okay to apply?

Minor nits below, otherwise fine, I think.
(As fine as it is in HEAD currently, which will change soon)

Regards,
Ralf

> Index: config/ltmain.m4sh
> ===================================================================
> RCS file: /cvsroot/libtool/libtool/config/ltmain.m4sh,v
> retrieving revision 1.1.2.51
> diff -u -r1.1.2.51 ltmain.m4sh
> --- config/ltmain.m4sh 27 Apr 2005 11:31:16 -0000 1.1.2.51
> +++ config/ltmain.m4sh 5 May 2005 12:59:16 -0000
> @@ -936,32 +936,7 @@
>      if ($AR t "$f_ex_an_ar_oldlib" | sort | sort -uc >/dev/null 2>&1); then
>       :
>      else
> -      f_ex_an_ar_lib=`$ECHO "X$f_ex_an_ar_oldlib" | $Xsed -e 's%^.*/%%'`
> -      func_warning "$modename: warning: object name conflicts; renaming 
> object files" 1>&2
> -      func_warning "$modename: warning: to ensure that they will not 
> overwrite" 1>&2
> -      $show "cp $f_ex_an_ar_oldlib $f_ex_an_ar_dir/$f_ex_an_ar_lib"
> -      $run eval "cp \$f_ex_an_ar_oldlib \$f_ex_an_ar_dir/\$f_ex_an_ar_lib"
> -      $AR t "$f_ex_an_ar_oldlib" | sort | uniq -c | while read count name
> -      do
> -     # We don't want to do anything to objects with unique names
> -        test "$count" -eq 1 && continue
> -     i=1
> -     while test "$i" -le "$count"
> -       do
> -       # Put our $i before any first dot (extension)
> -       # Never overwrite any file
> -       name_to="$name"
> -       while test "X$name_to" = "X$name" || test -f 
> "$f_ex_an_ar_dir/$name_to"
> -         do
> -         name_to=`$ECHO "X$name_to" | $Xsed -e "s/\([[^.]]*\)/\1-$i/"`
> -       done
> -       $show "(cd $f_ex_an_ar_dir && $AR x $f_ex_an_ar_lib '$name' && $MV 
> '$name' '$name_to' && $AR d $f_ex_an_ar_lib '$name')"
> -       $run eval "(cd \$f_ex_an_ar_dir && $AR x \$f_ex_an_ar_lib '$name' && 
> $MV '$name' '$name_to' && $AR d \$f_ex_an_ar_lib '$name')" || exit $?
> -       i=`expr $i + 1`
> -     done
> -      done
> -      $show "$RM $f_ex_an_ar_dir/$f_ex_an_ar_lib"
> -      $run eval "$RM \$f_ex_an_ar_dir/\$f_ex_an_ar_lib"
> +     func_fatal_error "object name conflicts in archive: 
> $f_ex_an_ar_dir/$f_ex_an_ar_oldlib"
>      fi
>  }
>  
> @@ -6262,6 +6237,52 @@
>         oldobjs="$oldobjs $func_extract_archives_result"
>       fi
>  
> +     # POSIX demands no paths to be encoded in archives.  We have
> +     # to avoid creating archives with duplicate basenames if we
> +     # might have to extract them afterwards, e.g., when creating a
> +     # static archive out of a convenience library, or when linking
> +     # the entirety of a libtool archive into another (currently
> +     # not supported by libtool).
> +     if (for obj in $oldobjs
> +         do
> +           $ECHO "X$obj" | $Xsed -e 's%^.*/%%'

Please do the Xsed outside the loop right before the first sort.
Since Xsed then does the wrong thing, please use
  $SED 's%X%%; s%.*/%%' 
instead.  No need to be slower than necessary here.

> +         done | sort | sort -uc >/dev/null 2>&1); then
> +       :
> +     else
> +       $ECHO "copying selected object files to avoid basename conflicts..."

Erm.  I think that should be func_echo.  I also think I missed this in
HEAD as well.

> +
> +       
> +       gentop="$output_objdir/${outputname}x"
> +       generated="$generated $gentop"
> +       func_mkdir_p "$gentop"
> +       save_oldobjs=$oldobjs
> +       oldobjs=
> +       counter=1
> +       for obj in $save_oldobjs
> +       do
> +         objbase=`$ECHO "X$obj" | $Xsed -e 's%^.*/%%'`
> +         case " $oldobjs " in
> +         " ") oldobjs=$obj ;;
> +         *[[\ /]]"$objbase "*)
> +           while :; do
> +             # Make sure we don't pick an alternate name that also
> +             # overlaps.
> +             newobj=lt$counter-$objbase
> +             counter=`expr $counter + 1`
> +             case " $oldobjs " in
> +             *[[\ /]]"$newobj "*) ;;
> +             *) if test ! -f "$gentop/$newobj"; then break; fi ;;
> +             esac
> +           done
> +           $show "ln $obj $gentop/$newobj || cp $obj $gentop/$newobj"
> +           $run ln "$obj" "$gentop/$newobj" ||
> +           $run cp "$obj" "$gentop/$newobj"
> +           oldobjs="$oldobjs $gentop/$newobj"
> +           ;;
> +         *) oldobjs="$oldobjs $obj" ;;
> +         esac
> +       done
> +     fi
>       eval cmds=\"$old_archive_cmds\"
>  
>       if len=`expr "X$cmds" : ".*"` &&
> @@ -6275,20 +6296,6 @@
>         objlist=
>         concat_cmds=
>         save_oldobjs=$oldobjs
> -       # GNU ar 2.10+ was changed to match POSIX; thus no paths are
> -       # encoded into archives.  This makes 'ar r' malfunction in
> -       # this piecewise linking case whenever conflicting object
> -       # names appear in distinct ar calls; check, warn and compensate.
> -         if (for obj in $save_oldobjs
> -         do
> -           $ECHO "X$obj" | $Xsed -e 's%^.*/%%'
> -         done | sort | sort -uc >/dev/null 2>&1); then
> -         :
> -       else
> -         func_warning "object name conflicts; overriding AR_FLAGS to 'cq'"
> -         func_warning "to ensure that POSIX-compatible ar will work"
> -         AR_FLAGS=cq
> -       fi
>         # Is there a better way of finding the last object in the list?
>         for obj in $save_oldobjs
>         do
> Index: tests/duplicate_members.test
> ===================================================================
> RCS file: tests/duplicate_members.test
> diff -N tests/duplicate_members.test
> --- /dev/null 1 Jan 1970 00:00:00 -0000
> +++ tests/duplicate_members.test 5 May 2005 12:59:16 -0000
> @@ -0,0 +1,55 @@
> +#! /bin/sh
> +# duplicate members
> +
> +. tests/defs || exit 1
> +
> +test -d duplicate_members.dir && rm -rf duplicate_members.dir
> +
> +mkdir duplicate_members.dir
> +# we don't want to use whole_archive_flag_spec, even if available
> +sed -e 's|^whole_archive_flag_spec=.*|whole_archive_flag_spec=|g' < $LIBTOOL 
> > duplicate_members.dir/libtool
> +
> +cd duplicate_members.dir
> +chmod +x ./libtool
> +
> +for a in 1 2 3 4 5 6
> +do
> +  mkdir $a
> +  echo "int foo$a() {return $a;}" > $a/a.c
> +done
> +eval `./libtool --config | grep ^CC`
> +
> +for a in 1 2 3 4 5 6
> +do
> +  ./libtool --mode=compile --tag=CC $CC -c $CFLAGS -o $a/a.lo $a/a.c
> +done
> +./libtool --mode=link --tag=CC $CC $CFLAGS $LDFLAGS -o libc0.la 6/a.lo
> +./libtool --mode=link --tag=CC $CC $CFLAGS $LDFLAGS -o libconv.la 1/a.lo 
> 2/a.lo 3/a.lo 4/a.lo 5/a.lo libc0.la
> +
> +exit 1

You most certainly don't want this in here!  :-)

> +
> +cat <<EOF > bar.c
> +int bar() {
> +    int result=foo1() +foo2() +foo3() +foo4() +foo5() +foo6();
> +    return result;
> +}
> +EOF
> +
> +./libtool --mode=compile --tag=CC $CC -c $CFLAGS -o bar.lo bar.c
> +./libtool --mode=link --tag=CC $CC $CFLAGS $LDFLAGS -o libbar.la bar.lo 
> libconv.la -rpath /notexist
> +
> +cat <<EOF > main.c

Really minor nit: I am used to the here doc delimiter being at the end
of the line (applies here and above).  Besides consistency my editor
also highlights it better.  No strong preference though.

> +int bar();
> +int main()
> +{
> +if (bar() == 21) return 0;
> +return 1;
> +}
> +EOF
> +./libtool --mode=compile --tag=CC $CC -c $CFLAGS -o main.lo main.c
> +./libtool --mode=link --tag=CC $CC $CFLAGS $LDFLAGS -o main main.lo 
> ./libbar.la
> +./main
> +retcode=$?
> +cd ..
> +rm -rf duplicate_members.dir
> +exit $retcode