Patchwork update-rc.d.bbclass: fix inhibit check

login
register
mail settings
Submitter Kang Kai
Date Feb. 19, 2014, 10:59 a.m.
Message ID <1392807566-15114-1-git-send-email-kai.kang@windriver.com>
Download mbox | patch
Permalink /patch/66975/
State Accepted
Commit 27b81efd3aa35f27456820634b5bbb0f764c7e8f
Headers show

Comments

Kang Kai - Feb. 19, 2014, 10:59 a.m.
In update-rc.d.bbclass it checks variable INITSCRIPT_PACKAGES to avoid
inherit this class. But it is wrong logic to check INITSCRIPT_PACKAGES.
When 'sysvinit' is in 'DISTRO_FEATURES', INITSCRIPT_PACKAGES will not be
checked.

Replace 'or' with 'and' to fix it.

Signed-off-by: Kai Kang <kai.kang@windriver.com>
---
 meta/classes/update-rc.d.bbclass | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Phil Blundell - Feb. 19, 2014, 11:43 a.m.
On Wed, 2014-02-19 at 18:59 +0800, Kai Kang wrote:
> In update-rc.d.bbclass it checks variable INITSCRIPT_PACKAGES to avoid
> inherit this class. But it is wrong logic to check INITSCRIPT_PACKAGES.
> When 'sysvinit' is in 'DISTRO_FEATURES', INITSCRIPT_PACKAGES will not be
> checked.

Er, really?  From the code you quoted...

>      # Check that this class isn't being inhibited (generally, by
>      # systemd.bbclass) before doing any work.
> -    if oe.utils.contains('DISTRO_FEATURES', 'sysvinit', True, False, d) or \
> +    if oe.utils.contains('DISTRO_FEATURES', 'sysvinit', True, False, d) and \
>         not d.getVar("INHIBIT_UPDATERCD_BBCLASS", True):
>          pkgs = d.getVar('INITSCRIPT_PACKAGES', True)
>          if pkgs == None:

... it seems that if sysvinit is in DISTRO_FEATURES then the outer "if"
will evaluate to true and it will indeed proceed to check
INITSCRIPT_PACKAGES in the last quoted line.  Can you clarify what
exactly is wrong here?

p.
Laurentiu Palcu - Feb. 19, 2014, 12:19 p.m.
On Wed, Feb 19, 2014 at 06:59:26PM +0800, Kai Kang wrote:
> In update-rc.d.bbclass it checks variable INITSCRIPT_PACKAGES to avoid
> inherit this class. But it is wrong logic to check INITSCRIPT_PACKAGES.
> When 'sysvinit' is in 'DISTRO_FEATURES', INITSCRIPT_PACKAGES will not be
> checked.
s/INITSCRIPT_PACKAGES/INHIBIT_UPDATERCD_BBCLASS/ above will make more
sense, I guess.

Because, as I see it, that 'if' will evaluate to True if 'sysvinit' is in
DISTRO_FEATURES, irrespective of how INHIBIT_UPDATERCD_BBCLASS is set...
which is a little bit wrong.

So, I think you just need to adjust the commit message.

laurentiu

> 
> Replace 'or' with 'and' to fix it.
> 
> Signed-off-by: Kai Kang <kai.kang@windriver.com>
> ---
>  meta/classes/update-rc.d.bbclass | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/meta/classes/update-rc.d.bbclass b/meta/classes/update-rc.d.bbclass
> index 4b92d8d..0ac2af7 100644
> --- a/meta/classes/update-rc.d.bbclass
> +++ b/meta/classes/update-rc.d.bbclass
> @@ -117,7 +117,7 @@ python populate_packages_updatercd () {
>  
>      # Check that this class isn't being inhibited (generally, by
>      # systemd.bbclass) before doing any work.
> -    if oe.utils.contains('DISTRO_FEATURES', 'sysvinit', True, False, d) or \
> +    if oe.utils.contains('DISTRO_FEATURES', 'sysvinit', True, False, d) and \
>         not d.getVar("INHIBIT_UPDATERCD_BBCLASS", True):
>          pkgs = d.getVar('INITSCRIPT_PACKAGES', True)
>          if pkgs == None:
> -- 
> 1.8.1.2
> 
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core
Kang Kai - Feb. 20, 2014, 1:32 a.m.
On 2014?02?19? 19:43, Phil Blundell wrote:
> On Wed, 2014-02-19 at 18:59 +0800, Kai Kang wrote:
>> In update-rc.d.bbclass it checks variable INITSCRIPT_PACKAGES to avoid
>> inherit this class. But it is wrong logic to check INITSCRIPT_PACKAGES.
>> When 'sysvinit' is in 'DISTRO_FEATURES', INITSCRIPT_PACKAGES will not be
>> checked.
> Er, really?  From the code you quoted...
>
>>       # Check that this class isn't being inhibited (generally, by
>>       # systemd.bbclass) before doing any work.
>> -    if oe.utils.contains('DISTRO_FEATURES', 'sysvinit', True, False, d) or \
>> +    if oe.utils.contains('DISTRO_FEATURES', 'sysvinit', True, False, d) and \
>>          not d.getVar("INHIBIT_UPDATERCD_BBCLASS", True):
>>           pkgs = d.getVar('INITSCRIPT_PACKAGES', True)
>>           if pkgs == None:
> ... it seems that if sysvinit is in DISTRO_FEATURES then the outer "if"
> will evaluate to true and it will indeed proceed to check
> INITSCRIPT_PACKAGES in the last quoted line.  Can you clarify what
> exactly is wrong here?

Sorry, as laurentiu comments, it is INHIBIT_UPDATERCD_BBCLASS which 
variable will not be checked.

I'll update the comment with V2.

Regards,
Kai

>
> p.
>
>
>
>

Patch

diff --git a/meta/classes/update-rc.d.bbclass b/meta/classes/update-rc.d.bbclass
index 4b92d8d..0ac2af7 100644
--- a/meta/classes/update-rc.d.bbclass
+++ b/meta/classes/update-rc.d.bbclass
@@ -117,7 +117,7 @@  python populate_packages_updatercd () {
 
     # Check that this class isn't being inhibited (generally, by
     # systemd.bbclass) before doing any work.
-    if oe.utils.contains('DISTRO_FEATURES', 'sysvinit', True, False, d) or \
+    if oe.utils.contains('DISTRO_FEATURES', 'sysvinit', True, False, d) and \
        not d.getVar("INHIBIT_UPDATERCD_BBCLASS", True):
         pkgs = d.getVar('INITSCRIPT_PACKAGES', True)
         if pkgs == None: