libtool-patches
[Top][All Lists]
Advanced

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

Re: Darwin status


From: Ralf Wildenhues
Subject: Re: Darwin status
Date: Tue, 7 Dec 2004 08:20:58 +0100
User-agent: Mutt/1.4.1i

[ to libtool-patches only ]

* Peter O'Gorman wrote on Tue, Dec 07, 2004 at 12:02:45AM CET:
> Ralf Wildenhues wrote:
> 
> >Only minor issues (apart from the fact that I can't test the patch
> >myself):
> 
> You can still test the patch, it affects all platforms, and I only tried it 
> on freebsd-4.8 (with gnu ar) and darwin8.

Yep, you're right (and I genuinely missed that).

> >I know the long variable names are ugly, and long lines are, too.
> >Still: can we keep indentation, please?
> 
> Made var names shorter and indented.

Good.

Maybe we should adopt a namespace for local function variables for small
functions which don't make use of other functions themselves.  Like my_*.
It's a dangerous thing to do, however (the function in this patch will
not be one of them for long..).

> >Could you state how the test fails before patching ltmain.in?
> >Does the test succeed on other systems, before and after patching
> >ltmain.in?
> 
> The test would likely pass on all systems using gnu ar. It would fail on 
> systems with a limited ar. Now it should pass on both. If a system uses an 
> ar which does not allow multiple objects (ok according to SUSv6) to have 
> the same name, it will fail.

Passes on gnu-linux, aix.

Minor nits (other hunks snipped):

> --- ltmain.in 1 Dec 2004 18:00:58 -0000 1.334.2.41
> +++ ltmain.in 6 Dec 2004 22:44:50 -0000
> @@ -299,10 +337,8 @@ func_extract_archives () {
>             # Remove the table of contents from the thin files.
>             $AR -d 
> "unfat-$$/${darwin_base_archive}-${darwin_arch}/${darwin_base_archive}" 
> __.SYMDEF 2>/dev/null || true
>             $AR -d 
> "unfat-$$/${darwin_base_archive}-${darwin_arch}/${darwin_base_archive}" 
> __.SYMDEF\ SORTED 2>/dev/null || true
> -           cd "unfat-$$/${darwin_base_archive}-${darwin_arch}"
> -           $AR -xo "${darwin_base_archive}"
> -           rm "${darwin_base_archive}"
> -           cd "$darwin_curdir"
> +           func_extract_an_archive 
> "unfat-$$/${darwin_base_archive}-${darwin_arch}" "${darwin_base_archive}"
> +           rm 
> "unfat-$$/${darwin_base_archive}-${darwin_arch}/${darwin_base_archive}"

Any reason not to use 'rm -f'?

>           done # $darwin_arches
>        ## Okay now we have a bunch of thin objects, gotta fatten them up :)
>           darwin_filelist=`find unfat-$$ -type f | xargs basename | sort -u | 
> $NL2SP`


> Index: tests/func_extract_archives.test
> ===================================================================
> RCS file: tests/func_extract_archives.test
> diff -N tests/func_extract_archives.test
> --- /dev/null 1 Jan 1970 00:00:00 -0000
> +++ tests/func_extract_archives.test 6 Dec 2004 22:44:50 -0000
> @@ -0,0 +1,83 @@
> +#! /bin/sh
> +# link.test - check that .lo files aren't made into programs.

Please delete this line or adjust.  :-)

> +
> +# Test script header.
> +need_prefix=no
> +if test -z "$srcdir"; then
> +  srcdir=`echo "$0" | sed 's%/[^/]*$%%'`
> +  test "$srcdir" = "$0" && srcdir=.
> +  test "${VERBOSE+set}" != "set" && VERBOSE=yes
> +fi
> +. $srcdir/defs || exit 1
> +case $host in 
> +*darwin*)
> +  makefatdarwin=yes
> +  ;;
> +  *)
> +  makefatdarwin=
> +  ;;
> +esac
> +rm -f foo.o bar.o libfoo.a
> +SED=${SED-sed}
> +Xsed="$SED -e s/^X//"
> +for afile in baz foobar foobaz
> +do
> +  if test -n "$makefatdarwin"; then
> +    echo "ppc $afile" > $afile.ppc.o
> +    echo "m68k $afile" > $afile.m68k.o
> +    echo "i386 $afile" > $afile.i386.o
> +    lipo -create -output $afile.o -arch ppc $afile.ppc.o -arch m68k \
> +      $afile.m68k.o -arch i386 $afile.i386.o
> +    rm -f $afile.*.o
> +    ar -q libfoo.a $afile.o
> +    rm -f $afile.o
> +  else
> +    echo "$afile" > $afile.o
> +    ar -q libfoo.a $afile.o
> +    rm -f $afile.o
> +  fi
> +done
> +for anum in 1 2 3 4 5 6 7 8 9 10 11 12
> +do
> +  if test -n "$makefatdarwin"; then
> +    echo "ppc foo $anum" > foo.ppc.o
> +    echo "m68k foo $anum" > foo.m68k.o
> +    echo "i386 foo $anum" > foo.i386.o
> +    lipo -create -output foo.o -arch ppc foo.ppc.o -arch m68k foo.m68k.o \
> +      -arch i386 foo.i386.o
> +    ar -q libfoo.a foo.o
> +    rm -f foo.o foo.*.o
> +    echo "ppc bar $anum" > bar.ppc.o
> +    echo "m68k bar $anum" > bar.m68k.o
> +    echo "i386 bar $anum" > bar.i386.o
> +    lipo -create -output bar.o -arch ppc bar.ppc.o -arch m68k bar.m68k.o \
> +      -arch i386 bar.i386.o
> +    ar -q libfoo.a bar.o
> +    rm -f bar.o bar.*.o
> +  else
> +    echo "foo $anum" > foo.o
> +    echo "bar $anum" > bar.o
> +    ar -q libfoo.a foo.o bar.o
> +  fi
> +done
> +test -d .libs || mkdir .libs
> +
> +eval "`$SED -n -e '/^# Shell function definitions:'$z'$/,/^# End of Shell 
> function definitions'$z'$/p' < $libtool`"
> +
> +show=echo
> +run=
> +mkdir=mkdir
> +rm="rm -"

Any reason you're not setting this to 'rm -f'?
I see warnings like this.
BTW, you could also set modename=libtool for nicer output.

> +mv=mv
> +echo=echo
> +AR=${AR-ar}
> +func_extract_archives ".libs/libfoo" "libfoo.a"
> +set -x
> +for anum in 1 2 3 4 5 6 7 8 9 10 11 12
> +do
> +  test -f ".libs/libfoo/libfoo.a/foo-$anum.o" || exit 1
> +  $EGREP -v "foo-$anum" ".libs/libfoo/libfoo.a/foo-$anum.o" || exit 1
> +  rm -f ".libs/libfoo/libfoo.a/foo-$anum.o"
> +done
> +rm -rf ".libs/libfoo"
> +exit 0

Feel free to commit after addressing these.

Regards,
Ralf




reply via email to

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