[PATCH] gdk-pixbuf: Fix libpng determinism issues

[PATCH] gdk-pixbuf: Fix libpng determinism issues

[Richard Purdie]

We now have libpng 1.6. If we build libpng12 as well as libpng 1.6, the 1.2
version gets preferred which is not desirable and does not give deterministic builds.

We really do want to use libpng since the item in DEPENDS will provide this so
manipulate the search list so the one we DEPEND on gets chosen. This was the cause of a
recent autobuilder failure.

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
diff --git a/meta/recipes-gnome/gdk-pixbuf/gdk-pixbuf-2.26.5/pngversion.patch b/meta/recipes-gnome/gdk-pixbuf/gdk-pixbuf-2.26.5/pngversion.patch
new file mode 100644
index 0000000..81a3d06
--- /dev/null
+++ b/meta/recipes-gnome/gdk-pixbuf/gdk-pixbuf-2.26.5/pngversion.patch
@@ -0,0 +1,23 @@
+We now have libpng 1.6. If we build libpng12 as well as libpng 1.6, the 1.2 version gets 
+preferred which is not desirable and does not give deterministic builds.
+
+We really do want to use libpng since the item in DEPENDS will provide this so
+manipulate the search list so the one we DEPEND on gets chosen.
+
+RP 2013/4/13
+
+Upstream-Status: Pending [worth discussing at least]
+
+Index: gdk-pixbuf-2.26.5/configure.ac
+===================================================================
+--- gdk-pixbuf-2.26.5.orig/configure.ac	2013-03-26 15:45:16.594820303 +0000
++++ gdk-pixbuf-2.26.5/configure.ac	2013-04-13 10:15:19.241433789 +0000
+@@ -588,7 +588,7 @@
+ 
+ dnl Test for libpng
+   if test x$with_libpng != xno && test -z "$LIBPNG"; then
+-    for l in libpng15 libpng14 libpng12 libpng13 libpng10 libpng ; do
++    for l in libpng libpng15 libpng14 libpng12 libpng13 libpng10 ; do
+       AC_MSG_CHECKING(for $l)
+       if $PKG_CONFIG --exists $l ; then
+         AC_MSG_RESULT(yes)
diff --git a/meta/recipes-gnome/gdk-pixbuf/gdk-pixbuf_2.26.5.bb b/meta/recipes-gnome/gdk-pixbuf/gdk-pixbuf_2.26.5.bb
index cc2ea50..b35f7c6 100644
--- a/meta/recipes-gnome/gdk-pixbuf/gdk-pixbuf_2.26.5.bb
+++ b/meta/recipes-gnome/gdk-pixbuf/gdk-pixbuf_2.26.5.bb
@@ -15,6 +15,7 @@ SRC_URI = "http://ftp.acc.umu.se/pub/GNOME/sources/gdk-pixbuf/2.26/gdk-pixbuf-${
            file://hardcoded_libtool.patch \
            file://configure_fix.patch \
            file://extending-libinstall-dependencies.patch \
+           file://pngversion.patch \
            "
 
 SRC_URI[md5sum] = "339329e6d619ee3e1cb93979111b04c0"

Re: [PATCH] gdk-pixbuf: Fix libpng determinism issues

[Colin Walters]

Is "libpng" the new canonical name for 1.6?  I assume there was a reason
it was listed last.  It looks like the current logic came from:

https://git.gnome.org/browse/gdk-pixbuf/commit/?id=ddedf5a2c2c63bfe8d6f04376cf2bba215a5eb19

Which is a not very enlightening commit message.  It looks like the
Fedora 18 "libpng" package provides both libpng.pc and libpng15.pc.
RHEL6 has the same except it's libpng12.pc too.  My Ubuntu 12.10 VM has
libpng12 with just libpng12.pc, no libpng.pc.

My main concern with this patch was ensuring that people aren't getting
a suddenly ancient and deprecated libpng, but that seems unlikely, so
unless there are other comments I can take care of turning this into
"git format-patch" style and pushing upstream.

On Sat, 2013-04-13 at 11:25 +0100, Richard Purdie wrote:
> We now have libpng 1.6. If we build libpng12 as well as libpng 1.6, the 1.2
> version gets preferred which is not desirable and does not give deterministic builds.
> 
> We really do want to use libpng since the item in DEPENDS will provide this so
> manipulate the search list so the one we DEPEND on gets chosen. This was the cause of a
> recent autobuilder failure.
> 
> Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
> ---
> diff --git a/meta/recipes-gnome/gdk-pixbuf/gdk-pixbuf-2.26.5/pngversion.patch b/meta/recipes-gnome/gdk-pixbuf/gdk-pixbuf-2.26.5/pngversion.patch
> new file mode 100644
> index 0000000..81a3d06
> --- /dev/null
> +++ b/meta/recipes-gnome/gdk-pixbuf/gdk-pixbuf-2.26.5/pngversion.patch
> @@ -0,0 +1,23 @@
> +We now have libpng 1.6. If we build libpng12 as well as libpng 1.6, the 1.2 version gets 
> +preferred which is not desirable and does not give deterministic builds.
> +
> +We really do want to use libpng since the item in DEPENDS will provide this so
> +manipulate the search list so the one we DEPEND on gets chosen.
> +
> +RP 2013/4/13
> +
> +Upstream-Status: Pending [worth discussing at least]
> +
> +Index: gdk-pixbuf-2.26.5/configure.ac
> +===================================================================
> +--- gdk-pixbuf-2.26.5.orig/configure.ac	2013-03-26 15:45:16.594820303 +0000
> ++++ gdk-pixbuf-2.26.5/configure.ac	2013-04-13 10:15:19.241433789 +0000
> +@@ -588,7 +588,7 @@
> + 
> + dnl Test for libpng
> +   if test x$with_libpng != xno && test -z "$LIBPNG"; then
> +-    for l in libpng15 libpng14 libpng12 libpng13 libpng10 libpng ; do
> ++    for l in libpng libpng15 libpng14 libpng12 libpng13 libpng10 ; do
> +       AC_MSG_CHECKING(for $l)
> +       if $PKG_CONFIG --exists $l ; then
> +         AC_MSG_RESULT(yes)
> diff --git a/meta/recipes-gnome/gdk-pixbuf/gdk-pixbuf_2.26.5.bb b/meta/recipes-gnome/gdk-pixbuf/gdk-pixbuf_2.26.5.bb
> index cc2ea50..b35f7c6 100644
> --- a/meta/recipes-gnome/gdk-pixbuf/gdk-pixbuf_2.26.5.bb
> +++ b/meta/recipes-gnome/gdk-pixbuf/gdk-pixbuf_2.26.5.bb
> @@ -15,6 +15,7 @@ SRC_URI = "http://ftp.acc.umu.se/pub/GNOME/sources/gdk-pixbuf/2.26/gdk-pixbuf-${
>             file://hardcoded_libtool.patch \
>             file://configure_fix.patch \
>             file://extending-libinstall-dependencies.patch \
> +           file://pngversion.patch \
>             "
>  
>  SRC_URI[md5sum] = "339329e6d619ee3e1cb93979111b04c0"
> 
> 

Re: [PATCH] gdk-pixbuf: Fix libpng determinism issues

[Richard Purdie]

On Sun, 2013-04-14 at 09:02 -0400, Colin Walters wrote:
> Is "libpng" the new canonical name for 1.6?

Its a symlink to libpng16.pc which is something libpng upstream's "make
install" provides.

>   I assume there was a reason
> it was listed last.  It looks like the current logic came from:
> 
> https://git.gnome.org/browse/gdk-pixbuf/commit/?id=ddedf5a2c2c63bfe8d6f04376cf2bba215a5eb19
>
> Which is a not very enlightening commit message.  It looks like the
> Fedora 18 "libpng" package provides both libpng.pc and libpng15.pc.
> RHEL6 has the same except it's libpng12.pc too.  My Ubuntu 12.10 VM has
> libpng12 with just libpng12.pc, no libpng.pc.

The more interesting change is:

https://git.gnome.org/browse/gdk-pixbuf/commit/configure.ac?id=d430bc4df3314a88cd538474d26ff7764d1f408c

and following that to the bugzilla 'For this to make sense, I changed
the order so that a version specific dep, such as libpng15 or libpng12,
is found before just "libpng".'

I'm not sure I entirely follow that logic.

> My main concern with this patch was ensuring that people aren't getting
> a suddenly ancient and deprecated libpng, but that seems unlikely, so
> unless there are other comments I can take care of turning this into
> "git format-patch" style and pushing upstream.

I think the intent of the symlink is to provide the system with a
default libpng to use in the absence of a specific version requirement.
As the code stands today, each time a new libpng comes out, gdk-pixbuf
will need changes before it will be able to use it. In the meantime, it
will potentially link against something old, e.g. 1.2, since 1.2 is in
the LSB 4.X spec so most LSB like systems would have 1.6 and 1.2.

If we can justify changing this upstream, that would be great :). It may
be worth adding libpng16 into the list too so everything is covered too.

Cheers,

Richard

Re: [PATCH] gdk-pixbuf: Fix libpng determinism issues

[Colin Walters]

[-- Attachment #1: Type: text/plain, Size: 1877 bytes --]

On Sun, 2013-04-14 at 16:33 +0100, Richard Purdie wrote:
> On Sun, 2013-04-14 at 09:02 -0400, Colin Walters wrote:
> The more interesting change is:
> 
> https://git.gnome.org/browse/gdk-pixbuf/commit/configure.ac?id=d430bc4df3314a88cd538474d26ff7764d1f408c
> 
> and following that to the bugzilla 'For this to make sense, I changed
> the order so that a version specific dep, such as libpng15 or
> libpng12,
> is found before just "libpng".'
> 
> I'm not sure I entirely follow that logic.

I added Matthias to CC as he touched this last then.

> I think the intent of the symlink is to provide the system with a
> default libpng to use in the absence of a specific version requirement.
> As the code stands today, each time a new libpng comes out, gdk-pixbuf
> will need changes before it will be able to use it. 

Right, we need configure.ac changes, but the rationale behind that is
that we'd also need *code* changes for each new major version of libpng.
But it sounds like what you're saying is that gdk-pixbuf compiles and
operates correctly with 1.6?  If that's the case, then the least
invasive change here is to simply add 1.6.

Blah, I tried changing the gnome-ostree build to fetch libpng's v1.6.1
git tag to test, but it hard requires Automake 1.13.

Anyways, if it works (looks like the latest oe-core has it), then
what about the attached?

> In the meantime, it
> will potentially link against something old, e.g. 1.2, since 1.2 is in
> the LSB 4.X spec so most LSB like systems would have 1.6 and 1.2.
> 
> If we can justify changing this upstream, that would be great :). It may
> be worth adding libpng16 into the list too so everything is covered too.

At this point I'm hoping the parade of libpng versions will
settle down, so hopefully no further tweaking of the configure script or
code will be required...



[-- Attachment #2: 0001-build-We-also-support-libpng16.patch --]
[-- Type: text/x-patch, Size: 862 bytes --]

From 829379cfa2b48e966125df2d070d2af40cb3f990 Mon Sep 17 00:00:00 2001
From: Colin Walters <walters@verbum.org>
Date: Mon, 15 Apr 2013 05:59:09 -0400
Subject: [PATCH] build: We also support libpng16

See http://lists.linuxtogo.org/pipermail/openembedded-core/2013-April/038321.html
---
 configure.ac |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/configure.ac b/configure.ac
index 8ec8b1f..106fab1 100644
--- a/configure.ac
+++ b/configure.ac
@@ -586,7 +586,7 @@ fi
 
 dnl Test for libpng
   if test x$with_libpng != xno && test -z "$LIBPNG"; then
-    for l in libpng15 libpng14 libpng12 libpng13 libpng10 libpng ; do
+    for l in libpng16 libpng15 libpng14 libpng12 libpng13 libpng10 libpng; do
       AC_MSG_CHECKING(for $l)
       if $PKG_CONFIG --exists $l ; then
         AC_MSG_RESULT(yes)
-- 
1.7.1

Re: [PATCH] gdk-pixbuf: Fix libpng determinism issues

[Koen Kooi]

Op 15 apr. 2013, om 12:08 heeft Colin Walters <walters@verbum.org> het volgende geschreven:

> On Sun, 2013-04-14 at 16:33 +0100, Richard Purdie wrote:
>> On Sun, 2013-04-14 at 09:02 -0400, Colin Walters wrote:
>> The more interesting change is:
>> 
>> https://git.gnome.org/browse/gdk-pixbuf/commit/configure.ac?id=d430bc4df3314a88cd538474d26ff7764d1f408c
>> 
>> and following that to the bugzilla 'For this to make sense, I changed
>> the order so that a version specific dep, such as libpng15 or
>> libpng12,
>> is found before just "libpng".'
>> 
>> I'm not sure I entirely follow that logic.
> 
> I added Matthias to CC as he touched this last then.
> 
>> I think the intent of the symlink is to provide the system with a
>> default libpng to use in the absence of a specific version requirement.
>> As the code stands today, each time a new libpng comes out, gdk-pixbuf
>> will need changes before it will be able to use it. 
> 
> Right, we need configure.ac changes, but the rationale behind that is
> that we'd also need *code* changes for each new major version of libpng.
> But it sounds like what you're saying is that gdk-pixbuf compiles and
> operates correctly with 1.6?  If that's the case, then the least
> invasive change here is to simply add 1.6.
> 
> Blah, I tried changing the gnome-ostree build to fetch libpng's v1.6.1
> git tag to test, but it hard requires Automake 1.13.

Only for parallel unit tests, I just sent a patch to update libpng to v1.6.1 to oe-core. It doesn't fix the problems I'm having with PNGs in weston, so please test it and see if it fixed any of your problems.

regards,

Koen

> 
> Anyways, if it works (looks like the latest oe-core has it), then
> what about the attached?
> 
>> In the meantime, it
>> will potentially link against something old, e.g. 1.2, since 1.2 is in
>> the LSB 4.X spec so most LSB like systems would have 1.6 and 1.2.
>> 
>> If we can justify changing this upstream, that would be great :). It may
>> be worth adding libpng16 into the list too so everything is covered too.
> 
> At this point I'm hoping the parade of libpng versions will
> settle down, so hopefully no further tweaking of the configure script or
> code will be required...
> 
> 
> <0001-build-We-also-support-libpng16.patch>_______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core

Re: [PATCH] gdk-pixbuf: Fix libpng determinism issues

[Richard Purdie]

On Mon, 2013-04-15 at 06:08 -0400, Colin Walters wrote:
> On Sun, 2013-04-14 at 16:33 +0100, Richard Purdie wrote:
> > On Sun, 2013-04-14 at 09:02 -0400, Colin Walters wrote:
> > The more interesting change is:
> > 
> > https://git.gnome.org/browse/gdk-pixbuf/commit/configure.ac?id=d430bc4df3314a88cd538474d26ff7764d1f408c
> > 
> > and following that to the bugzilla 'For this to make sense, I changed
> > the order so that a version specific dep, such as libpng15 or
> > libpng12,
> > is found before just "libpng".'
> > 
> > I'm not sure I entirely follow that logic.
> 
> I added Matthias to CC as he touched this last then.
> 
> > I think the intent of the symlink is to provide the system with a
> > default libpng to use in the absence of a specific version requirement.
> > As the code stands today, each time a new libpng comes out, gdk-pixbuf
> > will need changes before it will be able to use it. 
> 
> Right, we need configure.ac changes, but the rationale behind that is
> that we'd also need *code* changes for each new major version of libpng.
> But it sounds like what you're saying is that gdk-pixbuf compiles and
> operates correctly with 1.6?  If that's the case, then the least
> invasive change here is to simply add 1.6.

It compiles and operates correctly as far as we can tell, yes.

Given that rationale, I'd suggest "libpng" should be dropped from the
list.

> Blah, I tried changing the gnome-ostree build to fetch libpng's v1.6.1
> git tag to test, but it hard requires Automake 1.13.
> 
> Anyways, if it works (looks like the latest oe-core has it), then
> what about the attached?

It will make our builds work again for now until the next time someone
upgrades libpng and and then it will potentially silently start using an
old version in some builds :(.

We're therefore probably going to get stuck carrying a patch for
this :/.

> > In the meantime, it
> > will potentially link against something old, e.g. 1.2, since 1.2 is in
> > the LSB 4.X spec so most LSB like systems would have 1.6 and 1.2.
> > 
> > If we can justify changing this upstream, that would be great :). It may
> > be worth adding libpng16 into the list too so everything is covered too.
> 
> At this point I'm hoping the parade of libpng versions will
> settle down, so hopefully no further tweaking of the configure script or
> code will be required...

Its the case things silently break that really worry me.

Cheers,

Richard

Re: [PATCH] gdk-pixbuf: Fix libpng determinism issues

[Colin Walters]

[-- Attachment #1: Type: text/plain, Size: 358 bytes --]

On Mon, 2013-04-15 at 12:31 +0100, Richard Purdie wrote:

> It will make our builds work again for now until the next time someone
> upgrades libpng and and then it will potentially silently start using an
> old version in some builds :(.

So something like the attached on top of the previous patch?

If this looks good I'll push both to master.


[-- Attachment #2: 0001-build-drop-libpng-from-detected-list.patch --]
[-- Type: text/x-patch, Size: 1071 bytes --]

From 91074d01a0e2b4508f7f253f1e0a7669363cb128 Mon Sep 17 00:00:00 2001
From: Richard Purdie <richard.purdie@linuxfoundation.org>
Date: Mon, 15 Apr 2013 08:06:45 -0400
Subject: [PATCH] build: drop "libpng" from detected list

This gives us an arbitrary version depending on the build root, and we
may not be upwards compatible with future versions.  Or it's possible
that the ancient (but LSB mandated) 1.2 claims the "libpng.pc" name.

Signed-off-by: Colin Walters <walters@verbum.org>
---
 configure.ac |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/configure.ac b/configure.ac
index 106fab1..5efc1b6 100644
--- a/configure.ac
+++ b/configure.ac
@@ -586,7 +586,7 @@ fi
 
 dnl Test for libpng
   if test x$with_libpng != xno && test -z "$LIBPNG"; then
-    for l in libpng16 libpng15 libpng14 libpng12 libpng13 libpng10 libpng; do
+    for l in libpng16 libpng15 libpng14 libpng12 libpng13 libpng10; do
       AC_MSG_CHECKING(for $l)
       if $PKG_CONFIG --exists $l ; then
         AC_MSG_RESULT(yes)
-- 
1.7.1

Re: [PATCH] gdk-pixbuf: Fix libpng determinism issues

[Richard Purdie]

On Mon, 2013-04-15 at 08:12 -0400, Colin Walters wrote:
> On Mon, 2013-04-15 at 12:31 +0100, Richard Purdie wrote:
> 
> > It will make our builds work again for now until the next time someone
> > upgrades libpng and and then it will potentially silently start using an
> > old version in some builds :(.
> 
> So something like the attached on top of the previous patch?
> 
> If this looks good I'll push both to master.

Looks good to me, thanks!

Richard

Re: [PATCH] gdk-pixbuf: Fix libpng determinism issues

[Colin Walters]

On Mon, 2013-04-15 at 13:22 +0100, Richard Purdie wrote:
> On Mon, 2013-04-15 at 08:12 -0400, Colin Walters wrote:
> > On Mon, 2013-04-15 at 12:31 +0100, Richard Purdie wrote:
> > 
> > > It will make our builds work again for now until the next time someone
> > > upgrades libpng and and then it will potentially silently start using an
> > > old version in some builds :(.
> > 
> > So something like the attached on top of the previous patch?
> > 
> > If this looks good I'll push both to master.
> 
> Looks good to me, thanks!

Both pushed to git, thanks.

<nag>
If OE was building from git as it should, your original patch could have
been a ready-to-upstream "git format-patch" style instead of the default
"Upstream-Status: inappropriate" non-git patches =/
</nag>

Re: [PATCH] gdk-pixbuf: Fix libpng determinism issues

[Richard Purdie]

On Mon, 2013-04-15 at 08:59 -0400, Colin Walters wrote:
> On Mon, 2013-04-15 at 13:22 +0100, Richard Purdie wrote:
> > On Mon, 2013-04-15 at 08:12 -0400, Colin Walters wrote:
> > > On Mon, 2013-04-15 at 12:31 +0100, Richard Purdie wrote:
> > > 
> > > > It will make our builds work again for now until the next time someone
> > > > upgrades libpng and and then it will potentially silently start using an
> > > > old version in some builds :(.
> > > 
> > > So something like the attached on top of the previous patch?
> > > 
> > > If this looks good I'll push both to master.
> > 
> > Looks good to me, thanks!
> 
> Both pushed to git, thanks.

Much appreciated!

> <nag>
> If OE was building from git as it should, your original patch could have
> been a ready-to-upstream "git format-patch" style instead of the default
> "Upstream-Status: inappropriate" non-git patches =/
> </nag>

Noted, although I doubt we'll be using git SRC_URIs for everything any
time soon for a variety of reasons. I would love to see the option of
building most things from bleeding source but that would require a lot
of work which we don't have the bandwidth for.

Cheers,

Richard

Re: [PATCH] gdk-pixbuf: Fix libpng determinism issues

[Koen Kooi]

Op 19 apr. 2013, om 14:59 heeft Richard Purdie <richard.purdie@linuxfoundation.org> het volgende geschreven:

> On Mon, 2013-04-15 at 08:59 -0400, Colin Walters wrote:
>> On Mon, 2013-04-15 at 13:22 +0100, Richard Purdie wrote:
>>> On Mon, 2013-04-15 at 08:12 -0400, Colin Walters wrote:
>>>> On Mon, 2013-04-15 at 12:31 +0100, Richard Purdie wrote:
>>>> 
>>>>> It will make our builds work again for now until the next time someone
>>>>> upgrades libpng and and then it will potentially silently start using an
>>>>> old version in some builds :(.
>>>> 
>>>> So something like the attached on top of the previous patch?
>>>> 
>>>> If this looks good I'll push both to master.
>>> 
>>> Looks good to me, thanks!
>> 
>> Both pushed to git, thanks.
> 
> Much appreciated!
> 
>> <nag>
>> If OE was building from git as it should, your original patch could have
>> been a ready-to-upstream "git format-patch" style instead of the default
>> "Upstream-Status: inappropriate" non-git patches =/
>> </nag>
> 
> Noted, although I doubt we'll be using git SRC_URIs for everything any
> time soon for a variety of reasons. I would love to see the option of
> building most things from bleeding source but that would require a lot
> of work which we don't have the bandwidth for.

And would introduce some dependency hell (gtk-doc-native needed for every little gnome thing *and* udev) but more importantly it will expose us to that stupid, stupid, stupid practice of using git submodules to drag in autoconf stuff. Gstreamer is the worst offender, but there are a lot of others.

I try to have recipe build from a git tag instead of a tarball whenever possible, but it hurts a lot if it's one of that gtk-doc users, you can't disable that completely.

regards,

Koen

Re: [PATCH] gdk-pixbuf: Fix libpng determinism issues

[Colin Walters]

On Fri, 2013-04-19 at 15:41 +0200, Koen Kooi wrote:

> And would introduce some dependency hell (gtk-doc-native needed for every little gnome thing *and* udev) 

Nope, that's why gtk-doc-stub exists, and it's already used in OE:

http://git.gnome.org/browse/gtk-doc-stub

> but more importantly it will expose us to that stupid, stupid, stupid practice of using git submodules to drag in autoconf stuff. 

Correct handling of git submodules is definitely nontrivial.