diff mbox

[v2,1/1] pkg-autotools: generic configure fix for powerpc64

Message ID d005920f1b9f8acd1d32da2c68281020f6d28c2c.1479698374.git.sam.bobroff@au1.ibm.com
State Superseded
Headers show

Commit Message

Sam Bobroff Nov. 21, 2016, 3:19 a.m. UTC
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.

Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com>
---

v2:

* Patch host packages as well as target ones. (Feedback on the GNU Make code to
  detect this is welcome.)
* Folded long line in script.
* Removed patch backup files.
* Updated diff to unified format.
* Changed $pkg variable to $srcdir.

 package/pkg-autotools.mk                   | 19 ++++++++++++
 support/scripts/fix-configure-powerpc64.sh | 47 ++++++++++++++++++++++++++++++
 2 files changed, 66 insertions(+)
 create mode 100755 support/scripts/fix-configure-powerpc64.sh

Comments

Arnout Vandecappelle Nov. 26, 2016, 1:04 p.m. UTC | #1
On 21-11-16 04:19, 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.
> 
> Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com>
> ---
> 
> v2:
> 
> * Patch host packages as well as target ones. (Feedback on the GNU Make code to
>   detect this is welcome.)
> * Folded long line in script.
> * Removed patch backup files.
> * Updated diff to unified format.
> * Changed $pkg variable to $srcdir.
> 
>  package/pkg-autotools.mk                   | 19 ++++++++++++
>  support/scripts/fix-configure-powerpc64.sh | 47 ++++++++++++++++++++++++++++++
>  2 files changed, 66 insertions(+)
>  create mode 100755 support/scripts/fix-configure-powerpc64.sh
> 
> diff --git a/package/pkg-autotools.mk b/package/pkg-autotools.mk
> index d1cdb89..ada72c8 100644
> --- a/package/pkg-autotools.mk
> +++ b/package/pkg-autotools.mk
> @@ -79,6 +79,15 @@ define LIBTOOL_PATCH_HOOK
>  endef
>  
>  #
> +# Hook to patch common issue with configure on powerpc64{,le} failing
> +# to detect shared library support:
> +#
> +define CONFIGURE_FIX_POWERPC64_HOOK
> +	@$(call MESSAGE,"Checking configure (powerpc64/powerpc64le)")
> +	support/scripts/fix-configure-powerpc64.sh $($(PKG)_DIR)
> +endef
> +
> +#
>  # Hook to gettextize the package if needed
>  #
>  define GETTEXTIZE_HOOK
> @@ -255,6 +264,16 @@ endif
>  
>  endif
>  
> +# Append a configure hook if:
> +# * this is a target package and the target architecture is powerpc64 or powerpc64le,
> +# * or this is a host package and the host architecture is powerpc64 or powerpc64le.
> +# Must be added after other pre-configure hooks that might regenerate the
> +# configure script and overwrite the changes made here.
> +ifneq ($(or $(and $(filter target,$(4)), $(filter powerpc64 powerpc64le,$(ARCH))),
> +	$(and $(filter host,$(4)), $(filter powerpc64 powerpc64le,$(HOSTARCH)))),)

 We try to consistently double-dollar everything except $(1), $(2) etc. in the
inner- macros.

 This condition would IMHO be easier to read as (untested):
ifneq ($$(filter powerpc64%,$$(if $$(filter target,$(4)),$$(ARCH),$$(HOSTARCH))),)

 But I'm OK with the version you had as well.

 So
Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

 Regards,
 Arnout

> +$(2)_PRE_CONFIGURE_HOOKS += CONFIGURE_FIX_POWERPC64_HOOK
> +endif
> +
>  #
>  # Build step. Only define it if not already defined by the package .mk
>  # file.
> diff --git a/support/scripts/fix-configure-powerpc64.sh b/support/scripts/fix-configure-powerpc64.sh
> new file mode 100755
> index 0000000..ad71042
> --- /dev/null
> +++ b/support/scripts/fix-configure-powerpc64.sh
> @@ -0,0 +1,47 @@
> +#!/bin/bash
> +
> +# This is a script to find, and correct, a problem with old versions of
> +# configure that affect powerpc64 and powerpc64le.
> +
> +# The issue causes configure to incorrectly determine that shared library
> +# support is not present in the linker. This causes the package to build a
> +# static library rather than a dynamic one and although the build will succeed,
> +# it may cause packages that link with the static library it to fail due to
> +# undefined symbols.
> +
> +# This script searches for files named 'configure' that appear to have this
> +# issue (by searching for a known bad pattern) and patching them.
> +
> +set -e
> +
> +if [ $# -ne 1 ]; then
> +	echo "Usage: $0 <package build directory>"
> +	exit 2
> +fi
> +
> +srcdir="$1"
> +files=$(cd "$srcdir" && find . -name configure \
> +-exec grep -qF 'Generated by GNU Autoconf' {} \; \
> +-exec grep -qF 'ppc*-*linux*|powerpc*-*linux*)' {} \; -print)
> +
> +# --ignore-whitespace is needed because some packages have included
> +# copies of configure scripts where tabs have been replaced with spaces.
> +for c in $files; do
> +	patch --ignore-whitespace "$srcdir"/"$c" <<'EOF'
> +--- a/configure	2016-11-16 15:31:46.097447271 +1100
> ++++ b/configure	2008-07-21 12:17:23.000000000 +1000
> +@@ -4433,7 +4433,10 @@
> +         x86_64-*linux*)
> +           LD="${LD-ld} -m elf_x86_64"
> +           ;;
> +-        ppc*-*linux*|powerpc*-*linux*)
> ++	  powerpcle-*linux*)
> ++	    LD="${LD-ld} -m elf64lppc"
> ++	    ;;
> ++	  powerpc-*linux*)
> +           LD="${LD-ld} -m elf64ppc"
> +           ;;
> +         s390*-*linux*)
> +EOF
> +done
> +
>
Sam Bobroff Nov. 28, 2016, 3:42 a.m. UTC | #2
On Sat, Nov 26, 2016 at 02:04:08PM +0100, Arnout Vandecappelle wrote:
> 
> 
> On 21-11-16 04:19, 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.
> > 
> > Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com>
> > ---
> > 
> > v2:
> > 
> > * Patch host packages as well as target ones. (Feedback on the GNU Make code to
> >   detect this is welcome.)
> > * Folded long line in script.
> > * Removed patch backup files.
> > * Updated diff to unified format.
> > * Changed $pkg variable to $srcdir.
> > 
> >  package/pkg-autotools.mk                   | 19 ++++++++++++
> >  support/scripts/fix-configure-powerpc64.sh | 47 ++++++++++++++++++++++++++++++
> >  2 files changed, 66 insertions(+)
> >  create mode 100755 support/scripts/fix-configure-powerpc64.sh
> > 
> > diff --git a/package/pkg-autotools.mk b/package/pkg-autotools.mk
> > index d1cdb89..ada72c8 100644
> > --- a/package/pkg-autotools.mk
> > +++ b/package/pkg-autotools.mk
> > @@ -79,6 +79,15 @@ define LIBTOOL_PATCH_HOOK
> >  endef
> >  
> >  #
> > +# Hook to patch common issue with configure on powerpc64{,le} failing
> > +# to detect shared library support:
> > +#
> > +define CONFIGURE_FIX_POWERPC64_HOOK
> > +	@$(call MESSAGE,"Checking configure (powerpc64/powerpc64le)")
> > +	support/scripts/fix-configure-powerpc64.sh $($(PKG)_DIR)
> > +endef
> > +
> > +#
> >  # Hook to gettextize the package if needed
> >  #
> >  define GETTEXTIZE_HOOK
> > @@ -255,6 +264,16 @@ endif
> >  
> >  endif
> >  
> > +# Append a configure hook if:
> > +# * this is a target package and the target architecture is powerpc64 or powerpc64le,
> > +# * or this is a host package and the host architecture is powerpc64 or powerpc64le.
> > +# Must be added after other pre-configure hooks that might regenerate the
> > +# configure script and overwrite the changes made here.
> > +ifneq ($(or $(and $(filter target,$(4)), $(filter powerpc64 powerpc64le,$(ARCH))),
> > +	$(and $(filter host,$(4)), $(filter powerpc64 powerpc64le,$(HOSTARCH)))),)
> 
>  We try to consistently double-dollar everything except $(1), $(2) etc. in the
> inner- macros.

OK.

>  This condition would IMHO be easier to read as (untested):
> ifneq ($$(filter powerpc64%,$$(if $$(filter target,$(4)),$$(ARCH),$$(HOSTARCH))),)

Nice, looks good to me. I'll test it and send a new version with these changes.

>  But I'm OK with the version you had as well.
> 
>  So
> Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> 
>  Regards,
>  Arnout

And thanks for the review :-)

> 
> > +$(2)_PRE_CONFIGURE_HOOKS += CONFIGURE_FIX_POWERPC64_HOOK
> > +endif
> > +
> >  #
> >  # Build step. Only define it if not already defined by the package .mk
> >  # file.
> > diff --git a/support/scripts/fix-configure-powerpc64.sh b/support/scripts/fix-configure-powerpc64.sh
> > new file mode 100755
> > index 0000000..ad71042
> > --- /dev/null
> > +++ b/support/scripts/fix-configure-powerpc64.sh
> > @@ -0,0 +1,47 @@
> > +#!/bin/bash
> > +
> > +# This is a script to find, and correct, a problem with old versions of
> > +# configure that affect powerpc64 and powerpc64le.
> > +
> > +# The issue causes configure to incorrectly determine that shared library
> > +# support is not present in the linker. This causes the package to build a
> > +# static library rather than a dynamic one and although the build will succeed,
> > +# it may cause packages that link with the static library it to fail due to
> > +# undefined symbols.
> > +
> > +# This script searches for files named 'configure' that appear to have this
> > +# issue (by searching for a known bad pattern) and patching them.
> > +
> > +set -e
> > +
> > +if [ $# -ne 1 ]; then
> > +	echo "Usage: $0 <package build directory>"
> > +	exit 2
> > +fi
> > +
> > +srcdir="$1"
> > +files=$(cd "$srcdir" && find . -name configure \
> > +-exec grep -qF 'Generated by GNU Autoconf' {} \; \
> > +-exec grep -qF 'ppc*-*linux*|powerpc*-*linux*)' {} \; -print)
> > +
> > +# --ignore-whitespace is needed because some packages have included
> > +# copies of configure scripts where tabs have been replaced with spaces.
> > +for c in $files; do
> > +	patch --ignore-whitespace "$srcdir"/"$c" <<'EOF'
> > +--- a/configure	2016-11-16 15:31:46.097447271 +1100
> > ++++ b/configure	2008-07-21 12:17:23.000000000 +1000
> > +@@ -4433,7 +4433,10 @@
> > +         x86_64-*linux*)
> > +           LD="${LD-ld} -m elf_x86_64"
> > +           ;;
> > +-        ppc*-*linux*|powerpc*-*linux*)
> > ++	  powerpcle-*linux*)
> > ++	    LD="${LD-ld} -m elf64lppc"
> > ++	    ;;
> > ++	  powerpc-*linux*)
> > +           LD="${LD-ld} -m elf64ppc"
> > +           ;;
> > +         s390*-*linux*)
> > +EOF
> > +done
> > +
> > 
> 
> -- 
> Arnout Vandecappelle                          arnout at mind be
> Senior Embedded Software Architect            +32-16-286500
> Essensium/Mind                                http://www.mind.be
> G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
> LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
> GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF
diff mbox

Patch

diff --git a/package/pkg-autotools.mk b/package/pkg-autotools.mk
index d1cdb89..ada72c8 100644
--- a/package/pkg-autotools.mk
+++ b/package/pkg-autotools.mk
@@ -79,6 +79,15 @@  define LIBTOOL_PATCH_HOOK
 endef
 
 #
+# Hook to patch common issue with configure on powerpc64{,le} failing
+# to detect shared library support:
+#
+define CONFIGURE_FIX_POWERPC64_HOOK
+	@$(call MESSAGE,"Checking configure (powerpc64/powerpc64le)")
+	support/scripts/fix-configure-powerpc64.sh $($(PKG)_DIR)
+endef
+
+#
 # Hook to gettextize the package if needed
 #
 define GETTEXTIZE_HOOK
@@ -255,6 +264,16 @@  endif
 
 endif
 
+# Append a configure hook if:
+# * this is a target package and the target architecture is powerpc64 or powerpc64le,
+# * or this is a host package and the host architecture is powerpc64 or powerpc64le.
+# Must be added after other pre-configure hooks that might regenerate the
+# configure script and overwrite the changes made here.
+ifneq ($(or $(and $(filter target,$(4)), $(filter powerpc64 powerpc64le,$(ARCH))),
+	$(and $(filter host,$(4)), $(filter powerpc64 powerpc64le,$(HOSTARCH)))),)
+$(2)_PRE_CONFIGURE_HOOKS += CONFIGURE_FIX_POWERPC64_HOOK
+endif
+
 #
 # Build step. Only define it if not already defined by the package .mk
 # file.
diff --git a/support/scripts/fix-configure-powerpc64.sh b/support/scripts/fix-configure-powerpc64.sh
new file mode 100755
index 0000000..ad71042
--- /dev/null
+++ b/support/scripts/fix-configure-powerpc64.sh
@@ -0,0 +1,47 @@ 
+#!/bin/bash
+
+# This is a script to find, and correct, a problem with old versions of
+# configure that affect powerpc64 and powerpc64le.
+
+# The issue causes configure to incorrectly determine that shared library
+# support is not present in the linker. This causes the package to build a
+# static library rather than a dynamic one and although the build will succeed,
+# it may cause packages that link with the static library it to fail due to
+# undefined symbols.
+
+# This script searches for files named 'configure' that appear to have this
+# issue (by searching for a known bad pattern) and patching them.
+
+set -e
+
+if [ $# -ne 1 ]; then
+	echo "Usage: $0 <package build directory>"
+	exit 2
+fi
+
+srcdir="$1"
+files=$(cd "$srcdir" && find . -name configure \
+-exec grep -qF 'Generated by GNU Autoconf' {} \; \
+-exec grep -qF 'ppc*-*linux*|powerpc*-*linux*)' {} \; -print)
+
+# --ignore-whitespace is needed because some packages have included
+# copies of configure scripts where tabs have been replaced with spaces.
+for c in $files; do
+	patch --ignore-whitespace "$srcdir"/"$c" <<'EOF'
+--- a/configure	2016-11-16 15:31:46.097447271 +1100
++++ b/configure	2008-07-21 12:17:23.000000000 +1000
+@@ -4433,7 +4433,10 @@
+         x86_64-*linux*)
+           LD="${LD-ld} -m elf_x86_64"
+           ;;
+-        ppc*-*linux*|powerpc*-*linux*)
++	  powerpcle-*linux*)
++	    LD="${LD-ld} -m elf64lppc"
++	    ;;
++	  powerpc-*linux*)
+           LD="${LD-ld} -m elf64ppc"
+           ;;
+         s390*-*linux*)
+EOF
+done
+