[meta-browser][PATCH v2] chromium: List all PACKAGECONFIG settings to avoid warning

[meta-browser][PATCH v2] chromium: List all PACKAGECONFIG settings to avoid warning

[Gary Thomas]

OE-core now warns if PACKAGECONFIG is used to set an option that
does not have a corresponding PACKAGECONFIG[option]="xxx" line.
This recipe makes use of many such options & this patch suppresses
those warnings by listing the options.

Signed-off-by: Gary Thomas <gary@mlbassoc.com>
---
 recipes-browser/chromium/chromium.inc | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/recipes-browser/chromium/chromium.inc b/recipes-browser/chromium/chromium.inc
index 8e83f6a..973d1c9 100644
--- a/recipes-browser/chromium/chromium.inc
+++ b/recipes-browser/chromium/chromium.inc
@@ -17,6 +17,13 @@ PACKAGECONFIG ??= "use-egl"
 # automatically and silently fall back to GLX
 PACKAGECONFIG[use-egl] = ",,virtual/egl virtual/libgles2"
 
+# Additional PACKAGECONFIG options - listed here to avoid warnings
+PACKAGECONFIG[component-build] = ""
+PACKAGECONFIG[disable-api-keys-info-bar] = ""
+PACKAGECONFIG[ignore-lost-context] = ""
+PACKAGECONFIG[impl-side-painting] = ""
+PACKAGECONFIG[use-gl] = ""
+
 GYP_DEFINES += "${ARMFPABI} release_extra_cflags='-Wno-error=unused-local-typedefs' sysroot=''"
 
 do_configure() {
-- 
1.9.1

Re: [meta-browser][PATCH v2] chromium: List all PACKAGECONFIG settings to avoid warning

[Robert Yang]

Hi Gary,

On 07/01/2015 04:55 AM, Gary Thomas wrote:
> OE-core now warns if PACKAGECONFIG is used to set an option that
> does not have a corresponding PACKAGECONFIG[option]="xxx" line.
> This recipe makes use of many such options & this patch suppresses
> those warnings by listing the options.
>
> Signed-off-by: Gary Thomas <gary@mlbassoc.com>
> ---
>   recipes-browser/chromium/chromium.inc | 7 +++++++
>   1 file changed, 7 insertions(+)
>
> diff --git a/recipes-browser/chromium/chromium.inc b/recipes-browser/chromium/chromium.inc
> index 8e83f6a..973d1c9 100644
> --- a/recipes-browser/chromium/chromium.inc
> +++ b/recipes-browser/chromium/chromium.inc
> @@ -17,6 +17,13 @@ PACKAGECONFIG ??= "use-egl"
>   # automatically and silently fall back to GLX
>   PACKAGECONFIG[use-egl] = ",,virtual/egl virtual/libgles2"
>
> +# Additional PACKAGECONFIG options - listed here to avoid warnings
> +PACKAGECONFIG[component-build] = ""
> +PACKAGECONFIG[disable-api-keys-info-bar] = ""
> +PACKAGECONFIG[ignore-lost-context] = ""
> +PACKAGECONFIG[impl-side-painting] = ""
> +PACKAGECONFIG[use-gl] = ""

Since you don't really use PACKAGECONFIG, why not use
ANY_OTHER_VALUE_YOU_LIKE rather than PACKAGECONFIG to configure it ?

// Robert


> +
>   GYP_DEFINES += "${ARMFPABI} release_extra_cflags='-Wno-error=unused-local-typedefs' sysroot=''"
>
>   do_configure() {
>

Re: [meta-browser][PATCH v2] chromium: List all PACKAGECONFIG settings to avoid warning

[Gary Thomas]

On 2015-07-01 02:22, Robert Yang wrote:
> Hi Gary,
>
> On 07/01/2015 04:55 AM, Gary Thomas wrote:
>> OE-core now warns if PACKAGECONFIG is used to set an option that
>> does not have a corresponding PACKAGECONFIG[option]="xxx" line.
>> This recipe makes use of many such options & this patch suppresses
>> those warnings by listing the options.
>>
>> Signed-off-by: Gary Thomas <gary@mlbassoc.com>
>> ---
>>   recipes-browser/chromium/chromium.inc | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/recipes-browser/chromium/chromium.inc b/recipes-browser/chromium/chromium.inc
>> index 8e83f6a..973d1c9 100644
>> --- a/recipes-browser/chromium/chromium.inc
>> +++ b/recipes-browser/chromium/chromium.inc
>> @@ -17,6 +17,13 @@ PACKAGECONFIG ??= "use-egl"
>>   # automatically and silently fall back to GLX
>>   PACKAGECONFIG[use-egl] = ",,virtual/egl virtual/libgles2"
>>
>> +# Additional PACKAGECONFIG options - listed here to avoid warnings
>> +PACKAGECONFIG[component-build] = ""
>> +PACKAGECONFIG[disable-api-keys-info-bar] = ""
>> +PACKAGECONFIG[ignore-lost-context] = ""
>> +PACKAGECONFIG[impl-side-painting] = ""
>> +PACKAGECONFIG[use-gl] = ""
>
> Since you don't really use PACKAGECONFIG, why not use
> ANY_OTHER_VALUE_YOU_LIKE rather than PACKAGECONFIG to configure it ?

No, it's much better to use the standard mechanism (PACKAGECONFIG) rather
than making up something special for this recipe.  The patch is needed only
to suppress warnings about how it's being used.

>> +
>>   GYP_DEFINES += "${ARMFPABI} release_extra_cflags='-Wno-error=unused-local-typedefs' sysroot=''"
>>
>>   do_configure() {
>>

-- 
------------------------------------------------------------
Gary Thomas                 |  Consulting for the
MLB Associates              |    Embedded world
------------------------------------------------------------

Re: [meta-browser][PATCH v2] chromium: List all PACKAGECONFIG settings to avoid warning

[Burton, Ross]

On 1 July 2015 at 15:12, Gary Thomas <gary@mlbassoc.com> wrote:

> No, it's much better to use the standard mechanism (PACKAGECONFIG) rather
> than making up something special for this recipe.  The patch is needed only
> to suppress warnings about how it's being used.
>

I kinda of agree with Robert here - the standard method isn't being used,
but the variable is being used.

As the chromium recipe doesn't inherit autotools EXTRA_OECONF will only be
set by the PACKAGECONFIG handler, so it would be an improvement if the
enable/disable arguments were specified as usual in the flags and then
EXTRA_OEGYP just included EXTRA_OECONF.  (untested but might work, cmake
recipes certainly did this)

Ross

Re: [meta-browser][PATCH v2] chromium: List all PACKAGECONFIG settings to avoid warning

[Gary Thomas]

On 2015-07-01 09:47, Burton, Ross wrote:
> On 1 July 2015 at 15:12, Gary Thomas <gary@mlbassoc.com> wrote:
>
>> No, it's much better to use the standard mechanism (PACKAGECONFIG) rather
>> than making up something special for this recipe.  The patch is needed only
>> to suppress warnings about how it's being used.
>>
>
> I kinda of agree with Robert here - the standard method isn't being used,
> but the variable is being used.

Actually, it *is* using PACKAGECONFIG correctly for some of the settings.

>
> As the chromium recipe doesn't inherit autotools EXTRA_OECONF will only be
> set by the PACKAGECONFIG handler, so it would be an improvement if the
> enable/disable arguments were specified as usual in the flags and then
> EXTRA_OEGYP just included EXTRA_OECONF.  (untested but might work, cmake
> recipes certainly did this)

I didn't write this recipe and just wanted to reduce the warnings.
If someone else wants to rework it, that would be great.

-- 
------------------------------------------------------------
Gary Thomas                 |  Consulting for the
MLB Associates              |    Embedded world
------------------------------------------------------------

Re: [meta-browser][PATCH v2] chromium: List all PACKAGECONFIG settings to avoid warning

[Max Krummenacher]

Hi

Am Mittwoch, den 01.07.2015, 16:47 +0100 schrieb Burton, Ross:
> On 1 July 2015 at 15:12, Gary Thomas <gary@mlbassoc.com> wrote:
> 
> > No, it's much better to use the standard mechanism (PACKAGECONFIG) rather
> > than making up something special for this recipe.  The patch is needed only
> > to suppress warnings about how it's being used.
> >
> 
> I kinda of agree with Robert here - the standard method isn't being used,
> but the variable is being used.
> 
> As the chromium recipe doesn't inherit autotools EXTRA_OECONF will only be
> set by the PACKAGECONFIG handler, so it would be an improvement if the
> enable/disable arguments were specified as usual in the flags and then
> EXTRA_OEGYP just included EXTRA_OECONF.  (untested but might work, cmake
> recipes certainly did this)
> 
> Ross

I think that this view is guided by an inside view on PACKAGECONFIG.

For the guys wanting to influencing the build of a package they want to
set a list of options and then forget about it.
So having to write in your bbappend something like

PACKAGECONFIG += "use-egl"
PACKAGECONFIG_NON_CONFIGOPTION += "component-build"

instead of
PACKAGECONFIG += "use-egl component-build"

is sort of unintutive. 

I guess the warning which now pops up when not setting a value in
PACKAGECONFIG which does not have a corresponding PACKAGECONFIG[value]
is meant to warn of a typo in setting PACKAGECONFIG.
Gary's solution would even provide the same warning mechanism for free
while when moving to a PACKAGECONFIG_NON_CONFIGOPTION one would have to
implement another test to get a warning when providing a non supported
value to PACKAGECONFIG_NON_CONFIGOPTION.

Just my two cents.

Regards
Max