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

[Mark Hatle <mark.hatle@windriver.com>]

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
>
>
>
>