Patchwork libdrm: Explicitly disable the cairo dependency

login
register
mail settings
Submitter Richard Purdie
Date Oct. 5, 2012, 11:47 a.m.
Message ID <1349437649.15658.20.camel@ted>
Download mbox | patch
Permalink /patch/37811/
State Accepted
Commit 7f4a8dd914e35c2eaaba7bd953273d10a94d85eb
Headers show

Comments

Richard Purdie - Oct. 5, 2012, 11:47 a.m.
We don't want the cairo dependency. Unfortunately simply checking whether its present
isn't good enough. If its not in DEPENDS, it can disappear half way through building.
We therefore need to explicitly disable it.

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
Phil Blundell - Oct. 5, 2012, 11:54 a.m.
On Fri, 2012-10-05 at 12:47 +0100, Richard Purdie wrote:
> diff --git a/meta/recipes-graphics/drm/libdrm/nocairo.patch b/meta/recipes-graphics/drm/libdrm/nocairo.patch
> new file mode 100644
> index 0000000..f9b7f3a
> --- a/dev/null
> +++ b/meta/recipes-graphics/drm/libdrm/nocairo.patch
> @@ -0,0 +1,39 @@
> +We don't want the cairo dependency. Unfortunately simply checking whether its present 
> +isn't good enough. If its not in DEPENDS, it can disappear half way through building.
> +We therefore need to explictly disable it.
> +
> +RP
> +2012/10/5
> +
> +Index: libdrm-2.4.39/configure.ac

This new patch seems to be missing Upstream-Status and Signed-off-by.

p.
Otavio Salvador - Oct. 5, 2012, 1:03 p.m.
On Fri, Oct 5, 2012 at 8:47 AM, Richard Purdie
<richard.purdie@linuxfoundation.org> wrote:
> We don't want the cairo dependency. Unfortunately simply checking whether its present
> isn't good enough. If its not in DEPENDS, it can disappear half way through building.
> We therefore need to explicitly disable it.
>
> Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>

Richard,

Please use the Upstream-Status field in the patch so it is easier to
track later.
Richard Purdie - Oct. 5, 2012, 1:22 p.m.
On Fri, 2012-10-05 at 12:54 +0100, Phil Blundell wrote:
> On Fri, 2012-10-05 at 12:47 +0100, Richard Purdie wrote:
> > diff --git a/meta/recipes-graphics/drm/libdrm/nocairo.patch b/meta/recipes-graphics/drm/libdrm/nocairo.patch
> > new file mode 100644
> > index 0000000..f9b7f3a
> > --- a/dev/null
> > +++ b/meta/recipes-graphics/drm/libdrm/nocairo.patch
> > @@ -0,0 +1,39 @@
> > +We don't want the cairo dependency. Unfortunately simply checking whether its present 
> > +isn't good enough. If its not in DEPENDS, it can disappear half way through building.
> > +We therefore need to explictly disable it.
> > +
> > +RP
> > +2012/10/5
> > +
> > +Index: libdrm-2.4.39/configure.ac
> 
> This new patch seems to be missing Upstream-Status and Signed-off-by.

The rule is that it has author information which I did include.
Admittedly "RP" is a little cryptic but I think people can figure it out
by now and its an established convention for me by now. I've questioned
the rational of Signed-off-by in this context before but it seems people
find it easier and it doesn't particularly bother me...

As for the Upstream-Status, I should have included that and will add it,
thanks.

Richard
Daniel Stone - Oct. 7, 2012, 1:10 a.m.
Hi Richard,

On 5 October 2012 21:47, Richard Purdie
<richard.purdie@linuxfoundation.org> wrote:
> We don't want the cairo dependency. Unfortunately simply checking whether its present
> isn't good enough. If its not in DEPENDS, it can disappear half way through building.
> We therefore need to explicitly disable it.
>
> Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>

I've done roughly the same thing in this commit:
http://cgit.collabora.com/git/user/daniels/poky.git/commit/?id=96a6e8e9eb7c086be3fcbde6a38ac3b699fca008

which has already been submitted upstream:
http://lists.freedesktop.org/archives/dri-devel/2012-October/028514.html

Cheers,
Daniel
Daniel Stone - Oct. 8, 2012, 5:44 a.m.
On 7 October 2012 12:10, Daniel Stone <daniel@fooishbar.org> wrote:
> On 5 October 2012 21:47, Richard Purdie
> <richard.purdie@linuxfoundation.org> wrote:
>> We don't want the cairo dependency. Unfortunately simply checking whether its present
>> isn't good enough. If its not in DEPENDS, it can disappear half way through building.
>> We therefore need to explicitly disable it.
>
> I've done roughly the same thing in this commit:
> http://cgit.collabora.com/git/user/daniels/poky.git/commit/?id=96a6e8e9eb7c086be3fcbde6a38ac3b699fca008
>
> which has already been submitted upstream:
> http://lists.freedesktop.org/archives/dri-devel/2012-October/028514.html

My patch also has the advantage of not trying to link against Cairo if
it's present but disabled.  Without this, if you build libdrm, build
Cairo, clean libdrm and attempt to rebuild it again, the rebuild will
fail, because the test still attempts to link with -lcairo, which
fails as Cairo's .la file references libdrm.

Cheers,
Daniel
Richard Purdie - Oct. 8, 2012, 8:01 a.m.
On Mon, 2012-10-08 at 16:44 +1100, Daniel Stone wrote:
> On 7 October 2012 12:10, Daniel Stone <daniel@fooishbar.org> wrote:
> > On 5 October 2012 21:47, Richard Purdie
> > <richard.purdie@linuxfoundation.org> wrote:
> >> We don't want the cairo dependency. Unfortunately simply checking whether its present
> >> isn't good enough. If its not in DEPENDS, it can disappear half way through building.
> >> We therefore need to explicitly disable it.
> >
> > I've done roughly the same thing in this commit:
> > http://cgit.collabora.com/git/user/daniels/poky.git/commit/?id=96a6e8e9eb7c086be3fcbde6a38ac3b699fca008
> >
> > which has already been submitted upstream:
> > http://lists.freedesktop.org/archives/dri-devel/2012-October/028514.html

Thanks for that, sending something upstream was on my todo list but I
needed to fix the breaking builds more urgently.

> My patch also has the advantage of not trying to link against Cairo if
> it's present but disabled.  Without this, if you build libdrm, build
> Cairo, clean libdrm and attempt to rebuild it again, the rebuild will
> fail, because the test still attempts to link with -lcairo, which
> fails as Cairo's .la file references libdrm.

I'm assuming you mean libdrm's .la file references cario?

Since I pass in --disable-cairo, it never runs the pkgconfig test for
cairo and never puts the include/library options into play. I did test
that and just retested and it doesn't reference it.

Cheers,

Richard
Daniel Stone - Oct. 8, 2012, 8:32 a.m.
Hi,

On 8 October 2012 19:01, Richard Purdie
<richard.purdie@linuxfoundation.org> wrote:
> On Mon, 2012-10-08 at 16:44 +1100, Daniel Stone wrote:
>> > which has already been submitted upstream:
>> > http://lists.freedesktop.org/archives/dri-devel/2012-October/028514.html
>
> Thanks for that, sending something upstream was on my todo list but I
> needed to fix the breaking builds more urgently.

Understandable - happily the patch has already merged.  There's a
patchset in git://git.collabora.com/~daniels/poky which reverts your
original patch, adds a new patch for 2.4.39 with the backport of the
upstream patch, and bumps the git SRCREV to one with this patch
already merged.  Should I submit this to the Yocto list? (I'm told
that submitting Poky patches to oe-core is the worst kind of error.)

Cheers,
Daniel
Richard Purdie - Oct. 8, 2012, 9:19 a.m.
On Mon, 2012-10-08 at 19:32 +1100, Daniel Stone wrote:
> Hi,
> 
> On 8 October 2012 19:01, Richard Purdie
> <richard.purdie@linuxfoundation.org> wrote:
> > On Mon, 2012-10-08 at 16:44 +1100, Daniel Stone wrote:
> >> > which has already been submitted upstream:
> >> > http://lists.freedesktop.org/archives/dri-devel/2012-October/028514.html
> >
> > Thanks for that, sending something upstream was on my todo list but I
> > needed to fix the breaking builds more urgently.
> 
> Understandable - happily the patch has already merged.  There's a
> patchset in git://git.collabora.com/~daniels/poky which reverts your
> original patch, adds a new patch for 2.4.39 with the backport of the
> upstream patch, and bumps the git SRCREV to one with this patch
> already merged.  Should I submit this to the Yocto list? (I'm told
> that submitting Poky patches to oe-core is the worst kind of error.)

Please do send the patch. It likely won't go into the release branch as
I'm trying not to make more changes there than we have to but will make
it into master, if not this instant, shortly.

Cheers,

Richard

Patch

diff --git a/meta/recipes-graphics/drm/libdrm.inc b/meta/recipes-graphics/drm/libdrm.inc
index cc09791..2ed9c14 100644
--- a/meta/recipes-graphics/drm/libdrm.inc
+++ b/meta/recipes-graphics/drm/libdrm.inc
@@ -18,6 +18,8 @@  DEPENDS += " libpciaccess"
 
 inherit autotools pkgconfig
 
+EXTRA_OECONF += "--disable-cairo"
+
 PACKAGES =+ "${PN}-tests ${PN}-drivers ${PN}-kms"
 FILES_${PN}-tests = "${bindir}/dr* ${bindir}/mode*"
 FILES_${PN}-drivers = "${libdir}/libdrm_*.so.*"
diff --git a/meta/recipes-graphics/drm/libdrm/nocairo.patch b/meta/recipes-graphics/drm/libdrm/nocairo.patch
new file mode 100644
index 0000000..f9b7f3a
--- a/dev/null
+++ b/meta/recipes-graphics/drm/libdrm/nocairo.patch
@@ -0,0 +1,39 @@ 
+We don't want the cairo dependency. Unfortunately simply checking whether its present 
+isn't good enough. If its not in DEPENDS, it can disappear half way through building.
+We therefore need to explictly disable it.
+
+RP
+2012/10/5
+
+Index: libdrm-2.4.39/configure.ac
+===================================================================
+--- libdrm-2.4.39.orig/configure.ac	2012-08-24 14:54:42.000000000 +0000
++++ libdrm-2.4.39/configure.ac	2012-10-05 11:37:52.484821221 +0000
+@@ -63,6 +63,11 @@
+ 	      [Disable KMS mm abstraction library (default: auto)]),
+ 	      [LIBKMS=$enableval], [LIBKMS=auto])
+ 
++AC_ARG_ENABLE(cairo,
++	      AS_HELP_STRING([--disable-cairo],
++	      [Disable cairo (default: auto)]),
++	      [ENABLECAIRO=$enableval], [ENABLECAIRO=auto])
++
+ AC_ARG_ENABLE(intel,
+ 	      AS_HELP_STRING([--disable-intel],
+ 	      [Enable support for intel's KMS API (default: auto)]),
+@@ -201,9 +206,12 @@
+ 	AC_DEFINE(HAVE_EXYNOS, 1, [Have EXYNOS support])
+ fi
+ 
+-PKG_CHECK_MODULES(CAIRO, cairo, [HAVE_CAIRO=yes], [HAVE_CAIRO=no])
+-if test "x$HAVE_CAIRO" = xyes; then
+-	AC_DEFINE(HAVE_CAIRO, 1, [Have cairo support])
++HAVE_CAIRO=no
++if test "x$ENABLECAIRO" = xyes; then
++	PKG_CHECK_MODULES(CAIRO, cairo, [HAVE_CAIRO=yes], [HAVE_CAIRO=no])
++	if test "x$HAVE_CAIRO" = xyes; then
++		AC_DEFINE(HAVE_CAIRO, 1, [Have cairo support])
++	fi
+ fi
+ AM_CONDITIONAL(HAVE_CAIRO, [test "x$HAVE_CAIRO" = xyes])
+ 
diff --git a/meta/recipes-graphics/drm/libdrm_2.4.39.bb b/meta/recipes-graphics/drm/libdrm_2.4.39.bb
index 4e6a8d5..cb1f7f9 100644
--- a/meta/recipes-graphics/drm/libdrm_2.4.39.bb
+++ b/meta/recipes-graphics/drm/libdrm_2.4.39.bb
@@ -4,6 +4,7 @@  PR = "${INC_PR}.0"
 
 SRC_URI += "file://installtests.patch \
             file://GNU_SOURCE_definition.patch \
+            file://nocairo.patch \
            "
 
 SRC_URI[md5sum] = "9a299e021d81bab6c82307582c78319d"