[bitbake-devel] [1.44 02/18] runqueue/siggen: Allow handling of equivalent hashes

[Peter Kjellerstedt]

> -----Original Message-----
> From: bitbake-devel-bounces at lists.openembedded.org <bitbake-devel-
> bounces at lists.openembedded.org> On Behalf Of Armin Kuster
> Sent: den 23 december 2019 05:50
> To: bitbake-devel at lists.openembedded.org
> Subject: [bitbake-devel] [1.44 02/18] runqueue/siggen: Allow handling of
> equivalent hashes
> 
> From: Richard Purdie <richard.purdie at linuxfoundation.org>
> 
> Based on the hashserv's new ability to accept hash mappings, update
> runqueue
> to use this through a helper function in siggen.
> 
> This addresses problems with meta-extsdk-toolchain and its dependency on
> gdb-cross which caused errors when building eSDK. See the previous commit
> for more details.
> 
> Signed-off-by: Richard Purdie <richard.purdie at linuxfoundation.org>
> (cherry picked from commit 39098b4ba2133f4d9229a0aa4fcf4c3e1291286a)
> ---
>  lib/bb/runqueue.py | 31 +++++++++++++++++++------------
>  lib/bb/siggen.py   | 26 ++++++++++++++++++++++++++
>  2 files changed, 45 insertions(+), 12 deletions(-)
> 
> diff --git a/lib/bb/runqueue.py b/lib/bb/runqueue.py
> index bd7f03f..a869ba5 100644
> --- a/lib/bb/runqueue.py
> +++ b/lib/bb/runqueue.py
> @@ -2283,12 +2283,26 @@ class RunQueueExecute:
>                          for dep in self.rqdata.runtaskentries[tid].depends:
>                              procdep.append(dep)
>                          orighash = self.rqdata.runtaskentries[tid].hash
> -                        self.rqdata.runtaskentries[tid].hash = bb.parse.siggen.get_taskhash(tid, procdep, self.rqdata.dataCaches[mc_from_tid(tid)])
> +                        newhash = bb.parse.siggen.get_taskhash(tid, procdep, self.rqdata.dataCaches[mc_from_tid(tid)])
>                          origuni = self.rqdata.runtaskentries[tid].unihash
> -                        self.rqdata.runtaskentries[tid].unihash = bb.parse.siggen.get_unihash(tid)
> -                        logger.debug(1, "Task %s hash changes: %s->%s %s->%s" % (tid, orighash, self.rqdata.runtaskentries[tid].hash, origuni, self.rqdata.runtaskentries[tid].unihash))
> +                        newuni = bb.parse.siggen.get_unihash(tid)
> +                        # FIXME, need to check it can come from sstate at all for determinism?
> +                        remapped = False
> +                        if newuni == origuni:
> +                            # Nothing to do, we match, skip code below
> +                            remapped = True
> +                        elif tid in self.scenequeue_covered or tid in self.sq_live:
> +                            # Already ran this setscene task or it running. Report the new taskhash
> +                            remapped = bb.parse.siggen.report_unihash_equiv(tid, newhash, origuni, newuni, self.rqdata.dataCaches)
> +                            logger.info("Already covered setscene for %s so ignoring rehash (remap)" % (tid))

Do we really want this level of logging? I have not commented on it for 
master (yet), as I expect it is invaluable for RP & co that are currently 
working hard on the hash server, but for me as common user of bitbake, 
this is just noise (and a lot of it). I have a local patch that changes 
this to logger.debug(1, ...) together with another logger.info("Task %s 
unihash changed to %s" ...) a few lines earlier to get the cooker UI 
reasonably back to usable. Also see below.

> +
> +                        if not remapped:
> +                            logger.debug(1, "Task %s hash changes: %s->%s %s->%s" % (tid, orighash, newhash, origuni, newuni))
> +                            self.rqdata.runtaskentries[tid].hash = newhash
> +                            self.rqdata.runtaskentries[tid].unihash = newuni
> +                            changed.add(tid)
> +
>                          next |= self.rqdata.runtaskentries[tid].revdeps
> -                        changed.add(tid)
>                          total.remove(tid)
>                          next.intersection_update(total)
> 
> @@ -2307,18 +2321,11 @@ class RunQueueExecute:
>                  self.pending_migrations.add(tid)
> 
>          for tid in self.pending_migrations.copy():
> -            if tid in self.runq_running:
> +            if tid in self.runq_running or tid in self.sq_live:
>                  # Too late, task already running, not much we can do now
>                  self.pending_migrations.remove(tid)
>                  continue
> 
> -            if tid in self.scenequeue_covered or tid in self.sq_live:
> -                # Already ran this setscene task or it running
> -                # Potentially risky, should we report this hash as a match?
> -                logger.info("Already covered setscene for %s so ignoring rehash" % (tid))
> -                self.pending_migrations.remove(tid)
> -                continue
> -
>              valid = True
>              # Check no tasks this covers are running
>              for dep in self.sqdata.sq_covered_tasks[tid]:
> diff --git a/lib/bb/siggen.py b/lib/bb/siggen.py
> index e19812b..edf1010 100644
> --- a/lib/bb/siggen.py
> +++ b/lib/bb/siggen.py
> @@ -525,6 +525,32 @@ class SignatureGeneratorUniHashMixIn(object):
>                  except OSError:
>                      pass
> 
> +    def report_unihash_equiv(self, tid, taskhash, wanted_unihash, current_unihash, datacaches):
> +        try:
> +            extra_data = {}
> +            data = self.client().report_unihash_equiv(taskhash, self.method, wanted_unihash, extra_data)
> +            bb.note('Reported task %s as unihash %s to %s (%s)' % (tid, wanted_unihash, self.server, str(data)))

This one and ...

> +
> +            if data is None:
> +                bb.warn("Server unable to handle unihash report")
> +                return False
> +
> +            finalunihash = data['unihash']
> +
> +            if finalunihash == current_unihash:
> +                bb.note('Task %s unihash %s unchanged by server' % (tid, finalunihash))
> +            elif finalunihash == wanted_unihash:
> +                bb.note('Task %s unihash changed %s -> %s as wanted' % (tid, current_unihash, finalunihash))
> +                self.set_unihash(tid, finalunihash)
> +                return True
> +            else:
> +                # TODO: What to do here?
> +                bb.note('Task %s unihash reported as unwanted hash %s' % (tid, finalunihash))

... and these also seem excessive. I have changed them locally to 
bb.debug(1, ...) too.

> +        except hashserv.client.HashConnectionError as e:
> +            bb.warn('Error contacting Hash Equivalence Server %s: %s' % (self.server, str(e)))
> +
> +        return False
> 
>  #
>  # Dummy class used for bitbake-selftest
> --
> 2.7.4

//Peter