[PATCH 0/9] Add more signature cachability tests to the testsuite

38 views
Skip to first unread message

ker...@gmail.com

unread,
Apr 2, 2024, 1:28:57 PM4/2/24
to isar-...@googlegroups.com, Christopher Larson
From: Christopher Larson <chris....@siemens.com>

This series improves isar-sstate's lint command to support checking sigdata in tmp/stamps and to check for absolute paths in SRC_URI, and adds an additional test to the testsuite to exercise this capability. The new test checks for signature cachability issues much more quickly, as it does so without a full build, so it checks more target configurations. This uses bitbake's `-S none` option to avoid building anything, and then runs isar-sstate lint on the resulting sigdata in tmp/stamps.

This test will accept a verbose parameter on the command-line to pass --verbose to isar-sstate lint, which will show the "other" absolute paths found, not just source and build directory. This series also fixes two of the failures which were identified by the new test.

Here we can see the issues identified by the new test, which are fixed in this series:
```
$ avocado run testsuite/citest.py -t signatures --max-parallel-tasks=1
JOB ID : 3a3308967946663d9b239f638b030502fa80ef0a
JOB LOG : /builder/avocado/job-results/job-2024-03-29T19.24-3a33089/job.log
(1/1) testsuite/citest.py:SignatureTest.test_signature_lint: STARTED
(1/1) testsuite/citest.py:SignatureTest.test_signature_lint: FAIL: Detected cachability issues (42.93 s)
RESULTS : PASS 0 | ERROR 0 | FAIL 1 | SKIP 0 | WARN 0 | INTERRUPT 0 | CANCEL 0
JOB TIME : 44.63 s

Test summary:
1-testsuite/citest.py:SignatureTest.test_signature_lint: FAIL
```

Applicable section of the avocado full.log:
```
1-testsuite/citest.py:SignatureTest.test_signature_lint: 2024-03-29 19:25:10,818 avocado.test cibase L0142 ERROR| ==== issues found in ubuntu-focal-amd64:isar-bootstrap-target:unpack (95af4581) ====
1-testsuite/citest.py:SignatureTest.test_signature_lint: 2024-03-29 19:25:10,818 avocado.test cibase L0142 ERROR| -> path in sources-dir: SRC_URI entry "file:///home/kergoth/Code/industrial/signatures/isar/meta-isar/conf/distro/ubuntu.public.key;sha256sum=36a38199a4bf4eae1e7f574891f7dfcb79b91b87a33a499383265e1224b5e989"
1-testsuite/citest.py:SignatureTest.test_signature_lint: 2024-03-29 19:25:10,818 avocado.test cibase L0142 ERROR| ==== issues found in ubuntu-focal-amd64:isar-bootstrap-target:fetch (e8249cb2) ====
1-testsuite/citest.py:SignatureTest.test_signature_lint: 2024-03-29 19:25:10,818 avocado.test cibase L0142 ERROR| -> path in sources-dir: SRC_URI entry "file:///home/kergoth/Code/industrial/signatures/isar/meta-isar/conf/distro/ubuntu.public.key;sha256sum=36a38199a4bf4eae1e7f574891f7dfcb79b91b87a33a499383265e1224b5e989"
1-testsuite/citest.py:SignatureTest.test_signature_lint: 2024-03-29 19:25:10,818 avocado.test cibase L0142 ERROR| ==== issues found in debian-bullseye-amd64:isar-ci-ssh-setup:install (1c0a8b21) ====
1-testsuite/citest.py:SignatureTest.test_signature_lint: 2024-03-29 19:25:10,818 avocado.test cibase L0142 ERROR| -> path in sources-dir: TESTSUITEDIR = "/home/kergoth/Code/industrial/signatures/isar/testsuite"
1-testsuite/citest.py:SignatureTest.test_signature_lint: 2024-03-29 19:25:10,818 avocado.test cibase L0142 ERROR| found cachability issues (scanned 602 signatures)
1-testsuite/citest.py:SignatureTest.test_signature_lint: 2024-03-29 19:25:10,818 avocado.test cibase L0142 ERROR| -> absolute paths: sources-dir 3, build-dir 0, other 184
```

If we strip off the prefix for legibility:
```
ERROR| ==== issues found in ubuntu-focal-amd64:isar-bootstrap-target:unpack (95af4581) ====
ERROR| -> path in sources-dir: SRC_URI entry "file:///home/kergoth/Code/industrial/signatures/isar/meta-isar/conf/distro/ubuntu.public.key;sha256sum=36a38199a4bf4eae1e7f574891f7dfcb79b91b87a33a499383265e1224b5e989"
ERROR| ==== issues found in ubuntu-focal-amd64:isar-bootstrap-target:fetch (e8249cb2) ====
ERROR| -> path in sources-dir: SRC_URI entry "file:///home/kergoth/Code/industrial/signatures/isar/meta-isar/conf/distro/ubuntu.public.key;sha256sum=36a38199a4bf4eae1e7f574891f7dfcb79b91b87a33a499383265e1224b5e989"
ERROR| ==== issues found in debian-bullseye-amd64:isar-ci-ssh-setup:install (1c0a8b21) ====
ERROR| -> path in sources-dir: TESTSUITEDIR = "/home/kergoth/Code/industrial/signatures/isar/testsuite"
ERROR| found cachability issues (scanned 602 signatures)
ERROR| -> absolute paths: sources-dir 3, build-dir 0, other 184
```

The sstate/signatures tests were run for each commit in this series using `git rebase -x` to ensure it does not break a bisect.

I welcome any and all feedback on this. In particular I don't love extending the hardcoded list of image tasks being ignored in isar-sstate lint, but it's a start. I'm open to suggestions.

Christopher Larson (9):
isar-bootstrap: avoid forced early expansion of key vars
isar-ci-ssh-setup: avoid abs path in signatures
isar-sstate: lint: check for absolute paths in SRC_URI
isar-sstate: lint: add support for checking stamps
isar-sstate: lint: ignore more image tasks
isar-sstate: add --excluded-tasks argument
cibuilder.py: add -S support to the bitbake method
testsuite: add perform_signature_lint method
testsuite: add signature cachability checks

.../isar-ci-ssh-setup_0.1.bb | 3 +
.../isar-bootstrap/isar-bootstrap.inc | 8 ++-
scripts/isar-sstate | 60 +++++++++++++++----
testsuite/cibase.py | 19 ++++++
testsuite/cibuilder.py | 5 +-
testsuite/citest.py | 22 ++++++-
6 files changed, 100 insertions(+), 17 deletions(-)

--
2.39.2

ker...@gmail.com

unread,
Apr 2, 2024, 1:28:59 PM4/2/24
to isar-...@googlegroups.com, Christopher Larson, Christopher Larson
From: Christopher Larson <chris....@seimens.com>

Rather than appending the items from the expanded key variables into
SRC_URI individually, which means there's no way to use tools like
vardepvalue or vardepexclude to control signature generation, append the
unexpanded variables to the SRC_URI directly. This avoids issues with
shared state reuse for the isar-bootstrap packages.

Signed-off-by: Christopher Larson <chris....@siemens.com>
---
meta/recipes-core/isar-bootstrap/isar-bootstrap.inc | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc b/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc
index 17f19fd8..de14e946 100644
--- a/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc
+++ b/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc
@@ -10,7 +10,9 @@ LIC_FILES_CHKSUM = "file://${LAYERDIR_core}/licenses/COPYING.GPLv2;md5=751419260
FILESPATH:prepend := "${THISDIR}/files:"
SRC_URI = " \
file://locale \
- file://chroot-setup.sh"
+ file://chroot-setup.sh \
+ ${DISTRO_BOOTSTRAP_KEYS} \
+ ${THIRD_PARTY_APT_KEYS}"
PV = "1.0"

BOOTSTRAP_FOR_HOST ?= "0"
@@ -22,6 +24,8 @@ APTSRCS = "${WORKDIR}/apt-sources"
APTSRCS_INIT = "${WORKDIR}/apt-sources-init"
DISTRO_BOOTSTRAP_KEYFILES = ""
THIRD_PARTY_APT_KEYFILES = ""
+DISTRO_BOOTSTRAP_KEYS ?= ""
+THIRD_PARTY_APT_KEYS ?= ""
DEPLOY_ISAR_BOOTSTRAP ?= ""
DISTRO_BOOTSTRAP_BASE_PACKAGES = "locales"
DISTRO_BOOTSTRAP_BASE_PACKAGES:append:gnupg = ",gnupg"
@@ -48,13 +52,11 @@ python () {
distro_bootstrap_keys += own_pub_key.split()

for key in distro_bootstrap_keys:
- d.appendVar("SRC_URI", " %s" % key)
fetcher = bb.fetch2.Fetch([key], d)
filename = os.path.relpath(fetcher.localpath(key), topdir)
d.appendVar("DISTRO_BOOTSTRAP_KEYFILES", " ${TOPDIR}/%s" % filename)

for key in third_party_apt_keys:
- d.appendVar("SRC_URI", " %s" % key)
fetcher = bb.fetch2.Fetch([key], d)
filename = os.path.relpath(fetcher.localpath(key), topdir)
d.appendVar("THIRD_PARTY_APT_KEYFILES", " ${TOPDIR}/%s" % filename)
--
2.39.2

ker...@gmail.com

unread,
Apr 2, 2024, 1:29:00 PM4/2/24
to isar-...@googlegroups.com, Christopher Larson, Christopher Larson
From: Christopher Larson <chris....@seimens.com>

TESTSUITEDIR is a full absolute path to the testsuite directory in isar,
as set in the environment by the build setup scripts. This is referenced
in the install task, which prevents shared state reuse for this package.
While this is predominently used in CI, it's still a good idea to avoid
absolute paths in signatures, so we can reuse shared state for this
package in other contexts.

Rather than excluding the TESTSUITEDIR from signatures entirely with
vardepsexclude, we can retain some information about the path by using
os.path.relpath to make it relative to the top directory of the build.
This is the same approach used by isar-bootstrap for the keys, and the
vardepvalue approach is also used elsewhere for layer paths.

Signed-off-by: Christopher Larson <chris....@siemens.com>
---
.../recipes-ci/isar-ci-ssh-setup/isar-ci-ssh-setup_0.1.bb | 3 +++
1 file changed, 3 insertions(+)

diff --git a/meta-test/recipes-ci/isar-ci-ssh-setup/isar-ci-ssh-setup_0.1.bb b/meta-test/recipes-ci/isar-ci-ssh-setup/isar-ci-ssh-setup_0.1.bb
index 4693f647..89100444 100644
--- a/meta-test/recipes-ci/isar-ci-ssh-setup/isar-ci-ssh-setup_0.1.bb
+++ b/meta-test/recipes-ci/isar-ci-ssh-setup/isar-ci-ssh-setup_0.1.bb
@@ -13,6 +13,9 @@ DEBIAN_DEPENDS = "adduser, apt (>= 0.4.2), network-manager, sshd-regen-keys"

inherit dpkg-raw

+# Avoid absolute paths in signatures which prevent shared state reuse
+TESTSUITEDIR[vardepvalue] = "${@os.path.relpath('${TESTSUITEDIR}', '${TOPDIR}')}"
+
do_install() {
# Install authorized SSH keys
install -v -d ${D}/var/lib/isar-ci/.ssh/
--
2.39.2

ker...@gmail.com

unread,
Apr 2, 2024, 1:29:01 PM4/2/24
to isar-...@googlegroups.com, Christopher Larson, Christopher Larson
From: Christopher Larson <chris....@seimens.com>

In addition to the current checks for variables starting with an
absolute path, particularly those within the build or sources
directories, we should also check for absolute paths in SRC_URI
file entries.

Signed-off-by: Christopher Larson <chris....@siemens.com>
---
scripts/isar-sstate | 17 +++++++++++++++++
1 file changed, 17 insertions(+)

diff --git a/scripts/isar-sstate b/scripts/isar-sstate
index c14c2843..9b20cb8e 100755
--- a/scripts/isar-sstate
+++ b/scripts/isar-sstate
@@ -839,6 +839,23 @@ def sstate_lint(target, verbose, sources_dir, build_dir, exit_code, pedantic, **
continue
# remove leading whitespaces possibly added by appending
val = val.lstrip()
+ if name == 'SRC_URI':
+ src_uri = val.split()
+ for entry in src_uri:
+ if entry.startswith('file:///'):
+ entry_path = entry[7:]
+ if entry_path.startswith(build_dir):
+ pn_issues.append(f'\033[0;31m-> path in build-dir: SRC_URI entry "{entry}"\033[0m')
+ hits_builddir += 1
+ elif entry_path.startswith(sources_dir):
+ pn_issues.append(f'\033[0;31m-> path in sources-dir: SRC_URI entry "{entry}"\033[0m')
+ hits_srcdir += 1
+ else:
+ hits_other += 1
+ if verbose:
+ pn_issues.append(f'\033[0;34m-> other absolute path: SRC_URI entry "{entry}"\033[0m')
+ continue
+
if not val[0] == '/':
continue
if val.startswith(build_dir):
--
2.39.2

ker...@gmail.com

unread,
Apr 2, 2024, 1:29:02 PM4/2/24
to isar-...@googlegroups.com, Christopher Larson, Christopher Larson
From: Christopher Larson <chris....@seimens.com>

Bitbake supports writing signature data directly to the stamps directory
without having to build, so we should add the ability to lint this
signature data as well. This is useful for checking for cachability
issues without having to complete a build.

Signed-off-by: Christopher Larson <chris....@siemens.com>
---
scripts/isar-sstate | 34 +++++++++++++++++++++++-----------
1 file changed, 23 insertions(+), 11 deletions(-)

diff --git a/scripts/isar-sstate b/scripts/isar-sstate
index 9b20cb8e..b77f73eb 100755
--- a/scripts/isar-sstate
+++ b/scripts/isar-sstate
@@ -154,10 +154,12 @@ SstateCacheEntry = namedtuple(
# "${PV}:${PR}:${SSTATE_PKGARCH}:${SSTATE_VERSION}:"

# This regex extracts relevant fields:
-SstateRegex = re.compile(r'sstate:(?P<pn>[^:]*):[^:]*:[^:]*:[^:]*:'
+SstateRegex = re.compile(r'(.*/)?sstate:(?P<pn>[^:]*):[^:]*:[^:]*:[^:]*:'
r'(?P<arch>[^:]*):[^:]*:(?P<hash>[0-9a-f]*)_'
r'(?P<task>[^\.]*)\.(?P<suffix>.*)')
-
+StampsRegex = re.compile(
+ r"(.*/)?(?P<arch>[^/]+)/(?P<pn>[^/]+)/([^/]+)\.do_(?P<task>[^/]+)\.(?P<suffix>sigdata)\.(?P<hash>[0-9a-f]{64})"
+)

class SstateTargetBase(object):
def __init__(self, path, cached=False):
@@ -288,12 +290,13 @@ class SstateTargetBase(object):


class SstateFileTarget(SstateTargetBase):
- def __init__(self, path, **kwargs):
+ def __init__(self, path, regex=SstateRegex, **kwargs):
super().__init__(path, **kwargs)
if path.startswith('file://'):
path = path[len('file://'):]
self.path = path
self.basepath = os.path.abspath(path)
+ self.regex = regex

def __repr__(self):
return f"file://{self.path}"
@@ -334,12 +337,13 @@ class SstateFileTarget(SstateTargetBase):
for subdir, dirs, files in os.walk(self.basepath):
reldir = subdir[(len(self.basepath)+1):]
for f in files:
- m = SstateRegex.match(f)
+ relative = os.path.join(reldir, f)
+ m = self.regex.match(relative)
if m is not None:
islink = os.path.islink(os.path.join(subdir, f))
age = int(now - os.path.getmtime(os.path.join(subdir, f)))
all_files.append(SstateCacheEntry(
- path=os.path.join(reldir, f),
+ path=relative,
size=os.path.getsize(os.path.join(subdir, f)),
islink=islink,
age=age,
@@ -592,6 +596,9 @@ def arguments():
parser.add_argument(
'--exit-code', type=int, default=None,
help="lint: return this instead of number of found issues")
+ parser.add_argument(
+ '--lint-stamps', default=False, action='store_true',
+ help="lint: assume target is a stamps directory (target must be a local path)")

args = parser.parse_args()
if args.command in 'upload analyze'.split() and args.source is None:
@@ -798,7 +805,7 @@ def sstate_analyze(source, target, **kwargs):
print('\n'.join(out))


-def sstate_lint(target, verbose, sources_dir, build_dir, exit_code, pedantic, **kwargs):
+def sstate_lint(target, verbose, sources_dir, build_dir, exit_code, pedantic, lint_stamps, **kwargs):
ADDITIONAL_IGNORED_VARNAMES = 'PP'.split()
# only list non-cacheable tasks here
# note that these still can break caching of other tasks that depend on these.
@@ -809,7 +816,10 @@ def sstate_lint(target, verbose, sources_dir, build_dir, exit_code, pedantic, **
print(f"WARNING: target {target} does not exist. Nothing to analyze.")
return 0

- cache_sigs = {s.hash: s for s in target.list_all() if s.suffix.endswith('.siginfo')}
+ if lint_stamps:
+ cache_sigs = {s.hash: s for s in target.list_all()}
+ else:
+ cache_sigs = {s.hash: s for s in target.list_all() if s.suffix.endswith('.siginfo')}

hits_srcdir = 0
hits_builddir = 0
@@ -891,10 +901,12 @@ def main():
target = SstateDavTarget(args.target)
elif args.target.startswith('s3://'):
target = SstateS3Target(args.target)
- elif args.target.startswith('file://'):
- target = SstateFileTarget(args.target)
- else: # no protocol given, assume file://
- target = SstateFileTarget(args.target)
+ else: # Either file://, or no protocol given, assume file://
+ target = SstateFileTarget(args.target, StampsRegex if args.lint_stamps else SstateRegex)
+
+ if args.lint_stamps and not isinstance(target, SstateFileTarget):
+ print("ERROR: --lint-stamps only works with local file targets")
+ return 1

args.target = target
return globals()[f'sstate_{args.command}'](**vars(args))
--
2.39.2

ker...@gmail.com

unread,
Apr 2, 2024, 1:29:04 PM4/2/24
to isar-...@googlegroups.com, Christopher Larson, Christopher Larson
From: Christopher Larson <chris....@seimens.com>

Currently isar-sstate ignores image_wic, but not the other image tasks.
Add additional image tasks to align with this behavior.

Signed-off-by: Christopher Larson <chris....@siemens.com>
---
scripts/isar-sstate | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/scripts/isar-sstate b/scripts/isar-sstate
index b77f73eb..5270f944 100755
--- a/scripts/isar-sstate
+++ b/scripts/isar-sstate
@@ -146,6 +146,8 @@ try:
except ModuleNotFoundError:
s3_supported = False

+DEFAULT_IGNORED_TASKS = "rootfs_wicenv image_wic image_cpio image_tar image_ext4"
+
SstateCacheEntry = namedtuple(
'SstateCacheEntry', 'hash path arch pn task suffix islink age size'.split())

@@ -811,7 +813,7 @@ def sstate_lint(target, verbose, sources_dir, build_dir, exit_code, pedantic, li
# note that these still can break caching of other tasks that depend on these.
# Run in pedantic mode to also look for these issues (e.g. in image-in-image builds)
# To make a build fully cacheable, avoid absolute paths in BBLAYERS
- ADDITIONAL_IGNORED_TASKS = list() if pedantic else 'rootfs_wicenv image_wic'.split()
+ ADDITIONAL_IGNORED_TASKS = list() if pedantic else DEFAULT_IGNORED_TASKS.split()
if not target.exists():
print(f"WARNING: target {target} does not exist. Nothing to analyze.")
return 0
--
2.39.2

ker...@gmail.com

unread,
Apr 2, 2024, 1:29:05 PM4/2/24
to isar-...@googlegroups.com, Christopher Larson, Christopher Larson
From: Christopher Larson <chris....@seimens.com>

This allows the user to override the default lists of tasks to ignore
when linting the sstate cache.

Signed-off-by: Christopher Larson <chris....@siemens.com>
---
scripts/isar-sstate | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/scripts/isar-sstate b/scripts/isar-sstate
index 5270f944..dddfafcb 100755
--- a/scripts/isar-sstate
+++ b/scripts/isar-sstate
@@ -146,7 +146,7 @@ try:
except ModuleNotFoundError:
s3_supported = False

-DEFAULT_IGNORED_TASKS = "rootfs_wicenv image_wic image_cpio image_tar image_ext4"
+DEFAULT_IGNORED_TASKS = "rootfs_wicenv,image_wic,image_cpio,image_tar,image_ext4"

SstateCacheEntry = namedtuple(
'SstateCacheEntry', 'hash path arch pn task suffix islink age size'.split())
@@ -601,6 +601,9 @@ def arguments():
parser.add_argument(
'--lint-stamps', default=False, action='store_true',
help="lint: assume target is a stamps directory (target must be a local path)")
+ parser.add_argument(
+ '--excluded-tasks', type=str, default=DEFAULT_IGNORED_TASKS,
+ help="lint: comma-separated list of tasks to ignore (default: %(default)s)")

args = parser.parse_args()
if args.command in 'upload analyze'.split() and args.source is None:
@@ -609,6 +612,7 @@ def arguments():
elif args.command in 'info clean'.split() and args.source is not None:
print(f"ERROR: '{args.command}' must not have a source (only a target)")
sys.exit(1)
+ args.excluded_tasks = args.excluded_tasks.split(',')
return args


@@ -807,13 +811,14 @@ def sstate_analyze(source, target, **kwargs):
print('\n'.join(out))


-def sstate_lint(target, verbose, sources_dir, build_dir, exit_code, pedantic, lint_stamps, **kwargs):
+def sstate_lint(target, verbose, sources_dir, build_dir, exit_code, pedantic, lint_stamps,
+ excluded_tasks, **kwargs):
ADDITIONAL_IGNORED_VARNAMES = 'PP'.split()
# only list non-cacheable tasks here
# note that these still can break caching of other tasks that depend on these.
# Run in pedantic mode to also look for these issues (e.g. in image-in-image builds)
# To make a build fully cacheable, avoid absolute paths in BBLAYERS
- ADDITIONAL_IGNORED_TASKS = list() if pedantic else DEFAULT_IGNORED_TASKS.split()
+ ADDITIONAL_IGNORED_TASKS = list() if pedantic else excluded_tasks

ker...@gmail.com

unread,
Apr 2, 2024, 1:29:06 PM4/2/24
to isar-...@googlegroups.com, Christopher Larson, Christopher Larson
From: Christopher Larson <chris....@seimens.com>

This allows a test writer to call `bitbake -S none`, to generate
signature data in tmp/stamps.

Signed-off-by: Christopher Larson <chris....@siemens.com>
---
testsuite/cibuilder.py | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/testsuite/cibuilder.py b/testsuite/cibuilder.py
index fa30c2f5..e968d14d 100755
--- a/testsuite/cibuilder.py
+++ b/testsuite/cibuilder.py
@@ -188,7 +188,7 @@ def move_in_build_dir(self, src, dst):
if os.path.exists(self.build_dir + '/' + src):
shutil.move(self.build_dir + '/' + src, self.build_dir + '/' + dst)

- def bitbake(self, target, bitbake_cmd=None, **kwargs):
+ def bitbake(self, target, bitbake_cmd=None, sig_handler=None, **kwargs):
self.check_init()
self.log.info('===================================================')
self.log.info('Building ' + str(target))
@@ -200,6 +200,9 @@ def bitbake(self, target, bitbake_cmd=None, **kwargs):
if bitbake_cmd:
cmdline.append('-c')
cmdline.append(bitbake_cmd)
+ if sig_handler:
+ cmdline.append('-S')
+ cmdline.append(sig_handler)
if isinstance(target, list):
cmdline.extend(target)
else:
--
2.39.2

ker...@gmail.com

unread,
Apr 2, 2024, 1:29:07 PM4/2/24
to isar-...@googlegroups.com, Christopher Larson, Christopher Larson
From: Christopher Larson <chris....@seimens.com>

This method is provided to generate signature data for specified target
or targets and check for cachability issues without having to complete a
build.

Signed-off-by: Christopher Larson <chris....@siemens.com>
---
testsuite/cibase.py | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)

diff --git a/testsuite/cibase.py b/testsuite/cibase.py
index 90591f32..349a79f0 100755
--- a/testsuite/cibase.py
+++ b/testsuite/cibase.py
@@ -125,6 +125,25 @@ def perform_sstate_populate(self, image_target, **kwargs):
# Remove isar configuration so the the following test creates a new one
self.delete_from_build_dir('conf')

+ def perform_signature_lint(self, targets, verbose=False, sources_dir=isar_root,
+ excluded_tasks=None, **kwargs):
+ """Generate signature data for target(s) and check for cachability issues."""
+ self.configure(**kwargs)
+ self.move_in_build_dir("tmp", "tmp_before_sstate")
+ self.bitbake(targets, sig_handler="none")
+
+ verbose_arg = "--verbose" if verbose else ""
+ excluded_arg = f"--excluded-tasks {','.join(excluded_tasks)}" if excluded_tasks else ""
+ cmd = f"{isar_root}/scripts/isar-sstate lint --lint-stamps {self.build_dir}/tmp/stamps " \
+ f"--build-dir {self.build_dir} --sources-dir {sources_dir} {verbose_arg} {excluded_arg}"
+ self.log.info(f"Running: {cmd}")
+ exit_status, output = process.getstatusoutput(cmd, ignore_status=True)
+ if exit_status > 0:
+ ansi_escape = re.compile(r'\x1B(?:[@-Z\\-_]|\[[0-?]*[ -/]*[@-~])')
+ for line in output.splitlines():
+ self.log.error(ansi_escape.sub('', line))
+ self.fail("Detected cachability issues")
+
def perform_sstate_test(self, image_target, package_target, **kwargs):
def check_executed_tasks(target, expected):
taskorder_file = glob.glob(f'{self.build_dir}/tmp/work/*/{target}/*/temp/log.task_order')
--
2.39.2

ker...@gmail.com

unread,
Apr 2, 2024, 1:29:11 PM4/2/24
to isar-...@googlegroups.com, Christopher Larson, Christopher Larson
From: Christopher Larson <chris....@seimens.com>

The current sstate tests do call `isar-sstate lint`` after populating
the shared state cache, but those tests take some time to do so, and do
not check for cachability issues for other targets, so add a new test
which does so.

Signed-off-by: Christopher Larson <chris....@siemens.com>
---
testsuite/citest.py | 22 +++++++++++++++++++++-
1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/testsuite/citest.py b/testsuite/citest.py
index 7e24c498..b1c0b760 100755
--- a/testsuite/citest.py
+++ b/testsuite/citest.py
@@ -290,8 +290,28 @@ def test_container_sdk(self):
self.init()
self.perform_build_test(targets, bitbake_cmd='do_populate_sdk', container=True)

-class SstateTest(CIBaseTest):
+class SignatureTest(CIBaseTest):
+ """
+ Test for signature cachability issues which prevent shared state reuse.

+ SstateTest also checks for these, but this test is faster and will check more cases.
+
+ :avocado: tags=signatures,sstate
+ """
+ def test_signature_lint(self):
+ verbose = bool(int(self.params.get("verbose", default=0)))
+ targets = [
+ 'mc:qemuamd64-bullseye:isar-image-ci',
+ 'mc:qemuarm-bullseye:isar-image-base',
+ 'mc:qemuarm-bullseye:isar-image-base:do_populate_sdk',
+ 'mc:qemuarm64-bullseye:isar-image-base',
+ 'mc:qemuamd64-focal:isar-image-base'
+ ]
+
+ self.init()
+ self.perform_signature_lint(targets, verbose=verbose)
+
+class SstateTest(CIBaseTest):
"""
Test builds with artifacts taken from sstate cache

--
2.39.2

MOESSBAUER, Felix

unread,
Apr 3, 2024, 2:55:00 AM4/3/24
to ker...@gmail.com, isar-...@googlegroups.com, Larson, Chris, chris....@seimens.com
On Tue, 2024-04-02 at 17:28 +0000, ker...@gmail.com wrote:
> From: Christopher Larson <chris....@seimens.com>
------------------------------------------^

Hi, please resend the whole series with a correct "From:".

Felix
--
Siemens AG, Technology
Linux Expert Center


MOESSBAUER, Felix

unread,
Apr 3, 2024, 2:56:47 AM4/3/24
to ker...@gmail.com, isar-...@googlegroups.com, Larson, Chris
On Tue, 2024-04-02 at 17:28 +0000, ker...@gmail.com wrote:
> From: Christopher Larson <chris....@seimens.com>
>
> In addition to the current checks for variables starting with an
> absolute path, particularly those within the build or sources
> directories, we should also check for absolute paths in SRC_URI
> file entries.

Good idea. Thanks for adding.

Acked-by: Felix Moessbauer <felix.mo...@siemens.com>

--

MOESSBAUER, Felix

unread,
Apr 3, 2024, 3:02:46 AM4/3/24
to ker...@gmail.com, isar-...@googlegroups.com, Larson, Chris
On Tue, 2024-04-02 at 17:28 +0000, ker...@gmail.com wrote:
> From: Christopher Larson <chris....@seimens.com>
>
> Bitbake supports writing signature data directly to the stamps
> directory
> without having to build, so we should add the ability to lint this
> signature data as well. This is useful for checking for cachability
> issues without having to complete a build.

Hi,

I guess that is enabled with the '--dump-signatures=' parameter. Can we
please add a brief example (one-line) to the linter script for this
case.

>
> Signed-off-by: Christopher Larson <chris....@siemens.com>
> ---
>  scripts/isar-sstate | 34 +++++++++++++++++++++++-----------
>  1 file changed, 23 insertions(+), 11 deletions(-)
>
> diff --git a/scripts/isar-sstate b/scripts/isar-sstate
> index 9b20cb8e..b77f73eb 100755
> --- a/scripts/isar-sstate
> +++ b/scripts/isar-sstate
> @@ -154,10 +154,12 @@ SstateCacheEntry = namedtuple(
>  #                   
> "${PV}:${PR}:${SSTATE_PKGARCH}:${SSTATE_VERSION}:"
>  
>  # This regex extracts relevant fields:
> -SstateRegex = re.compile(r'sstate:(?P<pn>[^:]*):[^:]*:[^:]*:[^:]*:'
> +SstateRegex =
> re.compile(r'(.*/)?sstate:(?P<pn>[^:]*):[^:]*:[^:]*:[^:]*:'
>                           r'(?P<arch>[^:]*):[^:]*:(?P<hash>[0-9a-
> f]*)_'
>                           r'(?P<task>[^\.]*)\.(?P<suffix>.*)')
> -
> +StampsRegex = re.compile(
> + 

Is this regex hand-crafted or can it be found somewhere in bitbake as
well? Is the format documented in bitbake? I'm a bit worried that this
might change across versions. If so, please add a note that this might
need to be adjusted on bitbake updates.

Felix

--

MOESSBAUER, Felix

unread,
Apr 3, 2024, 3:08:13 AM4/3/24
to ker...@gmail.com, Schmidt, Adriaan, isar-...@googlegroups.com, Larson, Chris
On Tue, 2024-04-02 at 17:28 +0000, ker...@gmail.com wrote:
> From: Christopher Larson <chris....@seimens.com>
>
> Currently isar-sstate ignores image_wic, but not the other image
> tasks.
> Add additional image tasks to align with this behavior.
>
> Signed-off-by: Christopher Larson <chris....@siemens.com>
> ---
>  scripts/isar-sstate | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/scripts/isar-sstate b/scripts/isar-sstate
> index b77f73eb..5270f944 100755
> --- a/scripts/isar-sstate
> +++ b/scripts/isar-sstate
> @@ -146,6 +146,8 @@ try:
>  except ModuleNotFoundError:
>      s3_supported = False
>  
> +DEFAULT_IGNORED_TASKS = "rootfs_wicenv image_wic image_cpio
> image_tar image_ext4"

It might be better to ignore all image tasks, like "image_*". But as we
have more and more image typedeps chains (e.g. image_squashfs ->
image_verity -> image_wic), I'm wondering if ignoring this part still
makes sense. Maybe Adriaan can comment on this.

Felix

> +
>  SstateCacheEntry = namedtuple(
>          'SstateCacheEntry', 'hash path arch pn task suffix islink
> age size'.split())
>  
> @@ -811,7 +813,7 @@ def sstate_lint(target, verbose, sources_dir,
> build_dir, exit_code, pedantic, li
>      # note that these still can break caching of other tasks that
> depend on these.
>      # Run in pedantic mode to also look for these issues (e.g. in
> image-in-image builds)
>      # To make a build fully cacheable, avoid absolute paths in
> BBLAYERS
> -    ADDITIONAL_IGNORED_TASKS = list() if pedantic else
> 'rootfs_wicenv image_wic'.split()
> +    ADDITIONAL_IGNORED_TASKS = list() if pedantic else
> DEFAULT_IGNORED_TASKS.split()
>      if not target.exists():
>          print(f"WARNING: target {target} does not exist. Nothing to
> analyze.")
>          return 0
> --
> 2.39.2
>

MOESSBAUER, Felix

unread,
Apr 3, 2024, 3:10:45 AM4/3/24
to ker...@gmail.com, isar-...@googlegroups.com, Larson, Chris
On Tue, 2024-04-02 at 17:28 +0000, ker...@gmail.com wrote:
> From: Christopher Larson <chris....@seimens.com>
>
> This allows the user to override the default lists of tasks to ignore
> when linting the sstate cache.
>
> Signed-off-by: Christopher Larson <chris....@siemens.com>
> ---
>  scripts/isar-sstate | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/scripts/isar-sstate b/scripts/isar-sstate
> index 5270f944..dddfafcb 100755
> --- a/scripts/isar-sstate
> +++ b/scripts/isar-sstate
> @@ -146,7 +146,7 @@ try:
>  except ModuleNotFoundError:
>      s3_supported = False
>  
> -DEFAULT_IGNORED_TASKS = "rootfs_wicenv image_wic image_cpio
> image_tar image_ext4"
> +DEFAULT_IGNORED_TASKS =
> "rootfs_wicenv,image_wic,image_cpio,image_tar,image_ext4"

Also here, it might be good to support wildcards, e.g. 'image_*'.
Apart from that minor detail, this is a valuable addition.

Acked-by: Felix Moessbauer <felix.mo...@siemens.com>

Felix

MOESSBAUER, Felix

unread,
Apr 3, 2024, 3:12:55 AM4/3/24
to ker...@gmail.com, isar-...@googlegroups.com, Larson, Chris, chris....@seimens.com
On Tue, 2024-04-02 at 17:28 +0000, ker...@gmail.com wrote:
> From: Christopher Larson <chris....@seimens.com>
>
> This allows a test writer to call `bitbake -S none`, to generate
> signature data in tmp/stamps.

Is there a particular reason why we don't use the long version of the
parameters? In case this parameter ever changes, it is much easier to
find all locations if the long form is used.

Felix

Larson, Chris

unread,
Apr 4, 2024, 12:51:00 AM4/4/24
to MOESSBAUER, Felix, isar-...@googlegroups.com, chris....@seimens.com
No real reason, I'll update it. Thanks!

Larson, Chris

unread,
Apr 4, 2024, 12:51:00 AM4/4/24
to MOESSBAUER, Felix, ker...@gmail.com, isar-...@googlegroups.com, chris....@seimens.com
Actually the From was correct, as I really should be using my work address, I just need to actually send the emails from there instead to match. I'll do that for the v2. Thanks.

-----Original Message-----
From: Moessbauer, Felix (T CED OES-DE) <felix.mo...@siemens.com>
Sent: Tuesday, April 2, 2024 11:55 PM
To: ker...@gmail.com; isar-...@googlegroups.com
Cc: Larson, Chris (DI CTO FDS CES LX MEL) <chris....@siemens.com>; chris....@seimens.com

Larson, Chris

unread,
Apr 4, 2024, 12:51:00 AM4/4/24
to MOESSBAUER, Felix, ker...@gmail.com, isar-...@googlegroups.com
Will do, thanks. Your feedback has been quite helpful.

-----Original Message-----
From: Moessbauer, Felix (T CED OES-DE) <felix.mo...@siemens.com>
Sent: Wednesday, April 3, 2024 12:03 AM
To: ker...@gmail.com; isar-...@googlegroups.com
Cc: Larson, Chris (DI CTO FDS CES LX MEL) <chris....@siemens.com>

Larson, Chris

unread,
Apr 4, 2024, 12:51:00 AM4/4/24
to MOESSBAUER, Felix, ker...@gmail.com, isar-...@googlegroups.com
Good idea, I can include that in the v2 if there's no objection.

-----Original Message-----
From: Moessbauer, Felix (T CED OES-DE) <felix.mo...@siemens.com>
Sent: Wednesday, April 3, 2024 12:11 AM
To: ker...@gmail.com; isar-...@googlegroups.com
Cc: Larson, Chris (DI CTO FDS CES LX MEL) <chris....@siemens.com>

Larson, Chris

unread,
Apr 4, 2024, 12:51:11 AM4/4/24
to MOESSBAUER, Felix, ker...@gmail.com, isar-...@googlegroups.com
I have a quick question about this. In the code, there are constants using the "ignored" wording rather than excluded, so my adding the argument using excluded seems like it's probably not consistent. I should probably rename the argument to match, or adjust the variable naming for consistency. Any thoughts?

-----Original Message-----
From: Moessbauer, Felix (T CED OES-DE) <felix.mo...@siemens.com>
Sent: Wednesday, April 3, 2024 12:11 AM
To: ker...@gmail.com; isar-...@googlegroups.com
Cc: Larson, Chris (DI CTO FDS CES LX MEL) <chris....@siemens.com>
Subject: Re: [PATCH 6/9] isar-sstate: add --excluded-tasks argument

MOESSBAUER, Felix

unread,
Apr 4, 2024, 2:26:53 AM4/4/24
to ker...@gmail.com, Larson, Chris, isar-...@googlegroups.com, chris....@seimens.com
On Wed, 2024-04-03 at 21:42 +0000, Larson, Chris (DI CTO FDS CES LX
MEL) wrote:
> Actually the From was correct, as I really should be using my work
> address, I just need to actually send the emails from there instead
> to match. I'll do that for the v2. Thanks.

Sending from a different address is fine, but there is a typo in your
email: <chris....@seimens.com> != <chris....@siemens.com>
(SEIMENS vs. SIEMENS).

Felix

MOESSBAUER, Felix

unread,
Apr 4, 2024, 2:28:56 AM4/4/24
to ker...@gmail.com, Larson, Chris, isar-...@googlegroups.com
On Wed, 2024-04-03 at 21:44 +0000, Larson, Chris (DI CTO FDS CES LX
MEL) wrote:
> I have a quick question about this. In the code, there are constants
> using the "ignored" wording rather than excluded, so my adding the
> argument using excluded seems like it's probably not consistent. I
> should probably rename the argument to match, or adjust the variable
> naming for consistency. Any thoughts?

We should at least keep it consistent. Renaming the internal variables
is fine, but please try to not change the external interface (the
parameters).

Felix

chris....@siemens.com

unread,
Apr 5, 2024, 12:32:25 PM4/5/24
to isar-...@googlegroups.com, Christopher Larson
v2 updates:
- Explain the use of --dump-signatures in the docs on the lint command in isar-sstate
- Explain the stamps regex the way the sstate regex is explained, for consistency
- Support wildcards for the --excluded-tasks argument and default exclusions
- Drop the additional default task exclusions, as this should be revisited
- Fix a lint traceback if a variable's value is only whitespacxe

Christopher Larson (9):
isar-bootstrap: avoid forced early expansion of key vars
isar-ci-ssh-setup: avoid abs path in signatures
isar-sstate: lint: check for absolute paths in SRC_URI
isar-sstate: lint: add support for checking stamps
isar-sstate: add --excluded-tasks argument
isar-sstate: fix failures if a variable is set to just whitespace
cibuilder.py: add -S support to the bitbake method
testsuite: add perform_signature_lint method
testsuite: add signature cachability checks

.../isar-ci-ssh-setup_0.1.bb | 3 +
.../isar-bootstrap/isar-bootstrap.inc | 8 +-
scripts/isar-sstate | 85 +++++++++++++++----
testsuite/cibase.py | 19 +++++
testsuite/cibuilder.py | 5 +-
testsuite/citest.py | 22 ++++-
6 files changed, 121 insertions(+), 21 deletions(-)

--
2.39.2

chris....@siemens.com

unread,
Apr 5, 2024, 12:32:25 PM4/5/24
to isar-...@googlegroups.com, Christopher Larson
From: Christopher Larson <chris....@siemens.com>

In addition to the current checks for variables starting with an
absolute path, particularly those within the build or sources
directories, we should also check for absolute paths in SRC_URI
file entries.

Submitted at https://groups.google.com/g/isar-users/c/2NB-PXyswq8/m/C8LiWa1TAQAJ.

Signed-off-by: Christopher Larson <chris....@siemens.com>
---
scripts/isar-sstate | 17 +++++++++++++++++
1 file changed, 17 insertions(+)

diff --git a/scripts/isar-sstate b/scripts/isar-sstate
index c14c2843..9b20cb8e 100755
--- a/scripts/isar-sstate
+++ b/scripts/isar-sstate
@@ -839,6 +839,23 @@ def sstate_lint(target, verbose, sources_dir, build_dir, exit_code, pedantic, **
continue
# remove leading whitespaces possibly added by appending
val = val.lstrip()
+ if name == 'SRC_URI':
+ src_uri = val.split()
+ for entry in src_uri:
+ if entry.startswith('file:///'):
+ entry_path = entry[7:]
+ if entry_path.startswith(build_dir):
+ pn_issues.append(f'\033[0;31m-> path in build-dir: SRC_URI entry "{entry}"\033[0m')
+ hits_builddir += 1
+ elif entry_path.startswith(sources_dir):
+ pn_issues.append(f'\033[0;31m-> path in sources-dir: SRC_URI entry "{entry}"\033[0m')

chris....@siemens.com

unread,
Apr 5, 2024, 12:32:26 PM4/5/24
to isar-...@googlegroups.com, Christopher Larson
From: Christopher Larson <chris....@siemens.com>

Rather than appending the items from the expanded key variables into
SRC_URI individually, which means there's no way to use tools like
vardepvalue or vardepexclude to control signature generation, append the
unexpanded variables to the SRC_URI directly. This avoids issues with
shared state reuse for the isar-bootstrap packages.

Submitted at https://groups.google.com/g/isar-users/c/2NB-PXyswq8/m/xTTcxqxTAQAJ.

Signed-off-by: Christopher Larson <chris....@siemens.com>
---

chris....@siemens.com

unread,
Apr 5, 2024, 12:32:26 PM4/5/24
to isar-...@googlegroups.com, Christopher Larson
From: Christopher Larson <chris....@siemens.com>

Bitbake supports writing signature data directly to the stamps directory
without having to build, so we should add the ability to lint this
signature data as well. This is useful for checking for cachability
issues without having to complete a build.

Submitted at https://groups.google.com/g/isar-users/c/2NB-PXyswq8/m/GMuGk61TAQAJ.

Signed-off-by: Christopher Larson <chris....@siemens.com>
---
scripts/isar-sstate | 51 ++++++++++++++++++++++++++++++++++-----------
1 file changed, 39 insertions(+), 12 deletions(-)

diff --git a/scripts/isar-sstate b/scripts/isar-sstate
index 9b20cb8e..d68b8938 100755
--- a/scripts/isar-sstate
+++ b/scripts/isar-sstate
@@ -66,8 +66,16 @@ format as `bitbake-diffsigs`.

The `lint` command searches for common flaws that reduce the cachability
of a layer, e.g., signatures containing absolute paths from the host
-environment. Issues found this way usually indicate errors in recipes
-or in Isar itself.
+environment. Issues found this way usually indicate errors in recipes or
+in Isar itself.
+
+The `lint` command may be used to check a shared state cache folder, or,
+with the use of the --lint-stamps argument, will check the tmp/stamps
+folder and expects this as an argument. The signature data files must be
+populated in the stamps folder, which requires either a completed build
+or a `bitbake --dump-signatures=`. The value for this argument may be
+`none`, `printdiff`, or others defined in the handler, but `none` is the
+simplest to just generate signature data.

## Backends

@@ -154,10 +162,19 @@ SstateCacheEntry = namedtuple(
# "${PV}:${PR}:${SSTATE_PKGARCH}:${SSTATE_VERSION}:"

# This regex extracts relevant fields:
-SstateRegex = re.compile(r'sstate:(?P<pn>[^:]*):[^:]*:[^:]*:[^:]*:'
+SstateRegex = re.compile(r'(.*/)?sstate:(?P<pn>[^:]*):[^:]*:[^:]*:[^:]*:'
r'(?P<arch>[^:]*):[^:]*:(?P<hash>[0-9a-f]*)_'
r'(?P<task>[^\.]*)\.(?P<suffix>.*)')

+# The filename of stamps and associated signature data is defined in
+# Isar (ith an added suffix of `.<task>.sigdata.<hash>` by BitBake):
+# STAMPS_DIR ?= "${TMPDIR}/stamps"
+# STAMP = "${STAMPS_DIR}/${DISTRO}-${DISTRO_ARCH}/${PN}/${PV}-${PR}"
+
+# This regex extracts relevant fields:
+StampsRegex = re.compile(
+ r"(.*/)?(?P<arch>[^/]+)/(?P<pn>[^/]+)/([^/]+)\.do_(?P<task>[^/]+)\.(?P<suffix>sigdata)\.(?P<hash>[0-9a-f]{64})"
+)

class SstateTargetBase(object):
def __init__(self, path, cached=False):
@@ -288,12 +305,13 @@ class SstateTargetBase(object):


class SstateFileTarget(SstateTargetBase):
- def __init__(self, path, **kwargs):
+ def __init__(self, path, regex=SstateRegex, **kwargs):
super().__init__(path, **kwargs)
if path.startswith('file://'):
path = path[len('file://'):]
self.path = path
self.basepath = os.path.abspath(path)
+ self.regex = regex

def __repr__(self):
return f"file://{self.path}"
@@ -334,12 +352,13 @@ class SstateFileTarget(SstateTargetBase):
for subdir, dirs, files in os.walk(self.basepath):
reldir = subdir[(len(self.basepath)+1):]
for f in files:
- m = SstateRegex.match(f)
+ relative = os.path.join(reldir, f)
+ m = self.regex.match(relative)
if m is not None:
islink = os.path.islink(os.path.join(subdir, f))
age = int(now - os.path.getmtime(os.path.join(subdir, f)))
all_files.append(SstateCacheEntry(
- path=os.path.join(reldir, f),
+ path=relative,
size=os.path.getsize(os.path.join(subdir, f)),
islink=islink,
age=age,
@@ -592,6 +611,9 @@ def arguments():
parser.add_argument(
'--exit-code', type=int, default=None,
help="lint: return this instead of number of found issues")
+ parser.add_argument(
+ '--lint-stamps', default=False, action='store_true',
+ help="lint: assume target is a stamps directory (target must be a local path)")

args = parser.parse_args()
if args.command in 'upload analyze'.split() and args.source is None:
@@ -798,7 +820,7 @@ def sstate_analyze(source, target, **kwargs):
print('\n'.join(out))


-def sstate_lint(target, verbose, sources_dir, build_dir, exit_code, pedantic, **kwargs):
+def sstate_lint(target, verbose, sources_dir, build_dir, exit_code, pedantic, lint_stamps, **kwargs):
ADDITIONAL_IGNORED_VARNAMES = 'PP'.split()
# only list non-cacheable tasks here
# note that these still can break caching of other tasks that depend on these.
@@ -809,7 +831,10 @@ def sstate_lint(target, verbose, sources_dir, build_dir, exit_code, pedantic, **
print(f"WARNING: target {target} does not exist. Nothing to analyze.")
return 0

- cache_sigs = {s.hash: s for s in target.list_all() if s.suffix.endswith('.siginfo')}
+ if lint_stamps:
+ cache_sigs = {s.hash: s for s in target.list_all()}
+ else:
+ cache_sigs = {s.hash: s for s in target.list_all() if s.suffix.endswith('.siginfo')}

hits_srcdir = 0
hits_builddir = 0
@@ -891,10 +916,12 @@ def main():

chris....@siemens.com

unread,
Apr 5, 2024, 12:32:26 PM4/5/24
to isar-...@googlegroups.com, Christopher Larson
From: Christopher Larson <chris....@siemens.com>

TESTSUITEDIR is a full absolute path to the testsuite directory in isar,
as set in the environment by the build setup scripts. This is referenced
in the install task, which prevents shared state reuse for this package.
While this is predominently used in CI, it's still a good idea to avoid
absolute paths in signatures, so we can reuse shared state for this
package in other contexts.

Rather than excluding the TESTSUITEDIR from signatures entirely with
vardepsexclude, we can retain some information about the path by using
os.path.relpath to make it relative to the top directory of the build.
This is the same approach used by isar-bootstrap for the keys, and the
vardepvalue approach is also used elsewhere for layer paths.

Submitted at https://groups.google.com/g/isar-users/c/2NB-PXyswq8/m/oOIB_KxTAQAJ.

Signed-off-by: Christopher Larson <chris....@siemens.com>
---

chris....@siemens.com

unread,
Apr 5, 2024, 12:32:27 PM4/5/24
to isar-...@googlegroups.com, Christopher Larson
From: Christopher Larson <chris....@siemens.com>

This allows the user to override the default lists of tasks to ignore
when linting the sstate cache.

Submitted at https://groups.google.com/g/isar-users/c/2NB-PXyswq8/m/Ca1TO65TAQAJ.

Signed-off-by: Christopher Larson <chris....@siemens.com>
---
scripts/isar-sstate | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/scripts/isar-sstate b/scripts/isar-sstate
index d68b8938..a4429b7a 100755
--- a/scripts/isar-sstate
+++ b/scripts/isar-sstate
@@ -128,6 +128,7 @@ apt-get install python3-botocore
import argparse
from collections import namedtuple
import datetime
+from fnmatch import fnmatchcase
import os
import re
import shutil
@@ -154,6 +155,8 @@ try:
except ModuleNotFoundError:
s3_supported = False

+DEFAULT_IGNORED_TASKS = "rootfs_wicenv,image_wic"
+
SstateCacheEntry = namedtuple(
'SstateCacheEntry', 'hash path arch pn task suffix islink age size'.split())

@@ -614,6 +617,9 @@ def arguments():
parser.add_argument(
'--lint-stamps', default=False, action='store_true',
help="lint: assume target is a stamps directory (target must be a local path)")
+ parser.add_argument(
+ '--excluded-tasks', type=str, default=DEFAULT_IGNORED_TASKS,
+ help="lint: comma-separated list of tasks to ignore (default: %(default)s)")

args = parser.parse_args()
if args.command in 'upload analyze'.split() and args.source is None:
@@ -622,6 +628,7 @@ def arguments():
elif args.command in 'info clean'.split() and args.source is not None:
print(f"ERROR: '{args.command}' must not have a source (only a target)")
sys.exit(1)
+ args.excluded_tasks = args.excluded_tasks.split(',')
return args


@@ -820,13 +827,14 @@ def sstate_analyze(source, target, **kwargs):
print('\n'.join(out))


-def sstate_lint(target, verbose, sources_dir, build_dir, exit_code, pedantic, lint_stamps, **kwargs):
+def sstate_lint(target, verbose, sources_dir, build_dir, exit_code, pedantic, lint_stamps,
+ excluded_tasks, **kwargs):
ADDITIONAL_IGNORED_VARNAMES = 'PP'.split()
# only list non-cacheable tasks here
# note that these still can break caching of other tasks that depend on these.
# Run in pedantic mode to also look for these issues (e.g. in image-in-image builds)
# To make a build fully cacheable, avoid absolute paths in BBLAYERS
- ADDITIONAL_IGNORED_TASKS = list() if pedantic else 'rootfs_wicenv image_wic'.split()
+ ADDITIONAL_IGNORED_TASKS = list() if pedantic else excluded_tasks
if not target.exists():
print(f"WARNING: target {target} does not exist. Nothing to analyze.")
return 0
@@ -840,7 +848,7 @@ def sstate_lint(target, verbose, sources_dir, build_dir, exit_code, pedantic, li
hits_builddir = 0
hits_other = 0
for sig in cache_sigs.values():
- if sig.task in ADDITIONAL_IGNORED_TASKS:
+ if any(fnmatchcase(sig.task, pattern) for pattern in ADDITIONAL_IGNORED_TASKS):
continue

sig_file = target.download(sig.path)
--
2.39.2

chris....@siemens.com

unread,
Apr 5, 2024, 12:32:28 PM4/5/24
to isar-...@googlegroups.com, Christopher Larson
From: Christopher Larson <chris....@siemens.com>

We see this failure if a variable is set to the empty string, as whitespace is
being stripped off of the variable after we check if the value is empty or None,
not before.

Traceback (most recent call last):
File "/home/kergoth/Code/indos/signatures/isar/scripts/isar-sstate", line 941, in <module>
sys.exit(main())
^^^^^^
File "/home/kergoth/Code/indos/signatures/isar/scripts/isar-sstate", line 937, in main
return globals()[f'sstate_{args.command}'](**vars(args))
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/kergoth/Code/indos/signatures/isar/scripts/isar-sstate", line 894, in sstate_lint
if not val[0] == '/':
~~~^^^
IndexError: string index out of range

Signed-off-by: Christopher Larson <chris....@siemens.com>
---
scripts/isar-sstate | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/scripts/isar-sstate b/scripts/isar-sstate
index a4429b7a..4ea38bc8 100755
--- a/scripts/isar-sstate
+++ b/scripts/isar-sstate
@@ -868,10 +868,11 @@ def sstate_lint(target, verbose, sources_dir, build_dir, exit_code, pedantic, li
sigdata['taskhash_ignore_tasks'] and name in sigdata['taskhash_ignore_tasks'] or \
name in ADDITIONAL_IGNORED_VARNAMES:
continue
- if not val:
- continue
# remove leading whitespaces possibly added by appending
val = val.lstrip()
+ if not val:
+ continue
+
if name == 'SRC_URI':
src_uri = val.split()
for entry in src_uri:
--
2.39.2

chris....@siemens.com

unread,
Apr 5, 2024, 12:32:29 PM4/5/24
to isar-...@googlegroups.com, Christopher Larson
From: Christopher Larson <chris....@siemens.com>

This allows a test writer to call `bitbake -S none`, to generate
signature data in tmp/stamps.

Submitted at https://groups.google.com/g/isar-users/c/2NB-PXyswq8/m/gGz9b65TAQAJ.

Signed-off-by: Christopher Larson <chris....@siemens.com>
---

chris....@siemens.com

unread,
Apr 5, 2024, 12:32:29 PM4/5/24
to isar-...@googlegroups.com, Christopher Larson
From: Christopher Larson <chris....@siemens.com>

This method is provided to generate signature data for specified target
or targets and check for cachability issues without having to complete a
build.

Submitted at https://groups.google.com/g/isar-users/c/2NB-PXyswq8/m/42BMya5TAQAJ.

Signed-off-by: Christopher Larson <chris....@siemens.com>
---

chris....@siemens.com

unread,
Apr 5, 2024, 12:32:31 PM4/5/24
to isar-...@googlegroups.com, Christopher Larson
From: Christopher Larson <chris....@siemens.com>

The current sstate tests do call `isar-sstate lint`` after populating
the shared state cache, but those tests take some time to do so, and do
not check for cachability issues for other targets, so add a new test
which does so.

Submitted at https://groups.google.com/g/isar-users/c/2NB-PXyswq8/m/7Legiq9TAQAJ.

Signed-off-by: Christopher Larson <chris....@siemens.com>
---

Larson, Chris

unread,
Apr 5, 2024, 12:33:49 PM4/5/24
to isar-...@googlegroups.com
Apologies, the subject lines were supposed to be PATCHv2, missed an argument.

-----Original Message-----
From: Larson, Chris (DI CTO FDS CES LX MEL) <chris....@siemens.com>
Sent: Friday, April 5, 2024 9:31 AM
To: isar-...@googlegroups.com
Cc: Larson, Chris (DI CTO FDS CES LX MEL) <chris....@siemens.com>

Larson, Chris

unread,
Apr 11, 2024, 12:10:32 PM4/11/24
to isar-...@googlegroups.com
Were there any issues with the v2 updates? Thanks.

--
Christopher Larson
Siemens AG
www.siemens.com

-----Original Message-----
From: Larson, Chris (DI CTO FDS CES LX MEL) <chris....@siemens.com>
Sent: Friday, April 5, 2024 9:31 AM
To: isar-...@googlegroups.com
Cc: Larson, Chris (DI CTO FDS CES LX MEL) <chris....@siemens.com>
ERROR| ubuntu-focal-amd64:isar-bootstrap-target:unpack (95af4581) ====
ERROR| -> path in sources-dir: SRC_URI entry "file:///home/kergoth/Code/industrial/signatures/isar/meta-isar/conf/distro/ubuntu.public.key;sha256sum=36a38199a4bf4eae1e7f574891f7dfcb79b91b87a33a499383265e1224b5e989"
ERROR| ==== issues found in
ERROR| ubuntu-focal-amd64:isar-bootstrap-target:fetch (e8249cb2) ====
ERROR| -> path in sources-dir: SRC_URI entry "file:///home/kergoth/Code/industrial/signatures/isar/meta-isar/conf/distro/ubuntu.public.key;sha256sum=36a38199a4bf4eae1e7f574891f7dfcb79b91b87a33a499383265e1224b5e989"
ERROR| ==== issues found in
ERROR| debian-bullseye-amd64:isar-ci-ssh-setup:install (1c0a8b21) ====

MOESSBAUER, Felix

unread,
Apr 14, 2024, 8:39:39 AM4/14/24
to Larson, Chris, isar-...@googlegroups.com
On Fri, 2024-04-05 at 16:31 +0000, chris.larson via isar-users wrote:
> From: Christopher Larson <chris....@siemens.com>
>
> This series improves isar-sstate's lint command to support checking
> sigdata in tmp/stamps and to check for absolute paths in SRC_URI, and
> adds an additional test to the testsuite to exercise this capability.
> The new test checks for signature cachability issues much more
> quickly, as it does so without a full build, so it checks more target
> configurations. This uses bitbake's `-S none` option to avoid
> building anything, and then runs isar-sstate lint on the resulting
> sigdata in tmp/stamps.

Dear maintainers,

the feedback provided in v1 has been integrated and I re-tested this
series. By that, I propose to merge it.

Acked-by: felix.moessbauer <felix.mo...@siemens.com>

Best regards,
Felix
> target:fetch (e8249cb2) ====
> ERROR| -> path in sources-dir: SRC_URI entry
> "file:///home/kergoth/Code/industrial/signatures/isar/meta-
> isar/conf/distro/ubuntu.public.key;sha256sum=36a38199a4bf4eae1e7f5748
> 91f7dfcb79b91b87a33a499383265e1224b5e989"
> ERROR| ==== issues found in debian-bullseye-amd64:isar-ci-ssh-

Uladzimir Bely

unread,
Apr 15, 2024, 5:27:50 AM4/15/24
to chris....@siemens.com, isar-...@googlegroups.com
On Fri, 2024-04-05 at 16:31 +0000, chris.larson via isar-users wrote:
> target:fetch (e8249cb2) ====
> ERROR| -> path in sources-dir: SRC_URI entry
> "file:///home/kergoth/Code/industrial/signatures/isar/meta-
> isar/conf/distro/ubuntu.public.key;sha256sum=36a38199a4bf4eae1e7f5748
> 91f7dfcb79b91b87a33a499383265e1224b5e989"
> ERROR| ==== issues found in debian-bullseye-amd64:isar-ci-ssh-
Applied to next, thanks.
Reply all
Reply to author
Forward
0 new messages