[PATCH V3 0/3] sstatesig: add support to dump incremental locked signatures

[PATCH V3 0/3] sstatesig: add support to dump incremental locked signatures

[Hongxu Jia]

How to test incrementally locked signature dump:

1) Create a new build

2) Dump locked sig file for db
$ bitbake -S none db
...
Writing locked sigs to /home/jiahongxu/yocto/build-20140904-yocto/locked-sigs.inc
NOTE: Tasks Summary: Attempted 0 tasks of which 0 didn't need to be rerun and all succeeded.
...

3) Back up locked-sigs.inc
$ cp locked-sigs.inc locked-sigs.inc.bak

4) Dump locked sig file for db again
$ bitbake -S none db
...
Writing locked sigs to /home/jiahongxu/yocto/build-20140904-yocto/locked-sigs.inc
NOTE: Tasks Summary: Attempted 0 tasks of which 0 didn't need to be rerun and all succeeded.
...

5) Compare locked-sigs.inc and locked-sigs.inc.bak to verify nothing changed
$ vimdiff locked-sigs.inc locked-sigs.inc.bak

6) Require locked-sigs.inc
vim local.conf
...
require "${TOPDIR}/locked-sigs.inc"
...

7) Incrementally dump locked sig file for world
$ bitbake -S none world
...
Writing locked sigs to /home/jiahongxu/yocto/build-20140904-yocto/locked-sigs.inc
NOTE: Tasks Summary: Attempted 0 tasks of which 0 didn't need to be rerun and all succeeded.
...

8) Compare locked-sigs.inc and locked-sigs.inc.bak to verify incrementally dumping
$ vimdiff locked-sigs.inc locked-sigs.inc.bak

9) Back up locked-sigs.inc
$ cp locked-sigs.inc locked-sigs.inc.bak

10) Dump for world again
$ bitbake -S none world
...
Writing locked sigs to /home/jiahongxu/yocto/build-20140904-yocto/locked-sigs.inc
Nothing added to /home/jiahongxu/yocto/build-20140904-yocto/locked-sigs.inc
NOTE: Tasks Summary: Attempted 0 tasks of which 0 didn't need to be rerun and all succeeded.
...

11) Compare locked-sigs.inc and locked-sigs.inc.bak to verify nothing changed
$ vimdiff locked-sigs.inc locked-sigs.inc.bak

//Hongxu

The following changes since commit 280b6d0011f101e7a8edabd34c265b37a2f791ed:

  sstatesig: fix overrides behaviour to remove SIGGEN_LOCKEDSIGS_i586 (2014-09-17 22:00:26 +0100)

are available in the git repository at:

  git://git.pokylinux.org/poky-contrib hongxu/locked-sstate
  http://git.pokylinux.org/cgit.cgi/poky-contrib/log/?h=hongxu/locked-sstate

Hongxu Jia (3):
  sstatesig: Only dump incremental locked signatures
  sstatesig: add new item checking for locked signature dump
  sstatesig: fix to support unincremental locked signature dump

 meta/lib/oe/sstatesig.py | 33 ++++++++++++++++++++++++++++-----
 1 file changed, 28 insertions(+), 5 deletions(-)

-- 
1.9.1

[PATCH 1/3] sstatesig: Only dump incremental locked signatures

[Hongxu Jia]

The idea of incremental sig is:

New sig file = Old sig file (if available) + New sig items in current build.

The condition of incremental locked signature dump is an existed locked sig
file is required and it is also the dump sig file.

Signed-off-by: Hongxu Jia <hongxu.jia@windriver.com>
---
 meta/lib/oe/sstatesig.py | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/meta/lib/oe/sstatesig.py b/meta/lib/oe/sstatesig.py
index af7617e..56fd953 100644
--- a/meta/lib/oe/sstatesig.py
+++ b/meta/lib/oe/sstatesig.py
@@ -151,19 +151,23 @@ class SignatureGeneratorOEBasicHash(bb.siggen.SignatureGeneratorBasicHash):
                 types[t] = []
             types[t].append(k)
 
-        with open(sigfile, "w") as f:
+        with open(sigfile, "a") as f:
             for t in types:
-                f.write('SIGGEN_LOCKEDSIGS_%s = "\\\n' % t)
+                f.write('SIGGEN_LOCKEDSIGS_%s += "\\\n' % t)
                 types[t].sort()
                 sortedk = sorted(types[t], key=lambda k: self.lockedpnmap[k.rsplit(".",1)[0]])
                 for k in sortedk:
                     fn = k.rsplit(".",1)[0]
+                    pn = self.lockedpnmap[fn]
                     task = k.rsplit(".",1)[1]
                     if k not in self.taskhash:
                         continue
-                    f.write("    " + self.lockedpnmap[fn] + ":" + task + ":" + self.taskhash[k] + " \\\n")
+                    if pn in self.lockedsigs and task in self.lockedsigs[pn] and self.taskhash[k] == self.lockedsigs[pn][task]:
+                        continue
+                    sigentry = pn + ":" + task + ":" + self.taskhash[k]
+                    f.write("    " + sigentry + " \\\n")
                 f.write('    "\n')
-            f.write('SIGGEN_LOCKEDSIGS_TYPES_%s = "%s"' % (self.machine, " ".join(types.keys())))
+            f.write('SIGGEN_LOCKEDSIGS_TYPES_%s += "%s"\n' % (self.machine, " ".join(types.keys())))
 
     def checkhashes(self, missed, ret, sq_fn, sq_task, sq_hash, sq_hashfn, d):
         checklevel = d.getVar("SIGGEN_LOCKEDSIGS_CHECK_LEVEL", True)
-- 
1.9.1

[PATCH 2/3] sstatesig: add new item checking for locked signature dump

[Hongxu Jia]

While dumping locked signature, if there is nothing added
in the current build (such as build the same recipe again),
there are accumulating empty SIGGEN_LOCKEDSIGS added, such as:
...
SIGGEN_LOCKEDSIGS_t-x86-64 += "\
    "
SIGGEN_LOCKEDSIGS_TYPES_qemux86 += "t-x86-64"

SIGGEN_LOCKEDSIGS_t-x86-64 += "\
    "
SIGGEN_LOCKEDSIGS_TYPES_qemux86 += "t-x86-64"
...
Add new item checking to fix the issue.

Invoke set() to drop duplicated types

Signed-off-by: Hongxu Jia <hongxu.jia@windriver.com>
---
 meta/lib/oe/sstatesig.py | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/meta/lib/oe/sstatesig.py b/meta/lib/oe/sstatesig.py
index 56fd953..b2ffc7e 100644
--- a/meta/lib/oe/sstatesig.py
+++ b/meta/lib/oe/sstatesig.py
@@ -64,7 +64,7 @@ def sstate_rundepfilter(siggen, fn, recipename, task, dep, depname, dataCache):
 def sstate_lockedsigs(d):
     sigs = {}
     types = (d.getVar("SIGGEN_LOCKEDSIGS_TYPES", True) or "").split()
-    for t in types:
+    for t in set(types):
         lockedsigs = (d.getVar("SIGGEN_LOCKEDSIGS_%s" % t, True) or "").split()
         for ls in lockedsigs:
             pn, task, h = ls.split(":", 2)
@@ -143,6 +143,7 @@ class SignatureGeneratorOEBasicHash(bb.siggen.SignatureGeneratorBasicHash):
 
         bb.plain("Writing locked sigs to %s" % sigfile)
         types = {}
+        has_new_lockedsigs = False
         for k in self.runtaskdeps:
             fn = k.rsplit(".",1)[0]
             t = self.lockedhashfn[fn].split(" ")[1].split(":")[5]
@@ -151,6 +152,20 @@ class SignatureGeneratorOEBasicHash(bb.siggen.SignatureGeneratorBasicHash):
                 types[t] = []
             types[t].append(k)
 
+            if not has_new_lockedsigs:
+                pn = self.lockedpnmap[fn]
+                if pn not in self.lockedsigs:
+                    has_new_lockedsigs = True
+                    continue
+                task = k.rsplit(".",1)[1]
+                if task not in self.lockedsigs[pn]:
+                    has_new_lockedsigs = True
+                    continue
+
+        if not has_new_lockedsigs:
+            bb.plain("Nothing added to %s" % sigfile)
+            return
+
         with open(sigfile, "a") as f:
             for t in types:
                 f.write('SIGGEN_LOCKEDSIGS_%s += "\\\n' % t)
-- 
1.9.1

[PATCH 3/3] sstatesig: fix to support unincremental locked signature dump

[Hongxu Jia]

The condition of incremental locked signature dump is an
existed locked sig file is required and it is also the
dump sig file.

But if they are not the same file or simply no locked sig
file required, it should be unincremental.

In unincremental dump situation, we should write the locked
sig file rather than append.

Signed-off-by: Hongxu Jia <hongxu.jia@windriver.com>
---
 meta/lib/oe/sstatesig.py | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/meta/lib/oe/sstatesig.py b/meta/lib/oe/sstatesig.py
index b2ffc7e..ce7c960 100644
--- a/meta/lib/oe/sstatesig.py
+++ b/meta/lib/oe/sstatesig.py
@@ -93,6 +93,7 @@ class SignatureGeneratorOEBasicHash(bb.siggen.SignatureGeneratorBasicHash):
         self.lockedhashfn = {}
         self.machine = data.getVar("MACHINE", True)
         self.mismatch_msgs = []
+        self.bbinclude = (data.getVar("BBINCLUDED", True) or "").split()
         pass
     def rundep_check(self, fn, recipename, task, dep, depname, dataCache = None):
         return sstate_rundepfilter(self, fn, recipename, task, dep, depname, dataCache)
@@ -166,6 +167,9 @@ class SignatureGeneratorOEBasicHash(bb.siggen.SignatureGeneratorBasicHash):
             bb.plain("Nothing added to %s" % sigfile)
             return
 
+        if os.path.abspath(sigfile) not in self.bbinclude:
+            open(sigfile, "w").close()
+
         with open(sigfile, "a") as f:
             for t in types:
                 f.write('SIGGEN_LOCKEDSIGS_%s += "\\\n' % t)
-- 
1.9.1

Re: [PATCH 1/3] sstatesig: Only dump incremental locked signatures

[Richard Purdie]

On Thu, 2014-09-18 at 15:23 +0800, Hongxu Jia wrote:
> The idea of incremental sig is:
> 
> New sig file = Old sig file (if available) + New sig items in current build.
> 
> The condition of incremental locked signature dump is an existed locked sig
> file is required and it is also the dump sig file.
> 
> Signed-off-by: Hongxu Jia <hongxu.jia@windriver.com>
> ---
>  meta/lib/oe/sstatesig.py | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/meta/lib/oe/sstatesig.py b/meta/lib/oe/sstatesig.py
> index af7617e..56fd953 100644
> --- a/meta/lib/oe/sstatesig.py
> +++ b/meta/lib/oe/sstatesig.py
> @@ -151,19 +151,23 @@ class SignatureGeneratorOEBasicHash(bb.siggen.SignatureGeneratorBasicHash):
>                  types[t] = []
>              types[t].append(k)
>  
> -        with open(sigfile, "w") as f:
> +        with open(sigfile, "a") as f:
>              for t in types:
> -                f.write('SIGGEN_LOCKEDSIGS_%s = "\\\n' % t)
> +                f.write('SIGGEN_LOCKEDSIGS_%s += "\\\n' % t)
>                  types[t].sort()
>                  sortedk = sorted(types[t], key=lambda k: self.lockedpnmap[k.rsplit(".",1)[0]])
>                  for k in sortedk:
>                      fn = k.rsplit(".",1)[0]
> +                    pn = self.lockedpnmap[fn]
>                      task = k.rsplit(".",1)[1]
>                      if k not in self.taskhash:
>                          continue
> -                    f.write("    " + self.lockedpnmap[fn] + ":" + task + ":" + self.taskhash[k] + " \\\n")
> +                    if pn in self.lockedsigs and task in self.lockedsigs[pn] and self.taskhash[k] == self.lockedsigs[pn][task]:
> +                        continue
> +                    sigentry = pn + ":" + task + ":" + self.taskhash[k]
> +                    f.write("    " + sigentry + " \\\n")
>                  f.write('    "\n')
> -            f.write('SIGGEN_LOCKEDSIGS_TYPES_%s = "%s"' % (self.machine, " ".join(types.keys())))
> +            f.write('SIGGEN_LOCKEDSIGS_TYPES_%s += "%s"\n' % (self.machine, " ".join(types.keys())))
>  
>      def checkhashes(self, missed, ret, sq_fn, sq_task, sq_hash, sq_hashfn, d):
>          checklevel = d.getVar("SIGGEN_LOCKEDSIGS_CHECK_LEVEL", True)

I'm afraid I'm starting to feel very strongly this is not a direction we
should move in. Having the ability to write out a .inc file containing
on a delta is one thing, writing out a file for automatic inclusion and
trying to maintain that file is not something I feel comfortable with.

I think that at some point there needs to be external tooling handling
the inclusion and updating of this file and that the sigs code is not
the place for this.

For example, consider the case where you switch machines and want to
share an include file between these machines. With the changes proposed
in this patch series it will simply overwrite the file and remove the
entries for the other machine.

We could keep trying to patch up this code to cover every combination
and eventuality but in the end, I believe the maintenance of this file
should be something external, the sigs code should only be concerned
with the generation of the core entries.

Cheers,

Richard 

Re: [PATCH 1/3] sstatesig: Only dump incremental locked signatures

[Mark Hatle]

On 9/18/14, 8:47 AM, Richard Purdie wrote:
> On Thu, 2014-09-18 at 15:23 +0800, Hongxu Jia wrote:
>> The idea of incremental sig is:
>>
>> New sig file = Old sig file (if available) + New sig items in current build.
>>
>> The condition of incremental locked signature dump is an existed locked sig
>> file is required and it is also the dump sig file.
>>
>> Signed-off-by: Hongxu Jia <hongxu.jia@windriver.com>
>> ---
>>   meta/lib/oe/sstatesig.py | 12 ++++++++----
>>   1 file changed, 8 insertions(+), 4 deletions(-)
>>
>> diff --git a/meta/lib/oe/sstatesig.py b/meta/lib/oe/sstatesig.py
>> index af7617e..56fd953 100644
>> --- a/meta/lib/oe/sstatesig.py
>> +++ b/meta/lib/oe/sstatesig.py
>> @@ -151,19 +151,23 @@ class SignatureGeneratorOEBasicHash(bb.siggen.SignatureGeneratorBasicHash):
>>                   types[t] = []
>>               types[t].append(k)
>>
>> -        with open(sigfile, "w") as f:
>> +        with open(sigfile, "a") as f:
>>               for t in types:
>> -                f.write('SIGGEN_LOCKEDSIGS_%s = "\\\n' % t)
>> +                f.write('SIGGEN_LOCKEDSIGS_%s += "\\\n' % t)
>>                   types[t].sort()
>>                   sortedk = sorted(types[t], key=lambda k: self.lockedpnmap[k.rsplit(".",1)[0]])
>>                   for k in sortedk:
>>                       fn = k.rsplit(".",1)[0]
>> +                    pn = self.lockedpnmap[fn]
>>                       task = k.rsplit(".",1)[1]
>>                       if k not in self.taskhash:
>>                           continue
>> -                    f.write("    " + self.lockedpnmap[fn] + ":" + task + ":" + self.taskhash[k] + " \\\n")
>> +                    if pn in self.lockedsigs and task in self.lockedsigs[pn] and self.taskhash[k] == self.lockedsigs[pn][task]:
>> +                        continue
>> +                    sigentry = pn + ":" + task + ":" + self.taskhash[k]
>> +                    f.write("    " + sigentry + " \\\n")
>>                   f.write('    "\n')
>> -            f.write('SIGGEN_LOCKEDSIGS_TYPES_%s = "%s"' % (self.machine, " ".join(types.keys())))
>> +            f.write('SIGGEN_LOCKEDSIGS_TYPES_%s += "%s"\n' % (self.machine, " ".join(types.keys())))
>>
>>       def checkhashes(self, missed, ret, sq_fn, sq_task, sq_hash, sq_hashfn, d):
>>           checklevel = d.getVar("SIGGEN_LOCKEDSIGS_CHECK_LEVEL", True)
>
> I'm afraid I'm starting to feel very strongly this is not a direction we
> should move in. Having the ability to write out a .inc file containing
> on a delta is one thing, writing out a file for automatic inclusion and
> trying to maintain that file is not something I feel comfortable with.
>
> I think that at some point there needs to be external tooling handling
> the inclusion and updating of this file and that the sigs code is not
> the place for this.
>
> For example, consider the case where you switch machines and want to
> share an include file between these machines. With the changes proposed
> in this patch series it will simply overwrite the file and remove the
> entries for the other machine.
>
> We could keep trying to patch up this code to cover every combination
> and eventuality but in the end, I believe the maintenance of this file
> should be something external, the sigs code should only be concerned
> with the generation of the core entries.

This is why I was originally advocating a whitelist/blacklist approach with the 
read-only [or warn/error] sstate-cache approach.  It really is different then 
the locked signature.

The users want to use what they have that match the generated hashes, or get a 
warning/error -- unless the item is whitelisted.

They only need to manage a small list of the "things they changed" to support 
the white listing, and no complicated hashes are needed.

--Mark

> Cheers,
>
> Richard
>
>
>
>

Re: [PATCH 1/3] sstatesig: Only dump incremental locked signatures

[Hongxu Jia]

On 09/18/2014 09:47 PM, Richard Purdie wrote:
> I'm afraid I'm starting to feel very strongly this is not a direction we
> should move in. Having the ability to write out a .inc file containing
> on a delta is one thing, writing out a file for automatic inclusion and
> trying to maintain that file is not something I feel comfortable with.

Yes, I understand that your worry is reasonable, so many unexpected
exception needs to be maintained

> I think that at some point there needs to be external tooling handling
> the inclusion and updating of this file and that the sigs code is not
> the place for this.

I once coded a python script to handling the locked sig file from an
existed sstate, translated sstate item name (ends with '.tgz.siginfo')
to "<pn>:<task>:<hash>" tuple.

The main blocker is: for some tasks (such as do_patch/do_fetch/
do_unpack), the target and native have the same pn in sstate, such as:
''sstate-cache/c6/sstate:automake::1.14.1:r0::3:c667b87f5d4e15198afc744f525895fc_unpack.tgz.siginfo"
is used for "automake" and "automake-native", you could not figure
out arch (which used as type in locked sig file) from it neither.

> For example, consider the case where you switch machines and want to
> share an include file between these machines. With the changes proposed
> in this patch series it will simply overwrite the file and remove the
> entries for the other machine.

I think the swith machines case could work in this patch series,
It appends to the tail of the locked sig file, and not overwrite
the other machine's sig if incremetnal dump is set.

But I indeed understand what you worry about.

> We could keep trying to patch up this code to cover every combination
> and eventuality but in the end, I believe the maintenance of this file
> should be something external, the sigs code should only be concerned
> with the generation of the core entries.

Completely agree.

//Hongxu

> Cheers,
>
> Richard

Re: [PATCH 1/3] sstatesig: Only dump incremental locked signatures

[Hongxu Jia]

On 09/18/2014 09:47 PM, Richard Purdie wrote:
> I'm afraid I'm starting to feel very strongly this is not a direction we
> should move in. Having the ability to write out a .inc file containing
> on a delta is one thing, writing out a file for automatic inclusion and
> trying to maintain that file is not something I feel comfortable with.
>
> I think that at some point there needs to be external tooling handling
> the inclusion and updating of this file and that the sigs code is not
> the place for this.
>
> For example, consider the case where you switch machines and want to
> share an include file between these machines. With the changes proposed
> in this patch series it will simply overwrite the file and remove the
> entries for the other machine.

Yes, you are right, it will overwrite, not for the locked sig file,
but the core structure self.lockedsigs

It could explain the necessary why I add 'type' to self.lockedsigs
in previous path series.
{pn:{task:{hash}}} --> {type:{pn:{task:{hash}}}}

//Hongxu

>
> We could keep trying to patch up this code to cover every combination
> and eventuality but in the end, I believe the maintenance of this file
> should be something external, the sigs code should only be concerned
> with the generation of the core entries.
>
> Cheers,
>
> Richard