[PATCH] virglrenderer: correct REQUIRED_DISTRO_FEATURES

[PATCH] virglrenderer: correct REQUIRED_DISTRO_FEATURES

[Yu, Mingli]

From: Mingli Yu <mingli.yu@windriver.com>

Add opengl to REQUIRED_DISTRO_FEATURES only for target build.

Signed-off-by: Mingli Yu <mingli.yu@windriver.com>
---
 meta/recipes-graphics/virglrenderer/virglrenderer_0.8.2.bb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meta/recipes-graphics/virglrenderer/virglrenderer_0.8.2.bb b/meta/recipes-graphics/virglrenderer/virglrenderer_0.8.2.bb
index 29b12628d0..d97f0e78f1 100644
--- a/meta/recipes-graphics/virglrenderer/virglrenderer_0.8.2.bb
+++ b/meta/recipes-graphics/virglrenderer/virglrenderer_0.8.2.bb
@@ -21,4 +21,4 @@ inherit meson pkgconfig features_check
 
 BBCLASSEXTEND = "native nativesdk"
 
-REQUIRED_DISTRO_FEATURES = "opengl"
+REQUIRED_DISTRO_FEATURES_class-target = "opengl"
-- 
2.24.1

Re: [OE-core] [PATCH] virglrenderer: correct REQUIRED_DISTRO_FEATURES

[Alexander Kanavin]

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

Please explain why?

Alex

On Tue 9. Jun 2020 at 9.17, Yu, Mingli <mingli.yu@windriver.com> wrote:

> From: Mingli Yu <mingli.yu@windriver.com>
>
> Add opengl to REQUIRED_DISTRO_FEATURES only for target build.
>
> Signed-off-by: Mingli Yu <mingli.yu@windriver.com>
> ---
>  meta/recipes-graphics/virglrenderer/virglrenderer_0.8.2.bb | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/meta/recipes-graphics/virglrenderer/virglrenderer_0.8.2.bb
> b/meta/recipes-graphics/virglrenderer/virglrenderer_0.8.2.bb
> index 29b12628d0..d97f0e78f1 100644
> --- a/meta/recipes-graphics/virglrenderer/virglrenderer_0.8.2.bb
> +++ b/meta/recipes-graphics/virglrenderer/virglrenderer_0.8.2.bb
> @@ -21,4 +21,4 @@ inherit meson pkgconfig features_check
>
>  BBCLASSEXTEND = "native nativesdk"
>
> -REQUIRED_DISTRO_FEATURES = "opengl"
> +REQUIRED_DISTRO_FEATURES_class-target = "opengl"
> --
> 2.24.1
>
> 
>

[-- Attachment #2: Type: text/html, Size: 1931 bytes --]

Re: [OE-core] [PATCH] virglrenderer: correct REQUIRED_DISTRO_FEATURES

[Denys Dmytriyenko]

+1

The commit message should explain "why" and not "what", as "what" can be 
easily understood from the code itself.

Sometimes "why" is obvious - e.g. version update of CVE fix, but in many cases 
it's not at all clear why do you want everyone to be affected by your change.

PS. https://xkcd.com/1296/

-- 
Denys


On Tue, Jun 09, 2020 at 09:40:42AM +0200, Alexander Kanavin wrote:
> Please explain why?
> 
> Alex
> 
> On Tue 9. Jun 2020 at 9.17, Yu, Mingli <mingli.yu@windriver.com> wrote:
> 
> > From: Mingli Yu <mingli.yu@windriver.com>
> >
> > Add opengl to REQUIRED_DISTRO_FEATURES only for target build.
> >
> > Signed-off-by: Mingli Yu <mingli.yu@windriver.com>
> > ---
> >  meta/recipes-graphics/virglrenderer/virglrenderer_0.8.2.bb | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/meta/recipes-graphics/virglrenderer/virglrenderer_0.8.2.bb
> > b/meta/recipes-graphics/virglrenderer/virglrenderer_0.8.2.bb
> > index 29b12628d0..d97f0e78f1 100644
> > --- a/meta/recipes-graphics/virglrenderer/virglrenderer_0.8.2.bb
> > +++ b/meta/recipes-graphics/virglrenderer/virglrenderer_0.8.2.bb
> > @@ -21,4 +21,4 @@ inherit meson pkgconfig features_check
> >
> >  BBCLASSEXTEND = "native nativesdk"
> >
> > -REQUIRED_DISTRO_FEATURES = "opengl"
> > +REQUIRED_DISTRO_FEATURES_class-target = "opengl"
> > --
> > 2.24.1
> >
> > 
> >

> 

Re: [OE-core] [PATCH] virglrenderer: correct REQUIRED_DISTRO_FEATURES

[Yu, Mingli]

Hi Alex and Denys,

On 6/10/20 5:50 AM, Denys Dmytriyenko wrote:
> +1
> 
> The commit message should explain "why" and not "what", as "what" can be
> easily understood from the code itself.

In fact, the DISTRO_FEATURES setting shouldn't affect native build.

Initially this patch is used to fix below error when opengl not defined 
in DISTRO_FEATURES in my build environment.
$ bitbake virglrenderer-native
virglrenderer-native was skipped: missing required distro feature 
'opengl' (not in DISTRO_FEATURES)

And the value defined in DISTRO_FEATURES is only translated to the 
appropriate option supplied to the configure script during the 
do_configure task for recipes built for the target.

Thanks,
Mingli


> 
> Sometimes "why" is obvious - e.g. version update of CVE fix, but in many cases
> it's not at all clear why do you want everyone to be affected by your change.
> 
> PS. https://xkcd.com/1296/
> 
> 
> 
> 

Re: [OE-core] [PATCH] virglrenderer: correct REQUIRED_DISTRO_FEATURES

[Alexander Kanavin]

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

On Wed, 10 Jun 2020 at 04:10, Yu, Mingli <mingli.yu@windriver.com> wrote:

> Hi Alex and Denys,
>
> On 6/10/20 5:50 AM, Denys Dmytriyenko wrote:
> > +1
> >
> > The commit message should explain "why" and not "what", as "what" can be
> > easily understood from the code itself.
>
> In fact, the DISTRO_FEATURES setting shouldn't affect native build.
>
> Initially this patch is used to fix below error when opengl not defined
> in DISTRO_FEATURES in my build environment.
> $ bitbake virglrenderer-native
> virglrenderer-native was skipped: missing required distro feature
> 'opengl' (not in DISTRO_FEATURES)
>

This is not an error, but expected behavior. virglrenderer-native only
works when opengl is in DISTRO_FEATURES and should be skipped otherwise.

Alex

[-- Attachment #2: Type: text/html, Size: 1176 bytes --]

Re: [OE-core] [PATCH] virglrenderer: correct REQUIRED_DISTRO_FEATURES

[Yu, Mingli]

Hi Alex,

On 6/10/20 4:25 PM, Alexander Kanavin wrote:
> On Wed, 10 Jun 2020 at 04:10, Yu, Mingli <mingli.yu@windriver.com 
> <mailto:mingli.yu@windriver.com>> wrote:
> 
>     Hi Alex and Denys,
> 
>     On 6/10/20 5:50 AM, Denys Dmytriyenko wrote:
>      > +1
>      >
>      > The commit message should explain "why" and not "what", as "what"
>     can be
>      > easily understood from the code itself.
> 
>     In fact, the DISTRO_FEATURES setting shouldn't affect native build.
> 
>     Initially this patch is used to fix below error when opengl not defined
>     in DISTRO_FEATURES in my build environment.
>     $ bitbake virglrenderer-native
>     virglrenderer-native was skipped: missing required distro feature
>     'opengl' (not in DISTRO_FEATURES)
> 
> 
> This is not an error, but expected behavior. virglrenderer-native only 
> works when opengl is in DISTRO_FEATURES and should be skipped otherwise.

Does it mean the DISTRO_FEATURES setting also affects the native build? 
If so, please ignore this patch.

Thanks,

> 
> Alex

Re: [OE-core] [PATCH] virglrenderer: correct REQUIRED_DISTRO_FEATURES

[Alexander Kanavin]

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

On Thu, 11 Jun 2020 at 09:22, Yu, Mingli <mingli.yu@windriver.com> wrote:

> > This is not an error, but expected behavior. virglrenderer-native only
> > works when opengl is in DISTRO_FEATURES and should be skipped otherwise.
>
> Does it mean the DISTRO_FEATURES setting also affects the native build?
> If so, please ignore this patch.
>

By default it does not, but for specific items listed in bitbake.conf it
does:

# Normally target distro features will not be applied to native builds:
# Native distro features on this list will use the target feature value
DISTRO_FEATURES_FILTER_NATIVE ?= "api-documentation opengl"
DISTRO_FEATURES_FILTER_NATIVESDK ?= "api-documentation opengl"

Alex

[-- Attachment #2: Type: text/html, Size: 1082 bytes --]

Re: [OE-core] [PATCH] virglrenderer: correct REQUIRED_DISTRO_FEATURES

[Yu, Mingli]

On 6/11/20 4:14 PM, Alexander Kanavin wrote:
> On Thu, 11 Jun 2020 at 09:22, Yu, Mingli <mingli.yu@windriver.com 
> <mailto:mingli.yu@windriver.com>> wrote:
> 
>      > This is not an error, but expected behavior. virglrenderer-native
>     only
>      > works when opengl is in DISTRO_FEATURES and should be skipped
>     otherwise.
> 
>     Does it mean the DISTRO_FEATURES setting also affects the native build?
>     If so, please ignore this patch.
> 
> 
> By default it does not, but for specific items listed in bitbake.conf it 
> does:
> 
> # Normally target distro features will not be applied to native builds:
> # Native distro features on this list will use the target feature value
> DISTRO_FEATURES_FILTER_NATIVE ?= "api-documentation opengl"
> DISTRO_FEATURES_FILTER_NATIVESDK ?= "api-documentation opengl"

Understood, many thanks your explanation!

Thanks,

> 
> Alex