* [PATCH 1/9] isar-bootstrap: avoid forced early expansion of key vars
2024-04-02 17:28 [PATCH 0/9] Add more signature cachability tests to the testsuite kergoth
@ 2024-04-02 17:28 ` kergoth
2024-04-03 6:54 ` MOESSBAUER, Felix
2024-04-02 17:28 ` [PATCH 2/9] isar-ci-ssh-setup: avoid abs path in signatures kergoth
` (7 subsequent siblings)
8 siblings, 1 reply; 24+ messages in thread
From: kergoth @ 2024-04-02 17:28 UTC (permalink / raw)
To: isar-users; +Cc: Christopher Larson, Christopher Larson
From: Christopher Larson <chris.larson@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.larson@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
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH 1/9] isar-bootstrap: avoid forced early expansion of key vars
2024-04-02 17:28 ` [PATCH 1/9] isar-bootstrap: avoid forced early expansion of key vars kergoth
@ 2024-04-03 6:54 ` MOESSBAUER, Felix
2024-04-03 21:42 ` Larson, Chris
0 siblings, 1 reply; 24+ messages in thread
From: MOESSBAUER, Felix @ 2024-04-03 6:54 UTC (permalink / raw)
To: kergoth, isar-users; +Cc: Larson, Chris, chris.larson
On Tue, 2024-04-02 at 17:28 +0000, kergoth@gmail.com wrote:
> From: Christopher Larson <chris.larson@seimens.com>
------------------------------------------^
Hi, please resend the whole series with a correct "From:".
Felix
>
> 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.larson@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
>
--
Siemens AG, Technology
Linux Expert Center
^ permalink raw reply [flat|nested] 24+ messages in thread* RE: [PATCH 1/9] isar-bootstrap: avoid forced early expansion of key vars
2024-04-03 6:54 ` MOESSBAUER, Felix
@ 2024-04-03 21:42 ` Larson, Chris
2024-04-04 6:26 ` MOESSBAUER, Felix
0 siblings, 1 reply; 24+ messages in thread
From: Larson, Chris @ 2024-04-03 21:42 UTC (permalink / raw)
To: MOESSBAUER, Felix, kergoth, isar-users; +Cc: chris.larson
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.moessbauer@siemens.com>
Sent: Tuesday, April 2, 2024 11:55 PM
To: kergoth@gmail.com; isar-users@googlegroups.com
Cc: Larson, Chris (DI CTO FDS CES LX MEL) <chris.larson@siemens.com>; chris.larson@seimens.com
Subject: Re: [PATCH 1/9] isar-bootstrap: avoid forced early expansion of key vars
On Tue, 2024-04-02 at 17:28 +0000, kergoth@gmail.com wrote:
> From: Christopher Larson <chris.larson@seimens.com>
------------------------------------------^
Hi, please resend the whole series with a correct "From:".
Felix
>
> 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.larson@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
>
--
Siemens AG, Technology
Linux Expert Center
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH 1/9] isar-bootstrap: avoid forced early expansion of key vars
2024-04-03 21:42 ` Larson, Chris
@ 2024-04-04 6:26 ` MOESSBAUER, Felix
0 siblings, 0 replies; 24+ messages in thread
From: MOESSBAUER, Felix @ 2024-04-04 6:26 UTC (permalink / raw)
To: kergoth, Larson, Chris, isar-users; +Cc: chris.larson
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.larson@seimens.com> != <chris.larson@siemens.com>
(SEIMENS vs. SIEMENS).
Felix
>
> -----Original Message-----
> From: Moessbauer, Felix (T CED OES-DE) <felix.moessbauer@siemens.com>
> Sent: Tuesday, April 2, 2024 11:55 PM
> To: kergoth@gmail.com; isar-users@googlegroups.com
> Cc: Larson, Chris (DI CTO FDS CES LX MEL) <chris.larson@siemens.com>;
> chris.larson@seimens.com
> Subject: Re: [PATCH 1/9] isar-bootstrap: avoid forced early expansion
> of key vars
>
> On Tue, 2024-04-02 at 17:28 +0000, kergoth@gmail.com wrote:
> > From: Christopher Larson <chris.larson@seimens.com>
> ------------------------------------------^
>
> Hi, please resend the whole series with a correct "From:".
>
> Felix
>
> >
> > 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.larson@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
> >
>
> --
> Siemens AG, Technology
> Linux Expert Center
>
>
--
Siemens AG, Technology
Linux Expert Center
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 2/9] isar-ci-ssh-setup: avoid abs path in signatures
2024-04-02 17:28 [PATCH 0/9] Add more signature cachability tests to the testsuite kergoth
2024-04-02 17:28 ` [PATCH 1/9] isar-bootstrap: avoid forced early expansion of key vars kergoth
@ 2024-04-02 17:28 ` kergoth
2024-04-02 17:28 ` [PATCH 3/9] isar-sstate: lint: check for absolute paths in SRC_URI kergoth
` (6 subsequent siblings)
8 siblings, 0 replies; 24+ messages in thread
From: kergoth @ 2024-04-02 17:28 UTC (permalink / raw)
To: isar-users; +Cc: Christopher Larson, Christopher Larson
From: Christopher Larson <chris.larson@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.larson@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
^ permalink raw reply [flat|nested] 24+ messages in thread* [PATCH 3/9] isar-sstate: lint: check for absolute paths in SRC_URI
2024-04-02 17:28 [PATCH 0/9] Add more signature cachability tests to the testsuite kergoth
2024-04-02 17:28 ` [PATCH 1/9] isar-bootstrap: avoid forced early expansion of key vars kergoth
2024-04-02 17:28 ` [PATCH 2/9] isar-ci-ssh-setup: avoid abs path in signatures kergoth
@ 2024-04-02 17:28 ` kergoth
2024-04-03 6:56 ` MOESSBAUER, Felix
2024-04-02 17:28 ` [PATCH 4/9] isar-sstate: lint: add support for checking stamps kergoth
` (5 subsequent siblings)
8 siblings, 1 reply; 24+ messages in thread
From: kergoth @ 2024-04-02 17:28 UTC (permalink / raw)
To: isar-users; +Cc: Christopher Larson, Christopher Larson
From: Christopher Larson <chris.larson@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.larson@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
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH 3/9] isar-sstate: lint: check for absolute paths in SRC_URI
2024-04-02 17:28 ` [PATCH 3/9] isar-sstate: lint: check for absolute paths in SRC_URI kergoth
@ 2024-04-03 6:56 ` MOESSBAUER, Felix
0 siblings, 0 replies; 24+ messages in thread
From: MOESSBAUER, Felix @ 2024-04-03 6:56 UTC (permalink / raw)
To: kergoth, isar-users; +Cc: Larson, Chris
On Tue, 2024-04-02 at 17:28 +0000, kergoth@gmail.com wrote:
> From: Christopher Larson <chris.larson@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.moessbauer@siemens.com>
>
> Signed-off-by: Christopher Larson <chris.larson@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
>
--
Siemens AG, Technology
Linux Expert Center
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 4/9] isar-sstate: lint: add support for checking stamps
2024-04-02 17:28 [PATCH 0/9] Add more signature cachability tests to the testsuite kergoth
` (2 preceding siblings ...)
2024-04-02 17:28 ` [PATCH 3/9] isar-sstate: lint: check for absolute paths in SRC_URI kergoth
@ 2024-04-02 17:28 ` kergoth
2024-04-03 7:02 ` MOESSBAUER, Felix
2024-04-02 17:28 ` [PATCH 5/9] isar-sstate: lint: ignore more image tasks kergoth
` (4 subsequent siblings)
8 siblings, 1 reply; 24+ messages in thread
From: kergoth @ 2024-04-02 17:28 UTC (permalink / raw)
To: isar-users; +Cc: Christopher Larson, Christopher Larson
From: Christopher Larson <chris.larson@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.larson@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
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH 4/9] isar-sstate: lint: add support for checking stamps
2024-04-02 17:28 ` [PATCH 4/9] isar-sstate: lint: add support for checking stamps kergoth
@ 2024-04-03 7:02 ` MOESSBAUER, Felix
2024-04-03 21:42 ` Larson, Chris
0 siblings, 1 reply; 24+ messages in thread
From: MOESSBAUER, Felix @ 2024-04-03 7:02 UTC (permalink / raw)
To: kergoth, isar-users; +Cc: Larson, Chris
On Tue, 2024-04-02 at 17:28 +0000, kergoth@gmail.com wrote:
> From: Christopher Larson <chris.larson@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.larson@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
>
> 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
>
--
Siemens AG, Technology
Linux Expert Center
^ permalink raw reply [flat|nested] 24+ messages in thread* RE: [PATCH 4/9] isar-sstate: lint: add support for checking stamps
2024-04-03 7:02 ` MOESSBAUER, Felix
@ 2024-04-03 21:42 ` Larson, Chris
0 siblings, 0 replies; 24+ messages in thread
From: Larson, Chris @ 2024-04-03 21:42 UTC (permalink / raw)
To: MOESSBAUER, Felix, kergoth, isar-users
Will do, thanks. Your feedback has been quite helpful.
-----Original Message-----
From: Moessbauer, Felix (T CED OES-DE) <felix.moessbauer@siemens.com>
Sent: Wednesday, April 3, 2024 12:03 AM
To: kergoth@gmail.com; isar-users@googlegroups.com
Cc: Larson, Chris (DI CTO FDS CES LX MEL) <chris.larson@siemens.com>
Subject: Re: [PATCH 4/9] isar-sstate: lint: add support for checking stamps
On Tue, 2024-04-02 at 17:28 +0000, kergoth@gmail.com wrote:
> From: Christopher Larson <chris.larson@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.larson@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
>
> 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
>
--
Siemens AG, Technology
Linux Expert Center
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 5/9] isar-sstate: lint: ignore more image tasks
2024-04-02 17:28 [PATCH 0/9] Add more signature cachability tests to the testsuite kergoth
` (3 preceding siblings ...)
2024-04-02 17:28 ` [PATCH 4/9] isar-sstate: lint: add support for checking stamps kergoth
@ 2024-04-02 17:28 ` kergoth
2024-04-03 7:08 ` MOESSBAUER, Felix
2024-04-02 17:28 ` [PATCH 6/9] isar-sstate: add --excluded-tasks argument kergoth
` (3 subsequent siblings)
8 siblings, 1 reply; 24+ messages in thread
From: kergoth @ 2024-04-02 17:28 UTC (permalink / raw)
To: isar-users; +Cc: Christopher Larson, Christopher Larson
From: Christopher Larson <chris.larson@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.larson@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
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH 5/9] isar-sstate: lint: ignore more image tasks
2024-04-02 17:28 ` [PATCH 5/9] isar-sstate: lint: ignore more image tasks kergoth
@ 2024-04-03 7:08 ` MOESSBAUER, Felix
0 siblings, 0 replies; 24+ messages in thread
From: MOESSBAUER, Felix @ 2024-04-03 7:08 UTC (permalink / raw)
To: kergoth, Schmidt, Adriaan, isar-users; +Cc: Larson, Chris
On Tue, 2024-04-02 at 17:28 +0000, kergoth@gmail.com wrote:
> From: Christopher Larson <chris.larson@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.larson@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
>
--
Siemens AG, Technology
Linux Expert Center
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 6/9] isar-sstate: add --excluded-tasks argument
2024-04-02 17:28 [PATCH 0/9] Add more signature cachability tests to the testsuite kergoth
` (4 preceding siblings ...)
2024-04-02 17:28 ` [PATCH 5/9] isar-sstate: lint: ignore more image tasks kergoth
@ 2024-04-02 17:28 ` kergoth
2024-04-03 7:10 ` MOESSBAUER, Felix
2024-04-02 17:28 ` [PATCH 7/9] cibuilder.py: add -S support to the bitbake method kergoth
` (2 subsequent siblings)
8 siblings, 1 reply; 24+ messages in thread
From: kergoth @ 2024-04-02 17:28 UTC (permalink / raw)
To: isar-users; +Cc: Christopher Larson, Christopher Larson
From: Christopher Larson <chris.larson@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.larson@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
if not target.exists():
print(f"WARNING: target {target} does not exist. Nothing to analyze.")
return 0
--
2.39.2
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH 6/9] isar-sstate: add --excluded-tasks argument
2024-04-02 17:28 ` [PATCH 6/9] isar-sstate: add --excluded-tasks argument kergoth
@ 2024-04-03 7:10 ` MOESSBAUER, Felix
2024-04-03 21:41 ` Larson, Chris
2024-04-03 21:44 ` Larson, Chris
0 siblings, 2 replies; 24+ messages in thread
From: MOESSBAUER, Felix @ 2024-04-03 7:10 UTC (permalink / raw)
To: kergoth, isar-users; +Cc: Larson, Chris
On Tue, 2024-04-02 at 17:28 +0000, kergoth@gmail.com wrote:
> From: Christopher Larson <chris.larson@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.larson@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.moessbauer@siemens.com>
Felix
>
> 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
> if not target.exists():
> print(f"WARNING: target {target} does not exist. Nothing to
> analyze.")
> return 0
> --
> 2.39.2
>
--
Siemens AG, Technology
Linux Expert Center
^ permalink raw reply [flat|nested] 24+ messages in thread* RE: [PATCH 6/9] isar-sstate: add --excluded-tasks argument
2024-04-03 7:10 ` MOESSBAUER, Felix
@ 2024-04-03 21:41 ` Larson, Chris
2024-04-03 21:44 ` Larson, Chris
1 sibling, 0 replies; 24+ messages in thread
From: Larson, Chris @ 2024-04-03 21:41 UTC (permalink / raw)
To: MOESSBAUER, Felix, kergoth, isar-users
Good idea, I can include that in the v2 if there's no objection.
-----Original Message-----
From: Moessbauer, Felix (T CED OES-DE) <felix.moessbauer@siemens.com>
Sent: Wednesday, April 3, 2024 12:11 AM
To: kergoth@gmail.com; isar-users@googlegroups.com
Cc: Larson, Chris (DI CTO FDS CES LX MEL) <chris.larson@siemens.com>
Subject: Re: [PATCH 6/9] isar-sstate: add --excluded-tasks argument
On Tue, 2024-04-02 at 17:28 +0000, kergoth@gmail.com wrote:
> From: Christopher Larson <chris.larson@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.larson@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.moessbauer@siemens.com>
Felix
>
> 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
> if not target.exists():
> print(f"WARNING: target {target} does not exist. Nothing to
> analyze.")
> return 0
> --
> 2.39.2
>
--
Siemens AG, Technology
Linux Expert Center
^ permalink raw reply [flat|nested] 24+ messages in thread* RE: [PATCH 6/9] isar-sstate: add --excluded-tasks argument
2024-04-03 7:10 ` MOESSBAUER, Felix
2024-04-03 21:41 ` Larson, Chris
@ 2024-04-03 21:44 ` Larson, Chris
2024-04-04 6:28 ` MOESSBAUER, Felix
1 sibling, 1 reply; 24+ messages in thread
From: Larson, Chris @ 2024-04-03 21:44 UTC (permalink / raw)
To: MOESSBAUER, Felix, kergoth, isar-users
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.moessbauer@siemens.com>
Sent: Wednesday, April 3, 2024 12:11 AM
To: kergoth@gmail.com; isar-users@googlegroups.com
Cc: Larson, Chris (DI CTO FDS CES LX MEL) <chris.larson@siemens.com>
Subject: Re: [PATCH 6/9] isar-sstate: add --excluded-tasks argument
On Tue, 2024-04-02 at 17:28 +0000, kergoth@gmail.com wrote:
> From: Christopher Larson <chris.larson@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.larson@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.moessbauer@siemens.com>
Felix
>
> 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
> if not target.exists():
> print(f"WARNING: target {target} does not exist. Nothing to
> analyze.")
> return 0
> --
> 2.39.2
>
--
Siemens AG, Technology
Linux Expert Center
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH 6/9] isar-sstate: add --excluded-tasks argument
2024-04-03 21:44 ` Larson, Chris
@ 2024-04-04 6:28 ` MOESSBAUER, Felix
0 siblings, 0 replies; 24+ messages in thread
From: MOESSBAUER, Felix @ 2024-04-04 6:28 UTC (permalink / raw)
To: kergoth, Larson, Chris, isar-users
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
>
> -----Original Message-----
> From: Moessbauer, Felix (T CED OES-DE) <felix.moessbauer@siemens.com>
> Sent: Wednesday, April 3, 2024 12:11 AM
> To: kergoth@gmail.com; isar-users@googlegroups.com
> Cc: Larson, Chris (DI CTO FDS CES LX MEL) <chris.larson@siemens.com>
> Subject: Re: [PATCH 6/9] isar-sstate: add --excluded-tasks argument
>
> On Tue, 2024-04-02 at 17:28 +0000, kergoth@gmail.com wrote:
> > From: Christopher Larson <chris.larson@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.larson@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.moessbauer@siemens.com>
>
> Felix
>
> >
> > 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
> > if not target.exists():
> > print(f"WARNING: target {target} does not exist. Nothing
> > to
> > analyze.")
> > return 0
> > --
> > 2.39.2
> >
>
> --
> Siemens AG, Technology
> Linux Expert Center
>
>
--
Siemens AG, Technology
Linux Expert Center
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 7/9] cibuilder.py: add -S support to the bitbake method
2024-04-02 17:28 [PATCH 0/9] Add more signature cachability tests to the testsuite kergoth
` (5 preceding siblings ...)
2024-04-02 17:28 ` [PATCH 6/9] isar-sstate: add --excluded-tasks argument kergoth
@ 2024-04-02 17:28 ` kergoth
2024-04-03 7:12 ` MOESSBAUER, Felix
2024-04-02 17:28 ` [PATCH 8/9] testsuite: add perform_signature_lint method kergoth
2024-04-02 17:28 ` [PATCH 9/9] testsuite: add signature cachability checks kergoth
8 siblings, 1 reply; 24+ messages in thread
From: kergoth @ 2024-04-02 17:28 UTC (permalink / raw)
To: isar-users; +Cc: Christopher Larson, Christopher Larson
From: Christopher Larson <chris.larson@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.larson@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
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH 7/9] cibuilder.py: add -S support to the bitbake method
2024-04-02 17:28 ` [PATCH 7/9] cibuilder.py: add -S support to the bitbake method kergoth
@ 2024-04-03 7:12 ` MOESSBAUER, Felix
2024-04-03 21:41 ` Larson, Chris
0 siblings, 1 reply; 24+ messages in thread
From: MOESSBAUER, Felix @ 2024-04-03 7:12 UTC (permalink / raw)
To: kergoth, isar-users; +Cc: Larson, Chris, chris.larson
On Tue, 2024-04-02 at 17:28 +0000, kergoth@gmail.com wrote:
> From: Christopher Larson <chris.larson@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
>
> Signed-off-by: Christopher Larson <chris.larson@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
>
--
Siemens AG, Technology
Linux Expert Center
^ permalink raw reply [flat|nested] 24+ messages in thread* RE: [PATCH 7/9] cibuilder.py: add -S support to the bitbake method
2024-04-03 7:12 ` MOESSBAUER, Felix
@ 2024-04-03 21:41 ` Larson, Chris
0 siblings, 0 replies; 24+ messages in thread
From: Larson, Chris @ 2024-04-03 21:41 UTC (permalink / raw)
To: MOESSBAUER, Felix, isar-users; +Cc: chris.larson
No real reason, I'll update it. Thanks!
-----Original Message-----
From: Moessbauer, Felix (T CED OES-DE) <felix.moessbauer@siemens.com>
Sent: Wednesday, April 3, 2024 12:13 AM
To: kergoth@gmail.com; isar-users@googlegroups.com
Cc: Larson, Chris (DI CTO FDS CES LX MEL) <chris.larson@siemens.com>; chris.larson@seimens.com
Subject: Re: [PATCH 7/9] cibuilder.py: add -S support to the bitbake method
On Tue, 2024-04-02 at 17:28 +0000, kergoth@gmail.com wrote:
> From: Christopher Larson <chris.larson@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
>
> Signed-off-by: Christopher Larson <chris.larson@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
>
--
Siemens AG, Technology
Linux Expert Center
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 8/9] testsuite: add perform_signature_lint method
2024-04-02 17:28 [PATCH 0/9] Add more signature cachability tests to the testsuite kergoth
` (6 preceding siblings ...)
2024-04-02 17:28 ` [PATCH 7/9] cibuilder.py: add -S support to the bitbake method kergoth
@ 2024-04-02 17:28 ` kergoth
2024-04-02 17:28 ` [PATCH 9/9] testsuite: add signature cachability checks kergoth
8 siblings, 0 replies; 24+ messages in thread
From: kergoth @ 2024-04-02 17:28 UTC (permalink / raw)
To: isar-users; +Cc: Christopher Larson, Christopher Larson
From: Christopher Larson <chris.larson@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.larson@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
^ permalink raw reply [flat|nested] 24+ messages in thread* [PATCH 9/9] testsuite: add signature cachability checks
2024-04-02 17:28 [PATCH 0/9] Add more signature cachability tests to the testsuite kergoth
` (7 preceding siblings ...)
2024-04-02 17:28 ` [PATCH 8/9] testsuite: add perform_signature_lint method kergoth
@ 2024-04-02 17:28 ` kergoth
8 siblings, 0 replies; 24+ messages in thread
From: kergoth @ 2024-04-02 17:28 UTC (permalink / raw)
To: isar-users; +Cc: Christopher Larson, Christopher Larson
From: Christopher Larson <chris.larson@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.larson@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
^ permalink raw reply [flat|nested] 24+ messages in thread