[1.44 0/7] patch review

[1.44 0/7] patch review

[Armin Kuster]

Here are a few more changes targeted for the zeus 3.0.2 release.
Please have comments back by Tuesday.

The following changes since commit 1a026d4aca47ed1b0b74a8a818635e7520e8f9c8:

  runqueue: Use a set for the setscene tasks list (2020-01-18 12:20:38 -0800)

are available in the Git repository at:

  git://git.openembedded.org/bitbake-contrib stable/1.44-nut
  http://cgit.openembedded.org/bitbake-contrib/log/?h=stable/1.44-nut

Chris Laplante (3):
  fetch2: do not suffix srcrev cache key with PN
  fetch2/git: _revision_key: collapse adjacent slashes
  persist_data.py: Immediately get exclusive lock in __setitem__

Frazer Clews (1):
  cooker/toaster: replaced deprecated method warn() with warning()

Richard Purdie (3):
  siggen: Optimise get_unihash disk based cache handling
  siggen: Cache unihash values to avoid cache lookup
  siggen: Avoid cache mismatch issues with locked sigs

 lib/bb/cooker.py                              |  2 +-
 lib/bb/fetch2/__init__.py                     |  3 +-
 lib/bb/fetch2/git.py                          |  4 +-
 lib/bb/persist_data.py                        |  3 +
 lib/bb/siggen.py                              | 68 +++++++++++++------
 .../management/commands/runbuilds.py          |  6 +-
 6 files changed, 60 insertions(+), 26 deletions(-)

-- 
2.17.1

[1.44 1/7] fetch2: do not suffix srcrev cache key with PN

[Armin Kuster]

From: Chris Laplante <chris.laplante@agilent.com>

Prior to this change, two different recipes pulling from the same
exact repo could get a different SRCREV during a single parse session.

This was originally observed using git. For git at least, it still
allows recipes to pull from the same repo, but with different branches
or tags, since the form of the srcrev cache key for git is:

        "git:" + ud.host + ud.path.replace('/', '.') + ud.unresolvedrev[name]

Where the 'unresolvedrev' part is the branch or tag name.

Signed-off-by: Chris Laplante <chris.laplante@agilent.com>
Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
(cherry picked from commit 6c938e6fd29beebe09b32be839dae008fe6491d2)
Signed-off-by: Armin Kuster <akuster808@gmail.com>
---
 lib/bb/fetch2/__init__.py | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/lib/bb/fetch2/__init__.py b/lib/bb/fetch2/__init__.py
index 731c1608..4a781bde 100644
--- a/lib/bb/fetch2/__init__.py
+++ b/lib/bb/fetch2/__init__.py
@@ -1567,8 +1567,7 @@ class FetchMethod(object):
         return True, str(latest_rev)
 
     def generate_revision_key(self, ud, d, name):
-        key = self._revision_key(ud, d, name)
-        return "%s-%s" % (key, d.getVar("PN") or "")
+        return self._revision_key(ud, d, name)
 
     def latest_versionstring(self, ud, d):
         """
-- 
2.17.1

[1.44 2/7] fetch2/git: _revision_key: collapse adjacent slashes

[Armin Kuster]

From: Chris Laplante <chris.laplante@agilent.com>

From a SRCREV caching point of view, there is no reason to treat the
following upstreams as different:

    SRC_URI = "git://github.com/file/file.git"
    SRC_URI = "git://github.com//file/file.git"

Signed-off-by: Chris Laplante <chris.laplante@agilent.com>
Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
(cherry picked from commit 425e21c14955dd38868c6e97637df3bbe0f89fac)
Signed-off-by: Armin Kuster <akuster808@gmail.com>
---
 lib/bb/fetch2/git.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lib/bb/fetch2/git.py b/lib/bb/fetch2/git.py
index fa41b078..9ba9cdcc 100644
--- a/lib/bb/fetch2/git.py
+++ b/lib/bb/fetch2/git.py
@@ -594,7 +594,9 @@ class Git(FetchMethod):
         """
         Return a unique key for the url
         """
-        return "git:" + ud.host + ud.path.replace('/', '.') + ud.unresolvedrev[name]
+        # Collapse adjacent slashes
+        slash_re = re.compile(r"/+")
+        return "git:" + ud.host + slash_re.sub(".", ud.path) + ud.unresolvedrev[name]
 
     def _lsremote(self, ud, d, search):
         """
-- 
2.17.1

[1.44 3/7] persist_data.py: Immediately get exclusive lock in __setitem__

[Armin Kuster]

From: Chris Laplante <chris.laplante@agilent.com>

To avoid races, SQLTable::__setitem__ needs an exclusive lock for the
entire transaction, not just the INSERT/UPDATE part.

Signed-off-by: Chris Laplante <chris.laplante@agilent.com>
Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
(cherry picked from commit feb43e7c30f5bfab75d718896c45df621810d06f)
Signed-off-by: Armin Kuster <akuster808@gmail.com>
---
 lib/bb/persist_data.py | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lib/bb/persist_data.py b/lib/bb/persist_data.py
index de8f87a8..7357ab2d 100644
--- a/lib/bb/persist_data.py
+++ b/lib/bb/persist_data.py
@@ -179,6 +179,9 @@ class SQLTable(collections.MutableMapping):
         elif not isinstance(value, str):
             raise TypeError('Only string values are supported')
 
+        # Ensure the entire transaction (including SELECT) executes under write lock
+        cursor.execute("BEGIN EXCLUSIVE")
+
         cursor.execute("SELECT * from %s where key=?;" % self.table, [key])
         row = cursor.fetchone()
         if row is not None:
-- 
2.17.1

[1.44 4/7] cooker/toaster: replaced deprecated method warn() with warning()

[Armin Kuster]

From: Frazer Clews <frazer.clews@codethink.co.uk>

Removed the deprecated methods as it will only cause problems later on,
and since warn() just calls warning(), it shouldn't change anything

Signed-off-by: Frazer Clews <frazer.clews@codethink.co.uk>
Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
(cherry picked from commit a194f275235f22411cb2368f06a44f61ceb6a0f3)
Signed-off-by: Armin Kuster <akuster808@gmail.com>
---
 lib/bb/cooker.py                                        | 2 +-
 lib/toaster/bldcontrol/management/commands/runbuilds.py | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/bb/cooker.py b/lib/bb/cooker.py
index b74affa7..911805a6 100644
--- a/lib/bb/cooker.py
+++ b/lib/bb/cooker.py
@@ -1788,7 +1788,7 @@ class CookerCollectFiles(object):
                 # When constructing an older style single regex, it's possible for BBMASK
                 # to end up beginning with '|', which matches and masks _everything_.
                 if mask.startswith("|"):
-                    collectlog.warn("BBMASK contains regular expression beginning with '|', fixing: %s" % mask)
+                    collectlog.warning("BBMASK contains regular expression beginning with '|', fixing: %s" % mask)
                     mask = mask[1:]
                 try:
                     re.compile(mask)
diff --git a/lib/toaster/bldcontrol/management/commands/runbuilds.py b/lib/toaster/bldcontrol/management/commands/runbuilds.py
index 50ec4098..19f659ec 100644
--- a/lib/toaster/bldcontrol/management/commands/runbuilds.py
+++ b/lib/toaster/bldcontrol/management/commands/runbuilds.py
@@ -168,17 +168,17 @@ class Command(BaseCommand):
         try:
             self.cleanup()
         except Exception as e:
-            logger.warn("runbuilds: cleanup exception %s" % str(e))
+            logger.warning("runbuilds: cleanup exception %s" % str(e))
 
         try:
             self.archive()
         except Exception as e:
-            logger.warn("runbuilds: archive exception %s" % str(e))
+            logger.warning("runbuilds: archive exception %s" % str(e))
 
         try:
             self.schedule()
         except Exception as e:
-            logger.warn("runbuilds: schedule exception %s" % str(e))
+            logger.warning("runbuilds: schedule exception %s" % str(e))
 
     def handle(self, **options):
         pidfile_path = os.path.join(os.environ.get("BUILDDIR", "."),
-- 
2.17.1

[1.44 5/7] siggen: Optimise get_unihash disk based cache handling

[Armin Kuster]

From: Richard Purdie <richard.purdie@linuxfoundation.org>

Currently the cache can grow huge since any previously used hash is
retained in the cache. This change moves to use one hash per task
which improves the speed of the functions considerably. Currently
performance is an issue, as are very large cache files and cache
load time.

By moving to a single hash per task, the shorted filename as a key
is no longer usable as the same recipe has multiple variants for
the same filename so this has to change.

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
(cherry picked from commit ed764e7fcf04b6d0ba6b4cac7415b1ee8f492865)
Signed-off-by: Armin Kuster <akuster808@gmail.com>
---
 lib/bb/siggen.py | 50 +++++++++++++++++++++++++++++++-----------------
 1 file changed, 32 insertions(+), 18 deletions(-)

diff --git a/lib/bb/siggen.py b/lib/bb/siggen.py
index ded1da02..209a3428 100644
--- a/lib/bb/siggen.py
+++ b/lib/bb/siggen.py
@@ -44,6 +44,7 @@ class SignatureGenerator(object):
         self.file_checksum_values = {}
         self.taints = {}
         self.unitaskhashes = {}
+        self.tidtopn = {}
         self.setscenetasks = set()
 
     def finalise(self, fn, d, varient):
@@ -79,19 +80,19 @@ class SignatureGenerator(object):
         return
 
     def get_taskdata(self):
-        return (self.runtaskdeps, self.taskhash, self.file_checksum_values, self.taints, self.basehash, self.unitaskhashes, self.setscenetasks)
+        return (self.runtaskdeps, self.taskhash, self.file_checksum_values, self.taints, self.basehash, self.unitaskhashes, self.tidtopn, self.setscenetasks)
 
     def set_taskdata(self, data):
-        self.runtaskdeps, self.taskhash, self.file_checksum_values, self.taints, self.basehash, self.unitaskhashes, self.setscenetasks = data
+        self.runtaskdeps, self.taskhash, self.file_checksum_values, self.taints, self.basehash, self.unitaskhashes, self.tidtopn, self.setscenetasks = data
 
     def reset(self, data):
         self.__init__(data)
 
     def get_taskhashes(self):
-        return self.taskhash, self.unitaskhashes
+        return self.taskhash, self.unitaskhashes, self.tidtopn
 
     def set_taskhashes(self, hashes):
-        self.taskhash, self.unitaskhashes = hashes
+        self.taskhash, self.unitaskhashes, self.tidtopn = hashes
 
     def save_unitaskhashes(self):
         return
@@ -124,9 +125,10 @@ class SignatureGeneratorBasic(SignatureGenerator):
         else:
             self.checksum_cache = None
 
-        self.unihash_cache = bb.cache.SimpleCache("1")
+        self.unihash_cache = bb.cache.SimpleCache("3")
         self.unitaskhashes = self.unihash_cache.init_cache(data, "bb_unihashes.dat", {})
         self.localdirsexclude = (data.getVar("BB_SIGNATURE_LOCAL_DIRS_EXCLUDE") or "CVS .bzr .git .hg .osc .p4 .repo .svn").split()
+        self.tidtopn = {}
 
     def init_rundepcheck(self, data):
         self.taskwhitelist = data.getVar("BB_HASHTASK_WHITELIST") or None
@@ -210,6 +212,9 @@ class SignatureGeneratorBasic(SignatureGenerator):
         self.runtaskdeps[tid] = []
         self.file_checksum_values[tid] = []
         recipename = dataCache.pkg_fn[fn]
+
+        self.tidtopn[tid] = recipename
+
         for dep in sorted(deps, key=clean_basepath):
             (depmc, _, deptaskname, depfn) = bb.runqueue.split_tid_mcfn(dep)
             if mc != depmc:
@@ -407,24 +412,35 @@ class SignatureGeneratorUniHashMixIn(object):
             self._client = hashserv.create_client(self.server)
         return self._client
 
-    def __get_task_unihash_key(self, tid):
-        # TODO: The key only *needs* to be the taskhash, the tid is just
-        # convenient
-        return '%s:%s' % (tid.rsplit("/", 1)[1], self.taskhash[tid])
-
     def get_stampfile_hash(self, tid):
         if tid in self.taskhash:
             # If a unique hash is reported, use it as the stampfile hash. This
             # ensures that if a task won't be re-run if the taskhash changes,
             # but it would result in the same output hash
-            unihash = self.unitaskhashes.get(self.__get_task_unihash_key(tid), None)
+            unihash = self._get_unihash(tid)
             if unihash is not None:
                 return unihash
 
         return super().get_stampfile_hash(tid)
 
     def set_unihash(self, tid, unihash):
-        self.unitaskhashes[self.__get_task_unihash_key(tid)] = unihash
+        (mc, fn, taskname, taskfn) = bb.runqueue.split_tid_mcfn(tid)
+        key = mc + ":" + self.tidtopn[tid] + ":" + taskname
+        self.unitaskhashes[key] = (self.taskhash[tid], unihash)
+
+    def _get_unihash(self, tid, checkkey=None):
+        if tid not in self.tidtopn:
+            return None
+        (mc, fn, taskname, taskfn) = bb.runqueue.split_tid_mcfn(tid)
+        key = mc + ":" + self.tidtopn[tid] + ":" + taskname
+        if key not in self.unitaskhashes:
+            return None
+        if not checkkey:
+            checkkey = self.taskhash[tid]
+        (key, unihash) = self.unitaskhashes[key]
+        if key != checkkey:
+            return None
+        return unihash
 
     def get_unihash(self, tid):
         taskhash = self.taskhash[tid]
@@ -433,11 +449,9 @@ class SignatureGeneratorUniHashMixIn(object):
         if self.setscenetasks and tid not in self.setscenetasks:
             return taskhash
 
-        key = self.__get_task_unihash_key(tid)
-
         # TODO: This cache can grow unbounded. It probably only needs to keep
         # for each task
-        unihash = self.unitaskhashes.get(key, None)
+        unihash =  self._get_unihash(tid)
         if unihash is not None:
             return unihash
 
@@ -472,7 +486,7 @@ class SignatureGeneratorUniHashMixIn(object):
         except hashserv.client.HashConnectionError as e:
             bb.warn('Error contacting Hash Equivalence Server %s: %s' % (self.server, str(e)))
 
-        self.unitaskhashes[key] = unihash
+        self.set_unihash(tid, unihash)
         return unihash
 
     def report_unihash(self, path, task, d):
@@ -484,13 +498,13 @@ class SignatureGeneratorUniHashMixIn(object):
         tempdir = d.getVar('T')
         fn = d.getVar('BB_FILENAME')
         tid = fn + ':do_' + task
-        key = tid.rsplit("/", 1)[1] + ':' + taskhash
+        key = tid + ':' + taskhash
 
         if self.setscenetasks and tid not in self.setscenetasks:
             return
 
         # Sanity checks
-        cache_unihash = self.unitaskhashes.get(key, None)
+        cache_unihash = self._get_unihash(tid, checkkey=taskhash)
         if cache_unihash is None:
             bb.fatal('%s not in unihash cache. Please report this error' % key)
 
-- 
2.17.1

[1.44 6/7] siggen: Cache unihash values to avoid cache lookup

[Armin Kuster]

From: Richard Purdie <richard.purdie@linuxfoundation.org>

Add unihash cache of values to speed up cache lookup.

This avoids the overhead of the disk based check functions.

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
(cherry picked from commit 5c9cc45b60904a1c355db9bf9c4495f1b25aca37)
Signed-off-by: Armin Kuster <akuster808@gmail.com>
---
 lib/bb/siggen.py | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/lib/bb/siggen.py b/lib/bb/siggen.py
index 209a3428..96807c46 100644
--- a/lib/bb/siggen.py
+++ b/lib/bb/siggen.py
@@ -40,6 +40,7 @@ class SignatureGenerator(object):
     def __init__(self, data):
         self.basehash = {}
         self.taskhash = {}
+        self.unihash = {}
         self.runtaskdeps = {}
         self.file_checksum_values = {}
         self.taints = {}
@@ -80,19 +81,19 @@ class SignatureGenerator(object):
         return
 
     def get_taskdata(self):
-        return (self.runtaskdeps, self.taskhash, self.file_checksum_values, self.taints, self.basehash, self.unitaskhashes, self.tidtopn, self.setscenetasks)
+        return (self.runtaskdeps, self.taskhash, self.unihash, self.file_checksum_values, self.taints, self.basehash, self.unitaskhashes, self.tidtopn, self.setscenetasks)
 
     def set_taskdata(self, data):
-        self.runtaskdeps, self.taskhash, self.file_checksum_values, self.taints, self.basehash, self.unitaskhashes, self.tidtopn, self.setscenetasks = data
+        self.runtaskdeps, self.taskhash, self.unihash, self.file_checksum_values, self.taints, self.basehash, self.unitaskhashes, self.tidtopn, self.setscenetasks = data
 
     def reset(self, data):
         self.__init__(data)
 
     def get_taskhashes(self):
-        return self.taskhash, self.unitaskhashes, self.tidtopn
+        return self.taskhash, self.unihash, self.unitaskhashes, self.tidtopn
 
     def set_taskhashes(self, hashes):
-        self.taskhash, self.unitaskhashes, self.tidtopn = hashes
+        self.taskhash, self.unihash, self.unitaskhashes, self.tidtopn = hashes
 
     def save_unitaskhashes(self):
         return
@@ -108,6 +109,7 @@ class SignatureGeneratorBasic(SignatureGenerator):
     def __init__(self, data):
         self.basehash = {}
         self.taskhash = {}
+        self.unihash = {}
         self.taskdeps = {}
         self.runtaskdeps = {}
         self.file_checksum_values = {}
@@ -256,7 +258,13 @@ class SignatureGeneratorBasic(SignatureGenerator):
 
         data = self.basehash[tid]
         for dep in self.runtaskdeps[tid]:
-            data = data + self.get_unihash(dep)
+            if dep in self.unihash:
+                if self.unihash[dep] is None:
+                    data = data + self.taskhash[dep]
+                else:
+                    data = data + self.unihash[dep]
+            else:
+                data = data + self.get_unihash(dep)
 
         for (f, cs) in self.file_checksum_values[tid]:
             if cs:
@@ -427,6 +435,7 @@ class SignatureGeneratorUniHashMixIn(object):
         (mc, fn, taskname, taskfn) = bb.runqueue.split_tid_mcfn(tid)
         key = mc + ":" + self.tidtopn[tid] + ":" + taskname
         self.unitaskhashes[key] = (self.taskhash[tid], unihash)
+        self.unihash[tid] = unihash
 
     def _get_unihash(self, tid, checkkey=None):
         if tid not in self.tidtopn:
@@ -447,12 +456,14 @@ class SignatureGeneratorUniHashMixIn(object):
 
         # If its not a setscene task we can return
         if self.setscenetasks and tid not in self.setscenetasks:
+            self.unihash[tid] = None
             return taskhash
 
         # TODO: This cache can grow unbounded. It probably only needs to keep
         # for each task
         unihash =  self._get_unihash(tid)
         if unihash is not None:
+            self.unihash[tid] = unihash
             return unihash
 
         # In the absence of being able to discover a unique hash from the
@@ -487,6 +498,7 @@ class SignatureGeneratorUniHashMixIn(object):
             bb.warn('Error contacting Hash Equivalence Server %s: %s' % (self.server, str(e)))
 
         self.set_unihash(tid, unihash)
+        self.unihash[tid] = unihash
         return unihash
 
     def report_unihash(self, path, task, d):
-- 
2.17.1

[1.44 7/7] siggen: Avoid cache mismatch issues with locked sigs

[Armin Kuster]

From: Richard Purdie <richard.purdie@linuxfoundation.org>

If locked sigs are in use this function makes little sense, need to
avoid generating mismatch warnings.

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
(cherry picked from commit 27ad9c1d468fba858a4adeb56b605227b415ae0f)
Signed-off-by: Armin Kuster <akuster808@gmail.com>
---
 lib/bb/siggen.py | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/lib/bb/siggen.py b/lib/bb/siggen.py
index 96807c46..ffd8fcaf 100644
--- a/lib/bb/siggen.py
+++ b/lib/bb/siggen.py
@@ -515,6 +515,10 @@ class SignatureGeneratorUniHashMixIn(object):
         if self.setscenetasks and tid not in self.setscenetasks:
             return
 
+        # This can happen if locked sigs are in action. Detect and just abort
+        if taskhash != self.taskhash[tid]:
+            return
+
         # Sanity checks
         cache_unihash = self._get_unihash(tid, checkkey=taskhash)
         if cache_unihash is None:
-- 
2.17.1