From mboxrd@z Thu Jan  1 00:00:00 1970
From: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Date: Thu, 17 Nov 2016 11:05:29 +0100
Subject: [Buildroot] [RFC PATCH 1/1] pkg-autotools: generic configure
 fix for powerpc64
In-Reply-To: <56063c394c473bf4292658511bae0dd99583be07.1479356115.git.sam.bobroff@au1.ibm.com>
References: <56063c394c473bf4292658511bae0dd99583be07.1479356115.git.sam.bobroff@au1.ibm.com>
Message-ID: <20161117110529.55c3578e@free-electrons.com>
List-Id: <buildroot.busybox.net>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: buildroot@busybox.net

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@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"

> +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?

> +suffix=.powerpc64.orig

Do we really want to keep the original files?


> +*** 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).

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
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com