Patchwork gdk-pixbuf: Fix libpng determinism issues

login
register
mail settings
Submitter Richard Purdie
Date April 13, 2013, 10:25 a.m.
Message ID <1365848719.16702.74.camel@ted>
Download mbox | patch
Permalink /patch/48059/
State Accepted
Commit ce1d262ea36da9a9fdeeefc0ddc69833801d4d2d
Headers show

Comments

Richard Purdie - April 13, 2013, 10:25 a.m.
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>
---
Colin Walters - April 14, 2013, 1:02 p.m.
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"
> 
>
Richard Purdie - April 14, 2013, 3:33 p.m.
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

Patch

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"