Re: [OE-core] [PATCH 3/7] sstatesig/find_siginfo: unify a disjointed API
On Fri, 2024-01-05 at 12:42 +0100, Alexander Kanavin wrote: > On Fri, 5 Jan 2024 at 12:22, Richard Purdie > wrote: > > > I've a few ideas on how we might be able to detect potential problems, > > I'll continue to try and work something out but I want to make it clear > > there are some things it is hard to change :/. > > There's the option of adding another function (find_siginfo_v2) and > leaving this one as it was, this is how the kernel does things. They > don't shy away from adding v3, v4 and so on either. But I thought less > code is better than more code. That is effectively what I just merged, except I did break compatibility by dropping the v1. We need to give the user good messages about how to fix their problem which is why a number is needed rather than just a function name. Cheers, Richard -=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#193354): https://lists.openembedded.org/g/openembedded-core/message/193354 Mute This Topic: https://lists.openembedded.org/mt/103239349/21656 Group Owner: openembedded-core+ow...@lists.openembedded.org Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [OE-core] [PATCH 3/7] sstatesig/find_siginfo: unify a disjointed API
On Fri, 5 Jan 2024 at 12:22, Richard Purdie wrote: > I've a few ideas on how we might be able to detect potential problems, > I'll continue to try and work something out but I want to make it clear > there are some things it is hard to change :/. There's the option of adding another function (find_siginfo_v2) and leaving this one as it was, this is how the kernel does things. They don't shy away from adding v3, v4 and so on either. But I thought less code is better than more code. Alex -=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#193352): https://lists.openembedded.org/g/openembedded-core/message/193352 Mute This Topic: https://lists.openembedded.org/mt/103239349/21656 Group Owner: openembedded-core+ow...@lists.openembedded.org Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [OE-core] [PATCH 3/7] sstatesig/find_siginfo: unify a disjointed API
On Mon, 2023-12-18 at 09:43 +0100, Alexander Kanavin wrote: > 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 > --- > 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(-) I can see why you've done this. I had thought this might have been a compatibility issue but it wasn't, it was just bad design from the start (and I wasn't responsible for it for a change!). It gives me a real headache for merging though since it requires OE- Core and bitbake to change in lockstep. There is a risk people might just update bitbake but not OE-Core and also vice versa, they update OE-Core but not bitbake. This can happen for all kinds of reasons, e.g. mixing a newer bitbake with an older release. The latter is easy to address with a bitbake version check bit the former is not really possible to detect with our current code as bitbake can't know OE-Core doesn't support what it needs. With other changes in the pipeline like inherit_defer and the fact there is a bug related to this on a stable branch, it makes it all the more likely people will try and mix/match different versions. I've a few ideas on how we might be able to detect potential problems, I'll continue to try and work something out but I want to make it clear there are some things it is hard to change :/. Cheers, Richard -=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#193351): https://lists.openembedded.org/g/openembedded-core/message/193351 Mute This Topic: https://lists.openembedded.org/mt/103239349/21656 Group Owner: openembedded-core+ow...@lists.openembedded.org Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[OE-core] [PATCH 3/7] sstatesig/find_siginfo: unify a disjointed API
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
---
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
