[Buildroot] [RFC PATCH 1/1] pkg-autotools: generic configure fix for powerpc64

Sam Bobroff sam.bobroff at au1.ibm.com
Fri Nov 18 00:48:54 UTC 2016


On Thu, Nov 17, 2016 at 11:05:29AM +0100, Thomas Petazzoni wrote:
> Hello,
> 
> On Thu, 17 Nov 2016 15:15:23 +1100, Sam Bobroff wrote:
> > Many (100+) packages supported by buildroot contain old configure
> > scripts (or build them from old versions of autotools) that are unable
> > to determine how to link shared libraries on powerpc64 and
> > powerpc64le. This causes that test to erroneously fail on toolchains
> > that are not "bi-endian" (which is the case for toolchains built by
> > buildroot), which causes configure to build static libraries instead
> > of dynamic ones. Although these builds succeed, they tend to cause
> > linker failures in binaries later linked against them.
> > 
> > Because affected configure files can be discovered automatically, this
> > patch introduces a hook (enabled only when building for powerpc64 and
> > powerpc64le) that uses a script to scan and fix each package.
> > 
> > This applies only to packages built for the target.
> > 
> > Signed-off-by: Sam Bobroff <sam.bobroff at au1.ibm.com>
> 
> I think I like this approach. Yes, it looks a bit hackish, but we
> already patch libtool files in a fairly similar way, and this is
> actually fixing the real problem.
> 
> > +set -e
> > +
> > +if [ $# -ne 1 ]; then
> > +	echo "Usage: $0 <package build directory>"
> > +	exit 2
> > +fi
> > +
> > +pkg="$1"
> 
> It's not really pkg (which we normally use for the package name), but
> the package source directory. Maybe just:
> 
> srcdir="$1"

Good idea, will do.

> > +files=$(cd "$pkg" && find . -name configure -exec grep -qF 'Generated by GNU Autoconf' {} \; -exec grep -qF 'ppc*-*linux*|powerpc*-*linux*)' {} \; -print)
> 
> This line is a bit too long, can you split it? Also, why do you cd into
> the source directory, you can just as well do find ${srcdir}, right?

I'll split it. Doing the cd first means that the output in the build log
is more readable, since it's then relative to the srcdir (without it you
get the full absolute path). Which do you prefer?

> 
> > +suffix=.powerpc64.orig
> 
> Do we really want to keep the original files?

Well, no, but now that you mention it, what about this idea:

Use the patch backup file as a way to see if the patch has been applied
already and if it has been, skip patching.

At the moment, if you interrupt the configure process (which is quite
long) and then re-run the build, it will fail because the patching
fails.

> > +*** a/configure	2016-11-07 14:04:47.444117880 +1100
> > +--- b/configure	2016-11-07 14:05:03.652181547 +1100
> > +***************
> > +*** 1302,1308 ****
> > +  	  x86_64-*linux*)
> > +  	    LD="${LD-ld} -m elf_x86_64"
> > +  	    ;;
> > +! 	  ppc*-*linux*|powerpc*-*linux*)
> > +  	    LD="${LD-ld} -m elf64ppc"
> > +  	    ;;
> > +  	  s390*-*linux*|s390*-*tpf*)
> > +--- 1302,1311 ----
> > +  	  x86_64-*linux*)
> > +  	    LD="${LD-ld} -m elf_x86_64"
> > +  	    ;;
> > +! 	  powerpcle-*linux*)
> > +! 	    LD="${LD-ld} -m elf64lppc"
> > +! 	    ;;
> > +! 	  powerpc-*linux*)
> > +  	    LD="${LD-ld} -m elf64ppc"
> > +  	    ;;
> > +  	  s390*-*linux*|s390*-*tpf*)
> > +EOF
> 
> Is there any reason to use this diff format? You seem to be using it
> all the time. Please use unified diffs instead (diff -u).

Oops, will fix!

> I'm hesitating between this patch approach (with significant offsets)
> when applying, and an approach using a awk script. Here is an awk
> script that does the job:
> 
> /^\s*ppc\*-\*linux\*|powerpc\*-\*linux\*)$/ {
>     infix = 1;
>     print "         powerpcle-*linux*)";
>     print "           LD=\"${LD-ld} -m elf64lppc\"";
>     print "           ;;";
>     print "         powerpc-*linux*)";
>     print "           LD=\"${LD-ld} -m elf64ppc\"";
>     print "           ;;";
> }
> 
> /^\s*;;$/ {
>     if (infix) {
> 	infix = 0;
> 	next;
>     }
> }
> 
> // {
>     if (infix)
> 	next;
>     print;
> }
> 
> I'm not sure which of the two solutions is the most appropriate. The
> awk solution can probably be applied without doing the grep for the
> ppc*-*linux line, because the awk script only does the replacement if
> this line is found anyway.
> 
> Thomas

>From the other thread, it seems like you're happy to keep using patch.

I'll post a v2.

Cheers,
Sam.




More information about the buildroot mailing list