classes/insane: add check for PN in OVERRIDES

classes/insane: add check for PN in OVERRIDES

[Mikhail Durnev]

Hello Paul,

I have a question about your commit c331f0a:

-------------------------------------------
diff --git a/meta/classes/insane.bbclass b/meta/classes/insane.bbclass
index 4d2392e..fb18022 100644
--- a/meta/classes/insane.bbclass
+++ b/meta/classes/insane.bbclass
@@ -907,6 +907,11 @@ python () {
      if d.getVar('do_stage', True) is not None:
          bb.fatal("Legacy staging found for %s as it has a do_stage 
function. This will need conversion to a do_install or often simply 
removal to work with OE-core" % d.getVar("FILE", True))

+    overrides = d.getVar('OVERRIDES', True).split(':')
+    pn = d.getVar('PN', True)
+    if pn in overrides:
+        bb.warn('Recipe %s has PN of "%s" which is in OVERRIDES, this 
can result in unexpected behaviour.' % (d.getVar("FILE", True), pn))
+
      issues = []
      if (d.getVar('PACKAGES', True) or "").split():
          for var in 'RDEPENDS', 'RRECOMMENDS', 'RSUGGESTS', 
'RCONFLICTS', 'RPROVIDES', 'RREPLACES', 'FILES', 'pkg_preinst', 
'pkg_postinst', 'pkg_prerm', 'pkg_postrm', 'ALLOW_EMPTY':
----------------------------------------------

A recipe (lets say linux_git.bb) has its PN in OVERRIDES with prefix 
'pn-' (pn-linux). So, if we have 'linux' in OVERRIDES then it cannot 
interfere. Should we search for pn-${PN} instead? E.g.:

     overrides = d.getVar('OVERRIDES', True).split(':')
     pn = d.getVar('PN', True)
     if ("pn-%s" % pn) in overrides:
         bb.warn('Recipe %s has PN of "%s" which is in OVERRIDES, this 
can result in unexpected behaviour.' % (d.getVar("FILE", True), pn))

Thanks,
Mikhail

Re: classes/insane: add check for PN in OVERRIDES

[Paul Eggleton]

Hi Mikhail,

On Tuesday 27 August 2013 17:36:03 Mikhail Durnev wrote:
> I have a question about your commit c331f0a:
> 
> -------------------------------------------
> diff --git a/meta/classes/insane.bbclass b/meta/classes/insane.bbclass
> index 4d2392e..fb18022 100644
> --- a/meta/classes/insane.bbclass
> +++ b/meta/classes/insane.bbclass
> @@ -907,6 +907,11 @@ python () {
>       if d.getVar('do_stage', True) is not None:
>           bb.fatal("Legacy staging found for %s as it has a do_stage
> function. This will need conversion to a do_install or often simply
> removal to work with OE-core" % d.getVar("FILE", True))
> 
> +    overrides = d.getVar('OVERRIDES', True).split(':')
> +    pn = d.getVar('PN', True)
> +    if pn in overrides:
> +        bb.warn('Recipe %s has PN of "%s" which is in OVERRIDES, this
> can result in unexpected behaviour.' % (d.getVar("FILE", True), pn))
> +
>       issues = []
>       if (d.getVar('PACKAGES', True) or "").split():
>           for var in 'RDEPENDS', 'RRECOMMENDS', 'RSUGGESTS',
> 'RCONFLICTS', 'RPROVIDES', 'RREPLACES', 'FILES', 'pkg_preinst',
> 'pkg_postinst', 'pkg_prerm', 'pkg_postrm', 'ALLOW_EMPTY':
> ----------------------------------------------
> 
> A recipe (lets say linux_git.bb) has its PN in OVERRIDES with prefix
> 'pn-' (pn-linux). So, if we have 'linux' in OVERRIDES then it cannot
> interfere.

Unfortunately that is not the case. If PN is "linux" and TARGET_OS is "linux" 
(note, for ARM machines only TARGET_OS may be "linux-gnueabi" rather than 
"linux") then statements such as this:

FILES_${PN} = "/path/to/file"

will immediately become:

FILES = "/path/to/file"

Which itself will trigger a warning about FILES not being package specific. 
Without an explicit check on PN being in OVERRIDES it's not immediately 
obvious what has happened.

Additionally consider the following case in local.conf:

PREFERRED_VERSION_linux = "3.8.10"

This will turn into PREFERRED_VERSION = "3.8.10" and then you'll get a bunch 
of spewed notices about version "3.8.10" being unavailable for every recipe in 
the system that you're building.

PN in OVERRIDES is bad news, which is why we added this warning. You really do 
need to name your recipes so that they don't clash with items in OVERRIDES.

Cheers,
Paul

-- 

Paul Eggleton
Intel Open Source Technology Centre

Re: classes/insane: add check for PN in OVERRIDES

[Mikhail Durnev]

Hi Paul,

28.08.2013 19:41, Paul Eggleton wrote:
> Hi Mikhail,
>
> On Tuesday 27 August 2013 17:36:03 Mikhail Durnev wrote:
>> I have a question about your commit c331f0a:
>>
>> -------------------------------------------
>> diff --git a/meta/classes/insane.bbclass b/meta/classes/insane.bbclass
>> index 4d2392e..fb18022 100644
>> --- a/meta/classes/insane.bbclass
>> +++ b/meta/classes/insane.bbclass
>> @@ -907,6 +907,11 @@ python () {
>>        if d.getVar('do_stage', True) is not None:
>>            bb.fatal("Legacy staging found for %s as it has a do_stage
>> function. This will need conversion to a do_install or often simply
>> removal to work with OE-core" % d.getVar("FILE", True))
>>
>> +    overrides = d.getVar('OVERRIDES', True).split(':')
>> +    pn = d.getVar('PN', True)
>> +    if pn in overrides:
>> +        bb.warn('Recipe %s has PN of "%s" which is in OVERRIDES, this
>> can result in unexpected behaviour.' % (d.getVar("FILE", True), pn))
>> +
>>        issues = []
>>        if (d.getVar('PACKAGES', True) or "").split():
>>            for var in 'RDEPENDS', 'RRECOMMENDS', 'RSUGGESTS',
>> 'RCONFLICTS', 'RPROVIDES', 'RREPLACES', 'FILES', 'pkg_preinst',
>> 'pkg_postinst', 'pkg_prerm', 'pkg_postrm', 'ALLOW_EMPTY':
>> ----------------------------------------------
>>
>> A recipe (lets say linux_git.bb) has its PN in OVERRIDES with prefix
>> 'pn-' (pn-linux). So, if we have 'linux' in OVERRIDES then it cannot
>> interfere.
> Unfortunately that is not the case. If PN is "linux" and TARGET_OS is "linux"
> (note, for ARM machines only TARGET_OS may be "linux-gnueabi" rather than
> "linux") then statements such as this:
>
> FILES_${PN} = "/path/to/file"
>
> will immediately become:
>
> FILES = "/path/to/file"
>
> Which itself will trigger a warning about FILES not being package specific.
> Without an explicit check on PN being in OVERRIDES it's not immediately
> obvious what has happened.
>
> Additionally consider the following case in local.conf:
>
> PREFERRED_VERSION_linux = "3.8.10"
>
> This will turn into PREFERRED_VERSION = "3.8.10" and then you'll get a bunch
> of spewed notices about version "3.8.10" being unavailable for every recipe in
> the system that you're building.
>
> PN in OVERRIDES is bad news, which is why we added this warning. You really do
> need to name your recipes so that they don't clash with items in OVERRIDES.
>
> Cheers,
> Paul
>
Thank you for clarification. I build x86. My TARGET_OS, kernel recipe 
name and kernel PN is "linux". Fortunately, kernel.bbclass uses PN only 
once in:

FILES_${PN} = ""

For "linux" the problem is masked by usage of "virtual/kernel" in 
conditions. So I see only your warning about PN in OVERRIDES and no 
other warnings or problems. But I definitely should consider about 
changing PN.

Thanks,
Mikhail