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

[Richard Purdie <richard.purdie@linuxfoundation.org>]

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