libpng 1.6.13: fix build for aarch64

Message ID 1411387952-22717-1-git-send-email-koen.kooi@linaro.org
State Accepted
Commit 100a46e19da87964d11e11d1af1e59c27a1d5241
Headers show

Commit Message

Koen Kooi Sept. 22, 2014, 12:12 p.m.
The configure override was too restrictive, it needed both 'arm' and 'neon' to trigger, which breaks on aarch64. Since TUNE_FEATURES is the only qualifier that matters, drop the 'arm' override.

Buildtested for 'genericarmv8' and 'qemux86' machines.

Signed-off-by: Koen Kooi <koen.kooi@linaro.org>
---
 meta/recipes-multimedia/libpng/libpng_1.6.13.bb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Koen Kooi Nov. 3, 2014, 9:57 a.m. | #1
ping

> Op 22 sep. 2014, om 14:12 heeft Koen Kooi <koen.kooi@linaro.org> het volgende geschreven:
> 
> The configure override was too restrictive, it needed both 'arm' and 'neon' to trigger, which breaks on aarch64. Since TUNE_FEATURES is the only qualifier that matters, drop the 'arm' override.
> 
> Buildtested for 'genericarmv8' and 'qemux86' machines.
> 
> Signed-off-by: Koen Kooi <koen.kooi@linaro.org>
> ---
> meta/recipes-multimedia/libpng/libpng_1.6.13.bb | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/meta/recipes-multimedia/libpng/libpng_1.6.13.bb b/meta/recipes-multimedia/libpng/libpng_1.6.13.bb
> index 0c6fd1f..8798a96 100644
> --- a/meta/recipes-multimedia/libpng/libpng_1.6.13.bb
> +++ b/meta/recipes-multimedia/libpng/libpng_1.6.13.bb
> @@ -19,7 +19,7 @@ BINCONFIG = "${bindir}/libpng-config ${bindir}/libpng16-config"
> inherit autotools binconfig-disabled pkgconfig
> 
> # Work around missing symbols
> -EXTRA_OECONF_append_arm = " ${@bb.utils.contains("TUNE_FEATURES", "neon", "--enable-arm-neon=on", "--enable-arm-neon=off" ,d)}"
> +EXTRA_OECONF_append = " ${@bb.utils.contains("TUNE_FEATURES", "neon", "--enable-arm-neon=on", "--enable-arm-neon=off" ,d)}"
> 
> PACKAGES =+ "${PN}-tools"
> 
> -- 
> 1.9.3
>
Ross Burton Nov. 3, 2014, 10:25 a.m. | #2
On 3 November 2014 09:57, Koen Kooi <koen@dominion.thruhere.net> wrote:

> ping
>

Last night I was looking at my "post-1.7" tag and the number of aarch64
patches there.   Kai, did you continue work on that staging branch for all
of the aarch64 patches that were being posted?

Ross
Koen Kooi Nov. 3, 2014, 11:27 a.m. | #3
> Op 3 nov. 2014, om 11:25 heeft Burton, Ross <ross.burton@intel.com> het volgende geschreven:
> 
> On 3 November 2014 09:57, Koen Kooi <koen@dominion.thruhere.net> wrote:
> ping
> 
> Last night I was looking at my "post-1.7" tag and the number of aarch64 patches there.   Kai, did you continue work on that staging branch for all of the aarch64 patches that were being posted?

What does that have to do with this patch?
Ross Burton Nov. 3, 2014, 11:30 a.m. | #4
On 3 November 2014 11:27, Koen Kooi <koen@dominion.thruhere.net> wrote:

> > Last night I was looking at my "post-1.7" tag and the number of aarch64
> patches there.   Kai, did you continue work on that staging branch for all
> of the aarch64 patches that were being posted?
>
> What does that have to do with this patch?


Kai was collecting patches that were sent to oe-core, and this may have
been one of them.

Ross
Koen Kooi Nov. 3, 2014, 12:06 p.m. | #5
> Op 3 nov. 2014, om 12:30 heeft Burton, Ross <ross.burton@intel.com> het volgende geschreven:
> 
> 
> On 3 November 2014 11:27, Koen Kooi <koen@dominion.thruhere.net> wrote:
> > Last night I was looking at my "post-1.7" tag and the number of aarch64 patches there.   Kai, did you continue work on that staging branch for all of the aarch64 patches that were being posted?
> 
> What does that have to do with this patch?
> 
> Kai was collecting patches that were sent to oe-core, and this may have been one of them.

Is this some new requirement for OE-core patches?
Richard Purdie Nov. 3, 2014, 12:10 p.m. | #6
On Mon, 2014-11-03 at 13:06 +0100, Koen Kooi wrote:
> > Op 3 nov. 2014, om 12:30 heeft Burton, Ross <ross.burton@intel.com> het volgende geschreven:
> > 
> > 
> > On 3 November 2014 11:27, Koen Kooi <koen@dominion.thruhere.net> wrote:
> > > Last night I was looking at my "post-1.7" tag and the number of aarch64 patches there.   Kai, did you continue work on that staging branch for all of the aarch64 patches that were being posted?
> > 
> > What does that have to do with this patch?
> > 
> > Kai was collecting patches that were sent to oe-core, and this may have been one of them.
> 
> Is this some new requirement for OE-core patches?

Its someone being helpful and trying to consolidate a topic branch and
test things before it gets merged.

So its not a new requirement but it is helpful and appreciated.

Cheers,

Richard
Koen Kooi Nov. 3, 2014, 3:24 p.m. | #7
Op 3 nov. 2014, om 13:10 heeft Richard Purdie <richard.purdie@linuxfoundation.org> het volgende geschreven:

> On Mon, 2014-11-03 at 13:06 +0100, Koen Kooi wrote:
>>> Op 3 nov. 2014, om 12:30 heeft Burton, Ross <ross.burton@intel.com> het volgende geschreven:
>>> 
>>> 
>>> On 3 November 2014 11:27, Koen Kooi <koen@dominion.thruhere.net> wrote:
>>>> Last night I was looking at my "post-1.7" tag and the number of aarch64 patches there.   Kai, did you continue work on that staging branch for all of the aarch64 patches that were being posted?
>>> 
>>> What does that have to do with this patch?
>>> 
>>> Kai was collecting patches that were sent to oe-core, and this may have been one of them.
>> 
>> Is this some new requirement for OE-core patches?
> 
> Its someone being helpful and trying to consolidate a topic branch and
> test things before it gets merged.
> 
> So its not a new requirement but it is helpful and appreciated.

It's been over a month and libpng is still broken, so I refuse to classify this new process as 'helpful' or as 'appreciated' at this point. 

If this continues can the person doing such a branch reply to patches intended for it saying they will or won't be queued? And please run the patches without such a reply through the regular process. That would be both helpful and appreciated.

--
Koen Kooi
Builds and Baselines | Release Manager
Linaro.org | Open source software for ARM SoCs
Ross Burton Nov. 3, 2014, 4:08 p.m. | #8
On 3 November 2014 15:24, Koen Kooi <koen.kooi@linaro.org> wrote:

> It's been over a month and libpng is still broken, so I refuse to classify
> this new process as 'helpful' or as 'appreciated' at this point.


Remember that 1.7 was frozen solid for several weeks of that month, so no
it wouldn't have been merged in that time period.  It's only just
re-opened: master is currently 80 patches ahead of dizzy, 35 in the C-Pull
I sent last night, and another 33 in my staging branch right now so it's
not like we're taking it easy right now.

Some context: during September it was obvious that various parties had an
interest in aarch64 (Linaro and Wind River).  Patches to fix build failures
were being sent sporadically, a qemuarm64 machine was submitted, but there
was no kernel for said machine to actually build.   At the time there was
no qemuarm64 machine that actually worked, so even merging the "simple"
fixes on just faith that they're correct would be reckless considering we
were freezing.  This is why I suggested[1] that someone with a vested
interest in aarch64 collate all the patches into a single branch and keep
it rebased to master so that when master opens, it can be tested on *all*
qemu architectures and merged.  It appears that this hasn't happened.  Will
Linaro be able to create a branch that incorporates all of the patches and
new machine that's been tested as a whole?

Ross

[1]
http://lists.openembedded.org/pipermail/openembedded-core/2014-September/097211.html
Richard Purdie Nov. 3, 2014, 4:08 p.m. | #9
On Mon, 2014-11-03 at 16:24 +0100, Koen Kooi wrote:
> Op 3 nov. 2014, om 13:10 heeft Richard Purdie <richard.purdie@linuxfoundation.org> het volgende geschreven:
> 
> > On Mon, 2014-11-03 at 13:06 +0100, Koen Kooi wrote:
> >>> Op 3 nov. 2014, om 12:30 heeft Burton, Ross <ross.burton@intel.com> het volgende geschreven:
> >>> 
> >>> 
> >>> On 3 November 2014 11:27, Koen Kooi <koen@dominion.thruhere.net> wrote:
> >>>> Last night I was looking at my "post-1.7" tag and the number of aarch64 patches there.   Kai, did you continue work on that staging branch for all of the aarch64 patches that were being posted?
> >>> 
> >>> What does that have to do with this patch?
> >>> 
> >>> Kai was collecting patches that were sent to oe-core, and this may have been one of them.
> >> 
> >> Is this some new requirement for OE-core patches?
> > 
> > Its someone being helpful and trying to consolidate a topic branch and
> > test things before it gets merged.
> > 
> > So its not a new requirement but it is helpful and appreciated.
> 
> It's been over a month and libpng is still broken, so I refuse to
> classify this new process as 'helpful' or as 'appreciated' at this
> point. 

Well, for the past month we've been freezing for and working toward the
release and it was agreed this batch of patches were being queued for
post release. I think that was quite clear.

As you well know, partly this is because we don't have a way to test any
of it, nor is it an officially supported target. If we get a consistent
set of code and a qemu machine that can test it, we'll be in a better
position and I believe that is something people are working towards (but
was way too late for the 1.7 cycle).

We're now in a position where we've started merging patches again, there
is a backlog being worked through and I'm sure this batch will be
presented soon. We are sticking with the stated plan which was to have a
queued branch of those patches, until such times as there is a better
plan or offer.

> If this continues can the person doing such a branch reply to patches
> intended for it saying they will or won't be queued? And please run
> the patches without such a reply through the regular process. That
> would be both helpful and appreciated.

It would be nice if the folks at Linaro would actually present things
like a branch containing theire changes and help with this work. As it
is, it seems others need to do this for them and instead they just want
to complain why we don't take patches (which often don't even seem to
build). I can understand not everyone at Linaro is going to test patches
on a couple of other arches but having someone there aggregating patches
and doing some acid tests might massively improve both their reputation
and increase the chances of patches merging in a timely fashion.

Cheers,

Richard
Koen Kooi Nov. 3, 2014, 5:13 p.m. | #10
> Op 3 nov. 2014, om 17:08 heeft Burton, Ross <ross.burton@intel.com> het volgende geschreven:
> 
> 
> On 3 November 2014 15:24, Koen Kooi <koen.kooi@linaro.org> wrote:
> It's been over a month and libpng is still broken, so I refuse to classify this new process as 'helpful' or as 'appreciated' at this point.
> 
> Remember that 1.7 was frozen solid for several weeks of that month, so no it wouldn't have been merged in that time period.  It's only just re-opened: master is currently 80 patches ahead of dizzy, 35 in the C-Pull I sent last night, and another 33 in my staging branch right now so it's not like we're taking it easy right now.
> 
> Some context: during September it was obvious that various parties had an interest in aarch64 (Linaro and Wind River).  Patches to fix build failures were being sent sporadically, a qemuarm64 machine was submitted, but there was no kernel for said machine to actually build.   At the time there was no qemuarm64 machine that actually worked, so even merging the "simple" fixes on just faith that they're correct would be reckless considering we were freezing.  This is why I suggested[1] that someone with a vested interest in aarch64 collate all the patches into a single branch and keep it rebased to master so that when master opens, it can be tested on *all* qemu architectures and merged.  It appears that this hasn't happened.  Will Linaro be able to create a branch that incorporates all of the patches and new machine that's been tested as a whole?

Most likely not because we already have meta-aarch64 for the toolchain and kernel support. But aside from that, the past few *years* it wasn't a problem to get aarch64 related patches merged into OE-core, so why is there suddenly such a problem now?

Furthermore, this libpng patch isn't strictly an aarch64 patch, it fixes a previous commit that should have been flagged in review as being wrong for using double qualifiers in a bogus way. So why did the culprit go in and the fix submitted 3 days later not?
Ross Burton Nov. 3, 2014, 5:33 p.m. | #11
On 3 November 2014 17:13, Koen Kooi <koen@dominion.thruhere.net> wrote:

> Most likely not because we already have meta-aarch64 for the toolchain and
> kernel support. But aside from that, the past few *years* it wasn't a
> problem to get aarch64 related patches merged into OE-core, so why is there
> suddenly such a problem now?
>

Because oe-core was frozen and we were only taking serious fixes.  Now
there's a number of related patches that can and should be bundled up so
they can be tested as a whole.


> Furthermore, this libpng patch isn't strictly an aarch64 patch, it fixes a
> previous commit that should have been flagged in review as being wrong for
> using double qualifiers in a bogus way. So why did the culprit go in and
> the fix submitted 3 days later not?


That patch in particular?  Because humans are not infallible.  FWIW, this
is in my MUT now.

Ross
Mark Hatle Nov. 3, 2014, 5:38 p.m. | #12
On 11/3/14, 9:24 AM, Koen Kooi wrote:
>
> Op 3 nov. 2014, om 13:10 heeft Richard Purdie <richard.purdie@linuxfoundation.org> het volgende geschreven:
>
>> On Mon, 2014-11-03 at 13:06 +0100, Koen Kooi wrote:
>>>> Op 3 nov. 2014, om 12:30 heeft Burton, Ross <ross.burton@intel.com> het volgende geschreven:
>>>>
>>>>
>>>> On 3 November 2014 11:27, Koen Kooi <koen@dominion.thruhere.net> wrote:
>>>>> Last night I was looking at my "post-1.7" tag and the number of aarch64 patches there.   Kai, did you continue work on that staging branch for all of the aarch64 patches that were being posted?
>>>>
>>>> What does that have to do with this patch?
>>>>
>>>> Kai was collecting patches that were sent to oe-core, and this may have been one of them.
>>>
>>> Is this some new requirement for OE-core patches?
>>
>> Its someone being helpful and trying to consolidate a topic branch and
>> test things before it gets merged.
>>
>> So its not a new requirement but it is helpful and appreciated.
>
> It's been over a month and libpng is still broken, so I refuse to classify this new process as 'helpful' or as 'appreciated' at this point.
>
> If this continues can the person doing such a branch reply to patches intended for it saying they will or won't be queued? And please run the patches without such a reply through the regular process. That would be both helpful and appreciated.

We've been attempting to aggregate what we (WR) can in:

  http://git.yoctoproject.org/cgit/cgit.cgi/poky-contrib/log/?h=kangkai/qemuarm64

But as it's unofficial and we're simply trying to keep the patches we know of 
together..  you'll have to be patient.  The first order of business is getting a 
functional qemuarm64.. it's MOSTLY there, but we're having toolchain issues and 
other things that are simply higher priority for us.

As for libpng, we have a different version of the patch in that branch...  it 
was enough to get around the problem in question, but may not be the correct fix.

--Mark

> --
> Koen Kooi
> Builds and Baselines | Release Manager
> Linaro.org | Open source software for ARM SoCs
>
>
>
Ross Burton Nov. 3, 2014, 5:44 p.m. | #13
On 3 November 2014 17:38, Mark Hatle <mark.hatle@windriver.com> wrote:

> We've been attempting to aggregate what we (WR) can in:
>
>  http://git.yoctoproject.org/cgit/cgit.cgi/poky-contrib/
> log/?h=kangkai/qemuarm64
>
> But as it's unofficial and we're simply trying to keep the patches we know
> of together..  you'll have to be patient.  The first order of business is
> getting a functional qemuarm64.. it's MOSTLY there, but we're having
> toolchain issues and other things that are simply higher priority for us.
>

Great, thanks Mark.  A functional qemuarm64 that boots core-image-minimal
is a good step in the right direction, then we can experiment with it on
the autobuilder. :)

Patiently awaiting patches,
Ross
Kai Kang Nov. 4, 2014, 2:30 a.m. | #14
On 2014?11?04? 01:44, Burton, Ross wrote:
> On 3 November 2014 17:38, Mark Hatle <mark.hatle@windriver.com> wrote:
>
>> We've been attempting to aggregate what we (WR) can in:
>>
>>   http://git.yoctoproject.org/cgit/cgit.cgi/poky-contrib/
>> log/?h=kangkai/qemuarm64
>>
>> But as it's unofficial and we're simply trying to keep the patches we know
>> of together..  you'll have to be patient.  The first order of business is
>> getting a functional qemuarm64.. it's MOSTLY there, but we're having
>> toolchain issues and other things that are simply higher priority for us.
>>
> Great, thanks Mark.  A functional qemuarm64 that boots core-image-minimal
> is a good step in the right direction, then we can experiment with it on
> the autobuilder. :)

There will be a few more patches from Mark for multilib, then we could 
build it on autobuilder. I'll finish it today.

--Kai

>
> Patiently awaiting patches,
> Ross
>
>
>

Patch

diff --git a/meta/recipes-multimedia/libpng/libpng_1.6.13.bb b/meta/recipes-multimedia/libpng/libpng_1.6.13.bb
index 0c6fd1f..8798a96 100644
--- a/meta/recipes-multimedia/libpng/libpng_1.6.13.bb
+++ b/meta/recipes-multimedia/libpng/libpng_1.6.13.bb
@@ -19,7 +19,7 @@  BINCONFIG = "${bindir}/libpng-config ${bindir}/libpng16-config"
 inherit autotools binconfig-disabled pkgconfig
 
 # Work around missing symbols
-EXTRA_OECONF_append_arm = " ${@bb.utils.contains("TUNE_FEATURES", "neon", "--enable-arm-neon=on", "--enable-arm-neon=off" ,d)}"
+EXTRA_OECONF_append = " ${@bb.utils.contains("TUNE_FEATURES", "neon", "--enable-arm-neon=on", "--enable-arm-neon=off" ,d)}"
 
 PACKAGES =+ "${PN}-tools"