Patchwork [dora,PATCH_V3] mesa: double check for eglplatform.h

login
register
mail settings
Submitter Valentin Popa
Date April 14, 2014, 2:51 p.m.
Message ID <1397487085-8924-1-git-send-email-valentin.popa@intel.com>
Download mbox | patch
Permalink /patch/70589/
State New
Headers show

Comments

Valentin Popa - April 14, 2014, 2:51 p.m.
Even if 'egl' is in PACKAGECONFIG, mesa egl support
can be disabled explicitly (changing configure flags
using a .bbappend, for example).
On dora, meta-fsl-arm is an example of this kind.
On master there are no known cases, and we should
encourge package configuration through PACKAGECONFIG.

This patch adds another check for the existence
of eglplatform.h before 'sed' can alter it.

Signed-off-by: Valentin Popa <valentin.popa@intel.com>
---
 meta/recipes-graphics/mesa/mesa_9.1.6.bb | 4 +++-
 meta/recipes-graphics/mesa/mesa_git.bb   | 4 +++-
 2 files changed, 6 insertions(+), 2 deletions(-)
Robert Yang - April 16, 2014, 3 a.m.
On 04/14/2014 10:51 PM, Valentin Popa wrote:
> Even if 'egl' is in PACKAGECONFIG, mesa egl support
> can be disabled explicitly (changing configure flags
> using a .bbappend, for example).
> On dora, meta-fsl-arm is an example of this kind.
> On master there are no known cases, and we should
> encourge package configuration through PACKAGECONFIG.
>
> This patch adds another check for the existence
> of eglplatform.h before 'sed' can alter it.
>
> Signed-off-by: Valentin Popa <valentin.popa@intel.com>

Reviewed and Tested by: Robert Yang <liezhi.yang@windriver.com>

// Robert

> ---
>   meta/recipes-graphics/mesa/mesa_9.1.6.bb | 4 +++-
>   meta/recipes-graphics/mesa/mesa_git.bb   | 4 +++-
>   2 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/meta/recipes-graphics/mesa/mesa_9.1.6.bb b/meta/recipes-graphics/mesa/mesa_9.1.6.bb
> index 6e9cd82..388cfd7 100644
> --- a/meta/recipes-graphics/mesa/mesa_9.1.6.bb
> +++ b/meta/recipes-graphics/mesa/mesa_9.1.6.bb
> @@ -19,6 +19,8 @@ S = "${WORKDIR}/Mesa-${PV}"
>   #make eglplatform.h independent of MESA_EGL_NO_X11_HEADER
>   do_install_append() {
>       if ${@base_contains('PACKAGECONFIG', 'egl', 'true', 'false', d)}; then
> -        sed -i -e 's/^#ifdef MESA_EGL_NO_X11_HEADERS/#if ${@base_contains('DISTRO_FEATURES', 'x11', '0', '1', d)}/' ${D}${includedir}/EGL/eglplatform.h
> +        if [ -e "${D}${includedir}/EGL/eglplatform.h" ]; then
> +            sed -i -e 's/^#ifdef MESA_EGL_NO_X11_HEADERS/#if ${@base_contains('DISTRO_FEATURES', 'x11', '0', '1', d)}/' ${D}${includedir}/EGL/eglplatform.h
> +        fi
>       fi
>   }
> \ No newline at end of file
> diff --git a/meta/recipes-graphics/mesa/mesa_git.bb b/meta/recipes-graphics/mesa/mesa_git.bb
> index 1babcc0..714911f 100644
> --- a/meta/recipes-graphics/mesa/mesa_git.bb
> +++ b/meta/recipes-graphics/mesa/mesa_git.bb
> @@ -23,6 +23,8 @@ S = "${WORKDIR}/git"
>   #make eglplatform.h independent of MESA_EGL_NO_X11_HEADER
>   do_install_append() {
>       if ${@base_contains('PACKAGECONFIG', 'egl', 'true', 'false', d)}; then
> -        sed -i -e 's/^#ifdef MESA_EGL_NO_X11_HEADERS/#if ${@base_contains('DISTRO_FEATURES', 'x11', '0', '1', d)}/' ${D}${includedir}/EGL/eglplatform.h
> +        if [ -e "${D}${includedir}/EGL/eglplatform.h" ]; then
> +            sed -i -e 's/^#ifdef MESA_EGL_NO_X11_HEADERS/#if ${@base_contains('DISTRO_FEATURES', 'x11', '0', '1', d)}/' ${D}${includedir}/EGL/eglplatform.h
> +        fi
>       fi
>   }
>
Otavio Salvador - April 18, 2014, 11:38 p.m.
On Wed, Apr 16, 2014 at 12:00 AM, Robert Yang <liezhi.yang@windriver.com> wrote:
>
>
> On 04/14/2014 10:51 PM, Valentin Popa wrote:
>>
>> Even if 'egl' is in PACKAGECONFIG, mesa egl support
>> can be disabled explicitly (changing configure flags
>> using a .bbappend, for example).
>> On dora, meta-fsl-arm is an example of this kind.
>> On master there are no known cases, and we should
>> encourge package configuration through PACKAGECONFIG.
>>
>> This patch adds another check for the existence
>> of eglplatform.h before 'sed' can alter it.
>>
>> Signed-off-by: Valentin Popa <valentin.popa@intel.com>
>
>
> Reviewed and Tested by: Robert Yang <liezhi.yang@windriver.com>
>
> // Robert
>
>
>> ---
>>   meta/recipes-graphics/mesa/mesa_9.1.6.bb | 4 +++-
>>   meta/recipes-graphics/mesa/mesa_git.bb   | 4 +++-
>>   2 files changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/meta/recipes-graphics/mesa/mesa_9.1.6.bb
>> b/meta/recipes-graphics/mesa/mesa_9.1.6.bb
>> index 6e9cd82..388cfd7 100644
>> --- a/meta/recipes-graphics/mesa/mesa_9.1.6.bb
>> +++ b/meta/recipes-graphics/mesa/mesa_9.1.6.bb
>> @@ -19,6 +19,8 @@ S = "${WORKDIR}/Mesa-${PV}"
>>   #make eglplatform.h independent of MESA_EGL_NO_X11_HEADER
>>   do_install_append() {
>>       if ${@base_contains('PACKAGECONFIG', 'egl', 'true', 'false', d)};
>> then
>> -        sed -i -e 's/^#ifdef MESA_EGL_NO_X11_HEADERS/#if
>> ${@base_contains('DISTRO_FEATURES', 'x11', '0', '1', d)}/'
>> ${D}${includedir}/EGL/eglplatform.h
>> +        if [ -e "${D}${includedir}/EGL/eglplatform.h" ]; then
>> +            sed -i -e 's/^#ifdef MESA_EGL_NO_X11_HEADERS/#if
>> ${@base_contains('DISTRO_FEATURES', 'x11', '0', '1', d)}/'
>> ${D}${includedir}/EGL/eglplatform.h
>> +        fi
>>       fi
>>   }
>> \ No newline at end of file
>> diff --git a/meta/recipes-graphics/mesa/mesa_git.bb
>> b/meta/recipes-graphics/mesa/mesa_git.bb
>> index 1babcc0..714911f 100644
>> --- a/meta/recipes-graphics/mesa/mesa_git.bb
>> +++ b/meta/recipes-graphics/mesa/mesa_git.bb
>> @@ -23,6 +23,8 @@ S = "${WORKDIR}/git"
>>   #make eglplatform.h independent of MESA_EGL_NO_X11_HEADER
>>   do_install_append() {
>>       if ${@base_contains('PACKAGECONFIG', 'egl', 'true', 'false', d)};
>> then
>> -        sed -i -e 's/^#ifdef MESA_EGL_NO_X11_HEADERS/#if
>> ${@base_contains('DISTRO_FEATURES', 'x11', '0', '1', d)}/'
>> ${D}${includedir}/EGL/eglplatform.h
>> +        if [ -e "${D}${includedir}/EGL/eglplatform.h" ]; then
>> +            sed -i -e 's/^#ifdef MESA_EGL_NO_X11_HEADERS/#if
>> ${@base_contains('DISTRO_FEATURES', 'x11', '0', '1', d)}/'
>> ${D}${includedir}/EGL/eglplatform.h
>> +        fi
>>       fi
>>   }
>>

So WHAT is holding a bugfix for a regression to be merged? I am very
disappointed with the maintenance in the Dora branch.

1. a change has been added without testing other layers hosted in YP.
2. it has been merged without being ran in YP AB.
3. I reported the issue in the same day it has been merged
4. it has been 10 days and this has not yet been reverted/fixed.

So CAN THIS BE MERGED PLEASE?
Richard Purdie - April 19, 2014, 1:44 p.m.
On Fri, 2014-04-18 at 20:38 -0300, Otavio Salvador wrote:
> So WHAT is holding a bugfix for a regression to be merged? I am very
> disappointed with the maintenance in the Dora branch.
> 
> 1. a change has been added without testing other layers hosted in YP.
> 2. it has been merged without being ran in YP AB.
> 3. I reported the issue in the same day it has been merged
> 4. it has been 10 days and this has not yet been reverted/fixed.
> 
> So CAN THIS BE MERGED PLEASE?

WHY HAVE YOU NOT REPLIED TO BUG 6098? I ASKED YOU SOMETHING THERE 9
WHOLE DAYS AGO? IT IS A HIGH!!!! ITS BEEN OPEN 17 DAYS!!!!

I happen to know you've been travelling/busy and likely haven't had
time. Fair enough, it happens.

I have also been travelling, any spare cycles were directed to the 1.6
release and the beaglebone problem which was threatening to disrupt it.
Right now its a 4 day holiday weekend in the UK. I should be out trying
to relax, instead I'm horribly jetlagged and even better, reading all
caps emails from you.

This is to say nothing of being a bit distracted after dropping an 80kg
stone hearth on the end of one of my fingers a couple of weeks ago,
fracturing it, then reacting badly to some antibiotics to add to the
fun.

As it happens I merged the fix this morning before I read this, it was
already in the queue. I'd planned to do so sooner but hadn't got around
to it. Several of those days were looking into the regression and
figuring out a way which fixed it, and fixed the other bug report the
problem was solving.

In answer to your questions:

Its not policy to test every layer with a change in OE-Core. We do our
best to make sure things don't break. Not every change gets run through
the autobuilder. We do run things through the AB before a release
though. 1.5.2 has not been released yet and this issue will get resolved
before that happens.

I do wish we've been able to act sooner, equally most of the people
involved have various pressures on their time. We do our best.

Cheers,

Richard
Otavio Salvador - April 19, 2014, 2:24 p.m.
Hello,

On Sat, Apr 19, 2014 at 10:44 AM, Richard Purdie
<richard.purdie@linuxfoundation.org> wrote:
> On Fri, 2014-04-18 at 20:38 -0300, Otavio Salvador wrote:
>> So WHAT is holding a bugfix for a regression to be merged? I am very
>> disappointed with the maintenance in the Dora branch.
>>
>> 1. a change has been added without testing other layers hosted in YP.
>> 2. it has been merged without being ran in YP AB.
>> 3. I reported the issue in the same day it has been merged
>> 4. it has been 10 days and this has not yet been reverted/fixed.
>>
>> So CAN THIS BE MERGED PLEASE?
>
> WHY HAVE YOU NOT REPLIED TO BUG 6098? I ASKED YOU SOMETHING THERE 9
> WHOLE DAYS AGO? IT IS A HIGH!!!! ITS BEEN OPEN 17 DAYS!!!!

Great! But the difference it is not a regression and happen in a
/very/ specific host setup. I think this is quite different of a
broken build for EVERY meta-fsl-arm user in the STABLE BRANCH!

Sorry but this is not the same thing.

> I happen to know you've been travelling/busy and likely haven't had
> time. Fair enough, it happens.

Yes, I arrived two days ago and been dealing with my backlog.

> I have also been travelling, any spare cycles were directed to the 1.6
> release and the beaglebone problem which was threatening to disrupt it.
> Right now its a 4 day holiday weekend in the UK. I should be out trying
> to relax, instead I'm horribly jetlagged and even better, reading all
> caps emails from you.

Sorry but Robert is the Dora maintainer so this is not an excuse. He
ought, as maintainer, put priority in such a regression.

> This is to say nothing of being a bit distracted after dropping an 80kg
> stone hearth on the end of one of my fingers a couple of weeks ago,
> fracturing it, then reacting badly to some antibiotics to add to the
> fun.

I knew it and I am happy you been better now. However as I said above
Dora is a Robert's maintained branch so I was not expecting you to be
the one fixing it...

> As it happens I merged the fix this morning before I read this, it was
> already in the queue. I'd planned to do so sooner but hadn't got around
> to it. Several of those days were looking into the regression and
> figuring out a way which fixed it, and fixed the other bug report the
> problem was solving.

Thanks.

> In answer to your questions:
>
> Its not policy to test every layer with a change in OE-Core. We do our
> best to make sure things don't break. Not every change gets run through
> the autobuilder. We do run things through the AB before a release
> though. 1.5.2 has not been released yet and this issue will get resolved
> before that happens.
>
> I do wish we've been able to act sooner, equally most of the people
> involved have various pressures on their time. We do our best.

I'd expect all merges in a stable branch would be run in AB. If this
is not the policy, it should be made as one. One of biggest strengths
of Yocto Project is the maintenance and quality of its code so I think
we need to improve our policy to avoid this to happen in future.

Cheers,

Patch

diff --git a/meta/recipes-graphics/mesa/mesa_9.1.6.bb b/meta/recipes-graphics/mesa/mesa_9.1.6.bb
index 6e9cd82..388cfd7 100644
--- a/meta/recipes-graphics/mesa/mesa_9.1.6.bb
+++ b/meta/recipes-graphics/mesa/mesa_9.1.6.bb
@@ -19,6 +19,8 @@  S = "${WORKDIR}/Mesa-${PV}"
 #make eglplatform.h independent of MESA_EGL_NO_X11_HEADER
 do_install_append() {
     if ${@base_contains('PACKAGECONFIG', 'egl', 'true', 'false', d)}; then
-        sed -i -e 's/^#ifdef MESA_EGL_NO_X11_HEADERS/#if ${@base_contains('DISTRO_FEATURES', 'x11', '0', '1', d)}/' ${D}${includedir}/EGL/eglplatform.h
+        if [ -e "${D}${includedir}/EGL/eglplatform.h" ]; then
+            sed -i -e 's/^#ifdef MESA_EGL_NO_X11_HEADERS/#if ${@base_contains('DISTRO_FEATURES', 'x11', '0', '1', d)}/' ${D}${includedir}/EGL/eglplatform.h
+        fi
     fi
 }
\ No newline at end of file
diff --git a/meta/recipes-graphics/mesa/mesa_git.bb b/meta/recipes-graphics/mesa/mesa_git.bb
index 1babcc0..714911f 100644
--- a/meta/recipes-graphics/mesa/mesa_git.bb
+++ b/meta/recipes-graphics/mesa/mesa_git.bb
@@ -23,6 +23,8 @@  S = "${WORKDIR}/git"
 #make eglplatform.h independent of MESA_EGL_NO_X11_HEADER
 do_install_append() {
     if ${@base_contains('PACKAGECONFIG', 'egl', 'true', 'false', d)}; then
-        sed -i -e 's/^#ifdef MESA_EGL_NO_X11_HEADERS/#if ${@base_contains('DISTRO_FEATURES', 'x11', '0', '1', d)}/' ${D}${includedir}/EGL/eglplatform.h
+        if [ -e "${D}${includedir}/EGL/eglplatform.h" ]; then
+            sed -i -e 's/^#ifdef MESA_EGL_NO_X11_HEADERS/#if ${@base_contains('DISTRO_FEATURES', 'x11', '0', '1', d)}/' ${D}${includedir}/EGL/eglplatform.h
+        fi
     fi
 }