Openembedded Core Discussions
 help / color / mirror / Atom feed
From: Alexander Kanavin <alex.kanavin@gmail.com>
To: openembedded-core@lists.openembedded.org
Cc: Alexander Kanavin <alex@linutronix.de>
Subject: [PATCH 6/9] sstatesig/find_siginfo: unify a disjointed API
Date: Thu, 14 Dec 2023 14:45:25 +0100	[thread overview]
Message-ID: <20231214134528.1973602-6-alex@linutronix.de> (raw)
In-Reply-To: <20231214134528.1973602-1-alex@linutronix.de>

find_siginfo() returns two different data structures depending
on whether its third argument (list of hashes to find) is empty or
not:
- a dict of timestamps keyed by path
- a dict of paths keyed by hash

This is not a good API design; it's much better to return
a dict of dicts that include both timestamp and path, keyed by
hash. Then the API consumer can decide how they want to use these
fields, particularly for additional diagnostics or informational
output.

I also took the opportunity to add a binary field that
tells if the match came from sstate or local stamps dir, which
will help prioritize local stamps when looking up most
recent task signatures.

Signed-off-by: Alexander Kanavin <alex@linutronix.de>
---
 meta/lib/oe/buildhistory_analysis.py        |  2 +-
 meta/lib/oe/sstatesig.py                    | 31 +++++++++------------
 meta/lib/oeqa/selftest/cases/sstatetests.py | 10 +++----
 3 files changed, 19 insertions(+), 24 deletions(-)

diff --git a/meta/lib/oe/buildhistory_analysis.py b/meta/lib/oe/buildhistory_analysis.py
index b1856846b6a..4edad01580c 100644
--- a/meta/lib/oe/buildhistory_analysis.py
+++ b/meta/lib/oe/buildhistory_analysis.py
@@ -562,7 +562,7 @@ def compare_siglists(a_blob, b_blob, taskdiff=False):
                 elif not hash2 in hashfiles:
                     out.append("Unable to find matching sigdata for %s with hash %s" % (desc, hash2))
                 else:
-                    out2 = bb.siggen.compare_sigfiles(hashfiles[hash1], hashfiles[hash2], recursecb, collapsed=True)
+                    out2 = bb.siggen.compare_sigfiles(hashfiles[hash1]['path'], hashfiles[hash2]['path'], recursecb, collapsed=True)
                     for line in out2:
                         m = hashlib.sha256()
                         m.update(line.encode('utf-8'))
diff --git a/meta/lib/oe/sstatesig.py b/meta/lib/oe/sstatesig.py
index 8a97fb0c04b..0342bcdc87a 100644
--- a/meta/lib/oe/sstatesig.py
+++ b/meta/lib/oe/sstatesig.py
@@ -349,7 +349,6 @@ def find_siginfo(pn, taskname, taskhashlist, d):
            pn, taskname = key.split(':', 1)
 
     hashfiles = {}
-    filedates = {}
 
     def get_hashval(siginfo):
         if siginfo.endswith('.siginfo'):
@@ -357,6 +356,12 @@ def find_siginfo(pn, taskname, taskhashlist, d):
         else:
             return siginfo.rpartition('.')[2]
 
+    def get_time(fullpath):
+        try:
+            return os.stat(fullpath).st_mtime
+        except OSError:
+            return None
+
     # First search in stamps dir
     localdata = d.createCopy()
     localdata.setVar('MULTIMACH_TARGET_SYS', '*')
@@ -372,24 +377,21 @@ def find_siginfo(pn, taskname, taskhashlist, d):
     filespec = '%s.%s.sigdata.*' % (stamp, taskname)
     foundall = False
     import glob
+    bb.debug(1, "Calling glob.glob on {}".format(filespec))
     for fullpath in glob.glob(filespec):
         match = False
         if taskhashlist:
             for taskhash in taskhashlist:
                 if fullpath.endswith('.%s' % taskhash):
-                    hashfiles[taskhash] = fullpath
+                    hashfiles[taskhash] = {'path':fullpath, 'sstate':False, 'time':get_time(fullpath)}
                     if len(hashfiles) == len(taskhashlist):
                         foundall = True
                         break
         else:
-            try:
-                filedates[fullpath] = os.stat(fullpath).st_mtime
-            except OSError:
-                continue
             hashval = get_hashval(fullpath)
-            hashfiles[hashval] = fullpath
+            hashfiles[hashval] = {'path':fullpath, 'sstate':False, 'time':get_time(fullpath)}
 
-    if not taskhashlist or (len(filedates) < 2 and not foundall):
+    if not taskhashlist or (len(hashfiles) < 2 and not foundall):
         # That didn't work, look in sstate-cache
         hashes = taskhashlist or ['?' * 64]
         localdata = bb.data.createCopy(d)
@@ -412,22 +414,15 @@ def find_siginfo(pn, taskname, taskhashlist, d):
                 localdata.setVar('SSTATE_EXTRAPATH', "${NATIVELSBSTRING}/")
             filespec = '%s.siginfo' % localdata.getVar('SSTATE_PKG')
 
+            bb.debug(1, "Calling glob.glob on {}".format(filespec))
             matchedfiles = glob.glob(filespec)
             for fullpath in matchedfiles:
                 actual_hashval = get_hashval(fullpath)
                 if actual_hashval in hashfiles:
                     continue
-                hashfiles[hashval] = fullpath
-                if not taskhashlist:
-                    try:
-                        filedates[fullpath] = os.stat(fullpath).st_mtime
-                    except:
-                        continue
+                hashfiles[actual_hashval] = {'path':fullpath, 'sstate':True, 'time':get_time(fullpath)}
 
-    if taskhashlist:
-        return hashfiles
-    else:
-        return filedates
+    return hashfiles
 
 bb.siggen.find_siginfo = find_siginfo
 
diff --git a/meta/lib/oeqa/selftest/cases/sstatetests.py b/meta/lib/oeqa/selftest/cases/sstatetests.py
index f5b3437d86b..83abcdfdd62 100644
--- a/meta/lib/oeqa/selftest/cases/sstatetests.py
+++ b/meta/lib/oeqa/selftest/cases/sstatetests.py
@@ -765,14 +765,14 @@ addtask tmptask2 before do_tmptask1
                 hashes = [hash1, hash2]
                 hashfiles = find_siginfo(key, None, hashes)
                 self.assertCountEqual(hashes, hashfiles)
-                bb.siggen.compare_sigfiles(hashfiles[hash1], hashfiles[hash2], recursecb)
+                bb.siggen.compare_sigfiles(hashfiles[hash1]['path'], hashfiles[hash2]['path'], recursecb)
 
             for pn in pns:
                 recursecb_count = 0
-                filedates = find_siginfo(pn, "do_tmptask1")
-                self.assertGreaterEqual(len(filedates), 2)
-                latestfiles = sorted(filedates.keys(), key=lambda f: filedates[f])[-2:]
-                bb.siggen.compare_sigfiles(latestfiles[-2], latestfiles[-1], recursecb)
+                matches = find_siginfo(pn, "do_tmptask1")
+                self.assertGreaterEqual(len(matches), 2)
+                latesthashes = sorted(matches.keys(), key=lambda h: matches[h]['time'])[-2:]
+                bb.siggen.compare_sigfiles(matches[latesthashes[-2]]['path'], matches[latesthashes[-1]]['path'], recursecb)
                 self.assertEqual(recursecb_count,1)
 
 class SStatePrintdiff(SStateBase):
-- 
2.39.2



  parent reply	other threads:[~2023-12-14 13:46 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-14 13:45 [PATCH 1/9] oeqa/selftest/sstatetests: re-work CDN tests, add local cache tests Alexander Kanavin
2023-12-14 13:45 ` [PATCH 2/9] gnu-config: delete do_compile task Alexander Kanavin
2023-12-14 13:45 ` [PATCH 3/9] bitbake/runqueue: initialize RunQueueExecute before printdiff rather than after Alexander Kanavin
2023-12-14 14:39   ` [OE-core] " Richard Purdie
2023-12-14 17:28     ` Alexander Kanavin
2023-12-15 16:04       ` Richard Purdie
2023-12-15 16:49         ` Alexander Kanavin
2023-12-15 16:56           ` Richard Purdie
2023-12-14 13:45 ` [PATCH 4/9] bitbake/runqueue: rework 'bitbake -S printdiff' logic Alexander Kanavin
2023-12-14 13:45 ` [PATCH 5/9] selftest/sstatetests: fix up printdiff test to match rework of printdiff logic Alexander Kanavin
2023-12-14 13:45 ` Alexander Kanavin [this message]
2023-12-14 13:45 ` [PATCH 7/9] bitbake-diffsigs/runqueue: adapt to reworked find_siginfo() Alexander Kanavin
2023-12-14 13:45 ` [PATCH 8/9] bitbake/runqueue: prioritize local stamps over sstate signatures in printdiff Alexander Kanavin
2023-12-14 13:45 ` [PATCH 9/9] bitbake/runqueue: add debugging for find_siginfo() calls Alexander Kanavin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20231214134528.1973602-6-alex@linutronix.de \
    --to=alex.kanavin@gmail.com \
    --cc=alex@linutronix.de \
    --cc=openembedded-core@lists.openembedded.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox