Patchwork [bitbake-devel] Remove whitelisted vars from non-task deps

login
mail settings
Submitter Christopher Larson
Date April 3, 2012, 11:23 p.m.
Message ID <1333495429-17048-1-git-send-email-kergoth@gmail.com>
Download mbox | patch
Permalink /patch/25133/
State Accepted
Commit dac12560ac8431ee24609f8de25cb1645572d350
Headers show

Comments

Christopher Larson - April 3, 2012, 11:23 p.m.
From: Christopher Larson <chris_larson@mentor.com>

Though the value of variables in the BB_BASEHASH_WHITELIST is kept out of the
checksums, dependency on them is not, at least for variables and non-task
functions. In the code, the whitelist is removed from the overall task dep
list, but not the individual variable deps. The result of this is that
functions like sysroot_stage_all and oe_runmake end up with whitelisted
variables like TERM listed in their dependencies, which means that doing
a 'unset TERM' before building will result in all checksums for tasks that
depend on those changing, and shared state reuse not behaving correctly.

This is only really a potential issue for variables from the environment, as
it's the existance/removal of the variable that's an issue, not its value, and
the other whitelisted variables are set in our metadata. This which means in
practical terms the only cases where this is likely to be an issue are in
environments where one of the following are unset: TERM, LOGNAME, HOME, USER,
PWD, SHELL. This may seem like an unlikely circumstance, but is in fact a real
issue for those of us using autobuilders. Jenkins does not set TERM when
executing shell, which means shared state archives produced by your jenkins
server would not be fully reused by an actual user.

Fixed by removing the whitelisted elements from the individual variable deps,
not just the accumulated result.

Signed-off-by: Christopher Larson <chris_larson@mentor.com>
---
 lib/bb/siggen.py |    9 +++++----
 1 files changed, 5 insertions(+), 4 deletions(-)
Christopher Larson - April 3, 2012, 11:25 p.m.
On Tue, Apr 3, 2012 at 4:23 PM, Christopher Larson <kergoth@gmail.com> wrote:
> This is only really a potential issue for variables from the environment, as
> it's the existance/removal of the variable that's an issue, not its value, and
> the other whitelisted variables are set in our metadata. This which means in

Erm, s/This which/This/
Matthew McClintock - April 4, 2012, 5:02 p.m.
On Tue, Apr 3, 2012 at 4:23 PM, Christopher Larson <kergoth@gmail.com> wrote:
> From: Christopher Larson <chris_larson@mentor.com>
>
> Though the value of variables in the BB_BASEHASH_WHITELIST is kept out of the
> checksums, dependency on them is not, at least for variables and non-task
> functions. In the code, the whitelist is removed from the overall task dep
> list, but not the individual variable deps. The result of this is that
> functions like sysroot_stage_all and oe_runmake end up with whitelisted
> variables like TERM listed in their dependencies, which means that doing
> a 'unset TERM' before building will result in all checksums for tasks that
> depend on those changing, and shared state reuse not behaving correctly.
>
> This is only really a potential issue for variables from the environment, as
> it's the existance/removal of the variable that's an issue, not its value, and
> the other whitelisted variables are set in our metadata. This which means in
> practical terms the only cases where this is likely to be an issue are in
> environments where one of the following are unset: TERM, LOGNAME, HOME, USER,
> PWD, SHELL. This may seem like an unlikely circumstance, but is in fact a real
> issue for those of us using autobuilders. Jenkins does not set TERM when
> executing shell, which means shared state archives produced by your jenkins
> server would not be fully reused by an actual user.
>
> Fixed by removing the whitelisted elements from the individual variable deps,
> not just the accumulated result.

Which recipes/packages were you seeing this on? I've noticed the TERM
issue on several packages but not on all of them.

-M

>
> Signed-off-by: Christopher Larson <chris_larson@mentor.com>
> ---
>  lib/bb/siggen.py |    9 +++++----
>  1 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/lib/bb/siggen.py b/lib/bb/siggen.py
> index 8c79b17..7f5ae93 100644
> --- a/lib/bb/siggen.py
> +++ b/lib/bb/siggen.py
> @@ -89,6 +89,7 @@ class SignatureGeneratorBasic(SignatureGenerator):
>                 bb.error("Task %s from %s seems to be empty?!" % (task, fn))
>                 data = ''
>
> +            gendeps[task] -= self.basewhitelist
>             newdeps = gendeps[task]
>             seen = set()
>             while newdeps:
> @@ -98,12 +99,12 @@ class SignatureGeneratorBasic(SignatureGenerator):
>                 for dep in nextdeps:
>                     if dep in self.basewhitelist:
>                         continue
> +                    gendeps[dep] -= self.basewhitelist
>                     newdeps |= gendeps[dep]
>                 newdeps -= seen
>
> -            alldeps = seen - self.basewhitelist
> -
> -            for dep in sorted(alldeps):
> +            alldeps = sorted(seen)
> +            for dep in alldeps:
>                 data = data + dep
>                 if dep in lookupcache:
>                     var = lookupcache[dep]
> @@ -113,7 +114,7 @@ class SignatureGeneratorBasic(SignatureGenerator):
>                 if var:
>                     data = data + str(var)
>             self.basehash[fn + "." + task] = hashlib.md5(data).hexdigest()
> -            taskdeps[task] = sorted(alldeps)
> +            taskdeps[task] = alldeps
>
>         self.taskdeps[fn] = taskdeps
>         self.gendeps[fn] = gendeps
> --
> 1.7.7
>
>
> _______________________________________________
> bitbake-devel mailing list
> bitbake-devel@lists.openembedded.org
> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/bitbake-devel
Christopher Larson - April 4, 2012, 5:05 p.m.
On Wed, Apr 4, 2012 at 10:02 AM, Matthew McClintock
<msm-fsl@mcclintock.net> wrote:
> On Tue, Apr 3, 2012 at 4:23 PM, Christopher Larson <kergoth@gmail.com> wrote:
>> From: Christopher Larson <chris_larson@mentor.com>
>>
>> Though the value of variables in the BB_BASEHASH_WHITELIST is kept out of the
>> checksums, dependency on them is not, at least for variables and non-task
>> functions. In the code, the whitelist is removed from the overall task dep
>> list, but not the individual variable deps. The result of this is that
>> functions like sysroot_stage_all and oe_runmake end up with whitelisted
>> variables like TERM listed in their dependencies, which means that doing
>> a 'unset TERM' before building will result in all checksums for tasks that
>> depend on those changing, and shared state reuse not behaving correctly.
>>
>> This is only really a potential issue for variables from the environment, as
>> it's the existance/removal of the variable that's an issue, not its value, and
>> the other whitelisted variables are set in our metadata. This which means in
>> practical terms the only cases where this is likely to be an issue are in
>> environments where one of the following are unset: TERM, LOGNAME, HOME, USER,
>> PWD, SHELL. This may seem like an unlikely circumstance, but is in fact a real
>> issue for those of us using autobuilders. Jenkins does not set TERM when
>> executing shell, which means shared state archives produced by your jenkins
>> server would not be fully reused by an actual user.
>>
>> Fixed by removing the whitelisted elements from the individual variable deps,
>> not just the accumulated result.
>
> Which recipes/packages were you seeing this on? I've noticed the TERM
> issue on several packages but not on all of them.

One I know of off the top of my head was avahi. The sstate archives
for it which came out of jenkins were not being used. As one example,
the populate-sysroot archive wasn't being used due to the TERM
dependency being added/removed to/from the dependency list for
sysroot_stage_*.

Patch

diff --git a/lib/bb/siggen.py b/lib/bb/siggen.py
index 8c79b17..7f5ae93 100644
--- a/lib/bb/siggen.py
+++ b/lib/bb/siggen.py
@@ -89,6 +89,7 @@  class SignatureGeneratorBasic(SignatureGenerator):
                 bb.error("Task %s from %s seems to be empty?!" % (task, fn))
                 data = ''
 
+            gendeps[task] -= self.basewhitelist
             newdeps = gendeps[task]
             seen = set()
             while newdeps:
@@ -98,12 +99,12 @@  class SignatureGeneratorBasic(SignatureGenerator):
                 for dep in nextdeps:
                     if dep in self.basewhitelist:
                         continue
+                    gendeps[dep] -= self.basewhitelist
                     newdeps |= gendeps[dep]
                 newdeps -= seen
 
-            alldeps = seen - self.basewhitelist
-
-            for dep in sorted(alldeps):
+            alldeps = sorted(seen)
+            for dep in alldeps:
                 data = data + dep
                 if dep in lookupcache:
                     var = lookupcache[dep]
@@ -113,7 +114,7 @@  class SignatureGeneratorBasic(SignatureGenerator):
                 if var:
                     data = data + str(var)
             self.basehash[fn + "." + task] = hashlib.md5(data).hexdigest()
-            taskdeps[task] = sorted(alldeps)
+            taskdeps[task] = alldeps
 
         self.taskdeps[fn] = taskdeps
         self.gendeps[fn] = gendeps