[PATCH] sstate: Add optimizing logic for crosssdk setscene dependencies

[PATCH] sstate: Add optimizing logic for crosssdk setscene dependencies

[Ming Liu]

This patch mainly aims to add optimisation for crosssdk setscene dependency
validating which we haven't handled in current logic, and which I think we
could have as we've already implemented to native/cross, although there
are albeit not many crossdk tasks, we could still get some performance
enhancement.

And it also fix a vulnerability of some certain workflow, think about the
following scenario with current logic:
    bitbake nativesdk-eglibc-initial -c cleansstate
    bitbake gcc-crosssdk-initial -c clean
    bitbake gmp-native -c clean
    bitbake libmpc-native -c clean
    bitbake mpfr-native -c clean
    bitbake gcc-crosssdk-initial
    bitbake nativesdk-eglibc-initial

Aboving will fail for absence of a few native libraries required by
gcc-crosssdk-initial.

Also modified some places in current code except the optimisation, as
following:
1 Remove isNative function since no code is referring it.
2 Add do_package to the list that don't exist and are noexec.

Signed-off-by: Ming Liu <ming.liu@windriver.com>
---
 meta/classes/sstate.bbclass | 24 +++++++++++-------------
 1 file changed, 11 insertions(+), 13 deletions(-)

diff --git a/meta/classes/sstate.bbclass b/meta/classes/sstate.bbclass
index 517c100..49f92f9 100644
--- a/meta/classes/sstate.bbclass
+++ b/meta/classes/sstate.bbclass
@@ -666,12 +666,10 @@ def setscene_depvalid(task, taskdependees, notneeded, d):
 
     bb.debug(2, "Considering setscene task: %s" % (str(taskdependees[task])))
 
-    def isNative(x):
-        return x.endswith("-native")
-    def isNativeCross(x):
-        return x.endswith("-native") or x.endswith("-cross") or x.endswith("-cross-initial")
+    def isNativeCrossCrosssdk(x):
+        return x.endswith("-native") or x.endswith("-cross") or x.endswith("-cross-initial") or x.endswith("-crosssdk") or x.endswith("-crosssdk-initial")
     def isSafeDep(x):
-        if x in ["quilt-native", "autoconf-native", "automake-native", "gnu-config-native", "libtool-native", "pkgconfig-native", "gcc-cross", "binutils-cross", "gcc-cross-initial"]:
+        if x in ["quilt-native", "autoconf-native", "automake-native", "gnu-config-native", "libtool-native", "pkgconfig-native", "gcc-cross", "binutils-cross", "gcc-cross-initial", "gcc-crosssdk", "binutils-crosssdk", "gcc-crosssdk-initial"]:
             return True
         return False
     def isPostInstDep(x):
@@ -701,8 +699,8 @@ def setscene_depvalid(task, taskdependees, notneeded, d):
             if isPostInstDep(taskdependees[task][0]) and taskdependees[dep][1] in ['do_package_write_deb', 'do_package_write_ipk', 'do_package_write_rpm']:
                 return False
             continue
-        # Native/Cross packages don't exist and are noexec anyway
-        if isNativeCross(taskdependees[dep][0]) and taskdependees[dep][1] in ['do_package_write_deb', 'do_package_write_ipk', 'do_package_write_rpm', 'do_packagedata']:
+        # Native/Cross/Crosssdk packages don't exist and are noexec anyway
+        if isNativeCrossCrosssdk(taskdependees[dep][0]) and taskdependees[dep][1] in ['do_package_write_deb', 'do_package_write_ipk', 'do_package_write_rpm', 'do_packagedata', 'do_package']:
             continue
 
         # Consider sysroot depending on sysroot tasks
@@ -713,14 +711,14 @@ def setscene_depvalid(task, taskdependees, notneeded, d):
             # Nothing need depend on libc-initial/gcc-cross-initial
             if taskdependees[task][0].endswith("-initial"):
                 continue
-            # Native/Cross populate_sysroot need their dependencies
-            if isNativeCross(taskdependees[task][0]) and isNativeCross(taskdependees[dep][0]):
+            # Native/Cross/Crosssdk populate_sysroot need their dependencies
+            if isNativeCrossCrosssdk(taskdependees[task][0]) and isNativeCrossCrosssdk(taskdependees[dep][0]):
                 return False
-            # Target populate_sysroot depended on by cross tools need to be installed
-            if isNativeCross(taskdependees[dep][0]):
+            # Target/Nativesdk populate_sysroot depended on by cross/crosssdk tools need to be installed
+            if isNativeCrossCrosssdk(taskdependees[dep][0]):
                 return False
-            # Native/cross tools depended upon by target sysroot are not needed
-            if isNativeCross(taskdependees[task][0]):
+            # Native,Cross/Crosssdk tools depended upon by target/nativesdk sysroot are not needed
+            if isNativeCrossCrosssdk(taskdependees[task][0]):
                 continue
             # Target populate_sysroot need their dependencies
             return False
-- 
1.8.4.1

Re: [PATCH] sstate: Add optimizing logic for crosssdk setscene dependencies

[Ming Liu]

Hi, guys:

Would anybody take a tech review to my patch? Thanks!

the best,
thank you

On 11/14/2013 06:51 PM, Ming Liu wrote:
> This patch mainly aims to add optimisation for crosssdk setscene dependency
> validating which we haven't handled in current logic, and which I think we
> could have as we've already implemented to native/cross, although there
> are albeit not many crossdk tasks, we could still get some performance
> enhancement.
>
> And it also fix a vulnerability of some certain workflow, think about the
> following scenario with current logic:
>      bitbake nativesdk-eglibc-initial -c cleansstate
>      bitbake gcc-crosssdk-initial -c clean
>      bitbake gmp-native -c clean
>      bitbake libmpc-native -c clean
>      bitbake mpfr-native -c clean
>      bitbake gcc-crosssdk-initial
>      bitbake nativesdk-eglibc-initial
>
> Aboving will fail for absence of a few native libraries required by
> gcc-crosssdk-initial.
>
> Also modified some places in current code except the optimisation, as
> following:
> 1 Remove isNative function since no code is referring it.
> 2 Add do_package to the list that don't exist and are noexec.
>
> Signed-off-by: Ming Liu <ming.liu@windriver.com>
> ---
>   meta/classes/sstate.bbclass | 24 +++++++++++-------------
>   1 file changed, 11 insertions(+), 13 deletions(-)
>
> diff --git a/meta/classes/sstate.bbclass b/meta/classes/sstate.bbclass
> index 517c100..49f92f9 100644
> --- a/meta/classes/sstate.bbclass
> +++ b/meta/classes/sstate.bbclass
> @@ -666,12 +666,10 @@ def setscene_depvalid(task, taskdependees, notneeded, d):
>   
>       bb.debug(2, "Considering setscene task: %s" % (str(taskdependees[task])))
>   
> -    def isNative(x):
> -        return x.endswith("-native")
> -    def isNativeCross(x):
> -        return x.endswith("-native") or x.endswith("-cross") or x.endswith("-cross-initial")
> +    def isNativeCrossCrosssdk(x):
> +        return x.endswith("-native") or x.endswith("-cross") or x.endswith("-cross-initial") or x.endswith("-crosssdk") or x.endswith("-crosssdk-initial")
>       def isSafeDep(x):
> -        if x in ["quilt-native", "autoconf-native", "automake-native", "gnu-config-native", "libtool-native", "pkgconfig-native", "gcc-cross", "binutils-cross", "gcc-cross-initial"]:
> +        if x in ["quilt-native", "autoconf-native", "automake-native", "gnu-config-native", "libtool-native", "pkgconfig-native", "gcc-cross", "binutils-cross", "gcc-cross-initial", "gcc-crosssdk", "binutils-crosssdk", "gcc-crosssdk-initial"]:
>               return True
>           return False
>       def isPostInstDep(x):
> @@ -701,8 +699,8 @@ def setscene_depvalid(task, taskdependees, notneeded, d):
>               if isPostInstDep(taskdependees[task][0]) and taskdependees[dep][1] in ['do_package_write_deb', 'do_package_write_ipk', 'do_package_write_rpm']:
>                   return False
>               continue
> -        # Native/Cross packages don't exist and are noexec anyway
> -        if isNativeCross(taskdependees[dep][0]) and taskdependees[dep][1] in ['do_package_write_deb', 'do_package_write_ipk', 'do_package_write_rpm', 'do_packagedata']:
> +        # Native/Cross/Crosssdk packages don't exist and are noexec anyway
> +        if isNativeCrossCrosssdk(taskdependees[dep][0]) and taskdependees[dep][1] in ['do_package_write_deb', 'do_package_write_ipk', 'do_package_write_rpm', 'do_packagedata', 'do_package']:
>               continue
>   
>           # Consider sysroot depending on sysroot tasks
> @@ -713,14 +711,14 @@ def setscene_depvalid(task, taskdependees, notneeded, d):
>               # Nothing need depend on libc-initial/gcc-cross-initial
>               if taskdependees[task][0].endswith("-initial"):
>                   continue
> -            # Native/Cross populate_sysroot need their dependencies
> -            if isNativeCross(taskdependees[task][0]) and isNativeCross(taskdependees[dep][0]):
> +            # Native/Cross/Crosssdk populate_sysroot need their dependencies
> +            if isNativeCrossCrosssdk(taskdependees[task][0]) and isNativeCrossCrosssdk(taskdependees[dep][0]):
>                   return False
> -            # Target populate_sysroot depended on by cross tools need to be installed
> -            if isNativeCross(taskdependees[dep][0]):
> +            # Target/Nativesdk populate_sysroot depended on by cross/crosssdk tools need to be installed
> +            if isNativeCrossCrosssdk(taskdependees[dep][0]):
>                   return False
> -            # Native/cross tools depended upon by target sysroot are not needed
> -            if isNativeCross(taskdependees[task][0]):
> +            # Native,Cross/Crosssdk tools depended upon by target/nativesdk sysroot are not needed
> +            if isNativeCrossCrosssdk(taskdependees[task][0]):
>                   continue
>               # Target populate_sysroot need their dependencies
>               return False

Re: [PATCH] sstate: Add optimizing logic for crosssdk setscene dependencies

[Richard Purdie]

On Thu, 2013-11-14 at 18:51 +0800, Ming Liu wrote:
> This patch mainly aims to add optimisation for crosssdk setscene dependency
> validating which we haven't handled in current logic, and which I think we
> could have as we've already implemented to native/cross, although there
> are albeit not many crossdk tasks, we could still get some performance
> enhancement.
> 
> And it also fix a vulnerability of some certain workflow, think about the
> following scenario with current logic:
>     bitbake nativesdk-eglibc-initial -c cleansstate
>     bitbake gcc-crosssdk-initial -c clean
>     bitbake gmp-native -c clean
>     bitbake libmpc-native -c clean
>     bitbake mpfr-native -c clean
>     bitbake gcc-crosssdk-initial
>     bitbake nativesdk-eglibc-initial
> 
> Aboving will fail for absence of a few native libraries required by
> gcc-crosssdk-initial.
> 
> Also modified some places in current code except the optimisation, as
> following:
> 1 Remove isNative function since no code is referring it.
> 2 Add do_package to the list that don't exist and are noexec.

I've split this patch up as its doing too many things at once. In
particular, I think we should keep the "isNativeCross()" function name
instead of adding Crosssdk to the name since it just makes things more
confusing to read.

I've take a part for the crosssdk part in master-next which is being
tested at the moment, can you resent the do_package part by itself
please?

I don't quite understand how the sequence of commands above breaks
things or how this patch fixes it. Are you sure this wasn't fixed by:

http://git.yoctoproject.org/cgit.cgi/poky-contrib/commit/?h=ciorga/PUs&id=1dcbf3096d7d42032faade96dae89c25a4feca7a

which would be the real bug?

Cheers,

Richard

Re: [PATCH] sstate: Add optimizing logic for crosssdk setscene dependencies

[Ming Liu]

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

On 01/06/2014 08:25 PM, Richard Purdie wrote:
> On Thu, 2013-11-14 at 18:51 +0800, Ming Liu wrote:
>> This patch mainly aims to add optimisation for crosssdk setscene dependency
>> validating which we haven't handled in current logic, and which I think we
>> could have as we've already implemented to native/cross, although there
>> are albeit not many crossdk tasks, we could still get some performance
>> enhancement.
>>
>> And it also fix a vulnerability of some certain workflow, think about the
>> following scenario with current logic:
>>      bitbake nativesdk-eglibc-initial -c cleansstate
>>      bitbake gcc-crosssdk-initial -c clean
>>      bitbake gmp-native -c clean
>>      bitbake libmpc-native -c clean
>>      bitbake mpfr-native -c clean
>>      bitbake gcc-crosssdk-initial
>>      bitbake nativesdk-eglibc-initial
>>
>> Aboving will fail for absence of a few native libraries required by
>> gcc-crosssdk-initial.
>>
>> Also modified some places in current code except the optimisation, as
>> following:
>> 1 Remove isNative function since no code is referring it.
>> 2 Add do_package to the list that don't exist and are noexec.
> I've split this patch up as its doing too many things at once. In
> particular, I think we should keep the "isNativeCross()" function name
> instead of adding Crosssdk to the name since it just makes things more
> confusing to read.
I am on board with your comments.

>
> I've take a part for the crosssdk part in master-next which is being
> tested at the moment, can you resent the do_package part by itself
> please?
No problem, I will resend a patch soon with modifying the do_package 
part only.

>
> I don't quite understand how the sequence of commands above breaks
> things or how this patch fixes it. Are you sure this wasn't fixed by:
>
> http://git.yoctoproject.org/cgit.cgi/poky-contrib/commit/?h=ciorga/PUs&id=1dcbf3096d7d42032faade96dae89c25a4feca7a
>
> which would be the real bug?
I am pretty sure they are**NOT the same issue. The 1dcbf309 is about to 
fix gcc-crosssdk lacking its dependencies in bb, while in this case, 
it's due to some other setscene tasks(populate_sysroot) depended by 
gcc-crosssdk-initial's setscene task(populate_sysroot) were not 
validated correctly, without the modification to isNativeCross, the 
setscene_depvalid will return TRUE through:
......
             # Native/Cross populate_sysroot need their dependencies
             if isNativeCross(taskdependees[task][0]) and 
isNativeCross(taskdependees[dep][0]):
                 return False
             # Target populate_sysroot depended on by cross tools need 
to be installed
             if isNativeCross(taskdependees[dep][0]):
                 return False
             # Native/cross tools depended upon by target sysroot are 
not needed
             if isNativeCross(taskdependees[task][0]):
                 continue
......
     return True
......

for example, when the task is 'gmp-native', and the dep is 
'gcc-crosssdk-initial', it is absolutely wrong for gcc-crosssdk-initial 
needs its dependencies like gmp-native, libmpc-native, mpfr-native to be 
present in sysroot or its binaries will fail to be executed.

However, by double-checked the patch, I found that it had introduced a 
regression that binutils-crosssdk should not be added into safe dep list 
for it's in the DEPENDS of gcc-crosssdk-initial, I will send a fix for 
it too.

//Ming Liu
>
> Cheers,
>
> Richard
>
>
>


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