All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH 1/1] sdl: fix building on powerpc64 and powerpc64le
@ 2016-11-07  3:29 Sam Bobroff
  2016-11-07 21:51 ` Thomas Petazzoni
  0 siblings, 1 reply; 14+ messages in thread
From: Sam Bobroff @ 2016-11-07  3:29 UTC (permalink / raw)
  To: buildroot

Fixes
http://autobuild.buildroot.net/results/70659eead71faa82ccfd0016d04caed134707c24

(The problem was detected when building chocolate-doom but the issue
is in sdl.)

Old autotools included with SDL fails to detect dynamic linker support
on powerpc64 and powerpc64le.

See SDL bug 3481: https://bugzilla.libsdl.org/show_bug.cgi?id=3481

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

 package/sdl/0003-fix-configure-powerpc64.patch | 30 ++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)
 create mode 100644 package/sdl/0003-fix-configure-powerpc64.patch

diff --git a/package/sdl/0003-fix-configure-powerpc64.patch b/package/sdl/0003-fix-configure-powerpc64.patch
new file mode 100644
index 0000000..764a947
--- /dev/null
+++ b/package/sdl/0003-fix-configure-powerpc64.patch
@@ -0,0 +1,30 @@
+sdl: Add powerpc64 and powerpc64le support to old autotools.
+
+Fixes build on powerpc64le which doesn't fail but produces a static library
+rather than a dynamic one (which causes link errors in some packages using
+libsdl).
+
+Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com>
+
+*** a/acinclude/libtool.m4	2016-11-07 14:04:47.444117880 +1100
+--- b/acinclude/libtool.m4	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"
+  	    ;;
+! 	  powerpc64le-*linux*)
+! 	    LD="${LD-ld} -m elf64lppc"
+! 	    ;;
+! 	  powerpc64-*linux*)
+  	    LD="${LD-ld} -m elf64ppc"
+  	    ;;
+  	  s390*-*linux*|s390*-*tpf*)
-- 
2.10.0.297.gf6727b0

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Buildroot] [PATCH 1/1] sdl: fix building on powerpc64 and powerpc64le
  2016-11-07  3:29 [Buildroot] [PATCH 1/1] sdl: fix building on powerpc64 and powerpc64le Sam Bobroff
@ 2016-11-07 21:51 ` Thomas Petazzoni
  2016-11-08  1:04   ` Arnout Vandecappelle
  2016-11-08  3:44   ` Sam Bobroff
  0 siblings, 2 replies; 14+ messages in thread
From: Thomas Petazzoni @ 2016-11-07 21:51 UTC (permalink / raw)
  To: buildroot

Hello,

On Mon,  7 Nov 2016 14:29:44 +1100, Sam Bobroff wrote:
> Fixes
> http://autobuild.buildroot.net/results/70659eead71faa82ccfd0016d04caed134707c24
> 
> (The problem was detected when building chocolate-doom but the issue
> is in sdl.)
> 
> Old autotools included with SDL fails to detect dynamic linker support
> on powerpc64 and powerpc64le.
> 
> See SDL bug 3481: https://bugzilla.libsdl.org/show_bug.cgi?id=3481
> 
> Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com>

Thanks, but I have some comments, see below.

> ---
> 
>  package/sdl/0003-fix-configure-powerpc64.patch | 30 ++++++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
>  create mode 100644 package/sdl/0003-fix-configure-powerpc64.patch
> 
> diff --git a/package/sdl/0003-fix-configure-powerpc64.patch b/package/sdl/0003-fix-configure-powerpc64.patch
> new file mode 100644
> index 0000000..764a947
> --- /dev/null
> +++ b/package/sdl/0003-fix-configure-powerpc64.patch
> @@ -0,0 +1,30 @@
> +sdl: Add powerpc64 and powerpc64le support to old autotools.
> +
> +Fixes build on powerpc64le which doesn't fail but produces a static library
> +rather than a dynamic one (which causes link errors in some packages using
> +libsdl).
> +
> +Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com>
> +
> +*** a/acinclude/libtool.m4	2016-11-07 14:04:47.444117880 +1100
> +--- b/acinclude/libtool.m4	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"
> +  	    ;;
> +! 	  powerpc64le-*linux*)
> +! 	    LD="${LD-ld} -m elf64lppc"
> +! 	    ;;
> +! 	  powerpc64-*linux*)
> +  	    LD="${LD-ld} -m elf64ppc"
> +  	    ;;
> +  	  s390*-*linux*|s390*-*tpf*)

First, we want all patches formatted as unified diffs, so please use
this format when generating patches. Suggestion: for packages that
don't use Git upstream, use quilt to generate your patches.

Second, this change doesn't make the code look like what libtool.m4 has
in libtool upstream. It has:

          powerpc64le-*linux*)
            LD="${LD-ld} -m elf32lppclinux"
            ;;
          powerpc64-*linux*)
            LD="${LD-ld} -m elf32ppclinux"
            ;;
	[...]
          powerpcle-*linux*)
            LD="${LD-ld} -m elf64lppc"
            ;;

which is not the same as what your patch adds. Why do we have this
difference?

Also, are we going to need to patch libtool.m4 in each and every
package around? libtool.m4 from sdl is from libtool 2.2 which is not
_that_ old (by the standards of libtool upgrade speed, of course), so
we're likely to find many other packages in the same situation, aren't
we?

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Buildroot] [PATCH 1/1] sdl: fix building on powerpc64 and powerpc64le
  2016-11-07 21:51 ` Thomas Petazzoni
@ 2016-11-08  1:04   ` Arnout Vandecappelle
  2016-11-08  5:10     ` Sam Bobroff
  2016-11-08  3:44   ` Sam Bobroff
  1 sibling, 1 reply; 14+ messages in thread
From: Arnout Vandecappelle @ 2016-11-08  1:04 UTC (permalink / raw)
  To: buildroot



On 07-11-16 22:51, Thomas Petazzoni wrote:
> Also, are we going to need to patch libtool.m4 in each and every
> package around? libtool.m4 from sdl is from libtool 2.2 which is not
> _that_ old (by the standards of libtool upgrade speed, of course), so
> we're likely to find many other packages in the same situation, aren't
> we?

 Let me rephrase this.

 You should actually update support/libtool/buildroot-libtool-v1.5.patch and
support/libtool/buildroot-libtool-v2.2.patch which will fix all packages in one
fell swoop. (That is, assuming that v2.4 is OK already.)

 Regards,
 Arnout


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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Buildroot] [PATCH 1/1] sdl: fix building on powerpc64 and powerpc64le
  2016-11-07 21:51 ` Thomas Petazzoni
  2016-11-08  1:04   ` Arnout Vandecappelle
@ 2016-11-08  3:44   ` Sam Bobroff
  1 sibling, 0 replies; 14+ messages in thread
From: Sam Bobroff @ 2016-11-08  3:44 UTC (permalink / raw)
  To: buildroot

On Mon, Nov 07, 2016 at 10:51:06PM +0100, Thomas Petazzoni wrote:
> Hello,
> 
> On Mon,  7 Nov 2016 14:29:44 +1100, Sam Bobroff wrote:
> > Fixes
> > http://autobuild.buildroot.net/results/70659eead71faa82ccfd0016d04caed134707c24
> > 
> > (The problem was detected when building chocolate-doom but the issue
> > is in sdl.)
> > 
> > Old autotools included with SDL fails to detect dynamic linker support
> > on powerpc64 and powerpc64le.
> > 
> > See SDL bug 3481: https://bugzilla.libsdl.org/show_bug.cgi?id=3481
> > 
> > Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com>
> 
> Thanks, but I have some comments, see below.
> 
> > ---
> > 
> >  package/sdl/0003-fix-configure-powerpc64.patch | 30 ++++++++++++++++++++++++++
> >  1 file changed, 30 insertions(+)
> >  create mode 100644 package/sdl/0003-fix-configure-powerpc64.patch
> > 
> > diff --git a/package/sdl/0003-fix-configure-powerpc64.patch b/package/sdl/0003-fix-configure-powerpc64.patch
> > new file mode 100644
> > index 0000000..764a947
> > --- /dev/null
> > +++ b/package/sdl/0003-fix-configure-powerpc64.patch
> > @@ -0,0 +1,30 @@
> > +sdl: Add powerpc64 and powerpc64le support to old autotools.
> > +
> > +Fixes build on powerpc64le which doesn't fail but produces a static library
> > +rather than a dynamic one (which causes link errors in some packages using
> > +libsdl).
> > +
> > +Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com>
> > +
> > +*** a/acinclude/libtool.m4	2016-11-07 14:04:47.444117880 +1100
> > +--- b/acinclude/libtool.m4	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"
> > +  	    ;;
> > +! 	  powerpc64le-*linux*)
> > +! 	    LD="${LD-ld} -m elf64lppc"
> > +! 	    ;;
> > +! 	  powerpc64-*linux*)
> > +  	    LD="${LD-ld} -m elf64ppc"
> > +  	    ;;
> > +  	  s390*-*linux*|s390*-*tpf*)
> 
> First, we want all patches formatted as unified diffs, so please use
> this format when generating patches. Suggestion: for packages that
> don't use Git upstream, use quilt to generate your patches.
> 
> Second, this change doesn't make the code look like what libtool.m4 has
> in libtool upstream. It has:
> 
>           powerpc64le-*linux*)
>             LD="${LD-ld} -m elf32lppclinux"
>             ;;
>           powerpc64-*linux*)
>             LD="${LD-ld} -m elf32ppclinux"
>             ;;
> 	[...]
>           powerpcle-*linux*)
>             LD="${LD-ld} -m elf64lppc"
>             ;;
> 
> which is not the same as what your patch adds. Why do we have this
> difference?

Ah, because I had an incomplete understanding of the issue!

I'll restate the bug that needs to be fixed, just to be clear:

Unpatched, the old configure will create a binary and check it to see if
it's 32 bit or 64 bit. In this case it's 64 bit, and the toolchain spec
(which includes "powerpc64le") is matched against
"ppc*-*linux*|powerpc*-*linux*", which matches, and the linker is called
with "-m elf64ppc", which fails because that's big endian on a little
endian toolchain. Configure interprets this to mean that dynamic linking
isn't supported. (This isn't apparent with some distro toolchains, I
think because because they support BE and LE in the same toolchain, so
the wrong option is passed during this test but it works anyway, hiding
the problem.)

My patch added matches that would select the correct linker option but,
as you point out, that's not what was done upstream. What I didn't know
was that we should instead be omitting the emulation mode flag all
together unless we need to build 32 bit on 64 or the the other way
round. We can apparently rely on the toolchain doing the correct thing
without an emultion mode being specified, as long as it's producing the
desired 32 or 64 bit binaries by default.

So let's do it the upstream way. I'm not convinced that it will
work on every possible toolchain configuration but if autotools are
happy then it's fine with me. It certainly works for my tests with
buildroot. I'll either post a v2 or incorporate it into some more
generic fix if that's possible.

> Also, are we going to need to patch libtool.m4 in each and every
> package around? libtool.m4 from sdl is from libtool 2.2 which is not
> _that_ old (by the standards of libtool upgrade speed, of course), so
> we're likely to find many other packages in the same situation, aren't
> we?
> 
> Thanks,
> 
> Thomas
> -- 
> Thomas Petazzoni, CTO, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Buildroot] [PATCH 1/1] sdl: fix building on powerpc64 and powerpc64le
  2016-11-08  1:04   ` Arnout Vandecappelle
@ 2016-11-08  5:10     ` Sam Bobroff
  2016-11-08 12:24       ` Arnout Vandecappelle
  0 siblings, 1 reply; 14+ messages in thread
From: Sam Bobroff @ 2016-11-08  5:10 UTC (permalink / raw)
  To: buildroot

On Tue, Nov 08, 2016 at 02:04:31AM +0100, Arnout Vandecappelle wrote:
> 
> 
> On 07-11-16 22:51, Thomas Petazzoni wrote:
> > Also, are we going to need to patch libtool.m4 in each and every
> > package around? libtool.m4 from sdl is from libtool 2.2 which is not
> > _that_ old (by the standards of libtool upgrade speed, of course), so
> > we're likely to find many other packages in the same situation, aren't
> > we?

From the libtool git history it looks like the issue is fixed after
libtool 2.4.2.418.

>  Let me rephrase this.
> 
>  You should actually update support/libtool/buildroot-libtool-v1.5.patch and
> support/libtool/buildroot-libtool-v2.2.patch which will fix all packages in one
> fell swoop. (That is, assuming that v2.4 is OK already.)

That sounds great, but that system seems to be set up to patch only
ltmain.sh, and the code that needs patching isn't there.

The change needs to be in aclocal/libtool.m4  but I can't patch that and
then propagate the change to configure, because AUTORECONF doesn't work
for that package. (Maybe it would work for other packages, but I suspect
there would be problems updating a single file on it's own.)

I suppose I could try auto-patching configure and/or configure.in (or
.ac) along the same lines as ltmain.sh... what do you think?

Cheers,
Sam.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Buildroot] [PATCH 1/1] sdl: fix building on powerpc64 and powerpc64le
  2016-11-08  5:10     ` Sam Bobroff
@ 2016-11-08 12:24       ` Arnout Vandecappelle
  2016-11-09  4:30         ` Sam Bobroff
  0 siblings, 1 reply; 14+ messages in thread
From: Arnout Vandecappelle @ 2016-11-08 12:24 UTC (permalink / raw)
  To: buildroot



On 08-11-16 06:10, Sam Bobroff wrote:
> On Tue, Nov 08, 2016 at 02:04:31AM +0100, Arnout Vandecappelle wrote:
>>
>>
>> On 07-11-16 22:51, Thomas Petazzoni wrote:
>>> Also, are we going to need to patch libtool.m4 in each and every
>>> package around? libtool.m4 from sdl is from libtool 2.2 which is not
>>> _that_ old (by the standards of libtool upgrade speed, of course), so
>>> we're likely to find many other packages in the same situation, aren't
>>> we?
> 
> From the libtool git history it looks like the issue is fixed after
> libtool 2.4.2.418.
> 
>>  Let me rephrase this.
>>
>>  You should actually update support/libtool/buildroot-libtool-v1.5.patch and
>> support/libtool/buildroot-libtool-v2.2.patch which will fix all packages in one
>> fell swoop. (That is, assuming that v2.4 is OK already.)
> 
> That sounds great, but that system seems to be set up to patch only
> ltmain.sh, and the code that needs patching isn't there.
> 
> The change needs to be in aclocal/libtool.m4 

 Arg, my bad. I didn't look at the patch itself :-)

> but I can't patch that and
> then propagate the change to configure, because AUTORECONF doesn't work
> for that package. (Maybe it would work for other packages, but I suspect
> there would be problems updating a single file on it's own.)

 For most packages, AUTORECONF works. But it's really not nice to have to
autoreconf a large number of packages just because one platform which many
people don't use is not supported...


> I suppose I could try auto-patching configure and/or configure.in (or
> .ac) along the same lines as ltmain.sh... what do you think?

 Autopatching configure.in/ac will not work because it's actually libtool.m4,
right? You could autopatch that, but some packages will not bundly libtool.m4 I
guess.

 Autopatching configure is going to be very hard I expect, because the location
and context of the hunk to be patched is probably very volatile. But if it does
work, it's probably the way to go. You would also have to patch libtool.m4 if it
exists, to keep things consistent. And you'd probably want to ignore errors from
patch.

 Otherwise, I'm afraid I don't see how we can solve this in a generic way...

 Regards,
 Arnout

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Buildroot] [PATCH 1/1] sdl: fix building on powerpc64 and powerpc64le
  2016-11-08 12:24       ` Arnout Vandecappelle
@ 2016-11-09  4:30         ` Sam Bobroff
  2016-11-13 17:13           ` Bernd Kuhls
  0 siblings, 1 reply; 14+ messages in thread
From: Sam Bobroff @ 2016-11-09  4:30 UTC (permalink / raw)
  To: buildroot

On Tue, Nov 08, 2016 at 01:24:04PM +0100, Arnout Vandecappelle wrote:
> 
> 
> On 08-11-16 06:10, Sam Bobroff wrote:
> > On Tue, Nov 08, 2016 at 02:04:31AM +0100, Arnout Vandecappelle wrote:
> >>
> >>
> >> On 07-11-16 22:51, Thomas Petazzoni wrote:
> >>> Also, are we going to need to patch libtool.m4 in each and every
> >>> package around? libtool.m4 from sdl is from libtool 2.2 which is not
> >>> _that_ old (by the standards of libtool upgrade speed, of course), so
> >>> we're likely to find many other packages in the same situation, aren't
> >>> we?
> > 
> > From the libtool git history it looks like the issue is fixed after
> > libtool 2.4.2.418.
> > 
> >>  Let me rephrase this.
> >>
> >>  You should actually update support/libtool/buildroot-libtool-v1.5.patch and
> >> support/libtool/buildroot-libtool-v2.2.patch which will fix all packages in one
> >> fell swoop. (That is, assuming that v2.4 is OK already.)
> > 
> > That sounds great, but that system seems to be set up to patch only
> > ltmain.sh, and the code that needs patching isn't there.
> > 
> > The change needs to be in aclocal/libtool.m4 
> 
>  Arg, my bad. I didn't look at the patch itself :-)
> 
> > but I can't patch that and
> > then propagate the change to configure, because AUTORECONF doesn't work
> > for that package. (Maybe it would work for other packages, but I suspect
> > there would be problems updating a single file on it's own.)
> 
>  For most packages, AUTORECONF works. But it's really not nice to have to
> autoreconf a large number of packages just because one platform which many
> people don't use is not supported...

Agreed.

> > I suppose I could try auto-patching configure and/or configure.in (or
> > .ac) along the same lines as ltmain.sh... what do you think?
> 
>  Autopatching configure.in/ac will not work because it's actually libtool.m4,
> right? You could autopatch that, but some packages will not bundly libtool.m4 I
> guess.

Yes, and I've discovered that if you patch libtool.m4, some packages
build systems detect that and launch their own re-autoconf, which
crashes.

It looks like any fix will have to be directly to configure after any
autoreconfig is finished :-(

>  Autopatching configure is going to be very hard I expect, because the location
> and context of the hunk to be patched is probably very volatile. But if it does
> work, it's probably the way to go. You would also have to patch libtool.m4 if it
> exists, to keep things consistent. And you'd probably want to ignore errors from
> patch.
> 
>  Otherwise, I'm afraid I don't see how we can solve this in a generic way...

Right, but it seems that although the hunk moves around a lot it's content
is still a large static chunk that's easily found. Since patch is happy
to patch anywhere (if all the context matches), it looks like patch will
handle that OK. Of course it will always fail on configure scripts that
are already new enough or have already been fixed manually (and it looks
like some packages have done that). And I never like ignoring errors.

I can think of a few ways to implement this in buildroot:

(a) Patch pkg-autotools.mk to find any files named 'configure' and try
to patch them, ignoring failures.

(b) As above but only patch files that match a magic pattern that
indicates they need patching (there does happen to be such a pattern in
this case).

(c) As above but only patch packages that set a package configuration
variable (something like FOO_PATCH_CONFIGURE=YES).

For (b) and (c), I could hard-code it to run a single patch (from
support/libtool/... or support/configure?) or I could make it a bit more
generic.

For (b) we could have a pattern file and a patch file, applying the
patch if configure contains the pattern, or for (c) the variable could
contain the patch file name (relative to support/something/?).  For
example, have FOO_PATCH_CONFIGURE=powerpc64.patch. I'm not
aware of any other need for this at the moment but it also seems nice to
keep the arch specific stuff somewhat separated out.

For (c) We (me) would need to tag every package that needed patching,
but that doesn't seem too bad if it's just one line. The package files
would also be a good record of which packages still need fixing.

How do those options sound? Any other suggestions?

Cheers,
Sam.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Buildroot] [PATCH 1/1] sdl: fix building on powerpc64 and powerpc64le
  2016-11-09  4:30         ` Sam Bobroff
@ 2016-11-13 17:13           ` Bernd Kuhls
  2016-11-13 20:18             ` Arnout Vandecappelle
  0 siblings, 1 reply; 14+ messages in thread
From: Bernd Kuhls @ 2016-11-13 17:13 UTC (permalink / raw)
  To: buildroot

Am Wed, 09 Nov 2016 15:30:21 +1100 schrieb Sam Bobroff:

> How do those options sound? Any other suggestions?

Hi,

while investigating the build errors of the minidlna package
http://autobuild.buildroot.net/?reason=minidlna-1.1.5&arch=powerpc64le
I came across this discussion.

The root cause of the build error is the fact that both libexif (last 
release dates 2012-07-12) and libid3tag (last release dates 2004-02-18) 
contain a .m4 file which does not detect shared libs support because of 
the elf64ppc/elf64lppc problem. Autoreconf'ing libexif has its own 
problems wrt $MKINSTALLDIRS so I would like to avoid it.

What about doing something similar Debian did back in 2014?
https://lists.debian.org/debian-powerpc/2014/09/msg00041.html
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=760395

Regards, Bernd

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Buildroot] [PATCH 1/1] sdl: fix building on powerpc64 and powerpc64le
  2016-11-13 17:13           ` Bernd Kuhls
@ 2016-11-13 20:18             ` Arnout Vandecappelle
  2016-11-16 11:33               ` Thomas Petazzoni
  0 siblings, 1 reply; 14+ messages in thread
From: Arnout Vandecappelle @ 2016-11-13 20:18 UTC (permalink / raw)
  To: buildroot



On 13-11-16 18:13, Bernd Kuhls wrote:
> Am Wed, 09 Nov 2016 15:30:21 +1100 schrieb Sam Bobroff:
> 
>> How do those options sound? Any other suggestions?
> 
> Hi,
> 
> while investigating the build errors of the minidlna package
> http://autobuild.buildroot.net/?reason=minidlna-1.1.5&arch=powerpc64le
> I came across this discussion.
> 
> The root cause of the build error is the fact that both libexif (last 
> release dates 2012-07-12) and libid3tag (last release dates 2004-02-18) 
> contain a .m4 file which does not detect shared libs support because of 
> the elf64ppc/elf64lppc problem. Autoreconf'ing libexif has its own 
> problems wrt $MKINSTALLDIRS so I would like to avoid it.
> 
> What about doing something similar Debian did back in 2014?
> https://lists.debian.org/debian-powerpc/2014/09/msg00041.html
> https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=760395

 Nice idea, but it's only possible for internal toolchains. Unless if we merge
the ld-wrapper patches (last news of which was: it shouldn't be needed).

 I'm slightly inclined to go for patching configure or libtool.m4 instead.

 Regards,
 Arnout


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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Buildroot] [PATCH 1/1] sdl: fix building on powerpc64 and powerpc64le
  2016-11-13 20:18             ` Arnout Vandecappelle
@ 2016-11-16 11:33               ` Thomas Petazzoni
  2016-11-16 22:42                 ` Arnout Vandecappelle
  2016-11-16 22:49                 ` Sam Bobroff
  0 siblings, 2 replies; 14+ messages in thread
From: Thomas Petazzoni @ 2016-11-16 11:33 UTC (permalink / raw)
  To: buildroot

Hello,

On Sun, 13 Nov 2016 21:18:46 +0100, Arnout Vandecappelle wrote:

>  Nice idea, but it's only possible for internal toolchains. Unless if we merge
> the ld-wrapper patches (last news of which was: it shouldn't be needed).
> 
>  I'm slightly inclined to go for patching configure or libtool.m4 instead.

Can we move forward with this topic? Could somone summarize the problem
and options we have with advantages/drawbacks?

We've got quite a lot of build failures because of that. In the latest
autobuild report, we've got 7 build failures on powerpc64le (on a total
of 41 build failures), and I suspect a significant number of them are
due to this libtool issue.

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Buildroot] [PATCH 1/1] sdl: fix building on powerpc64 and powerpc64le
  2016-11-16 11:33               ` Thomas Petazzoni
@ 2016-11-16 22:42                 ` Arnout Vandecappelle
  2016-11-18  0:22                   ` Sam Bobroff
  2016-11-16 22:49                 ` Sam Bobroff
  1 sibling, 1 reply; 14+ messages in thread
From: Arnout Vandecappelle @ 2016-11-16 22:42 UTC (permalink / raw)
  To: buildroot



On 16-11-16 12:33, Thomas Petazzoni wrote:
> Hello,
> 
> On Sun, 13 Nov 2016 21:18:46 +0100, Arnout Vandecappelle wrote:
> 
>>  Nice idea, but it's only possible for internal toolchains. Unless if we merge
>> the ld-wrapper patches (last news of which was: it shouldn't be needed).
>>
>>  I'm slightly inclined to go for patching configure or libtool.m4 instead.
> 
> Can we move forward with this topic? Could somone summarize the problem
> and options we have with advantages/drawbacks?

 That I can.

 Support for powerpc64le is a relatively recent addition to the libtool autoconf
macro. This macro is used in the configure script to detect shared library
support in the toolchain. The wrong version will do 'ld -m elf64ppc', which is
not a supported emulation for powerpc64le (it should be elf64lppc, notice the
extra l).

 The possible solutions:

1. Patch each package individually. This is a lot of work. Debian [1] reported a
few hundred packages that were affected.

2. In autotools-package, patch the configure script. The lines to change can be
anywhere in the configure script so the patch will always have a (rather large)
offset, but a few samples indicate that the context doesn't change so it applies
pretty easily. Care has to be taken however that a configure script that is
already correct is not patched, and also a configure script that doesn't contain
libtool.m4 at all shouldn't be patched.

2b. Like 2), but patch libtool.m4 instead of configure when we're doing an
autoreconf. libtool.m4 is safer to patch - the ppc hunk is certainly there, so
we just have to check if it's already corrected and if not apply the patch. But
of course for packages which we don't autoreconf, we're in the situation of 2.

2c. Like 2b), but always autoreconf (for ppc64le of course). Problem is that
some packages don't autoreconf cleanly, and we probably still have a few
packages where we have patches for configure but not configure.ac. I think this
one is a no go.

2d. Like 2, 2b or 2c, but specify per package that this has to be done. That
eliminates the risks involved: we are sure that the patch has to be applied, and
we know that it can be applied. But it means that all packages have to be fixed
explicitly.

3. Make sure LD accepts -m elf64ppc (and interpret it as -m elflppc). Nobody
else is going to pass this option so it should be safe. This requires an ld
wrapper. It could be the generic ld wrapper [2], or it could be custom wrapper
script that only gets installed for ppc64le. The same issues as for the current
toolchain wrapper have to be taken into account (i.e. when used as an external
toolchain, the wrapper should be wrapped again).

3b. Patch our binutils to accept -m elf64ppc (using patch from [1]), and assume
an external toolchain has this patch applied. Since we don't have an existing
ppc64le external toolchain now, this is not too exotic of an assumption. But it
means maintaining an arch-specific patch which is never great.


 Now that I wrote everything down, it looks like option 3 as suggested by Bernd
is the simplest and cleanest. It keeps things rather localized (which is less
the case for the variants of 2, and also 3b means patches for the different
binutils versions). It could be done either in the INSTALL_WRAPPER loops, or
perhaps as a TOOLCHAIN_POST_INSTALL_TARGET hook.


 Regards,
 Arnout

[1] https://lists.debian.org/debian-powerpc/2014/09/msg00041.html
[2] http://patchwork.ozlabs.org/patch/606688/

> 
> We've got quite a lot of build failures because of that. In the latest
> autobuild report, we've got 7 build failures on powerpc64le (on a total
> of 41 build failures), and I suspect a significant number of them are
> due to this libtool issue.
> 
> Thanks,
> 
> Thomas
> 

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Buildroot] [PATCH 1/1] sdl: fix building on powerpc64 and powerpc64le
  2016-11-16 11:33               ` Thomas Petazzoni
  2016-11-16 22:42                 ` Arnout Vandecappelle
@ 2016-11-16 22:49                 ` Sam Bobroff
  2016-11-16 22:53                   ` Thomas Petazzoni
  1 sibling, 1 reply; 14+ messages in thread
From: Sam Bobroff @ 2016-11-16 22:49 UTC (permalink / raw)
  To: buildroot

On Wed, Nov 16, 2016 at 12:33:31PM +0100, Thomas Petazzoni wrote:
> Hello,
> 
> On Sun, 13 Nov 2016 21:18:46 +0100, Arnout Vandecappelle wrote:
> 
> >  Nice idea, but it's only possible for internal toolchains. Unless if we merge
> > the ld-wrapper patches (last news of which was: it shouldn't be needed).
> > 
> >  I'm slightly inclined to go for patching configure or libtool.m4 instead.
> 
> Can we move forward with this topic? Could somone summarize the problem
> and options we have with advantages/drawbacks?
> 
> We've got quite a lot of build failures because of that. In the latest
> autobuild report, we've got 7 build failures on powerpc64le (on a total
> of 41 build failures), and I suspect a significant number of them are
> due to this libtool issue.

Sorry I've been quiet but I've been working on this :-)

I've built and used scripts to scan about 1,000 packages, and it looks
like about 100 of them have this problem. I'll post an RFC patch shortly
and answer your questions, above.

However, I've also run into several failures that are different issues,
so this isn't going to fix them all.

Cheers,
Sam.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Buildroot] [PATCH 1/1] sdl: fix building on powerpc64 and powerpc64le
  2016-11-16 22:49                 ` Sam Bobroff
@ 2016-11-16 22:53                   ` Thomas Petazzoni
  0 siblings, 0 replies; 14+ messages in thread
From: Thomas Petazzoni @ 2016-11-16 22:53 UTC (permalink / raw)
  To: buildroot

Hello,

On Thu, 17 Nov 2016 09:49:50 +1100, Sam Bobroff wrote:

> > We've got quite a lot of build failures because of that. In the latest
> > autobuild report, we've got 7 build failures on powerpc64le (on a total
> > of 41 build failures), and I suspect a significant number of them are
> > due to this libtool issue.  
> 
> Sorry I've been quiet but I've been working on this :-)

Great!

> I've built and used scripts to scan about 1,000 packages, and it looks
> like about 100 of them have this problem. I'll post an RFC patch shortly
> and answer your questions, above.

OK. Arnout has also posted a great summary of the possible solutions.
It's great to have the info that about 10% of the packages seem to be
affected, this sort of number can help in choosing between the
different options we have.

> However, I've also run into several failures that are different issues,
> so this isn't going to fix them all.

Sure, that's expected and perfectly OK.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Buildroot] [PATCH 1/1] sdl: fix building on powerpc64 and powerpc64le
  2016-11-16 22:42                 ` Arnout Vandecappelle
@ 2016-11-18  0:22                   ` Sam Bobroff
  0 siblings, 0 replies; 14+ messages in thread
From: Sam Bobroff @ 2016-11-18  0:22 UTC (permalink / raw)
  To: buildroot

On Wed, Nov 16, 2016 at 11:42:28PM +0100, Arnout Vandecappelle wrote:
> 
> 
> On 16-11-16 12:33, Thomas Petazzoni wrote:
> > Hello,
> > 
> > On Sun, 13 Nov 2016 21:18:46 +0100, Arnout Vandecappelle wrote:
> > 
> >>  Nice idea, but it's only possible for internal toolchains. Unless if we merge
> >> the ld-wrapper patches (last news of which was: it shouldn't be needed).
> >>
> >>  I'm slightly inclined to go for patching configure or libtool.m4 instead.
> > 
> > Can we move forward with this topic? Could somone summarize the problem
> > and options we have with advantages/drawbacks?
> 
>  That I can.
> 
>  Support for powerpc64le is a relatively recent addition to the libtool autoconf
> macro. This macro is used in the configure script to detect shared library
> support in the toolchain. The wrong version will do 'ld -m elf64ppc', which is
> not a supported emulation for powerpc64le (it should be elf64lppc, notice the
> extra l).

This is perhaps slightly off topic but I'm not sure that "it should be
elf64lppc" is correct. I initially thought that it was, and I based a patch on
it but someone pointed out that it didn't match upstream. This is what the
test looks like upstream (from libtool.m4, which seems to be the source of the
block in question):

          powerpcle-*linux*)
            LD="${LD-ld} -m elf64lppc"
            ;;
          powerpc-*linux*)
            LD="${LD-ld} -m elf64ppc"
            ;;

And this will not match either powerpc64- or powerpc64le-. What happens in
practice is that neither case matches, the linker is run without an emulation
flag and the test passes. When I looked at the whole block:

    case `/usr/bin/file conftest.o` in
      *32-bit*)
        case $host in
...
          powerpc64le-*linux*)
            LD="${LD-ld} -m elf32lppclinux"
            ;;
          powerpc64-*linux*)
            LD="${LD-ld} -m elf32ppclinux"
            ;;
...
        esac
        ;;
      *64-bit*)
        case $host in
...
          powerpcle-*linux*)
            LD="${LD-ld} -m elf64lppc"
            ;;
          powerpc-*linux*)
            LD="${LD-ld} -m elf64ppc"
            ;;
...
        esac
        ;;
    esac

It looks to me like the code is designed to use an emulation flag only when the
word size is not the default. In that case, it passes the correct BE or LE
emulation mode, but if the word size is the default it doesn't pass anything.

I can see that it works for buildroot toolchains, which aren't biendian, but it
doesn't seem obvious that it will work for any toolchain.

Should we still use the upstream version? (My personal feeling is that we
should because it works for buildroot and I don't want to go any deeper down
this rabbit hole!)

>  The poussible solutions:
> 
> 1. Patch each package individually. This is a lot of work. Debian [1] reported a
> few hundred packages that were affected.

Yep. I've found 125 so far out of about 1,000.

> 2. In autotools-package, patch the configure script. The lines to change can be
> anywhere in the configure script so the patch will always have a (rather large)
> offset, but a few samples indicate that the context doesn't change so it applies
> pretty easily. Care has to be taken however that a configure script that is
> already correct is not patched, and also a configure script that doesn't contain
> libtool.m4 at all shouldn't be patched.

I did find a way to detect the broken configure scripts, luckily there is a
pattern and context to look for. So I've tried this in a patch (see the other
thread).

> 2b. Like 2), but patch libtool.m4 instead of configure when we're doing an
> autoreconf. libtool.m4 is safer to patch - the ppc hunk is certainly there, so
> we just have to check if it's already corrected and if not apply the patch. But
> of course for packages which we don't autoreconf, we're in the situation of 2.
> 
> 2c. Like 2b), but always autoreconf (for ppc64le of course). Problem is that
> some packages don't autoreconf cleanly, and we probably still have a few
> packages where we have patches for configure but not configure.ac. I think this
> one is a no go.
> 
> 2d. Like 2, 2b or 2c, but specify per package that this has to be done. That
> eliminates the risks involved: we are sure that the patch has to be applied, and
> we know that it can be applied. But it means that all packages have to be fixed
> explicitly.

I liked these solutions but I had trouble implementing them: some packages ship
a configure script but no libtool files from which to regenerate it, and at
least one ships the files (and uses Make to detect changes in them) but
the build crashes while trying to run autoconf.

> 3. Make sure LD accepts -m elf64ppc (and interpret it as -m elflppc). Nobody
> else is going to pass this option so it should be safe. This requires an ld
> wrapper. It could be the generic ld wrapper [2], or it could be custom wrapper
> script that only gets installed for ppc64le. The same issues as for the current
> toolchain wrapper have to be taken into account (i.e. when used as an external
> toolchain, the wrapper should be wrapped again).
> 
> 3b. Patch our binutils to accept -m elf64ppc (using patch from [1]), and assume
> an external toolchain has this patch applied. Since we don't have an existing
> ppc64le external toolchain now, this is not too exotic of an assumption. But it
> means maintaining an arch-specific patch which is never great.

This is really interesting.

>  Now that I wrote everything down, it looks like option 3 as suggested by Bernd
> is the simplest and cleanest. It keeps things rather localized (which is less
> the case for the variants of 2, and also 3b means patches for the different
> binutils versions). It could be done either in the INSTALL_WRAPPER loops, or
> perhaps as a TOOLCHAIN_POST_INSTALL_TARGET hook.
> 
> 
>  Regards,
>  Arnout
> 
> [1] https://lists.debian.org/debian-powerpc/2014/09/msg00041.html
> [2] http://patchwork.ozlabs.org/patch/606688/
> 
> > 
> > We've got quite a lot of build failures because of that. In the latest
> > autobuild report, we've got 7 build failures on powerpc64le (on a total
> > of 41 build failures), and I suspect a significant number of them are
> > due to this libtool issue.
> > 
> > Thanks,
> > 
> > Thomas
> > 
> 
> -- 
> 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

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2016-11-18  0:22 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-07  3:29 [Buildroot] [PATCH 1/1] sdl: fix building on powerpc64 and powerpc64le Sam Bobroff
2016-11-07 21:51 ` Thomas Petazzoni
2016-11-08  1:04   ` Arnout Vandecappelle
2016-11-08  5:10     ` Sam Bobroff
2016-11-08 12:24       ` Arnout Vandecappelle
2016-11-09  4:30         ` Sam Bobroff
2016-11-13 17:13           ` Bernd Kuhls
2016-11-13 20:18             ` Arnout Vandecappelle
2016-11-16 11:33               ` Thomas Petazzoni
2016-11-16 22:42                 ` Arnout Vandecappelle
2016-11-18  0:22                   ` Sam Bobroff
2016-11-16 22:49                 ` Sam Bobroff
2016-11-16 22:53                   ` Thomas Petazzoni
2016-11-08  3:44   ` Sam Bobroff

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.